Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Chen Gang]

Oh, really it is. Originally, I skipped it (do not know it should be as an issue, too). Also sorry for my poor English: misunderstand what you said (this patch can still continue, although gcc 5 has another issue).

And next, I shall try to fix it, based on what bugzilla has done. But excuse me, I have no enough time resource on it, so maybe can not finish within this month (try to finish within next month) .

Thanks

Send from Lenovo A788t.

Iain Buclaw <ibuclaw@gdcproject.org> wrote:

>On 12 October 2014 15:47, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
>> On 10/12/14 22:13, Iain Buclaw wrote:
>>> On 12 October 2014 14:28, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
>>>> gdb requires "-Werror", and I387_ST0_REGNUM (tdep) is 'variable', then
>>>> compiler can think that I387_ST0_REGNUM (tdep) may be a large number,
>>>> which may cause issue, so report warning.
>>>>
>>>> Need fix this warning, and still keep the code clear enough for readers.
>>>> The related warning under Darwin with gnu built gcc:
>>>>
>>>
>>> I had noted the same on GCC 5.0.0 development, found that the line
>>> number in the warning was wrong and raised a bug
>>> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63510), just didn't get
>>> round to submitting a patch for gdb.
>>>
>>
>> But for me, what compiler has done is correct: "-Werror=strict-overflow"
>> need include "(X + c) >= X" for signed overflow. And our case matches
>> this case:
>>
>
>The compiler has done right, but that still doesn't stop the reported
>line number being wrong.
>
>-- Iain.

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Chen Gang]

Hello Iain:

Is this bug still alive (I joined in bug 63510)? If it is, I shall try
to fix it within this month (within 2014-11-30).

At present, its status is:

 - gcc members think that what compiler has done is correct, but need
   improve its report line number:

     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63510

 - gdb members might think what compiler has done is incorrect (need not
   report warning).

 - for me, I agree with gcc members' opinion, and shall try to improve
   its report line number.


Welcome any ideas, suggestions and completions, in time.

Thanks.

On 10/13/14 7:45, Chen Gang wrote:
> Oh, really it is. Originally, I skipped it (do not know it should be as an issue, too). Also sorry for my poor English: misunderstand what you said (this patch can still continue, although gcc 5 has another issue).
> 
> And next, I shall try to fix it, based on what bugzilla has done. But excuse me, I have no enough time resource on it, so maybe can not finish within this month (try to finish within next month) .
> 
> Thanks
> 
> Send from Lenovo A788t.
> 
> 
> Iain Buclaw <ibuclaw@gdcproject.org> wrote:
> 
> On 12 October 2014 15:47, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
>> On 10/12/14 22:13, Iain Buclaw wrote:
>>> On 12 October 2014 14:28, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
>>>> gdb requires "-Werror", and I387_ST0_REGNUM (tdep) is 'variable', then
>>>> compiler can think that I387_ST0_REGNUM (tdep) may be a large number,
>>>> which may cause issue, so report warning.
>>>>
>>>> Need fix this warning, and still keep the code clear enough for readers.
>>>> The related warning under Darwin with gnu built gcc:
>>>>
>>>
>>> I had noted the same on GCC 5.0.0 development, found that the line
>>> number in the warning was wrong and raised a bug
>>> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63510), just didn't get
>>> round to submitting a patch for gdb.
>>>
>>
>> But for me, what compiler has done is correct: "-Werror=strict-overflow"
>> need include "(X + c) >= X" for signed overflow. And our case matches
>> this case:
>>
> 
> The compiler has done right, but that still doesn't stop the reported
> line number being wrong.
> 
> -- Iain.
> 

Thanks.
-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Chen Gang S]

At present, for gcc version 5.0.0 20150109 (experimental) (GCC), can not
find this warning again. So this thread can be ended. If it will appear
again in the future, I shall analyze it again only within gcc wide.

And sorry for having delayed a long time for it.

Thanks.

On 11/10/14 09:45, Chen Gang wrote:
> Hello Iain:
> 
> Is this bug still alive (I joined in bug 63510)? If it is, I shall try
> to fix it within this month (within 2014-11-30).
> 
> At present, its status is:
> 
>  - gcc members think that what compiler has done is correct, but need
>    improve its report line number:
> 
>      https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63510
> 
>  - gdb members might think what compiler has done is incorrect (need not
>    report warning).
> 
>  - for me, I agree with gcc members' opinion, and shall try to improve
>    its report line number.
> 
> 
> Welcome any ideas, suggestions and completions, in time.
> 
> Thanks.
> 
> On 10/13/14 7:45, Chen Gang wrote:
>> Oh, really it is. Originally, I skipped it (do not know it should be as an issue, too). Also sorry for my poor English: misunderstand what you said (this patch can still continue, although gcc 5 has another issue).
>>
>> And next, I shall try to fix it, based on what bugzilla has done. But excuse me, I have no enough time resource on it, so maybe can not finish within this month (try to finish within next month) .
>>
>> Thanks
>>
>> Send from Lenovo A788t.
>>
>>
>> Iain Buclaw <ibuclaw@gdcproject.org> wrote:
>>
>> On 12 October 2014 15:47, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
>>> On 10/12/14 22:13, Iain Buclaw wrote:
>>>> On 12 October 2014 14:28, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
>>>>> gdb requires "-Werror", and I387_ST0_REGNUM (tdep) is 'variable', then
>>>>> compiler can think that I387_ST0_REGNUM (tdep) may be a large number,
>>>>> which may cause issue, so report warning.
>>>>>
>>>>> Need fix this warning, and still keep the code clear enough for readers.
>>>>> The related warning under Darwin with gnu built gcc:
>>>>>
>>>>
>>>> I had noted the same on GCC 5.0.0 development, found that the line
>>>> number in the warning was wrong and raised a bug
>>>> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63510), just didn't get
>>>> round to submitting a patch for gdb.
>>>>
>>>
>>> But for me, what compiler has done is correct: "-Werror=strict-overflow"
>>> need include "(X + c) >= X" for signed overflow. And our case matches
>>> this case:
>>>
>>
>> The compiler has done right, but that still doesn't stop the reported
>> line number being wrong.
>>
>> -- Iain.
>>
> 
> Thanks.
> 

-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Chen Gang]

OK, I will send patch v3 for it.

Send from Lenovo A788t.

Mark Kettenis <mark.kettenis@xs4all.nl> wrote:

>> Date: Wed, 15 Oct 2014 11:47:11 -0700
>> From: Joel Brobecker <brobecker@adacore.com>
>> 
>> Hi Mark,
>> 
>> > > Sorry, I disagree with this approach.  I think your compiler is really
>> > > being unhelpful here.  See if a newer GCC version of the compiler
>> > > still triggers that warning.  If so, complain to the GCC people.
>> 
>> Would you be OK with Pedro's suggested approach where we have
>> an additional macro that provides the number of ST registers?
>> That should get rid of the warning, and it seems like the code
>> might actually even be slightly better for it; it would make it
>> obvious that you iterate over a fixed number of registers, and
>> which ones you are interating over; [ST0 .. XMM0[ does not make
>> it as clear that the range only refers to ST registers, for instance.
>
>Pedro's suggestion made sense to me.

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Mark Kettenis]

> Date: Wed, 15 Oct 2014 11:47:11 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> 
> Hi Mark,
> 
> > > Sorry, I disagree with this approach.  I think your compiler is really
> > > being unhelpful here.  See if a newer GCC version of the compiler
> > > still triggers that warning.  If so, complain to the GCC people.
> 
> Would you be OK with Pedro's suggested approach where we have
> an additional macro that provides the number of ST registers?
> That should get rid of the warning, and it seems like the code
> might actually even be slightly better for it; it would make it
> obvious that you iterate over a fixed number of registers, and
> which ones you are interating over; [ST0 .. XMM0[ does not make
> it as clear that the range only refers to ST registers, for instance.

Pedro's suggestion made sense to me.

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Joel Brobecker]

Hi Mark,

> > Sorry, I disagree with this approach.  I think your compiler is really
> > being unhelpful here.  See if a newer GCC version of the compiler
> > still triggers that warning.  If so, complain to the GCC people.

Would you be OK with Pedro's suggested approach where we have
an additional macro that provides the number of ST registers?
That should get rid of the warning, and it seems like the code
might actually even be slightly better for it; it would make it
obvious that you iterate over a fixed number of registers, and
which ones you are interating over; [ST0 .. XMM0[ does not make
it as clear that the range only refers to ST registers, for instance.

-- 
Joel

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Chen Gang]

On 10/14/2014 12:05 AM, Chen Gang wrote:
> On 10/13/14 23:16, Mark Kettenis wrote:
>>> Date: Sun, 12 Oct 2014 21:28:40 +0800
>>> From: Chen Gang <gang.chen.5i5j@gmail.com>
>>>
>>> gdb requires "-Werror", and I387_ST0_REGNUM (tdep) is 'variable', then
>>> compiler can think that I387_ST0_REGNUM (tdep) may be a large number,
>>> which may cause issue, so report warning.
>>>
>>> Need fix this warning, and still keep the code clear enough for readers.
>>> The related warning under Darwin with gnu built gcc:
>>
>> Sorry, I disagree with this approach.  I think your compiler is really
>> being unhelpful here.  See if a newer GCC version of the compiler
>> still triggers that warning.  If so, complain to the GCC people.
>>
> 
> Excuse me, I do not dare to complain it to gcc members, because for me,
> the compiler's printing is obviously, and proofs itself is correct. If I
> ask a quite obvious question to them, I guess, I will be complained.
> 
> Hmm... But I may have an additional trying: Use original gcc and provide
> "-Werror=strict-overflow" to see what will happen.
> 
>  - If it will report warning too, I can ask gcc members why integrate
>    "-Wstrict-overflow" to -Wall.
> 
>  - If it will not report warning, it means original gcc also integrate
>    "-Wstrict-overflow" or have no this option. And for me, I prefer to
>    use "#pragma GCC diagnostic ignored ... " for our code.

After use "gcc version 4.8.3 20140624 (Red Hat 4.8.3-1) (GCC)" with
"-Wstrict-overflow", it supports this flag, and does not report warning.

For me, either "$pragma GCC diagnostic ignored ..." or continuing this
patch. At present, I don't know any other ways for it.

Thanks.
-- 
Chen Gang

Open share and attitude like air water and life which God blessed

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Chen Gang]

On 10/13/14 23:16, Mark Kettenis wrote:
>> Date: Sun, 12 Oct 2014 21:28:40 +0800
>> From: Chen Gang <gang.chen.5i5j@gmail.com>
>>
>> gdb requires "-Werror", and I387_ST0_REGNUM (tdep) is 'variable', then
>> compiler can think that I387_ST0_REGNUM (tdep) may be a large number,
>> which may cause issue, so report warning.
>>
>> Need fix this warning, and still keep the code clear enough for readers.
>> The related warning under Darwin with gnu built gcc:
> 
> Sorry, I disagree with this approach.  I think your compiler is really
> being unhelpful here.  See if a newer GCC version of the compiler
> still triggers that warning.  If so, complain to the GCC people.
> 

Excuse me, I do not dare to complain it to gcc members, because for me,
the compiler's printing is obviously, and proofs itself is correct. If I
ask a quite obvious question to them, I guess, I will be complained.

Hmm... But I may have an additional trying: Use original gcc and provide
"-Werror=strict-overflow" to see what will happen.

 - If it will report warning too, I can ask gcc members why integrate
   "-Wstrict-overflow" to -Wall.

 - If it will not report warning, it means original gcc also integrate
   "-Wstrict-overflow" or have no this option. And for me, I prefer to
   use "#pragma GCC diagnostic ignored ... " for our code.

Thanks.

>>   gcc -g -O2   -I. -I../../binutils-gdb/gdb -I../../binutils-gdb/gdb/common -I../../binutils-gdb/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../binutils-gdb/gdb/../include/opcode -I../../binutils-gdb/gdb/../opcodes/..  -I../../binutils-gdb/gdb/../readline/.. -I../bfd -I../../binutils-gdb/gdb/../bfd -I../../binutils-gdb/gdb/../include -I../libdecnumber -I../../binutils-gdb/gdb/../libdecnumber -I../../binutils-gdb/gdb/gnulib/import -Ibuild-gnulib/import   -DTUI=1 -D_THREAD_SAFE -I/usr/local/Cellar/guile/2.0.11/include/guile/2.0 -I/usr/local/Cellar/gmp/6.0.0a/include -I/usr/local/Cellar/readline/6.3.5/include -I/usr/local/Cellar/bdw-gc/7.2e/include -I/System/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7 -I/System/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7 -Wall -Wdeclaration-after-statement -Wpointer-arith -Wpointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmi!
>  ssing-protot
>> ypes -Wdeclaration-after-statement -Wempty-body -Wmissing-parameter-type -Wold-style-declaration -Wold-style-definition -Wformat-nonliteral -Werror -c -o i387-tdep.o -MT i387-tdep.o -MMD -MP -MF .deps/i387-tdep.Tpo ../../binutils-gdb/gdb/i387-tdep.c
>>   ../../binutils-gdb/gdb/i387-tdep.c: In function 'i387_supply_fsave':
>>   ../../binutils-gdb/gdb/i387-tdep.c:447:1: error: assuming signed overflow does not occur when assuming that (X + c) >= X is always true [-Werror=strict-overflow]
>>    i387_supply_fsave (struct regcache *regcache, int regnum, const void *fsave)
>>    ^
>>   ../../binutils-gdb/gdb/i387-tdep.c: In function 'i387_collect_fsave':
>>   ../../binutils-gdb/gdb/i387-tdep.c:502:1: error: assuming signed overflow does not occur when assuming that (X + c) >= X is always true [-Werror=strict-overflow]
>>    i387_collect_fsave (const struct regcache *regcache, int regnum, void *fsave)
>>    ^
>>   cc1: all warnings being treated as errors
>>
>> 2014-10-13  Chen Gang  <gang.chen.5i5j@gmail.com>
>>
>> 	* i387-tdep.c (i387_supply_fsave): Avoid warning for
>> 	"-Werror=strict-overflow"
>> ---
>>  gdb/i387-tdep.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
>> index d66ac6a..609b5be 100644
>> --- a/gdb/i387-tdep.c
>> +++ b/gdb/i387-tdep.c
>> @@ -450,11 +450,12 @@ i387_supply_fsave (struct regcache *regcache, int regnum, const void *fsave)
>>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>    const gdb_byte *regs = fsave;
>> -  int i;
>> +  int i, end;
>>  
>>    gdb_assert (tdep->st0_regnum >= I386_ST0_REGNUM);
>>  
>> -  for (i = I387_ST0_REGNUM (tdep); i < I387_XMM0_REGNUM (tdep); i++)
>> +  end = I387_XMM0_REGNUM (tdep); /* -Werror=strict-overflow for (X + c) >= X */
>> +  for (i = I387_ST0_REGNUM (tdep); i < end; i++)
>>      if (regnum == -1 || regnum == i)
>>        {
>>  	if (fsave == NULL)
>> @@ -503,11 +504,12 @@ i387_collect_fsave (const struct regcache *regcache, int regnum, void *fsave)
>>  {
>>    struct gdbarch_tdep *tdep = gdbarch_tdep (get_regcache_arch (regcache));
>>    gdb_byte *regs = fsave;
>> -  int i;
>> +  int i, end;
>>  
>>    gdb_assert (tdep->st0_regnum >= I386_ST0_REGNUM);
>>  
>> -  for (i = I387_ST0_REGNUM (tdep); i < I387_XMM0_REGNUM (tdep); i++)
>> +  end = I387_XMM0_REGNUM (tdep); /* -Werror=strict-overflow for (X + c) >= X */
>> +  for (i = I387_ST0_REGNUM (tdep); i < end; i++)
>>      if (regnum == -1 || regnum == i)
>>        {
>>  	/* Most of the FPU control registers occupy only 16 bits in
>> -- 
>> 1.8.5.2 (Apple Git-48)
>>

-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Mark Kettenis]

> Date: Sun, 12 Oct 2014 21:28:40 +0800
> From: Chen Gang <gang.chen.5i5j@gmail.com>
> 
> gdb requires "-Werror", and I387_ST0_REGNUM (tdep) is 'variable', then
> compiler can think that I387_ST0_REGNUM (tdep) may be a large number,
> which may cause issue, so report warning.
> 
> Need fix this warning, and still keep the code clear enough for readers.
> The related warning under Darwin with gnu built gcc:

Sorry, I disagree with this approach.  I think your compiler is really
being unhelpful here.  See if a newer GCC version of the compiler
still triggers that warning.  If so, complain to the GCC people.

>   gcc -g -O2   -I. -I../../binutils-gdb/gdb -I../../binutils-gdb/gdb/common -I../../binutils-gdb/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../binutils-gdb/gdb/../include/opcode -I../../binutils-gdb/gdb/../opcodes/..  -I../../binutils-gdb/gdb/../readline/.. -I../bfd -I../../binutils-gdb/gdb/../bfd -I../../binutils-gdb/gdb/../include -I../libdecnumber -I../../binutils-gdb/gdb/../libdecnumber -I../../binutils-gdb/gdb/gnulib/import -Ibuild-gnulib/import   -DTUI=1 -D_THREAD_SAFE -I/usr/local/Cellar/guile/2.0.11/include/guile/2.0 -I/usr/local/Cellar/gmp/6.0.0a/include -I/usr/local/Cellar/readline/6.3.5/include -I/usr/local/Cellar/bdw-gc/7.2e/include -I/System/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7 -I/System/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7 -Wall -Wdeclaration-after-statement -Wpointer-arith -Wpointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmi!
 ssing-protot
> ypes -Wdeclaration-after-statement -Wempty-body -Wmissing-parameter-type -Wold-style-declaration -Wold-style-definition -Wformat-nonliteral -Werror -c -o i387-tdep.o -MT i387-tdep.o -MMD -MP -MF .deps/i387-tdep.Tpo ../../binutils-gdb/gdb/i387-tdep.c
>   ../../binutils-gdb/gdb/i387-tdep.c: In function 'i387_supply_fsave':
>   ../../binutils-gdb/gdb/i387-tdep.c:447:1: error: assuming signed overflow does not occur when assuming that (X + c) >= X is always true [-Werror=strict-overflow]
>    i387_supply_fsave (struct regcache *regcache, int regnum, const void *fsave)
>    ^
>   ../../binutils-gdb/gdb/i387-tdep.c: In function 'i387_collect_fsave':
>   ../../binutils-gdb/gdb/i387-tdep.c:502:1: error: assuming signed overflow does not occur when assuming that (X + c) >= X is always true [-Werror=strict-overflow]
>    i387_collect_fsave (const struct regcache *regcache, int regnum, void *fsave)
>    ^
>   cc1: all warnings being treated as errors
> 
> 2014-10-13  Chen Gang  <gang.chen.5i5j@gmail.com>
> 
> 	* i387-tdep.c (i387_supply_fsave): Avoid warning for
> 	"-Werror=strict-overflow"
> ---
>  gdb/i387-tdep.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
> index d66ac6a..609b5be 100644
> --- a/gdb/i387-tdep.c
> +++ b/gdb/i387-tdep.c
> @@ -450,11 +450,12 @@ i387_supply_fsave (struct regcache *regcache, int regnum, const void *fsave)
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    const gdb_byte *regs = fsave;
> -  int i;
> +  int i, end;
>  
>    gdb_assert (tdep->st0_regnum >= I386_ST0_REGNUM);
>  
> -  for (i = I387_ST0_REGNUM (tdep); i < I387_XMM0_REGNUM (tdep); i++)
> +  end = I387_XMM0_REGNUM (tdep); /* -Werror=strict-overflow for (X + c) >= X */
> +  for (i = I387_ST0_REGNUM (tdep); i < end; i++)
>      if (regnum == -1 || regnum == i)
>        {
>  	if (fsave == NULL)
> @@ -503,11 +504,12 @@ i387_collect_fsave (const struct regcache *regcache, int regnum, void *fsave)
>  {
>    struct gdbarch_tdep *tdep = gdbarch_tdep (get_regcache_arch (regcache));
>    gdb_byte *regs = fsave;
> -  int i;
> +  int i, end;
>  
>    gdb_assert (tdep->st0_regnum >= I386_ST0_REGNUM);
>  
> -  for (i = I387_ST0_REGNUM (tdep); i < I387_XMM0_REGNUM (tdep); i++)
> +  end = I387_XMM0_REGNUM (tdep); /* -Werror=strict-overflow for (X + c) >= X */
> +  for (i = I387_ST0_REGNUM (tdep); i < end; i++)
>      if (regnum == -1 || regnum == i)
>        {
>  	/* Most of the FPU control registers occupy only 16 bits in
> -- 
> 1.8.5.2 (Apple Git-48)
> 

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Iain Buclaw]

On 12 October 2014 15:47, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
> On 10/12/14 22:13, Iain Buclaw wrote:
>> On 12 October 2014 14:28, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
>>> gdb requires "-Werror", and I387_ST0_REGNUM (tdep) is 'variable', then
>>> compiler can think that I387_ST0_REGNUM (tdep) may be a large number,
>>> which may cause issue, so report warning.
>>>
>>> Need fix this warning, and still keep the code clear enough for readers.
>>> The related warning under Darwin with gnu built gcc:
>>>
>>
>> I had noted the same on GCC 5.0.0 development, found that the line
>> number in the warning was wrong and raised a bug
>> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63510), just didn't get
>> round to submitting a patch for gdb.
>>
>
> But for me, what compiler has done is correct: "-Werror=strict-overflow"
> need include "(X + c) >= X" for signed overflow. And our case matches
> this case:
>

The compiler has done right, but that still doesn't stop the reported
line number being wrong.

-- Iain.

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Chen Gang]

On 10/12/14 22:13, Iain Buclaw wrote:
> On 12 October 2014 14:28, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
>> gdb requires "-Werror", and I387_ST0_REGNUM (tdep) is 'variable', then
>> compiler can think that I387_ST0_REGNUM (tdep) may be a large number,
>> which may cause issue, so report warning.
>>
>> Need fix this warning, and still keep the code clear enough for readers.
>> The related warning under Darwin with gnu built gcc:
>>
> 
> I had noted the same on GCC 5.0.0 development, found that the line
> number in the warning was wrong and raised a bug
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63510), just didn't get
> round to submitting a patch for gdb.
> 

But for me, what compiler has done is correct: "-Werror=strict-overflow"
need include "(X + c) >= X" for signed overflow. And our case matches
this case:

  int st0_regnum;
  
  void i387_supply_fsave ()
  {
    int i;
    for (i = st0_regnum; i < st0_regnum + 16;)
      {
      }
  }

> 
>>
>> 2014-10-13  Chen Gang  <gang.chen.5i5j@gmail.com>
>>
>>         * i387-tdep.c (i387_supply_fsave): Avoid warning for
>>         "-Werror=strict-overflow"
>> ---
>>  gdb/i387-tdep.c | 10 ++++++----
>>  1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
>> index d66ac6a..609b5be 100644
>> --- a/gdb/i387-tdep.c
>> +++ b/gdb/i387-tdep.c
>> @@ -450,11 +450,12 @@ i387_supply_fsave (struct regcache *regcache, int regnum, const void *fsave)
>>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>    const gdb_byte *regs = fsave;
>> -  int i;
>> +  int i, end;
>>
> 
> Also changing from int -> unsigned int fixes the warning.
> 
> -- Iain.
> 

-- 
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Re: [PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Iain Buclaw]

On 12 October 2014 14:28, Chen Gang <gang.chen.5i5j@gmail.com> wrote:
> gdb requires "-Werror", and I387_ST0_REGNUM (tdep) is 'variable', then
> compiler can think that I387_ST0_REGNUM (tdep) may be a large number,
> which may cause issue, so report warning.
>
> Need fix this warning, and still keep the code clear enough for readers.
> The related warning under Darwin with gnu built gcc:
>

I had noted the same on GCC 5.0.0 development, found that the line
number in the warning was wrong and raised a bug
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63510), just didn't get
round to submitting a patch for gdb.


>
> 2014-10-13  Chen Gang  <gang.chen.5i5j@gmail.com>
>
>         * i387-tdep.c (i387_supply_fsave): Avoid warning for
>         "-Werror=strict-overflow"
> ---
>  gdb/i387-tdep.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
> index d66ac6a..609b5be 100644
> --- a/gdb/i387-tdep.c
> +++ b/gdb/i387-tdep.c
> @@ -450,11 +450,12 @@ i387_supply_fsave (struct regcache *regcache, int regnum, const void *fsave)
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    const gdb_byte *regs = fsave;
> -  int i;
> +  int i, end;
>

Also changing from int -> unsigned int fixes the warning.

-- Iain.

[PATCH v2] gdb/i387-tdep.c: Avoid warning for "-Werror=strict-overflow"

[Chen Gang]

gdb requires "-Werror", and I387_ST0_REGNUM (tdep) is 'variable', then
compiler can think that I387_ST0_REGNUM (tdep) may be a large number,
which may cause issue, so report warning.

Need fix this warning, and still keep the code clear enough for readers.
The related warning under Darwin with gnu built gcc:

  gcc -g -O2   -I. -I../../binutils-gdb/gdb -I../../binutils-gdb/gdb/common -I../../binutils-gdb/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../binutils-gdb/gdb/../include/opcode -I../../binutils-gdb/gdb/../opcodes/..  -I../../binutils-gdb/gdb/../readline/.. -I../bfd -I../../binutils-gdb/gdb/../bfd -I../../binutils-gdb/gdb/../include -I../libdecnumber -I../../binutils-gdb/gdb/../libdecnumber -I../../binutils-gdb/gdb/gnulib/import -Ibuild-gnulib/import   -DTUI=1 -D_THREAD_SAFE -I/usr/local/Cellar/guile/2.0.11/include/guile/2.0 -I/usr/local/Cellar/gmp/6.0.0a/include -I/usr/local/Cellar/readline/6.3.5/include -I/usr/local/Cellar/bdw-gc/7.2e/include -I/System/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7 -I/System/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7 -Wall -Wdeclaration-after-statement -Wpointer-arith -Wpointer-sign -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wmissing-protot
ypes -Wdeclaration-after-statement -Wempty-body -Wmissing-parameter-type -Wold-style-declaration -Wold-style-definition -Wformat-nonliteral -Werror -c -o i387-tdep.o -MT i387-tdep.o -MMD -MP -MF .deps/i387-tdep.Tpo ../../binutils-gdb/gdb/i387-tdep.c
  ../../binutils-gdb/gdb/i387-tdep.c: In function 'i387_supply_fsave':
  ../../binutils-gdb/gdb/i387-tdep.c:447:1: error: assuming signed overflow does not occur when assuming that (X + c) >= X is always true [-Werror=strict-overflow]
   i387_supply_fsave (struct regcache *regcache, int regnum, const void *fsave)
   ^
  ../../binutils-gdb/gdb/i387-tdep.c: In function 'i387_collect_fsave':
  ../../binutils-gdb/gdb/i387-tdep.c:502:1: error: assuming signed overflow does not occur when assuming that (X + c) >= X is always true [-Werror=strict-overflow]
   i387_collect_fsave (const struct regcache *regcache, int regnum, void *fsave)
   ^
  cc1: all warnings being treated as errors

2014-10-13  Chen Gang  <gang.chen.5i5j@gmail.com>

	* i387-tdep.c (i387_supply_fsave): Avoid warning for
	"-Werror=strict-overflow"
---
 gdb/i387-tdep.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index d66ac6a..609b5be 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -450,11 +450,12 @@ i387_supply_fsave (struct regcache *regcache, int regnum, const void *fsave)
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   const gdb_byte *regs = fsave;
-  int i;
+  int i, end;
 
   gdb_assert (tdep->st0_regnum >= I386_ST0_REGNUM);
 
-  for (i = I387_ST0_REGNUM (tdep); i < I387_XMM0_REGNUM (tdep); i++)
+  end = I387_XMM0_REGNUM (tdep); /* -Werror=strict-overflow for (X + c) >= X */
+  for (i = I387_ST0_REGNUM (tdep); i < end; i++)
     if (regnum == -1 || regnum == i)
       {
 	if (fsave == NULL)
@@ -503,11 +504,12 @@ i387_collect_fsave (const struct regcache *regcache, int regnum, void *fsave)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (get_regcache_arch (regcache));
   gdb_byte *regs = fsave;
-  int i;
+  int i, end;
 
   gdb_assert (tdep->st0_regnum >= I386_ST0_REGNUM);
 
-  for (i = I387_ST0_REGNUM (tdep); i < I387_XMM0_REGNUM (tdep); i++)
+  end = I387_XMM0_REGNUM (tdep); /* -Werror=strict-overflow for (X + c) >= X */
+  for (i = I387_ST0_REGNUM (tdep); i < end; i++)
     if (regnum == -1 || regnum == i)
       {
 	/* Most of the FPU control registers occupy only 16 bits in
-- 
1.8.5.2 (Apple Git-48)