Re: -Werror,-Wtautological-overlap-compare error in h8300-tdep.c

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2020-05-18 8:51 a.m., Yoshinori Sato wrote:
> Yes.
> register number 6 (er6) is frame pointer in h8300 ABI.
> Since we are analyzing the code that accesses the stack frame,
> we will only look at the frame pointer.

Ah ok, thanks for the explanation.  Here's what I pushed.  Note that I also
changed the read_memory_integer to read_memory_unsigned_integer.


From 1d6ce4d31257e1ea49b3a1b257055bf8f7ff16af Mon Sep 17 00:00:00 2001
From: Yoshinori Sato <ysato@users.sourceforge.jp>
Date: Tue, 19 May 2020 13:33:08 -0400
Subject: [PATCH] gdb: fix -Wtautological-overlap-compare error in h8300-tdep.c

Compiling with clang 11 gives us:

      CXX    h8300-tdep.o
    /home/smarchi/src/binutils-gdb/gdb/h8300-tdep.c:225:21: error: overlapping comparisons always evaluate to false [-Werror,-Wtautological-overlap-compare]
                  if (disp < 0 && disp > 0xffffff)
                      ~~~~~~~~~^~~~~~~~~~~~~~~~~~
    /home/smarchi/src/binutils-gdb/gdb/h8300-tdep.c:203:17: error: overlapping comparisons always evaluate to false [-Werror,-Wtautological-overlap-compare]
              if (disp < 0 && disp > 0xffffff)
                  ~~~~~~~~~^~~~~~~~~~~~~~~~~~
    /home/smarchi/src/binutils-gdb/gdb/h8300-tdep.c:184:17: error: overlapping comparisons always evaluate to false [-Werror,-Wtautological-overlap-compare]
              if (disp < 0 && disp > 0xffffff)
                  ~~~~~~~~~^~~~~~~~~~~~~~~~~~

Indeed, disp (of type LONGEST) can't be less than 0 and greater than
0xffffff.

Fix it by changing the way we check if disp is negative.  Check the sign
bit of disp, which is a 24-bit number.

gdb/ChangeLog:

	* h8300-tdep.c (h8300_is_argument_spill): Change how we check
	whether disp is negative.
---
 gdb/ChangeLog    |  5 +++++
 gdb/h8300-tdep.c | 13 +++++++------
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f086b5cf908c..c4393182dd95 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-05-19  Yoshinori Sato  <ysato@users.sourceforge.jp>
+
+	* h8300-tdep.c (h8300_is_argument_spill): Change how we check
+	whether disp is negative.
+
 2020-05-19  Simon Marchi  <simon.marchi@efficios.com>

 	* symfile.h (struct symfile_segment_data)
diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index 3c2f502a124d..b569a23f2631 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -178,10 +178,10 @@ h8300_is_argument_spill (struct gdbarch *gdbarch, CORE_ADDR pc)
       if (IS_MOVB_Rn24_SP (read_memory_unsigned_integer (pc + 2,
 							 2, byte_order)))
 	{
-	  LONGEST disp = read_memory_integer (pc + 4, 4, byte_order);
+	  ULONGEST disp = read_memory_unsigned_integer (pc + 4, 4, byte_order);

 	  /* ... and d:24 is negative.  */
-	  if (disp < 0 && disp > 0xffffff)
+	  if ((disp & 0x00800000) != 0)
 	    return 8;
 	}
     }
@@ -197,10 +197,10 @@ h8300_is_argument_spill (struct gdbarch *gdbarch, CORE_ADDR pc)
       if (IS_MOVW_Rn24_SP (read_memory_unsigned_integer (pc + 2,
 							 2, byte_order)))
 	{
-	  LONGEST disp = read_memory_integer (pc + 4, 4, byte_order);
+	  ULONGEST disp = read_memory_unsigned_integer (pc + 4, 4, byte_order);

 	  /* ... and d:24 is negative.  */
-	  if (disp < 0 && disp > 0xffffff)
+	  if ((disp & 0x00800000) != 0)
 	    return 8;
 	}
     }
@@ -219,10 +219,11 @@ h8300_is_argument_spill (struct gdbarch *gdbarch, CORE_ADDR pc)
 	{
 	  if (IS_MOVL_Rn24_SP (read_memory_integer (pc + 4, 2, byte_order)))
 	    {
-	      LONGEST disp = read_memory_integer (pc + 6, 4, byte_order);
+	      ULONGEST disp = read_memory_unsigned_integer (pc + 6, 4,
+							    byte_order);

 	      /* ... and d:24 is negative.  */
-	      if (disp < 0 && disp > 0xffffff)
+	      if ((disp & 0x00800000) != 0)
 		return 10;
 	    }
 	}
-- 
2.26.2