GCC turns &~ into | due to undefined bit-shift without warning

classic Classic list List threaded Threaded
34 messages Options
12
Reply | Threaded
Open this post in threaded view
|

GCC turns &~ into | due to undefined bit-shift without warning

Moritz Strübe
Hey,

I have the following code:

#include <stdint.h>

void LL_ADC_SetChannelSingleDiff(uint32_t * val, uint32_t Channel,
uint32_t SingleDiff)
{
     *val = (*val & (~(Channel & 0x7FFFFU))) | ((Channel & 0x7FFFFU ) &
(0x7FFFFU << (SingleDiff & 0x20U)));
}

void test(uint32_t * testvar) {
     LL_ADC_SetChannelSingleDiff(testvar, 0x2 ,0x7FU );
}

Starting with gcc 6 and -O2 this code produces an or-instruction instead
of an and-not-instruction:

https://godbolt.org/z/kGtBfW

x86-64 -O1:
LL_ADC_SetChannelSingleDiff:
         and     esi, 524287
         or      DWORD PTR [rdi], esi
         ret
test:
         and     DWORD PTR [rdi], -3
         ret

x86-64 -O1:
LL_ADC_SetChannelSingleDiff:
         and     esi, 524287
         or      DWORD PTR [rdi], esi
         ret
test:
         or      DWORD PTR [rdi], 2
         ret




Considering that C11 6.5.7#3 ("If  the  value  of  the  right operand 
is  negative  or  is greater than or equal to the width of the promoted
left operand, the behavior is undefined.") is not very widely known, as
it "normally" just works, inverting the intent is quite unexpected.

Is there any option that would have helped me with this?

Should this be a bug? I know, from the C standard point of view this is
ok, but inverting the behavior without warning is really bad in terms of
user experience.

Clang does the same, but IMO that does not make things any better.

Cheers
Morty


--
Redheads Ltd. Softwaredienstleistungen
Schillerstr. 14
90409 Nürnberg

Telefon: +49 (0)911 180778-50
E-Mail: [hidden email] | Web: www.redheads.de

Geschäftsführer: Andreas Hanke
Sitz der Gesellschaft: Lauf
Amtsgericht Nürnberg HRB 22681
Ust-ID: DE 249436843

Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Jakub Jelinek
On Mon, Mar 11, 2019 at 08:49:30AM +0000, Moritz Strübe wrote:
> Considering that C11 6.5.7#3 ("If  the  value  of  the  right operand 
> is  negative  or  is greater than or equal to the width of the promoted
> left operand, the behavior is undefined.") is not very widely known, as
> it "normally" just works, inverting the intent is quite unexpected.
>
> Is there any option that would have helped me with this?

You could build with -fsanitize=undefined, that would tell you at runtime you
have undefined behavior in your code (if the SingleDiff has bit ever 0x20
set).

The fact that negative or >= bit precision shifts are UB is widely known,
and even if it wouldn't, for the compiler all the UBs are just UBs, the
compiler optimizes on the assumption that UB does not happen, so when it
sees 32-bit int << (x & 32), it can assume x must be 0 at that point,
anything else is UB.

GCC has warnings for the simple cases, where one uses negative or too large
constant shift, warning in cases like you have would be a false positive for
many people, there is nothing wrong with that (if x & 32 always results in
0).

        Jakub
Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Moritz Strübe
On 11.03.2019 at 10:14 Jakub Jelinek wrote:
> You could build with -fsanitize=undefined, that would tell you at runtime you
> have undefined behavior in your code (if the SingleDiff has bit ever 0x20
> set).

Yes, that helps. Unfortunately I'm on an embedded system, thus the code
size increase is just too big.

> The fact that negative or >= bit precision shifts are UB is widely known,
> and even if it wouldn't, for the compiler all the UBs are just UBs, the
> compiler optimizes on the assumption that UB does not happen, so when it
> sees 32-bit int << (x & 32), it can assume x must be 0 at that point,
> anything else is UB.

Thanks for that explanation. None the less, a compile time warning would
be nice. Especially as I this was caused by a library provided by ST. :(
Seems like we really need to add more sophisticated static analysis to
our CI.

Morty


--
Redheads Ltd. Softwaredienstleistungen
Schillerstr. 14
90409 Nürnberg

Telefon: +49 (0)911 180778-50
E-Mail: [hidden email] | Web: www.redheads.de

Geschäftsführer: Andreas Hanke
Sitz der Gesellschaft: Lauf
Amtsgericht Nürnberg HRB 22681
Ust-ID: DE 249436843

Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Jakub Jelinek
On Mon, Mar 11, 2019 at 11:06:37AM +0000, Moritz Strübe wrote:
> On 11.03.2019 at 10:14 Jakub Jelinek wrote:
> > You could build with -fsanitize=undefined, that would tell you at runtime you
> > have undefined behavior in your code (if the SingleDiff has bit ever 0x20
> > set).
>
> Yes, that helps. Unfortunately I'm on an embedded system, thus the code
> size increase is just too big.

You can -fsanitize-undefined-trap-on-error, which doesn't increase size too
much, it is less user-friendly, but still should catch the UB.

> > The fact that negative or >= bit precision shifts are UB is widely known,
> > and even if it wouldn't, for the compiler all the UBs are just UBs, the
> > compiler optimizes on the assumption that UB does not happen, so when it
> > sees 32-bit int << (x & 32), it can assume x must be 0 at that point,
> > anything else is UB.
>
> Thanks for that explanation. None the less, a compile time warning would
> be nice. Especially as I this was caused by a library provided by ST. :(
> Seems like we really need to add more sophisticated static analysis to
> our CI.

What you think the code would do for int << 32?  The probable reason why it
is UB in the standard is that each CPU handles that differently, on some
shift left by large count results in 0, on others the shift count is modulo
the bitsize, on yet others the shift count is also masked, but e.g. by
wordsize even when the shifted count is smaller (say int << 32 is 0, but int
<< 64 is like int << 0).

A warning is a bad idea generally, we'd need to warn for all cases where the
shift count is not compile time constant, all of those could be out of
bounds in theory.

        Jakub
Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Vincent Lefevre-3
In reply to this post by Moritz Strübe
On 2019-03-11 11:06:37 +0000, Moritz Strübe wrote:
> On 11.03.2019 at 10:14 Jakub Jelinek wrote:
> > The fact that negative or >= bit precision shifts are UB is widely known,
[...]

And even in the case where the compiler maps the shift directly to
the asm shift (without optimizations), the behavior may depend on
the processor.

> Thanks for that explanation. None the less, a compile time warning
> would be nice.

It already does by default:

       -Wshift-count-negative
           Warn if shift count is negative. This warning is enabled
           by default.

       -Wshift-count-overflow
           Warn if shift count >= width of type. This warning is
           enabled by default.

Of course, if the compiler cannot guess that there will be such
an issue, it will not emit the warning. You certainly don't want
a warning for each non-trivial shift just because the compiler
cannot know whether the constraint on the shift count will be
satisfied.

--
Vincent Lefèvre <[hidden email]> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

David Brown-4
On 11/03/2019 12:24, Vincent Lefevre wrote:

> On 2019-03-11 11:06:37 +0000, Moritz Strübe wrote:
>> On 11.03.2019 at 10:14 Jakub Jelinek wrote:
>>> The fact that negative or >= bit precision shifts are UB is widely known,
> [...]
>
> And even in the case where the compiler maps the shift directly to
> the asm shift (without optimizations), the behavior may depend on
> the processor.
>
>> Thanks for that explanation. None the less, a compile time warning
>> would be nice.
>
> It already does by default:
>
>        -Wshift-count-negative
>            Warn if shift count is negative. This warning is enabled
>            by default.
>
>        -Wshift-count-overflow
>            Warn if shift count >= width of type. This warning is
>            enabled by default.
>
> Of course, if the compiler cannot guess that there will be such
> an issue, it will not emit the warning. You certainly don't want
> a warning for each non-trivial shift just because the compiler
> cannot know whether the constraint on the shift count will be
> satisfied.
>

While the compiler clearly can't give a warning on calculated shifts
without massive amounts of false positives, it is able to give warnings
when there is a shift by a compile-time known constant value that is
invalid.  In the case of the OP's test function, inlining and constant
propagation means that the shift value /is/ known to the compiler - it
uses it for optimisation (in this case, it uses the undefined behaviour
to "simplify" the calculations).

Am I right in thinking that this is because the pass that checks the
shift sizes for warnings here comes before the relevant inlining or
constant propagation passes?  And if so, would it be possible to change
the ordering here?

Perhaps it would be possible to have a warning for when the compiler
optimises based on undefined behaviour, when the undefined behaviour
comes from values that are known to the compiler at compile time?  (When
the values are not known at compile time, optimising on the assumption
that the undefined behaviour doesn't happen is fair enough, and you
can't warn there without lots of false positives.)

Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Vincent Lefevre-3
On 2019-03-11 13:51:21 +0100, David Brown wrote:

> On 11/03/2019 12:24, Vincent Lefevre wrote:
> > It already does by default:
> >
> >        -Wshift-count-negative
> >            Warn if shift count is negative. This warning is enabled
> >            by default.
> >
> >        -Wshift-count-overflow
> >            Warn if shift count >= width of type. This warning is
> >            enabled by default.
> >
> > Of course, if the compiler cannot guess that there will be such
> > an issue, it will not emit the warning. You certainly don't want
> > a warning for each non-trivial shift just because the compiler
> > cannot know whether the constraint on the shift count will be
> > satisfied.
>
> While the compiler clearly can't give a warning on calculated shifts
> without massive amounts of false positives, it is able to give warnings
> when there is a shift by a compile-time known constant value that is
> invalid.  In the case of the OP's test function, inlining and constant
> propagation means that the shift value /is/ known to the compiler - it
> uses it for optimisation (in this case, it uses the undefined behaviour
> to "simplify" the calculations).
>
> Am I right in thinking that this is because the pass that checks the
> shift sizes for warnings here comes before the relevant inlining or
> constant propagation passes?  And if so, would it be possible to change
> the ordering here?

To generate a warning, the compiler would also have to make sure
that the inlined code with an out-of-range shift is actually not
dead code. In practice, it may happen that constraints are not
satisfied on some platforms, but the reason is that on such
platforms the code will never be executed (this is code written
to take care of other platforms).

--
Vincent Lefèvre <[hidden email]> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

David Brown-40
On 12/03/2019 16:40, Vincent Lefevre wrote:

> On 2019-03-11 13:51:21 +0100, David Brown wrote:
>> On 11/03/2019 12:24, Vincent Lefevre wrote:
>>> It already does by default:
>>>
>>>         -Wshift-count-negative
>>>             Warn if shift count is negative. This warning is enabled
>>>             by default.
>>>
>>>         -Wshift-count-overflow
>>>             Warn if shift count >= width of type. This warning is
>>>             enabled by default.
>>>
>>> Of course, if the compiler cannot guess that there will be such
>>> an issue, it will not emit the warning. You certainly don't want
>>> a warning for each non-trivial shift just because the compiler
>>> cannot know whether the constraint on the shift count will be
>>> satisfied.
>>
>> While the compiler clearly can't give a warning on calculated shifts
>> without massive amounts of false positives, it is able to give warnings
>> when there is a shift by a compile-time known constant value that is
>> invalid.  In the case of the OP's test function, inlining and constant
>> propagation means that the shift value /is/ known to the compiler - it
>> uses it for optimisation (in this case, it uses the undefined behaviour
>> to "simplify" the calculations).
>>
>> Am I right in thinking that this is because the pass that checks the
>> shift sizes for warnings here comes before the relevant inlining or
>> constant propagation passes?  And if so, would it be possible to change
>> the ordering here?
>
> To generate a warning, the compiler would also have to make sure
> that the inlined code with an out-of-range shift is actually not
> dead code. In practice, it may happen that constraints are not
> satisfied on some platforms, but the reason is that on such
> platforms the code will never be executed (this is code written
> to take care of other platforms).
>

I disagree.  To generate an unconditional error (rejecting the program),
the compiler would need such proof - such as by tracing execution from
main().  But to generate a warning activated specifically by the user,
there is no such requirement.  It's fine to give a warning based on the
code written, rather than on code that the compiler knows without doubt
will be executed.

(The warning here would be on the function "test" which calls the
function with the shift, not on that function itself - since it is only
when used in "test" that the compiler can see that there is undefined
behaviour.)


Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Vincent Lefevre-3
On 2019-03-12 21:56:59 +0100, David Brown wrote:
> I disagree.  To generate an unconditional error (rejecting the program), the
> compiler would need such proof - such as by tracing execution from main().
> But to generate a warning activated specifically by the user, there is no
> such requirement.  It's fine to give a warning based on the code written,
> rather than on code that the compiler knows without doubt will be executed.

There's already a bug about spurious warnings on shift counts:

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

--
Vincent Lefèvre <[hidden email]> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

David Brown-4
On 13/03/2019 03:25, Vincent Lefevre wrote:

> On 2019-03-12 21:56:59 +0100, David Brown wrote:
>> I disagree.  To generate an unconditional error (rejecting the program), the
>> compiler would need such proof - such as by tracing execution from main().
>> But to generate a warning activated specifically by the user, there is no
>> such requirement.  It's fine to give a warning based on the code written,
>> rather than on code that the compiler knows without doubt will be executed.
>
> There's already a bug about spurious warnings on shift counts:
>
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=4210
>

You can divide code into three groups (with the exact divisions varying
by compiler switches and version):

1. Code that the compiler knows for sure will run in every execution of
the program, generally because it can track the code flow from main().

2. Code that the compiler knows will /not/ run, due to things like
constant propagation, inlining, etc.

3. Code that the compiler does not know if it will run or not.



Code in group 1 here is usually quite small.  Code in group 2 can be
large, especially with C++ header libraries, templates, etc.  The
compiler will often eliminate such code and avoid generating any object
code.  gcc used to have a warning for when it found "group 2" code and
eliminated it - that warning was removed as gcc got smarter, and the
false positives were overwhelming.

Most code is in group 3.


I would say that if the compiler finds undefined behaviour in group 1
code, it should give an unconditional error message, almost regardless
of compiler switches.  (Many people will disagree with me here - that's
okay.  Fortunately for everyone else, I am not the one who decides these
things in gcc!).  Certainly that is standards-condoned behaviour.

To be useful to the developer, warnings have to be applied to group 3
code.  That does mean a risk of false positives - some code will be
group 2 (never run) though the compiler doesn't know it.

I am arguing here that a warning like this should be applied to group 3
code - you are suggesting it should only apply to group 1.

The bug report you linked was for code in group 2 - code that the
compiler can (or should be able to) see is never run.  I can see it
makes sense to disable or hide warnings from such code, but it may be
useful to have them anyway.  I expect people to have different
preferences here.


(I see in that bug report, solutions are complicated because C lets you
"disable" a block by writing "if (0)", and then lets you jump into it
from outside with labels and goto's.  Perhaps that should automatically
trigger a warning saying "Your code is made of spaghetti.  Any other
warnings may be unreliable with many false positives and false negatives".)

Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Moritz Strübe
In reply to this post by Jakub Jelinek
Hey.

Am 11.03.2019 um 12:17 schrieb Jakub Jelinek:

On Mon, Mar 11, 2019 at 11:06:37AM +0000, Moritz Strübe wrote:


On 11.03.2019 at 10:14 Jakub Jelinek wrote:


You could build with -fsanitize=undefined, that would tell you at runtime you
have undefined behavior in your code (if the SingleDiff has bit ever 0x20
set).


Yes, that helps. Unfortunately I'm on an embedded system, thus the code
size increase is just too big.


You can -fsanitize-undefined-trap-on-error, which doesn't increase size too
much, it is less user-friendly, but still should catch the UB.



Ok, I played around a bit. Interestingly, if I set -fsanitize=udefined and -fsanitize-undefined-trap-on-error the compiler detects that it will always trap, and optimizes the code accordingly (the code after the trap is removed).*  Which kind of brings me to David's argument: Shouldn't the compiler warn if there is undefined behavior it certainly knows of?
I do assume though that fsanitize just injects the test-code everywhere and relies on the compiler to remove it at unnecessary places. Would be nice, though. :)

Cheers
Morty

*After fixing the code, it got too big to fit.


--
Redheads Ltd. Softwaredienstleistungen
Schillerstr. 14
90409 Nürnberg

Telefon: +49 (0)911 180778-50
E-Mail: [hidden email]<mailto:[hidden email]> | Web: www.redheads.de<http://www.redheads.de>

Geschäftsführer: Andreas Hanke
Sitz der Gesellschaft: Lauf
Amtsgericht Nürnberg HRB 22681
Ust-ID: DE 249436843

Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Christophe Lyon
On 20/03/2019 15:08, Moritz Strübe wrote:

> Hey.
>
> Am 11.03.2019 um 12:17 schrieb Jakub Jelinek:
>
> On Mon, Mar 11, 2019 at 11:06:37AM +0000, Moritz Strübe wrote:
>
>
> On 11.03.2019 at 10:14 Jakub Jelinek wrote:
>
>
> You could build with -fsanitize=undefined, that would tell you at runtime you
> have undefined behavior in your code (if the SingleDiff has bit ever 0x20
> set).
>
>
> Yes, that helps. Unfortunately I'm on an embedded system, thus the code
> size increase is just too big.
>
>
> You can -fsanitize-undefined-trap-on-error, which doesn't increase size too
> much, it is less user-friendly, but still should catch the UB.
>
>

Wouldn't this fail to link? I thought the sanitizers need some runtime libraries which are only available under linux/macos/android. What do you mean by embedded? Isn't it arm-eabi?

>
> Ok, I played around a bit. Interestingly, if I set -fsanitize=udefined and -fsanitize-undefined-trap-on-error the compiler detects that it will always trap, and optimizes the code accordingly (the code after the trap is removed).*  Which kind of brings me to David's argument: Shouldn't the compiler warn if there is undefined behavior it certainly knows of?
> I do assume though that fsanitize just injects the test-code everywhere and relies on the compiler to remove it at unnecessary places. Would be nice, though. :)
>

Could you confirm in which version of the ST libraries you noticed this bug?
I'm told it was fixed on 23-march-2018.

Thanks,

Christophe


> Cheers
> Morty
>
> *After fixing the code, it got too big to fit.
>
>
> --
> Redheads Ltd. Softwaredienstleistungen
> Schillerstr. 14
> 90409 Nürnberg
>
> Telefon: +49 (0)911 180778-50
> E-Mail: [hidden email]<mailto:[hidden email]> | Web: www.redheads.de<http://www.redheads.de>
>
> Geschäftsführer: Andreas Hanke
> Sitz der Gesellschaft: Lauf
> Amtsgericht Nürnberg HRB 22681
> Ust-ID: DE 249436843
>

Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Moritz Strübe
Hey.

Am 20.03.2019 um 15:26 schrieb Christophe Lyon:

You can -fsanitize-undefined-trap-on-error, which doesn't increase size too
much, it is less user-friendly, but still should catch the UB.



Wouldn't this fail to link? I thought the sanitizers need some runtime libraries which are only available under linux/macos/android. What do you mean by embedded? Isn't it arm-eabi?


Nope. It inserts a trap, triggering a hard fault (as the manual says). Works just fine.

Moritz

--
Redheads Ltd. Softwaredienstleistungen
Schillerstr. 14
90409 Nürnberg

Telefon: +49 (0)911 180778-50
E-Mail: [hidden email]<mailto:[hidden email]> | Web: www.redheads.de<http://www.redheads.de>

Geschäftsführer: Andreas Hanke
Sitz der Gesellschaft: Lauf
Amtsgericht Nürnberg HRB 22681
Ust-ID: DE 249436843

Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Jakub Jelinek
In reply to this post by Moritz Strübe
On Wed, Mar 20, 2019 at 02:08:09PM +0000, Moritz Strübe wrote:
> Ok, I played around a bit. Interestingly, if I set -fsanitize=udefined and -fsanitize-undefined-trap-on-error the compiler detects that it will always trap, and optimizes the code accordingly (the code after the trap is removed).*  Which kind of brings me to David's argument: Shouldn't the compiler warn if there is undefined behavior it certainly knows of?

What does it mean certainly knows of?
The sanitization inserts (conditional) traps for all the constructs
that it sanitizes, you certainly don't want warning for that.
Even if the compiler can simplify or optimize out some of the guarding
conditionals around the traps, that doesn't mean it isn't in dead code that
will never be executed.
The only safe warning might be if the compiler can prove that whenever main
is called, there will be a trap executed later on, but that is not the case
in most programs, as one can't prove for most functions they actually never
loop and always return to the caller instead of say exiting, aborting, etc.
(and even if main traps immediately, one could have work done in
constructors and exit from there).  Otherwise, would you like to warn if
there is unconditional trap in some function?  That function could not be
ever called, or it could make some function calls before the trap that would
never return (exit, abort, throw exception, infinite loop).

        Jakub
Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Andrew Haley
In reply to this post by Moritz Strübe
On 3/20/19 2:08 PM, Moritz Strübe wrote:
>
> Ok, I played around a bit. Interestingly, if I set
> -fsanitize=udefined and -fsanitize-undefined-trap-on-error the
> compiler detects that it will always trap, and optimizes the code
> accordingly (the code after the trap is removed).* Which kind of
> brings me to David's argument: Shouldn't the compiler warn if there
> is undefined behavior it certainly knows of?

Maybe an example would help.

Consider this code:

for (int i = start; i < limit; i++) {
  foo(i * 5);
}

Should GCC be entitled to turn it into

int limit_tmp = i * 5;
for (int i = start * 5; i < limit_tmp; i += 5) {
  foo(i);
}

If you answered "Yes, GCC should be allowed to do this", would you
want a warning? And how many such warnings might there be in a typical
program?

--
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Richard Biener-2
On Wed, Mar 20, 2019 at 6:36 PM Andrew Haley <[hidden email]> wrote:

>
> On 3/20/19 2:08 PM, Moritz Strübe wrote:
> >
> > Ok, I played around a bit. Interestingly, if I set
> > -fsanitize=udefined and -fsanitize-undefined-trap-on-error the
> > compiler detects that it will always trap, and optimizes the code
> > accordingly (the code after the trap is removed).* Which kind of
> > brings me to David's argument: Shouldn't the compiler warn if there
> > is undefined behavior it certainly knows of?
>
> Maybe an example would help.
>
> Consider this code:
>
> for (int i = start; i < limit; i++) {
>   foo(i * 5);
> }
>
> Should GCC be entitled to turn it into
>
> int limit_tmp = i * 5;
> for (int i = start * 5; i < limit_tmp; i += 5) {
>   foo(i);
> }
>
> If you answered "Yes, GCC should be allowed to do this", would you
> want a warning? And how many such warnings might there be in a typical
> program?

I assume i is signed int.  Even then GCC may not do this unless it knows
the loop is entered (start < limit).

Richard.

>
> --
> Andrew Haley
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Alexander Monakov-4
On Thu, 21 Mar 2019, Richard Biener wrote:

> > Maybe an example would help.
> >
> > Consider this code:
> >
> > for (int i = start; i < limit; i++) {
> >   foo(i * 5);
> > }
> >
> > Should GCC be entitled to turn it into
> >
> > int limit_tmp = i * 5;
> > for (int i = start * 5; i < limit_tmp; i += 5) {
> >   foo(i);
> > }
> >
> > If you answered "Yes, GCC should be allowed to do this", would you
> > want a warning? And how many such warnings might there be in a typical
> > program?
>
> I assume i is signed int.  Even then GCC may not do this unless it knows
> the loop is entered (start < limit).

Additionally, the compiler needs to prove that 'foo' always returns normally
(i.e. cannot invoke exit/longjmp or such).

Alexander
Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Richard Biener-2
On Thu, Mar 21, 2019 at 9:25 AM Alexander Monakov <[hidden email]> wrote:

>
> On Thu, 21 Mar 2019, Richard Biener wrote:
> > > Maybe an example would help.
> > >
> > > Consider this code:
> > >
> > > for (int i = start; i < limit; i++) {
> > >   foo(i * 5);
> > > }
> > >
> > > Should GCC be entitled to turn it into
> > >
> > > int limit_tmp = i * 5;
> > > for (int i = start * 5; i < limit_tmp; i += 5) {
> > >   foo(i);
> > > }
> > >
> > > If you answered "Yes, GCC should be allowed to do this", would you
> > > want a warning? And how many such warnings might there be in a typical
> > > program?
> >
> > I assume i is signed int.  Even then GCC may not do this unless it knows
> > the loop is entered (start < limit).
>
> Additionally, the compiler needs to prove that 'foo' always returns normally
> (i.e. cannot invoke exit/longjmp or such).

Ah, yes.  Andrews example was probably meaning limit_tmp = limit * 5, not i * 5.
Computing start * 5 is fine if the loop is entered.

Richard.

>
> Alexander
Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Moritz Strübe
In reply to this post by Andrew Haley
Hey.

Am 20.03.2019 um 18:36 schrieb Andrew Haley:

> On 3/20/19 2:08 PM, Moritz Strübe wrote:
>> Ok, I played around a bit. Interestingly, if I set
>> -fsanitize=udefined and -fsanitize-undefined-trap-on-error the
>> compiler detects that it will always trap, and optimizes the code
>> accordingly (the code after the trap is removed).* Which kind of
>> brings me to David's argument: Shouldn't the compiler warn if there
>> is undefined behavior it certainly knows of?
> Maybe an example would help.
>
> Consider this code:
>
> for (int i = start; i < limit; i++) {
>    foo(i * 5);
> }
>
> Should GCC be entitled to turn it into
>
> int limit_tmp = i * 5;
> for (int i = start * 5; i < limit_tmp; i += 5) {
>    foo(i);
> }
>
> If you answered "Yes, GCC should be allowed to do this", would you
> want a warning? And how many such warnings might there be in a typical
> program?

Ok, let me see whether I get your point. I assume that should be "int
limit_tmp = limit * 5;".
In the original version I have a potential integer overflow while
passing a parameter. While in the second version, I have a potential
overflow in limit_tmp and therefore the loop range and number of calls
of foo is changed.
I think I start getting your point, but I none the less think it would
be really nice to have an option(!) to warn me about such things
nonetheless. Use cases would be libraries, or at least their interfaces
and critical software or just support finding potential bugs. Especially
when using third party libraries this would can help find potential issues.
Would it be possible to annotate the inserted checks with a debug symbol
or similar? That way one could compile using LTO and then search for the
remaining symbols? That would allow static analysis tools to search for
these symbols and annotate the code.

Cheers
Moritz


--
Redheads Ltd. Softwaredienstleistungen
Schillerstr. 14
90409 Nürnberg

Telefon: +49 (0)911 180778-50
E-Mail: [hidden email] | Web: www.redheads.de

Geschäftsführer: Andreas Hanke
Sitz der Gesellschaft: Lauf
Amtsgericht Nürnberg HRB 22681
Ust-ID: DE 249436843

Reply | Threaded
Open this post in threaded view
|

Re: GCC turns &~ into | due to undefined bit-shift without warning

Andrew Haley
On 3/21/19 8:53 AM, Moritz Strübe wrote:

> Hey.
>
> Am 20.03.2019 um 18:36 schrieb Andrew Haley:
>> On 3/20/19 2:08 PM, Moritz Strübe wrote:
>>> Ok, I played around a bit. Interestingly, if I set
>>> -fsanitize=udefined and -fsanitize-undefined-trap-on-error the
>>> compiler detects that it will always trap, and optimizes the code
>>> accordingly (the code after the trap is removed).* Which kind of
>>> brings me to David's argument: Shouldn't the compiler warn if there
>>> is undefined behavior it certainly knows of?
>> Maybe an example would help.
>>
>> Consider this code:
>>
>> for (int i = start; i < limit; i++) {
>>    foo(i * 5);
>> }
>>
>> Should GCC be entitled to turn it into
>>
>> int limit_tmp = i * 5;
>> for (int i = start * 5; i < limit_tmp; i += 5) {
>>    foo(i);
>> }
>>
>> If you answered "Yes, GCC should be allowed to do this", would you
>> want a warning? And how many such warnings might there be in a typical
>> program?
>
> Ok, let me see whether I get your point. I assume that should be "int
> limit_tmp = limit * 5;".

Yes, sorry.

> In the original version I have a potential integer overflow while
> passing a parameter. While in the second version, I have a potential
> overflow in limit_tmp and therefore the loop range and number of calls
> of foo is changed.

That's right.

> I think I start getting your point, but I none the less think it would
> be really nice to have an option(!) to warn me about such things
> nonetheless.

There aren't necesarily points in the compiler where GCC says "look,
this would be UB, so delete the code." Sometimes GCC simply assumes
that things like overflows cannot happen, so it ignores the
possibility. The code I provided is an example of that.

I suppose we could utilize the sanitize=undefined framework and emit a
warning everywhere a runtime check was inserted. That will at least
allow you to check in every case that the overflow, null pointer
exception, etc, cannot happen.

There would be a lot of warnings.

--
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
12