Re: [PATCH] gdb: fix -Wtautological-overlap-compare warning in mips-linux-tdep.c

[Simon Marchi <simark@simark.ca>]

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