[Bug c/115326] New: __builtin_sub_overflow reports incorrect overflow value

[Bug c/115326] New: __builtin_sub_overflow reports incorrect overflow value

[cody at tapscott dot me]

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

            Bug ID: 115326
           Summary: __builtin_sub_overflow reports incorrect overflow
                    value
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: cody at tapscott dot me
  Target Milestone: ---

Created attachment 58334
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58334&action=edit
reduced test case

The `__builtin_sub_overflow` intrinsic appears to report an incorrect overflow
value, in a specific case when it is used with pointers to the field of a
union.

I would expect the two evaluations in the attached test program to be
identical, but instead on my machine the abort() is triggered:
$ gcc test2.c -O0 -fno-strict-aliasing
$ ./a.out
[1]    1099972 IOT instruction  ./a.out

$ gcc -v:
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/opt/gcc-latest/libexec/gcc/x86_64-pc-linux-gnu/13.0.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../configure --prefix=/opt/gcc-latest --enable-languages=c,c++
--enable-libstdcxx-debug --enable-libstdcxx-backtrace --disable-bootstrap
--disable-multilib --disable-libvtv --with-system-zlib --without-isl
--enable-multiarch
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 13.0.1 20230416 (experimental) (GCC)

[Bug c/115326] __builtin_sub_overflow reports incorrect overflow value

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
We lower it as

  int overflow1 = r->as_u64[0] = REALPART_EXPR <.SUB_OVERFLOW ((uint64_t)
a->as_u64[0], (uint64_t) b->as_u64[0])>, (int) (_Bool) IMAGPART_EXPR
<.SUB_OVERFLOW ((uint64_t) a->as_u64[0], (uint64_t) b->as_u64[0])>;

where the assignment to r->as_u64[0] is done before the re-evaluation
for the overflow bit.  A SAVE_EXPR is missing here?  Jakub?

[Bug c/115326] __builtin_sub_overflow reports incorrect overflow value

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think this is just invalid testcase.
The compiler is told that *a is const, but it is changed through a different
lvalue.

[Bug c/115326] __builtin_sub_overflow reports incorrect overflow value

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #2)
> I think this is just invalid testcase.
> The compiler is told that *a is const, but it is changed through a different
> lvalue.

No the testcase is valid, it is just a dup of bug 108789.

*** This bug has been marked as a duplicate of bug 108789 ***

[Bug c/115326] __builtin_sub_overflow reports incorrect overflow value

[jakub at gcc dot gnu.org]

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Note, fold_builtin_arith_overflow just does
      tree call = build_call_expr_internal_loc (loc, ifn, ctype, 2,
                                                arg0, arg1);
      tree tgt = save_expr (call);
and then uses tgt twice, so it is save_expr decision whether it can be
evaluated twice or needs to be done once.
And save_expr adds SAVE_EXPR only to the second case when it sees non-const
VAR_DECLs,
in the first case the arguments are TREE_READONLY and so the ifn call is as
well and so
causes tree_invariant_p_1 to return true.