Re: assuming signed overflow does not occur
On 09/04/17 08:54, Manuel López-Ibáñez wrote: > I wrote an explanation of the current status of Wstrict-overflow to the > best of my knowledge: > https://gcc.gnu.org/wiki/VerboseDiagnostics#Wstrict_overflow > > I didn't mention GIMPLE because it is often the case that the root of > the problem is not obvious in gimple unless one also debugs the compiler > at the same time. > > I hope it is useful! Very much so, thank you!
Re: assuming signed overflow does not occur
On 03/09/17 23:00, Bruce Korb wrote: RFE's are for this list: please improve the message. The message does not have to be a dissertation, but messages nowadays can certainly include URL's to direct people to reasonable places. I'd suggest something like: gcc.gnu.org/gcc-messages/xxx WRT looking at a GIMPLE dump, I'd first have to learn what GIMPLE is, then learn how to decipher one, then figure out how to get it out of the compiler and finally plod through it. Guess what? I won't be doing that. I'll squish the warning first. :( Effective messages are very important. I wrote an explanation of the current status of Wstrict-overflow to the best of my knowledge: https://gcc.gnu.org/wiki/VerboseDiagnostics#Wstrict_overflow I didn't mention GIMPLE because it is often the case that the root of the problem is not obvious in gimple unless one also debugs the compiler at the same time. I hope it is useful! Cheers, Manuel.
Re: assuming signed overflow does not occur
Hi,
On Sun, Sep 3, 2017 at 1:48 PM, Florian Weimer wrote:
> * Bruce Korb:
>
>> I know about all these theoretical possibilities of numbers behaving
>> in strange ways when arithmetic optimizations assume that signed
>> overflow won't occur when they actually might. Yep, it creates subtle
>> bugs. The warning is worthwhile. Still and all:
>>
>> 485 tvdiff = abs_tval(sub_tval(timetv, tvlast));
>> 486 if (tvdiff.tv_sec != 0) {
>>
>> systime.c: In function 'step_systime':
>> systime.c:486:5: warning: assuming signed overflow does not occur when
>> simplifying conditional to constant [-Wstrict-overflow]
>>
>> What possible optimization might be going on to cause an overflow
>> problem when I simply want to know if the "tv_sec" field is zero or
>> not? (BTW, in current source, "tvdiff" is a structure that is returned
>> by abs_tval())
>
> This usually happens after inlining and other optimizations. You'll
> have to look at GIMPLE dumps to figure out what is going on.
RFE's are for this list: please improve the message.
The message does not have to be a dissertation, but messages
nowadays can certainly include URL's to direct people to
reasonable places. I'd suggest something like:
gcc.gnu.org/gcc-messages/xxx
WRT looking at a GIMPLE dump, I'd first have to learn what GIMPLE is,
then learn how to decipher one, then figure out how to get it out of
the compiler and
finally plod through it. Guess what? I won't be doing that. I'll
squish the warning first. :( Effective messages are very important.
Re: assuming signed overflow does not occur
* Bruce Korb:
> I know about all these theoretical possibilities of numbers behaving
> in strange ways when arithmetic optimizations assume that signed
> overflow won't occur when they actually might. Yep, it creates subtle
> bugs. The warning is worthwhile. Still and all:
>
> 485 tvdiff = abs_tval(sub_tval(timetv, tvlast));
> 486 if (tvdiff.tv_sec != 0) {
>
> systime.c: In function 'step_systime':
> systime.c:486:5: warning: assuming signed overflow does not occur when
> simplifying conditional to constant [-Wstrict-overflow]
>
> What possible optimization might be going on to cause an overflow
> problem when I simply want to know if the "tv_sec" field is zero or
> not? (BTW, in current source, "tvdiff" is a structure that is returned
> by abs_tval())
This usually happens after inlining and other optimizations. You'll
have to look at GIMPLE dumps to figure out what is going on.
(This is more a question for the gcc-help list.)
Re: assuming signed overflow does not occur
Per request, the inlined functions
On Sat, Sep 2, 2017 at 12:59 PM, Bruce Korb wrote:
> I know about all these theoretical possibilities of numbers behaving
> in strange ways when arithmetic optimizations assume that signed
> overflow won't occur when they actually might. Yep, it creates subtle
> bugs. The warning is worthwhile. Still and all:
>
> 485 tvdiff = abs_tval(sub_tval(timetv, tvlast));
> 486 if (tvdiff.tv_sec != 0) {
>
> systime.c: In function 'step_systime':
> systime.c:486:5: warning: assuming signed overflow does not occur when
> simplifying conditional to constant [-Wstrict-overflow]
>
> What possible optimization might be going on to cause an overflow
> problem when I simply want to know if the "tv_sec" field is zero or
> not? (BTW, in current source, "tvdiff" is a structure that is returned
> by abs_tval())
>
> $ gcc --version
> gcc (SUSE Linux) 4.8.5
> Copyright (C) 2015 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
/* make sure microseconds are in nominal range */
static inline struct timeval
normalize_tval(
struct timeval x
)
{
longz;
if (x.tv_usec < -3l * MICROSECONDS ||
x.tv_usec > 3l * MICROSECONDS ) {
z = x.tv_usec / MICROSECONDS;
x.tv_usec -= z * MICROSECONDS;
x.tv_sec += z;
}
if (x.tv_usec < 0)
do {
x.tv_usec += MICROSECONDS;
x.tv_sec--;
} while (x.tv_usec < 0);
else if (x.tv_usec >= MICROSECONDS)
do {
x.tv_usec -= MICROSECONDS;
x.tv_sec++;
} while (x.tv_usec >= MICROSECONDS);
return x;
}
/* x = a - b */
static inline struct timeval
sub_tval(
struct timeval a,
struct timeval b
)
{
struct timeval x;
x = a;
x.tv_sec -= b.tv_sec;
x.tv_usec -= b.tv_usec;
return normalize_tval(x);
}
/* x = abs(a) */
static inline struct timeval
abs_tval(
struct timeval a
)
{
struct timeval c;
c = normalize_tval(a);
if (c.tv_sec < 0) {
if (c.tv_usec != 0) {
c.tv_sec = -c.tv_sec - 1;
c.tv_usec = MICROSECONDS - c.tv_usec;
} else {
c.tv_sec = -c.tv_sec;
}
}
return c;
}
And the larger code fragment:
/* get the current time as l_fp (without fuzz) and as struct timeval */
get_ostime(&timets);
fp_sys = tspec_stamp_to_lfp(timets);
tvlast.tv_sec = timets.tv_sec;
tvlast.tv_usec = (timets.tv_nsec + 500) / 1000;
/* get the target time as l_fp */
L_ADD(&fp_sys, &fp_ofs);
/* unfold the new system time */
timetv = lfp_stamp_to_tval(fp_sys, &pivot);
/* now set new system time */
if (ntp_set_tod(&timetv, NULL) != 0) {
msyslog(LOG_ERR, "step-systime: %m");
if (enable_panic_check && allow_panic) {
msyslog(LOG_ERR, "step_systime: allow_panic is TRUE!");
}
return FALSE;
}
/* <--- time-critical path ended with 'ntp_set_tod()' <--- */
sys_residual = 0;
lamport_violated = (step < 0);
if (step_callback)
(*step_callback)();
tvdiff = abs_tval(sub_tval(timetv, tvlast));
if (tvdiff.tv_sec != 0) {
assuming signed overflow does not occur
I know about all these theoretical possibilities of numbers behaving
in strange ways when arithmetic optimizations assume that signed
overflow won't occur when they actually might. Yep, it creates subtle
bugs. The warning is worthwhile. Still and all:
485 tvdiff = abs_tval(sub_tval(timetv, tvlast));
486 if (tvdiff.tv_sec != 0) {
systime.c: In function 'step_systime':
systime.c:486:5: warning: assuming signed overflow does not occur when
simplifying conditional to constant [-Wstrict-overflow]
What possible optimization might be going on to cause an overflow
problem when I simply want to know if the "tv_sec" field is zero or
not? (BTW, in current source, "tvdiff" is a structure that is returned
by abs_tval())
$ gcc --version
gcc (SUSE Linux) 4.8.5
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Re: BUG: assuming signed overflow does not occur when simplifying conditional to constant
(Tarball attachment (75K) stripped.) On 12/29/12 10:56, Florian Weimer wrote: >> Not easily. git clone git://git.savannah.gnu.org/autogen.git > > Uhm, I get: > > configure.ac:30: error: AC_INIT should be called with package and version > arguments I ought to have directed you to a pre-release tarball. Sorry. The GIT source needs to be bootstrapped. > libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I../../autoopts \ > -I.. -I.. -I../.. -I../autoopts -I../../autoopts \ > -DPKGDATADIR=\"/u/bkorb/ag/ag/autogen-5.17.1pre3/_inst/share/autogen\" \ > -g -O2 -Wall -Werror -Wcast-align -Wmissing-prototypes -Wpointer-arith \ > -Wshadow -Wstrict-prototypes -Wwrite-strings -Wno-format-contains-nul \ > -fno-strict-aliasing -Wstrict-aliasing=2 -Wextra -Wconversion \ > -Wsign-conversion -Wstrict-overflow -MT libopts_la-libopts.lo \ > -MD -MP -MF .deps/libopts_la-libopts.Tpo \ > -c libopts.c -fPIC -DPIC \ > -o .libs/libopts_la-libopts.o > In file included from libopts.c:22:0: > ../../autoopts/configfile.c: In function 'intern_file_load': > ../../autoopts/configfile.c:1025:12: error: assuming signed overflow does not > occur when simplifying conditional to constant [-Werror=strict-overflow] > cc1: all warnings being treated as errors > make[4]: *** [libopts_la-libopts.lo] Error 1 > make[4]: Leaving directory `/u/bkorb/ag/ag/autogen-5.17.1pre3/_build/autoopts' > make[3]: *** [all-recursive] Error 1 > make[3]: Leaving directory `/u/bkorb/ag/ag/autogen-5.17.1pre3/_build/autoopts' >> I'd have to prune code until I can't prune more. > > Preprocessed sources would be helpful. Interesting. Capturing the above command line, preprocessing and then compiling yields different messages: > libopts.i: In function 'gnu_dev_major': > libopts.i:137:3: error: negative integer implicitly converted to unsigned > type [-Werror=sign-conversion] > libopts.i:137:33: error: conversion to 'unsigned int' from 'long long > unsigned int' may alter its value [-Werror=conversion] > libopts.i: In function 'gnu_dev_minor': > libopts.i:142:3: error: negative integer implicitly converted to unsigned > type [-Werror=sign-conversion] > libopts.i:142:25: error: conversion to 'unsigned int' from 'long long > unsigned int' may alter its value [-Werror=conversion] > libopts.i: In function 'gnu_dev_makedev': > libopts.i:148:4: error: negative integer implicitly converted to unsigned > type [-Werror=sign-conversion] > libopts.i:149:4: error: negative integer implicitly converted to unsigned > type [-Werror=sign-conversion] > libopts.i: In function '__sigismember': > libopts.i:424:174: error: conversion to 'long unsigned int' from 'int' may > change the sign of the result [-Werror=sign-conversion] > libopts.i:424:254: error: conversion to 'long unsigned int' from 'int' may > change the sign of the result [-Werror=sign-conversion] > libopts.i: In function '__sigaddset': > libopts.i:425:165: error: conversion to 'long unsigned int' from 'int' may > change the sign of the result [-Werror=sign-conversion] > libopts.i:425:245: error: conversion to 'long unsigned int' from 'int' may > change the sign of the result [-Werror=sign-conversion] > libopts.i: In function '__sigdelset': > libopts.i:426:165: error: conversion to 'long unsigned int' from 'int' may > change the sign of the result [-Werror=sign-conversion] > libopts.i:426:245: error: conversion to 'long unsigned int' from 'int' may > change the sign of the result [-Werror=sign-conversion] > libopts.i: In function 'fputc_unlocked': > libopts.i:1253:3: error: conversion to 'char' from 'int' may alter its value > [-Werror=conversion] > libopts.i: In function 'putc_unlocked': > libopts.i:1258:3: error: conversion to 'char' from 'int' may alter its value > [-Werror=conversion] > libopts.i: In function 'putchar_unlocked': > libopts.i:1263:3: error: conversion to 'char' from 'int' may alter its value > [-Werror=conversion] > cc1: all warnings being treated as error I stripped away the line information. Ah. Of course. GCC no longer knows what is derived from system headers. :) configfile.c:1025 maps to libopts.i:5416 and the compiler doesn't get that far. $ gcc --version gcc (SUSE Linux) 4.7.1 20120723 [gcc-4_7-branch revision 189773] Copyright (C) 2012 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for
Re: BUG: assuming signed overflow does not occur when simplifying conditional to constant
* Bruce Korb: > Hi Florian, > > On Sat, Dec 29, 2012 at 2:38 AM, Florian Weimer wrote: >>> ../../autoopts/configfile.c: In function 'intern_file_load': >>> ../../autoopts/configfile.c:1025:12: error: assuming signed overflow does >>> not occur \ >>> when simplifying conditional to constant [-Werror=strict-overflow] >> >> I can't reproduce this. Can you post a minimal example that actually >> shows this warning? > > Not easily. git clone git://git.savannah.gnu.org/autogen.git Uhm, I get: configure.ac:30: error: AC_INIT should be called with package and version arguments > I'd have to prune code until I can't prune more. Preprocessed sources would be helpful. It seems that the header file I need is generated and is not valid C99 without suitable preprocessor definitions, so I can't get to this point myself.
Re: BUG: assuming signed overflow does not occur when simplifying conditional to constant
Hi Florian, On Sat, Dec 29, 2012 at 2:38 AM, Florian Weimer wrote: >> ../../autoopts/configfile.c: In function 'intern_file_load': >> ../../autoopts/configfile.c:1025:12: error: assuming signed overflow does >> not occur \ >> when simplifying conditional to constant [-Werror=strict-overflow] > > I can't reproduce this. Can you post a minimal example that actually > shows this warning? Not easily. git clone git://git.savannah.gnu.org/autogen.git I'd have to prune code until I can't prune more. Unfortunately, I don't have that kind of time. I do wish I did. >> My guess is that some code somewhere presumes that "idx" never gets >> decremented. Not true. > > The warning can trigger after inlining and constant propogation, > somewhat obscuring its cause. PR55616 is one such example. It was clear it was some sort of optimization triggered issue. I'd guess that since the program would not function if the condition expression were optimized into a constant, then, therefore, the warning trigger was faulty. The message itself was also obtuse. Thank you for taking the time to think over the issue. Regards, Bruce
Re: BUG: assuming signed overflow does not occur when simplifying conditional to constant
* Bruce Korb:
> I wrote a loop that figures out how many items are in a list,
> counts down from that count to -1, changes direction and counts
> up from 0 to the limit, a la:
>
>
> inc = -1;
> int idx = 0;
> while (opts->papzHomeList[idx+1] != NULL)
> idx++;
> for (;;) {
> if (idx < 0) { <<<=== line 1025
> inc = 1;
> idx = 0;
> }
> char const * path = opts->papzHomeList[ idx ];
> if (path == NULL)
> break;
> idx += inc;
> // do a bunch of stuff
> }
>
> ../../autoopts/configfile.c: In function 'intern_file_load':
> ../../autoopts/configfile.c:1025:12: error: assuming signed overflow does not
> occur \
> when simplifying conditional to constant [-Werror=strict-overflow]
I can't reproduce this. Can you post a minimal example that actually
shows this warning?
> My guess is that some code somewhere presumes that "idx" never gets
> decremented. Not true.
The warning can trigger after inlining and constant propogation,
somewhat obscuring its cause. PR55616 is one such example.
BUG: assuming signed overflow does not occur when simplifying conditional to constant
I wrote a loop that figures out how many items are in a list,
counts down from that count to -1, changes direction and counts
up from 0 to the limit, a la:
inc = -1;
int idx = 0;
while (opts->papzHomeList[idx+1] != NULL)
idx++;
for (;;) {
if (idx < 0) { <<<=== line 1025
inc = 1;
idx = 0;
}
char const * path = opts->papzHomeList[ idx ];
if (path == NULL)
break;
idx += inc;
// do a bunch of stuff
}
../../autoopts/configfile.c: In function 'intern_file_load':
../../autoopts/configfile.c:1025:12: error: assuming signed overflow does not
occur \
when simplifying conditional to constant [-Werror=strict-overflow]
I do not think GCC should be "simplifying" the expression "idx < 0" to a
constant.
The number of entries in "papzHomeList" is known to be less than INT_MAX (64K
actually),
but not by the code in question. My guess is that some code somewhere presumes
that "idx" never gets decremented. Not true.
