* [PATCH] gdb: fix -Wtautological-overlap-compare warning in mips-linux-tdep.c
@ 2020-05-16 2:01 Simon Marchi
2020-05-16 6:42 ` Andreas Schwab
2020-05-16 13:29 ` Maciej W. Rozycki
0 siblings, 2 replies; 4+ messages in thread
From: Simon Marchi @ 2020-05-16 2:01 UTC (permalink / raw)
To: gdb-patches; +Cc: macro, Simon Marchi
When building with clang 11, I get:
CXX mips-linux-tdep.o
/home/smarchi/src/binutils-gdb/gdb/mips-linux-tdep.c:643:30: error: overlapping comparisons always evaluate to true [-Werror,-Wtautological-overlap-compare]
if (insn != 0x03e07821 || insn != 0x03e07825)
~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
/home/smarchi/src/binutils-gdb/gdb/mips-linux-tdep.c:636:30: error: overlapping comparisons always evaluate to true [-Werror,-Wtautological-overlap-compare]
if (insn != 0x03e0782d || insn != 0x03e07825)
~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
Indeed, given two different values, `insn` will always be different to
one of them, and these conditions always be true. I suppose that the
original intent was "if insn is equal to this instruction or equal to
that instruction". Therefore the `!=` should be changed to `==`.
gdb/ChangeLog:
* mips-linux-tdep.c (mips_linux_in_dynsym_stub): Fix condition.
---
gdb/mips-linux-tdep.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/gdb/mips-linux-tdep.c b/gdb/mips-linux-tdep.c
index aa7b8d11f3fb..ae067c19dfa7 100644
--- a/gdb/mips-linux-tdep.c
+++ b/gdb/mips-linux-tdep.c
@@ -633,16 +633,14 @@ mips_linux_in_dynsym_stub (CORE_ADDR pc)
if (n64)
{
/* 'daddu t7,ra' or 'or t7, ra, zero'*/
- if (insn != 0x03e0782d || insn != 0x03e07825)
+ if (insn == 0x03e0782d || insn == 0x03e07825)
return 0;
-
}
else
{
/* 'addu t7,ra' or 'or t7, ra, zero'*/
- if (insn != 0x03e07821 || insn != 0x03e07825)
+ if (insn == 0x03e07821 || insn == 0x03e07825)
return 0;
-
}
insn = extract_unsigned_integer (p + 8, 4, byte_order);
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb: fix -Wtautological-overlap-compare warning in mips-linux-tdep.c
2020-05-16 2:01 [PATCH] gdb: fix -Wtautological-overlap-compare warning in mips-linux-tdep.c Simon Marchi
@ 2020-05-16 6:42 ` Andreas Schwab
2020-05-16 13:29 ` Maciej W. Rozycki
1 sibling, 0 replies; 4+ messages in thread
From: Andreas Schwab @ 2020-05-16 6:42 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi
On Mai 15 2020, Simon Marchi via Gdb-patches wrote:
> Indeed, given two different values, `insn` will always be different to
> one of them, and these conditions always be true. I suppose that the
> original intent was "if insn is equal to this instruction or equal to
> that instruction". Therefore the `!=` should be changed to `==`.
I don't think that makes sense. Rather || should be fixed to &&.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb: fix -Wtautological-overlap-compare warning in mips-linux-tdep.c
2020-05-16 2:01 [PATCH] gdb: fix -Wtautological-overlap-compare warning in mips-linux-tdep.c Simon Marchi
2020-05-16 6:42 ` Andreas Schwab
@ 2020-05-16 13:29 ` Maciej W. Rozycki
2020-05-16 15:23 ` Simon Marchi
1 sibling, 1 reply; 4+ messages in thread
From: Maciej W. Rozycki @ 2020-05-16 13:29 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On Fri, 15 May 2020, Simon Marchi wrote:
> diff --git a/gdb/mips-linux-tdep.c b/gdb/mips-linux-tdep.c
> index aa7b8d11f3fb..ae067c19dfa7 100644
> --- a/gdb/mips-linux-tdep.c
> +++ b/gdb/mips-linux-tdep.c
> @@ -633,16 +633,14 @@ mips_linux_in_dynsym_stub (CORE_ADDR pc)
> if (n64)
> {
> /* 'daddu t7,ra' or 'or t7, ra, zero'*/
> - if (insn != 0x03e0782d || insn != 0x03e07825)
> + if (insn == 0x03e0782d || insn == 0x03e07825)
Nope, this ought to be:
if (insn != 0x03e0782d && insn != 0x03e07825)
> return 0;
> -
> }
> else
> {
> /* 'addu t7,ra' or 'or t7, ra, zero'*/
> - if (insn != 0x03e07821 || insn != 0x03e07825)
> + if (insn == 0x03e07821 || insn == 0x03e07825)
Likewise:
if (insn != 0x03e07821 && insn != 0x03e07825)
Thanks for looking into it (and for cc-ing me)!
Maciej
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gdb: fix -Wtautological-overlap-compare warning in mips-linux-tdep.c
2020-05-16 13:29 ` Maciej W. Rozycki
@ 2020-05-16 15:23 ` Simon Marchi
0 siblings, 0 replies; 4+ messages in thread
From: Simon Marchi @ 2020-05-16 15:23 UTC (permalink / raw)
To: Maciej W. Rozycki, Simon Marchi; +Cc: gdb-patches, Andreas Schwab
On 2020-05-16 9:29 a.m., Maciej W. Rozycki wrote:
> On Fri, 15 May 2020, Simon Marchi wrote:
>
>> diff --git a/gdb/mips-linux-tdep.c b/gdb/mips-linux-tdep.c
>> index aa7b8d11f3fb..ae067c19dfa7 100644
>> --- a/gdb/mips-linux-tdep.c
>> +++ b/gdb/mips-linux-tdep.c
>> @@ -633,16 +633,14 @@ mips_linux_in_dynsym_stub (CORE_ADDR pc)
>> if (n64)
>> {
>> /* 'daddu t7,ra' or 'or t7, ra, zero'*/
>> - if (insn != 0x03e0782d || insn != 0x03e07825)
>> + if (insn == 0x03e0782d || insn == 0x03e07825)
>
> Nope, this ought to be:
>
> if (insn != 0x03e0782d && insn != 0x03e07825)
>
>> return 0;
>> -
>> }
>> else
>> {
>> /* 'addu t7,ra' or 'or t7, ra, zero'*/
>> - if (insn != 0x03e07821 || insn != 0x03e07825)
>> + if (insn == 0x03e07821 || insn == 0x03e07825)
>
> Likewise:
>
> if (insn != 0x03e07821 && insn != 0x03e07825)
>
> Thanks for looking into it (and for cc-ing me)!
>
> Maciej
>
Ah so I misinterpreted the intent. Thanks to you and Andreas for pointing
it out!
I modified the patch and pushed it as below.
Simon
From 59f7bd8d2b855162db6784c9724ead9e2377f32c Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@efficios.com>
Date: Sat, 16 May 2020 11:21:41 -0400
Subject: [PATCH] gdb: fix -Wtautological-overlap-compare warning in
mips-linux-tdep.c
When building with clang 11, I get:
CXX mips-linux-tdep.o
/home/smarchi/src/binutils-gdb/gdb/mips-linux-tdep.c:643:30: error: overlapping comparisons always evaluate to true [-Werror,-Wtautological-overlap-compare]
if (insn != 0x03e07821 || insn != 0x03e07825)
~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
/home/smarchi/src/binutils-gdb/gdb/mips-linux-tdep.c:636:30: error: overlapping comparisons always evaluate to true [-Werror,-Wtautological-overlap-compare]
if (insn != 0x03e0782d || insn != 0x03e07825)
~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
Indeed, given two different values, `insn` will always be different to
one of them, and these conditions always be true.
This code is meant to return if `insn` isn't one of these two values, so
the `||` should be replaced with `&&`.
gdb/ChangeLog:
* mips-linux-tdep.c (mips_linux_in_dynsym_stub): Fix condition.
---
gdb/ChangeLog | 4 ++++
gdb/mips-linux-tdep.c | 6 ++----
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1c4dc5c94c2a..8d6901efe67c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-05-16 Simon Marchi <simon.marchi@efficios.com>
+
+ * mips-linux-tdep.c (mips_linux_in_dynsym_stub): Fix condition.
+
2020-05-16 Pedro Alves <palves@redhat.com>
* ia64-linux-nat.c
diff --git a/gdb/mips-linux-tdep.c b/gdb/mips-linux-tdep.c
index aa7b8d11f3fb..3ffd53db9ead 100644
--- a/gdb/mips-linux-tdep.c
+++ b/gdb/mips-linux-tdep.c
@@ -633,16 +633,14 @@ mips_linux_in_dynsym_stub (CORE_ADDR pc)
if (n64)
{
/* 'daddu t7,ra' or 'or t7, ra, zero'*/
- if (insn != 0x03e0782d || insn != 0x03e07825)
+ if (insn != 0x03e0782d && insn != 0x03e07825)
return 0;
-
}
else
{
/* 'addu t7,ra' or 'or t7, ra, zero'*/
- if (insn != 0x03e07821 || insn != 0x03e07825)
+ if (insn != 0x03e07821 && insn != 0x03e07825)
return 0;
-
}
insn = extract_unsigned_integer (p + 8, 4, byte_order);
--
2.26.2
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-16 15:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-16 2:01 [PATCH] gdb: fix -Wtautological-overlap-compare warning in mips-linux-tdep.c Simon Marchi
2020-05-16 6:42 ` Andreas Schwab
2020-05-16 13:29 ` Maciej W. Rozycki
2020-05-16 15:23 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).