[PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning

[PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning

[Chen Gang]

'(inst >> 6) & 0xf)' need compare with both '0x8' and '0x9', original
implementation missed additional '(' and ')' for it, which will cause
logical bug (will skip '0x8' checking).

The related warning under gcc5:

  gcc -g -O2   -I. -I../../binutils-gdb/gdb -I../../binutils-gdb/gdb/common -I../../binutils-gdb/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../binutils-gdb/gdb/../include/opcode -I../../binutils-gdb/gdb/../opcodes/.. -I../../binutils-gdb/gdb/../readline/.. -I../bfd -I../../binutils-gdb/gdb/../bfd -I../../binutils-gdb/gdb/../include -I../libdecnumber -I../../binutils-gdb/gdb/../libdecnumber  -I../../binutils-gdb/gdb/gnulib/import -Ibuild-gnulib/import   -DTUI=1  -I/usr/include/python2.7 -I/usr/include/python2.7 -Wall -Wdeclaration-after-statement -Wpointer-arith -Wpointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmissing-prototypes -Wdeclaration-after-statement -Wempty-body -Wmissing-parameter-type -Wold-style-declaration -Wold-style-definition -Wformat-nonliteral -Werror -c -o hppa-tdep.o -MT hppa-tdep.o -MMD -MP -MF .deps/hppa-tdep.Tpo ../../binutils-gdb/gdb/hppa-tdep.c
  ../../binutils-gdb/gdb/hppa-tdep.c: In function 'inst_saves_gr':
  ../../binutils-gdb/gdb/hppa-tdep.c:1406:30: error: comparison of constant '9' with boolean expression is always false [-Werror=bool-compare]
          || (inst >> 6) & 0xf) == 0x9))
                              ^
  cc1: all warnings being treated as errors

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
---
 gdb/hppa-tdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 627f31a..51edead 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -1403,7 +1403,7 @@ inst_saves_gr (unsigned long inst)
   if ((inst >> 26) == 0x19 || (inst >> 26) == 0x18
       || ((inst >> 26) == 0x3
 	  && (((inst >> 6) & 0xf) == 0x8
-	      || (inst >> 6) & 0xf) == 0x9))
+	      || ((inst >> 6) & 0xf) == 0x9)))
     return hppa_extract_5R_store (inst);
 
   return 0;
-- 
1.9.3

Re: [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning

[Andreas Schwab]

Chen Gang <gang.chen.5i5j@gmail.com> writes:

>  	  && (((inst >> 6) & 0xf) == 0x8
> -	      || (inst >> 6) & 0xf) == 0x9))
> +	      || ((inst >> 6) & 0xf) == 0x9)))

             ((inst >> 6) & 0xe) == 8

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning

[Chen Gang]

On 10/15/2014 05:29 AM, Andreas Schwab wrote:
> Chen Gang <gang.chen.5i5j@gmail.com> writes:
> 
>>  	  && (((inst >> 6) & 0xf) == 0x8
>> -	      || (inst >> 6) & 0xf) == 0x9))
>> +	      || ((inst >> 6) & 0xf) == 0x9)))
> 
>              ((inst >> 6) & 0xe) == 8
> 
> Andreas.
> 

I guess, your fixing may like below, which will be a different logical
working flow.

diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 627f31a..3112732 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -1402,8 +1402,8 @@ inst_saves_gr (unsigned long inst)
      too.  */
   if ((inst >> 26) == 0x19 || (inst >> 26) == 0x18
       || ((inst >> 26) == 0x3
-         && (((inst >> 6) & 0xf) == 0x8
-             || (inst >> 6) & 0xf) == 0x9))
+         && ((inst >> 6) & 0xf) == 0x8
+             || ((inst >> 6) & 0xf) == 0x9))
     return hppa_extract_5R_store (inst);
 
   return 0;

If you are sure it is, please help send related patch with more details
comments for it (excuse me, I am not quite familiar the related logical
details).


Thanks.
-- 
Chen Gang

Open share and attitude like air water and life which God blessed

Re: [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning

[Joel Brobecker]

> On 10/15/2014 05:29 AM, Andreas Schwab wrote:
> > Chen Gang <gang.chen.5i5j@gmail.com> writes:
> > 
> >>  	  && (((inst >> 6) & 0xf) == 0x8
> >> -	      || (inst >> 6) & 0xf) == 0x9))
> >> +	      || ((inst >> 6) & 0xf) == 0x9)))
> > 
> >              ((inst >> 6) & 0xe) == 8
> > 
> > Andreas.
> > 
> 
> I guess, your fixing may like below, which will be a different logical
> working flow.

I think Andreas is telling you that...

     ((inst >> 6) & 0xf) == 0x8
     || ((inst >> 6) & 0xf) == 0x9

... is logically equivalent to ...

     ((inst >> 6) & 0xe) == 8

In other word, if it does not matter if bit 7 is set or not
(the difference between 0x8 and 0x9) all you have to do is mask it.
That way, you test both conditions with one comparison instead of 2.

-- 
Joel

Re: [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning

[Andreas Schwab]

There's another bug in this function:

  /* Does it look like a stw?  */
  if ((inst >> 26) == 0x1a || (inst >> 26) == 0x1b
      || (inst >> 26) == 0x1f
      || ((inst >> 26) == 0x1f
	  && ((inst >> 6) == 0xa)))
    return hppa_extract_5R_store (inst);

The last condition is redundant.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning

[Chen Gang]

On 10/16/14 15:06, Andreas Schwab wrote:
> There's another bug in this function:
> 
>   /* Does it look like a stw?  */
>   if ((inst >> 26) == 0x1a || (inst >> 26) == 0x1b
>       || (inst >> 26) == 0x1f
>       || ((inst >> 26) == 0x1f
> 	  && ((inst >> 6) == 0xa)))
>     return hppa_extract_5R_store (inst);
> 
> The last condition is redundant.
> 

OK, thanks.

For me, it is another typo issue bug, according to the code within
inst_saves_gr(), need use "(inst >> 26) == 0x03" instead of the 2nd
"(inst >> 26) == 0x1f".

And are any members familiar with the related logical working flow? If
no reply within this week end, I shall try to be familiar with the
related working flow firstly, then make patch v2 within this month.


Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed