* -Werror,-Wtautological-overlap-compare error in h8300-tdep.c
@ 2020-05-15 21:05 Simon Marchi
2020-05-17 13:54 ` Yoshinori Sato
0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2020-05-15 21:05 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: -Werror,-Wtautological-overlap-compare error in h8300-tdep.c
2020-05-15 21:05 -Werror,-Wtautological-overlap-compare error in h8300-tdep.c Simon Marchi
@ 2020-05-17 13:54 ` Yoshinori Sato
2020-05-17 15:01 ` Simon Marchi
0 siblings, 1 reply; 6+ messages in thread
From: Yoshinori Sato @ 2020-05-17 13:54 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: -Werror,-Wtautological-overlap-compare error in h8300-tdep.c
2020-05-17 13:54 ` Yoshinori Sato
@ 2020-05-17 15:01 ` Simon Marchi
2020-05-18 12:51 ` Yoshinori Sato
0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2020-05-17 15:01 UTC (permalink / raw)
To: Yoshinori Sato; +Cc: gdb-patches
On 2020-05-17 9:54 a.m., Yoshinori Sato wrote:
> 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.
Thank you for providing the patch and providing the link to the relevant
documentation.
Could you please explain to me what this macro does?
#define IS_MOVB_EXT(x) ((x) == 0x7860)
If I understand correctly, the relevant encoding is described at page 125
of the document you linked.
It says that in the first byte, we should have 0x78 (we do). In the second
byte, we should have `erd`, a register number, in the upper nibble. The macro
above checks for a constant value, so does that mean it checks for a particulier
register number?
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: -Werror,-Wtautological-overlap-compare error in h8300-tdep.c
2020-05-17 15:01 ` Simon Marchi
@ 2020-05-18 12:51 ` Yoshinori Sato
2020-05-19 17:35 ` Simon Marchi
0 siblings, 1 reply; 6+ messages in thread
From: Yoshinori Sato @ 2020-05-18 12:51 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On Mon, 18 May 2020 00:01:02 +0900,
Simon Marchi wrote:
>
> On 2020-05-17 9:54 a.m., Yoshinori Sato wrote:
> > 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.
>
> Thank you for providing the patch and providing the link to the relevant
> documentation.
>
> Could you please explain to me what this macro does?
>
> #define IS_MOVB_EXT(x) ((x) == 0x7860)
>
> If I understand correctly, the relevant encoding is described at page 125
> of the document you linked.
>
> It says that in the first byte, we should have 0x78 (we do). In the second
> byte, we should have `erd`, a register number, in the upper nibble. The macro
> above checks for a constant value, so does that mean it checks for a particulier
> register number?
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.
> Simon
--
Yosinori Sato
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: -Werror,-Wtautological-overlap-compare error in h8300-tdep.c
2020-05-18 12:51 ` Yoshinori Sato
@ 2020-05-19 17:35 ` Simon Marchi
2020-05-25 9:52 ` Yoshinori Sato
0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2020-05-19 17:35 UTC (permalink / raw)
To: Yoshinori Sato; +Cc: gdb-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: -Werror,-Wtautological-overlap-compare error in h8300-tdep.c
2020-05-19 17:35 ` Simon Marchi
@ 2020-05-25 9:52 ` Yoshinori Sato
0 siblings, 0 replies; 6+ messages in thread
From: Yoshinori Sato @ 2020-05-25 9:52 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On Wed, 20 May 2020 02:35:12 +0900,
Simon Marchi wrote:
>
> 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.
>
Sorry too late reply.
It looks good. Thanks.
>
> 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
>
--
Yosinori Sato
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-25 9:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15 21:05 -Werror,-Wtautological-overlap-compare error in h8300-tdep.c Simon Marchi
2020-05-17 13:54 ` Yoshinori Sato
2020-05-17 15:01 ` Simon Marchi
2020-05-18 12:51 ` Yoshinori Sato
2020-05-19 17:35 ` Simon Marchi
2020-05-25 9:52 ` Yoshinori Sato
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).