* Re: [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning
[not found] <d6q0acpdxmm7g52oqylkjjb2.1413419448973@email.android.com>
@ 2014-10-16 7:06 ` Andreas Schwab
2014-10-16 7:38 ` Chen Gang
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2014-10-16 7:06 UTC (permalink / raw)
To: Chen Gang; +Cc: Joel Brobecker, gdb-patches
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."
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning
2014-10-16 7:06 ` [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning Andreas Schwab
@ 2014-10-16 7:38 ` Chen Gang
0 siblings, 0 replies; 6+ messages in thread
From: Chen Gang @ 2014-10-16 7:38 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Joel Brobecker, gdb-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning
2014-10-14 21:43 ` Chen Gang
@ 2014-10-15 15:00 ` Joel Brobecker
0 siblings, 0 replies; 6+ messages in thread
From: Joel Brobecker @ 2014-10-15 15:00 UTC (permalink / raw)
To: Chen Gang; +Cc: Andreas Schwab, gdb-patches
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning
2014-10-14 21:29 ` Andreas Schwab
@ 2014-10-14 21:43 ` Chen Gang
2014-10-15 15:00 ` Joel Brobecker
0 siblings, 1 reply; 6+ messages in thread
From: Chen Gang @ 2014-10-14 21:43 UTC (permalink / raw)
To: Andreas Schwab; +Cc: gdb-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning
2014-10-14 21:21 Chen Gang
@ 2014-10-14 21:29 ` Andreas Schwab
2014-10-14 21:43 ` Chen Gang
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2014-10-14 21:29 UTC (permalink / raw)
To: Chen Gang; +Cc: gdb-patches
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."
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning
@ 2014-10-14 21:21 Chen Gang
2014-10-14 21:29 ` Andreas Schwab
0 siblings, 1 reply; 6+ messages in thread
From: Chen Gang @ 2014-10-14 21:21 UTC (permalink / raw)
To: gdb-patches
'(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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-16 7:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <d6q0acpdxmm7g52oqylkjjb2.1413419448973@email.android.com>
2014-10-16 7:06 ` [PATCH] gdb/hppa-tdep.c: Fix a logical typo bug found by compiler warning Andreas Schwab
2014-10-16 7:38 ` Chen Gang
2014-10-14 21:21 Chen Gang
2014-10-14 21:29 ` Andreas Schwab
2014-10-14 21:43 ` Chen Gang
2014-10-15 15:00 ` Joel Brobecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox