[Bug c/106333] New: Required condition omitted from generated code

[Bug c/106333] New: Required condition omitted from generated code

[eran.kornblau at kaltura dot com]

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

            Bug ID: 106333
           Summary: Required condition omitted from generated code
           Product: gcc
           Version: 12.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: eran.kornblau at kaltura dot com
  Target Milestone: ---

Hi,

We bumped into a case where a required condition gets omitted from our code, we
validated the issue on versions 11.1.0, 11.2.0, 11.3.0, 12.1.0. 
Version 10.3.0 is working as expected.

The issue happens only when compiling with optimizations (I tested -O2), when
running without any -Oxxx flag, it works as expected.

The issue can be reproduced with these commands -
$ docker run -it gcc:12.1.0 /bin/sh

# cat > /tmp/x.c
#include <stdio.h>
#include <limits.h>
#include <stdint.h>
#include <string.h>

#define NGX_LIVE_INVALID_PTS                 (LLONG_MAX)

typedef struct {
    int64_t                 created;
    int64_t                 dts;
    uint32_t                flags;
    int32_t                 pts_delay;
} kmp_frame_t;

static int test(kmp_frame_t *frame)
{
    if (frame->dts >= NGX_LIVE_INVALID_PTS - frame->pts_delay &&
frame->pts_delay >= 0) {
        printf("bad!\n");

    } else {
        printf("good!\n");
    }
}

int main()
{
    kmp_frame_t  frame;

    memset(&frame, 0xff, sizeof(frame));
    frame.dts = 149201707622903;

    test(&frame);

    return 0;
}
^C
# gcc -O2 /tmp/x.c -o /tmp/x ; /tmp/x
bad!
# gcc /tmp/x.c -o /tmp/x ; /tmp/x
good!
#

Following the memset with 0xff, the value of frame.pts_delay is -1. Therefore,
the second part of the if condition (frame->pts_delay >= 0) is expected to be
false (the field is a signed int32_t), so we should get the string 'good!'
printed.
However, it seems the second part of the condition (following the &&) gets
optimized out, and we get 'bad!' printed out.

Thank you!

Eran

[Bug c/106333] Required condition omitted from generated code

[schwab@linux-m68k.org]

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

Andreas Schwab <schwab@linux-m68k.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #1 from Andreas Schwab <schwab@linux-m68k.org> ---
If frame->pts_delay < 0 then NGX_LIVE_INVALID_PTS - frame->pts_delay overflows,
thus this assumed to not happen.  You need to check frame->pts_delay >= 0 first
to get defined behaviour.

[Bug c/106333] Required condition omitted from generated code

[eran.kornblau at kaltura dot com]

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

--- Comment #2 from Eran Kornblau <eran.kornblau at kaltura dot com> ---
Thanks Andreas, I understand there's an overflow here, but isn't it a bug that
it affects the second part of the condition?
I mean, any value is legit for the first part of the condition, if the behavior
is undefined... but the second part of the condition is well defined. 

I personally find it a bit troubling that -
1. The result of the condition depends on the order of the operands (while they
don't have any side-effects...), and -
2. The optimizations change the behavior of the code in this case

Btw, the reason I chose this order is that the first part of the condition is
false in 99% of the time, while the second part is true 99% of the time. So, in
my case there is a tiny optimization in keeping the order as is.

Thank you,

Eran

[Bug c/106333] Required condition omitted from generated code

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Eran Kornblau from comment #2)
> Thanks Andreas, I understand there's an overflow here, but isn't it a bug
> that it affects the second part of the condition?


No.


> I mean, any value is legit for the first part of the condition, if the
> behavior is undefined... but the second part of the condition is well
> defined. 


That's not how undefined behaviour works. If the program has undefined
behaviour, the entire program has undefined behaviour. The unexpected outcome
is not limited to a single expression.

> I personally find it a bit troubling that -
> 1. The result of the condition depends on the order of the operands (while
> they don't have any side-effects...), and -

They don't have side effects, but evaluating one condition might have undefined
behaviour, so it's up to you to avoid that.


> 2. The optimizations change the behavior of the code in this case

No, the behaviour is always undefined. Optimization changes the observed
outcomes of that undefined behaviour, but that's allowed. Undefined behaviour
means anything can happen.

"This undefined behaviour surprises me" is not a bug.

[Bug c/106333] Required condition omitted from generated code

[eran.kornblau at kaltura dot com]

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

--- Comment #4 from Eran Kornblau <eran.kornblau at kaltura dot com> ---
Ok, thank you both.

One last point - maybe it makes sense to at least output a warning in this
case?
I added '-Wall -pedantic -Wextra' to the command, and didn't get any warning
about this.

The end result here was that some condition that I wrote didn't make it to the
binary, would have been good to know about it, before it happened to us on
production... something like the 'condition is always true/false' warnings.

Thanks,

Eran

[Bug c/106333] Required condition omitted from generated code

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
There was -Wstrict-overflow but many of those warnings are gone in recent
versions of the compiler and it was way too noisy. -fsanitize=undefined should
be able to detect the problem at runtime instead.

[Bug c/106333] Required condition omitted from generated code

[eran.kornblau at kaltura dot com]

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

--- Comment #6 from Eran Kornblau <eran.kornblau at kaltura dot com> ---
Indeed!
/tmp/x.c:18:44: runtime error: signed integer overflow: 9223372036854775807 -
-1 cannot be represented in type 'long long int'

Thanks!

Eran