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

[Yoshinori Sato]

On Sat, 16 May 2020 06:05:44 +0900,
Simon Marchi wrote:
> 
> When building with clang 11, we get:
> 
>   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.
> 
> The closest thing I could find to an instruction set reference was this:
> 
>   https://www.renesas.com/cn/en/doc/products/mpumcu/001/e602025_h8300.pdf
> 
> ... but it didn't really help me decode this code.  I'm reporting it in hope that
> somebody that knows what they are doing would know how to fix it.
> 
> Yoshinori, I CCed you because you happen to be the last person who did a meaningful commit
> in h8300-tdep.c, so maybe you have an idea.
> 
> Simon

This instruction enhanced on h8300h.
https://www.renesas.com/us/en/doc/products/mpumcu/001/rej09b0213_h8300h.pdf

disp have 24bit value.
So always 0 in upper byte.

I think it's good to check like this.

diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index 79c74001bc..d3d2bf84ab 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -178,10 +178,10 @@ h8300_is_argument_spill (struct gdbarch *gdbarch, CORE_ADD
R 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_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_ADD
R 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_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,10 @@ h8300_is_argument_spill (struct gdbarch *gdbarch, CORE_ADD
R 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_integer (pc + 6, 4, byte_order);
 
              /* ... and d:24 is negative.  */
-             if (disp < 0 && disp > 0xffffff)
+             if ((disp & 0x00800000) != 0)
                return 10;
            }
        }

-- 
Yosinori Sato