[Bug middle-end/110228] New: [14 Regression] llvm-16 miscompilation on small case switch, minimized

[Bug middle-end/110228] New: [14 Regression] llvm-16 miscompilation on small case switch, minimized

[slyfox at gcc dot gnu.org]

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

            Bug ID: 110228
           Summary: [14 Regression] llvm-16 miscompilation on small case
                    switch, minimized
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: slyfox at gcc dot gnu.org
  Target Milestone: ---

I initially observed the failure as an llvm-16 (and older) single test failure
when built by r14-1704-gff83d1b47aadcd : 

    - TEST 'LLVM :: ExecutionEngine/JITLink/X86/MachO_weak_references.s' FAILED

It is caused by  ObjectLinkingLayerJITLinkContext::lookup() miscompilation.

I extracted the following minimized example from it:

#include <vector>

using M = std::vector<unsigned>;

static M s_lm{1};

__attribute__((noipa))
static const M & clm(void) {
    return s_lm;
}

__attribute__((noipa))
static void bug(unsigned * p) {
    const M & Symbols = clm();

    for (unsigned v : Symbols) {
      int LookupFlags;
      switch (v) {
      // never used
      case 0:
        LookupFlags = 0;
        break;
      // always executed for a given input
      case 1:
        LookupFlags = 1;
        break;
      }
      *p = LookupFlags;
    }
}

__attribute__((noipa))
int main() {
    unsigned r = 42;
    bug(&r);
    if (r != 1) __builtin_trap();
}

Triggering:

$ g++ bug.cc -o bug -O1 -std=c++11 && ./bug
Illegal instruction (core dumped)
$ g++ bug.cc -o bug -O0 -std=c++11 && ./bug

Note: the program traverses vector of 1 element of value 1. Somehow on -O1 it
manages to miss `LookupFlags = 1;` store and does something else.

$ g++ -v
Using built-in specs.
COLLECT_GCC=/<<NIX>>/gcc-14.0.0/bin/g++
COLLECT_LTO_WRAPPER=/<<NIX>>/gcc-14.0.0/libexec/gcc/x86_64-unknown-linux-gnu/14.0.0/lto-wrapper
Target: x86_64-unknown-linux-gnu
Configured with:
--with-gmp-include=/0wz52f16y71ywjvc2rs9r7lw2axyl3fw3jma12azg88jia6k1srk/include
--with-gmp-lib=/1ri6zpg87f38gjqjg3hxxfwg9i4fk3hwr1d4c8saiic61pkmzvza/lib
--with-mpfr-include=/18rzsbmvdk9f7qcy0iynsiabalafwfhjjv9i38q7h5ckvz3z86hz/include
--with-mpfr-lib=/1dza8r46l2racn05l78c9iy3ibj1mp02bn45cwrfrg2rxqf05icg/lib
--with-mpc=/1pg6d47qa8b4l9mq51cvzkkd1jjjbqbvmlm6c9cyzw3kmsf4a10i
--with-native-system-header-dir=/16q46gpr64lcgbhkdnyigap3mc07g762vn2ckl6zqa12c1ww1kmp/include
--with-build-sysroot=/ --program-prefix= --enable-lto --disable-libstdcxx-pch
--without-included-gettext --with-system-zlib --enable-checking=release
--enable-static --enable-languages=c,c++ --disable-multilib --enable-plugin
--disable-libcc1
--with-isl=/16g549izckw0akjcgs6x4rps7swlqi0ff8i4xfs4cj36l7kdilpv
--disable-bootstrap --build=x86_64-unknown-linux-gnu
--host=x86_64-unknown-linux-gnu --target=x86_64-unknown-linux-gnu
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 14.0.0 99999999 (experimental) (GCC)

[Bug middle-end/110228] [14 Regression] llvm-16 miscompilation on small case switch, minimized

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
   Target Milestone|---                         |14.0

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
This has to do with uninitialized variables ....

[Bug middle-end/110228] [14 Regression] llvm-16 miscompilation on small case switch, minimized

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55311
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55311&action=edit
reduced testcase without vector

[Bug middle-end/110228] [14 Regression] llvm-16 miscompilation on small case switch, minimized

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
                 CC|                            |pinskia at gcc dot gnu.org
   Last reconfirmed|                            |2023-06-12
     Ever confirmed|0                           |1

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I am 99% sure this was exposed by r14-1597-g64d90d06d2db43538c8a45 .

[Bug middle-end/110228] [14 Regression] llvm-16 miscompilation on small case switch, minimized

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Phiopt does this:
```
        v_13 == 1 ? 1 : LookupFlags_6
Matching expression match.pd:1990, gimple-match-5.cc:23
Matching expression match.pd:1990, gimple-match-5.cc:23
Matching expression match.pd:2479, gimple-match-4.cc:35
Matching expression match.pd:2482, gimple-match-3.cc:66
Matching expression match.pd:2489, gimple-match-2.cc:58
Matching expression match.pd:1947, gimple-match-7.cc:20
Applying pattern match.pd:4742, gimple-match-7.cc:15326
Folded into the sequence:
_17 = v_13 == 1;
_18 = LookupFlags_6 | _17;
Removing basic block 5
;; basic block 5, loop depth 1
;;  pred:       4
;;  succ:       6
```
As zero_one_valued_p returns true for LookupFlags_6 because
tree_nonzero_bits/get_nonzero_bits returns 1 for it.

So it is ranger in the end that returns that it has a nonzero bits of 1.
Which is completely wrong as LookupFlags_6 is defined by:
  # LookupFlags_6 = PHI <LookupFlags_9(D)(2), LookupFlags_5(5)>


Which has an uninitialized variable in it which is not defined at what its
value would be ....

[Bug middle-end/110228] [14 Regression] llvm-16 miscompilation on small case switch, minimized

[pinskia at gcc dot gnu.org]

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

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note the original code definitely has an uninitialized variable in it.
The enum is defined as:
enum class SymbolLookupFlags { RequiredSymbol, WeaklyReferencedSymbol };

Which always has an underlying type  so all values (not just 0/1) are valid for
SymbolLookupFlags .

[Bug middle-end/110228] [14 Regression] llvm-16 miscompilation on small case switch, minimized

[pinskia at gcc dot gnu.org]

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

--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Adding :

        default:__builtin_unreachable();

Makes the testcase in this report work.
Maybe there should be an assert in LLVM's code too ...

[Bug middle-end/110228] [14 Regression] llvm-16 miscompiled due to an maybe uninitialized and optimizations saying that the uninitialized has a nonzero bits of 1.

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=71762

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
bug 71762 comment #7 is very much related to the issue here (and maybe even the
same).

[Bug middle-end/110228] [14 Regression] llvm-16 miscompiled due to an maybe uninitialized and optimizations saying that the uninitialized has a nonzero bits of 1.

[pinskia at gcc dot gnu.org]

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

--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So the `&` case with r14-1597-g64d90d06d2db43538c8a45 should be ok always I
think as you are anding with a known non-uninitialized variable (that is always
0 or 1) that will produce a value which is known to be 0 or 1. It is one that
produces `|` that exposes the issue here. Even if gimple level says it is 0/1,
doing a `|` will produce an uninitialized value still and the upper bits are
not defined and things go off in the weeds then.

[Bug middle-end/110228] [14 Regression] llvm-16 miscompiled due to an maybe uninitialized and optimizations saying that the uninitialized has a nonzero bits of 1.

[pinskia at gcc dot gnu.org]

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

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
That is these exports:
2->3  (T) c.5_7 :       [irange] int [1334323000, +INF] NONZERO 0x7fffffff
2->3  (T) _8 :  [irange] long unsigned int [1334323000, 2147483647] NONZERO
0x7fffffff
2->3  (T) b.6_9 :       [irange] int [1334323000, +INF] NONZERO 0x7fffffff
2->3  (T) _10 :         [irange] long unsigned int [1334323000, 2147483647]
NONZERO 0x7fffffff
2->3  (T) _11 :         [irange] long unsigned int [3481806647, 3481806649]
NONZERO 0xcf88273f
2->3  (T) _12 :         [irange] long unsigned int [0, 2] NONZERO 0x3

seems wrong.
We originally had:
    c.5_7 = c;
    _8 = (long unsigned int) c.5_7;
    b.6_9 = b;
    _10 = (long unsigned int) b.6_9;
    _11 = _8 + _10;
    _12 = _11 + 18446744070227744969;
    if (_12 <= 2)

so _11 export seems correct but _8 and _10 added together needs to be make _11.
Since it is unsigned, the whole range for both numbers can make _11's range;
that is we can't say anything about _8 or _10. Even if _11 is signed, I don't
see how _8 and _10 could be described at all ... since _8 could be 0 and _10
could be 3481806647 and the range that is exported for _8 does not even include
0.

Unless I am missing something obvious here.

[Bug middle-end/110228] [14 Regression] llvm-16 miscompiled due to an maybe uninitialized and optimizations saying that the uninitialized has a nonzero bits of 1.

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |shaohua.li at inf dot ethz.ch

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 110286 has been marked as a duplicate of this bug. ***

[Bug middle-end/110228] [14 Regression] llvm-16 miscompiled due to an maybe uninitialized and optimizations saying that the uninitialized has a nonzero bits of 1.

[pinskia at gcc dot gnu.org]

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

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
From the duplicate bug report analysis:
  # RANGE [irange] short int [0, 1] NONZERO 0x1
  # n_lsm.35_15 = PHI <_10(9), n_lsm.35_45(12)>
...
  # RANGE [irange] short int [0, 1] NONZERO 0x1
  # n_lsm.35_45 = PHI <n_lsm.35_15(11), _69(D)(6)>

  # RANGE [irange] short int [0, 1] NONZERO 0x1
  _10 = (short intD.17) _9;

But the range of _69(D) is really undefined ....

There is nothing phiopt/match is doing incorrectly due to the above, rather it
is ranger that might be considering the wrong/incorrect information for
uninitialized variables ...

[Bug middle-end/110228] [14 Regression] llvm-16 miscompiled due to an maybe uninitialized and optimizations saying that the uninitialized has a nonzero bits of 1.

[pinskia at gcc dot gnu.org]

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

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I should note my reduced testcase does not even need the ranger to tell me that
the range is [0,1] because it uses a bool which has a PRECISION of 1.
If an expand solution is not implemented in a few months, I will submit a patch
which disables the `bit_ior` case and leave a comment there on referencing PR
71762 and this bug.

[Bug middle-end/110228] [14 Regression] llvm-16 miscompiled due to an maybe uninitialized and optimizations saying that the uninitialized has a nonzero bits of 1.

[pinskia at gcc dot gnu.org]

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

--- Comment #13 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The big question is this code here:
```
unsigned a[4] = {1,1,1,1};


__attribute__((noipa))
void sink(int){}
__attribute__((noipa))
static void bug(unsigned & p, unsigned *t, int n) {
      bool LookupFlags ;

    for(int i = 0; i < n; i++) {
        unsigned v = t[i];
      if (v == 0)
        LookupFlags = 0;
     else {
        bool t = v == 1;

        LookupFlags = LookupFlags | t;
     }
     sink(LookupFlags);
       p = LookupFlags;
    }
}


int main() {
    unsigned r = 42;
    bug(r,a, sizeof(a)/sizeof(a[0]));
    __builtin_printf("%u\n", r);
    if (r != 1) __builtin_abort();
}
```
has undefined behavior in it? Right now we remove the whole `!= 0` if we use
with -O2.

[Bug middle-end/110228] [14 Regression] llvm-16 miscompiled due to an maybe uninitialized and optimizations saying that the uninitialized has a nonzero bits of 1.

[slyfox at gcc dot gnu.org]

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

--- Comment #14 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
> LookupFlags = LookupFlags | t;

That makes `LookupFlags` to guarantee to contain uninitialized bits. Did you
mean `LookupFlags = t;`?

[Bug middle-end/110228] [14 Regression] llvm-16 miscompiled due to an maybe uninitialized and optimizations saying that the uninitialized has a nonzero bits of 1.

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #13)
> has undefined behavior in it? Right now we remove the whole `!= 0` if we use
> with -O2.

Yes, it is UB.  You'd need first array element of 0 to make it valid.

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized and optimizations saying that the uninitialized has a nonzero bits of 1.

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[14 Regression] llvm-16     |[13/14 Regression] llvm-16
                   |miscompiled due to an maybe |miscompiled due to an maybe
                   |uninitialized and           |uninitialized and
                   |optimizations saying that   |optimizations saying that
                   |the uninitialized has a     |the uninitialized has a
                   |nonzero bits of 1.          |nonzero bits of 1.
   Target Milestone|14.0                        |13.2

--- Comment #16 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
here is a (x86_64 as it requires the inline-asm to force an undefined value
inside LookupFlags but LookupFlags is not used until the next iteration)
program which we have been miscompiling -O2 since r13-4459-g6508d5e5a1a8:
```
unsigned a[4] = {1,1,1,1};

unsigned tt1 = 0;
__attribute__((noipa))
static void bug(unsigned & p, unsigned *t, int n, int t2) {

    for(int i = 0; i < n; i++) {
        bool LookupFlags ;
        unsigned v = t[i];
        unsigned tt = tt1;
      if (v == 0)
        LookupFlags = 0;
     else if (v == 1)
        LookupFlags = 1;
     if (LookupFlags) {
        tt|=3u;
        LookupFlags = 0;
     }
     asm("movq $-1, %q1":"+a"(LookupFlags));
       p = tt;
    }
}


int main() {
    unsigned r = 42;
    bug(r,a, sizeof(a)/sizeof(a[0]), 1);
    __builtin_printf("%u\n", r);
    if (r != 3) __builtin_abort();
}
```

Yes it is a bit odd but that was the only way I could get the code added in
r13-4459-g6508d5e5a1a8 to be invoked and even have LookupFlags|=v==1 happening
and get what could get undefined behavior in the loop.

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized and optimizations saying that the uninitialized has a nonzero bits of 1.

[aldyh at gcc dot gnu.org]

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

--- Comment #17 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #4)
> Phiopt does this:
> ```
> 	v_13 == 1 ? 1 : LookupFlags_6
> Matching expression match.pd:1990, gimple-match-5.cc:23
> Matching expression match.pd:1990, gimple-match-5.cc:23
> Matching expression match.pd:2479, gimple-match-4.cc:35
> Matching expression match.pd:2482, gimple-match-3.cc:66
> Matching expression match.pd:2489, gimple-match-2.cc:58
> Matching expression match.pd:1947, gimple-match-7.cc:20
> Applying pattern match.pd:4742, gimple-match-7.cc:15326
> Folded into the sequence:
> _17 = v_13 == 1;
> _18 = LookupFlags_6 | _17;
> Removing basic block 5
> ;; basic block 5, loop depth 1
> ;;  pred:       4
> ;;  succ:       6
> ```
> As zero_one_valued_p returns true for LookupFlags_6 because
> tree_nonzero_bits/get_nonzero_bits returns 1 for it.
> 
> So it is ranger in the end that returns that it has a nonzero bits of 1.
> Which is completely wrong as LookupFlags_6 is defined by:
>   # LookupFlags_6 = PHI <LookupFlags_9(D)(2), LookupFlags_5(5)>
> 
> 
> Which has an uninitialized variable in it which is not defined at what its
> value would be ....

BTW, it doesn't seem ranger would be involved here at all, since this bug
happens with -O1, and evrp nor VRP* run at this level.  The only other way
ranger could be involved at -O1 is through DOM threading's use of ranger, but
phiopt1 from which your dump above comes from, runs before DOM.

I realize further downthread y'all conclude this is an unitialized issue, but
I'm just trying to understand how ranger could have been involved.

FWIW, there are other passes which set global nonzero bits at -O1, most notably
CCP.  Theoretically any pass which calls set_range_info() or set_nonzero_bits()
would alter everyone's view of a range (strlen pass, PRE, DOM, sprintf, CCP,
etc).

And way down on the list of likely scenarios is fold_range() being called with
a global query object.  This would get ranger involved in folding something
with known global values (no additional lookups), but it's unlikely.  You
probably got a nonzero bits from some other pass.

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[pinskia at gcc dot gnu.org]

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

--- Comment #18 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 110376 has been marked as a duplicate of this bug. ***

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[pinskia at gcc dot gnu.org]

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

--- Comment #19 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 110491 has been marked as a duplicate of this bug. ***

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zhendong.su at inf dot ethz.ch

--- Comment #20 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 110509 has been marked as a duplicate of this bug. ***

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[zhendong.su at inf dot ethz.ch]

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

--- Comment #21 from Zhendong Su <zhendong.su at inf dot ethz.ch> ---
Another very likely related reproducer.

[628] % gcctk -v
Using built-in specs.
COLLECT_GCC=gcctk
COLLECT_LTO_WRAPPER=/local/home/suz/suz-local/software/local/gcc-trunk/bin/../libexec/gcc/x86_64-pc-linux-gnu/14.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../gcc-trunk/configure --disable-bootstrap
--enable-checking=yes --prefix=/local/suz-local/software/local/gcc-trunk
--enable-sanitizers --enable-languages=c,c++ --disable-werror
--disable-multilib
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 14.0.0 20230701 (experimental) [master r14-924-gd709841ae0f] (GCC)
[629] %
[629] % gcctk -O3 small.c
[630] % for i in $(seq 1 10); do
> echo $i
> ./a.out
> done
1
Floating point exception
2
Floating point exception
3
Floating point exception
4
Floating point exception
5
Floating point exception
6
Floating point exception
7
Floating point exception
8
Floating point exception
9
10
[631] % cat small.c
int a, b, c, d, e, f = 1, g, h;
int main () {
  int i;
  for (; g < 1; g++) {
      if (!a)
        i = 1;
      for (; b < 1; b++)
      L:
        ;
    }
  int k = -1 / f;
  while (d)
    i = 0;
  c = k;
  e = ~i;
  if (e > h) {
      f = 0;
      goto L;
    }
  return 0;
}

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[pinskia at gcc dot gnu.org]

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

--- Comment #22 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Zhendong Su from comment #21)
> Another very likely related reproducer.
Yes:
  <bb 3> [local count: 82077551]:
  a.0_1 = a;
  _27 = a.0_1 == 0;
  _5 = (int) _27;
  _28 = _5 | i_14;
  pretmp_45 = b;
...
  <bb 7> [local count: 200189151]:
  # i_14 = PHI <i_19(D)(2), i_13(6)>

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[pinskia at gcc dot gnu.org]

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

--- Comment #23 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #22)
> (In reply to Zhendong Su from comment #21)
> > Another very likely related reproducer.
> Yes:
>   <bb 3> [local count: 82077551]:
>   a.0_1 = a;
>   _27 = a.0_1 == 0;
>   _5 = (int) _27;
>   _28 = _5 | i_14;
>   pretmp_45 = b;
> ...
>   <bb 7> [local count: 200189151]:
>   # i_14 = PHI <i_19(D)(2), i_13(6)>

I wonder if ccp1 in this case could cause the 1 to proped out of the loop. and
maybe we need some more isolation going on when it comes to using uninitialized
variables ...

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[slyfox at gcc dot gnu.org]

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

Sergei Trofimovich <slyfox at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #24 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
Trying to understand the failure mode here:

In bug.c.033t.early_objsz I still see the explicit stores to LocalFlags:

  <bb 4> :
  LookupFlags_15 = 0;
  goto <bb 7>; [INV]

  <bb 5> :
  if (v_13 == 1)
    goto <bb 6>; [INV]
  else
    goto <bb 7>; [INV]

  <bb 6> :
  LookupFlags_14 = 1;

  <bb 7> :
  # LookupFlags_4 = PHI <LookupFlags_15(4), LookupFlags_5(5),
LookupFlags_14(6)>
  *p_16(D) = LookupFlags_4;

But in bug.c.034t.ccp1 I see no stores at all:

  if (v_13 == 0)
    goto <bb 6>; [INV]
  else
    goto <bb 4>; [INV]

  <bb 4> :
  if (v_13 == 1)
    goto <bb 5>; [INV]
  else
    goto <bb 6>; [INV]

  <bb 5> :

  <bb 6> :
  # LookupFlags_4 = PHI <0(3), LookupFlags_5(4), 1(5)>

Specifically '# LookupFlags_4 = PHI <0(3), LookupFlags_5(4), 1(5)>' claims that
somehow gets values '0' and '1' into a PHI node.

AFAIU PHI is an equivalent of a mutable variable in an otherwise immutable SSA
form. It has to be a write it needed value is not there yet. Why the store of
'1' was removed? Is it because variable 'v_13' already happens to have 0 or 1
value?

I wonder why PHI does not look like something below:

    # LookupFlags_4 = PHI <v_13(3), LookupFlags_5(4), v_13(5)>

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[slyfox at gcc dot gnu.org]

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

--- Comment #25 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
Specifically this bug.c.034t.ccp1's bit looks fishy:

...
Folding statement: LookupFlags_14 = 1;
Queued stmt for removal.  Folds to: 1
Folding statement: LookupFlags_15 = 0;
Queued stmt for removal.  Folds to: 0
Folding PHI node: LookupFlags_4 = PHI <0(4), LookupFlags_5(5), 1(6)>
No folding possible
Folding statement: *p_16(D) = LookupFlags_4;
Not folded
Folding statement: i_18 = i_6 + 1;
Not folded
Removing dead stmt:i_8 = 0;
Removing dead stmt:LookupFlags_14 = 1;
Removing dead stmt:LookupFlags_15 = 0;

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[rguenth at gcc dot gnu.org]

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

--- Comment #26 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Sergei Trofimovich from comment #24)
> Trying to understand the failure mode here:
> 
> In bug.c.033t.early_objsz I still see the explicit stores to LocalFlags:
> 
>   <bb 4> :
>   LookupFlags_15 = 0;
>   goto <bb 7>; [INV]
> 
>   <bb 5> :
>   if (v_13 == 1)
>     goto <bb 6>; [INV]
>   else
>     goto <bb 7>; [INV]
> 
>   <bb 6> :
>   LookupFlags_14 = 1;
> 
>   <bb 7> :
>   # LookupFlags_4 = PHI <LookupFlags_15(4), LookupFlags_5(5),
> LookupFlags_14(6)>
>   *p_16(D) = LookupFlags_4;
> 
> But in bug.c.034t.ccp1 I see no stores at all:
> 
>   if (v_13 == 0)
>     goto <bb 6>; [INV]
>   else
>     goto <bb 4>; [INV]
> 
>   <bb 4> :
>   if (v_13 == 1)
>     goto <bb 5>; [INV]
>   else
>     goto <bb 6>; [INV]
> 
>   <bb 5> :
> 
>   <bb 6> :
>   # LookupFlags_4 = PHI <0(3), LookupFlags_5(4), 1(5)>
> 
> Specifically '# LookupFlags_4 = PHI <0(3), LookupFlags_5(4), 1(5)>' claims
> that somehow gets values '0' and '1' into a PHI node.
> 
> AFAIU PHI is an equivalent of a mutable variable in an otherwise immutable
> SSA form. It has to be a write it needed value is not there yet. Why the
> store of '1' was removed? Is it because variable 'v_13' already happens to
> have 0 or 1 value?
> 
> I wonder why PHI does not look like something below:
> 
>     # LookupFlags_4 = PHI <v_13(3), LookupFlags_5(4), v_13(5)>

LookupFlags is in SSA form thus we are dealing with registers and no "stores".
What happens is just constant propagation, LookupFlags_1{4,5} can be
replaced with their assigned constant.

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #27 from Richard Biener <rguenth at gcc dot gnu.org> ---
For the testcase in comment#16 we have ifcombine doing

   <bb 4> [local count: 477815112]:
-  if (v_12 == 1)
-    goto <bb 6>; [34.00%]
-  else
-    goto <bb 5>; [66.00%]
-
-  <bb 5> [local count: 315357972]:
-  if (LookupFlags_5 != 0)
-    goto <bb 6>; [50.00%]
+  _19 = v_12 == 1;
+  _20 = LookupFlags_5 | _19;
+  if (_20 != 0)
+    goto <bb 5>; [67.00%]

that at least accesses 'LookupFlags_5' without checking v_12 is either zero
or one.  This exposes an uninitialized read of LookupFlags_5 on the path
from entry, even actually at runtime.  PHI-OPT then makes tt value
dependent on this uninitialized variable and I suspect things go downhill
from there.

Note it's not obvious at all that 'LookupFlags_5' may be uninitialized here,
so ifcombine might need to use mark_ssa_maybe_undefs to try to conservatively
catch those.

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[cvs-commit at gcc dot gnu.org]

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

--- Comment #28 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:b083203f053f1666e9cc1ded2abdf4e1688d1ec0

commit r14-2289-gb083203f053f1666e9cc1ded2abdf4e1688d1ec0
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Jul 4 10:29:26 2023 +0200

    tree-optimization/110228 - avoid undefs in ifcombine more thoroughly

    The following replaces the simplistic gimple_uses_undefined_value_p
    with the conservative mark_ssa_maybe_undefs approach as already
    used by LIM and IVOPTs.  This is to avoid exposing an unconditional
    uninitialized read on a path from entry by if-combine.

            PR tree-optimization/110228
            * tree-ssa-ifcombine.cc (pass_tree_ifcombine::execute):
            Mark SSA may-undefs.
            (bb_no_side_effects_p): Check stmt uses for undefs.

            * gcc.dg/torture/pr110228.c: New testcase.
            * gcc.dg/uninit-pr101912.c: Un-XFAIL.

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

--- Comment #29 from Richard Biener <rguenth at gcc dot gnu.org> ---
I can't reproduce the original failure on trunk or with GCC 13.1, but the
testcase from comment#16 is now fixed.  I also cannot reproduce the testcase
from comment#21 before the fix.

Can people double-check this is now fixed, maybe also on the branch when this
fix is cherry-picked?

Thanks.

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[zhendong.su at inf dot ethz.ch]

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

--- Comment #30 from Zhendong Su <zhendong.su at inf dot ethz.ch> ---
(In reply to Richard Biener from comment #29)
> I can't reproduce the original failure on trunk or with GCC 13.1, but the
> testcase from comment#16 is now fixed.  I also cannot reproduce the testcase
> from comment#21 before the fix.
> 
> Can people double-check this is now fixed, maybe also on the branch when this
> fix is cherry-picked?
> 
> Thanks.

I can still reproduce the test from comment#21 as well as PR 110491 and PR
110509.

PR 110376 doesn't reproduce for me anymore.

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[rguenth at gcc dot gnu.org]

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

--- Comment #31 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Zhendong Su from comment #30)
> (In reply to Richard Biener from comment #29)
> > I can't reproduce the original failure on trunk or with GCC 13.1, but the
> > testcase from comment#16 is now fixed.  I also cannot reproduce the testcase
> > from comment#21 before the fix.
> > 
> > Can people double-check this is now fixed, maybe also on the branch when this
> > fix is cherry-picked?
> > 
> > Thanks.
> 
> I can still reproduce the test from comment#21 as well as PR 110491 and PR
> 110509.

PR110491 still reproduces after my fix (and PHI-OPT fixed the same way).
The two other non-deterministic ones don't reproduce before/after
(or I misunderstand how they work).

> PR 110376 doesn't reproduce for me anymore.

I'm going to add this as testcase.

So I think there's more analysis necessary and I didn't fix the underlying
issues in the other PRs.

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[slyfox at gcc dot gnu.org]

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

--- Comment #32 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #29)
> I can't reproduce the original failure on trunk or with GCC 13.1, but the
> testcase from comment#16 is now fixed.  I also cannot reproduce the testcase
> from comment#21 before the fix.
> 
> Can people double-check this is now fixed, maybe also on the branch when this
> fix is cherry-picked?

Test suite on llvm-16 still fails the same single 'LLVM ::
ExecutionEngine/JITLink/X86/MachO_weak_references.s' test for me. 

gcc was built from 2c12ccf800fc7890925402d30a02f0fa9e2627cc.

The following test still fails for me:

$ cat bug.c
static unsigned a[4] = {1,1,1,1};

__attribute__((noipa))
static void bug(unsigned * p, unsigned *t, int n) {
     unsigned LookupFlags;

    for(int i = 0; i < n; i++) {
      unsigned v = t[i];
      if      (v == 0) LookupFlags = 0;
      else if (v == 1) LookupFlags = 1;
      *p = LookupFlags;
    }
}

int main() {
    unsigned r = 42;
    bug(&r, a, 4);
    if (r != 1) __builtin_abort();
    return 0;
}

$ gcc bug.c -o bug -O1 -Wall && ./bug
bug.c: In function 'bug':
bug.c:10:15: warning: 'LookupFlags' may be used uninitialized
[-Wmaybe-uninitialized]
   10 |       else if (v == 1) LookupFlags = 1;
      |               ^
bug.c:5:15: note: 'LookupFlags' was declared here
    5 |      unsigned LookupFlags;
      |               ^~~~~~~~~~~
Aborted (core dumped)

bug.c.255t.optimized still turns stores into '|=' against uninit value:


__attribute__((noipa, noinline, noclone, no_icf))
void bug (unsigned int * p, unsigned int * t, int n)
{
  unsigned long ivtmp.8;
  unsigned int v;
  unsigned int LookupFlags;
  _Bool _16;
  unsigned int _17;
  unsigned int _18;
  void * _19;
  unsigned long _20;
  unsigned long _26;
  unsigned long _28;

  <bb 2> [local count: 118111600]:
  if (n_10(D) > 0)
    goto <bb 3>; [89.00%]
  else
    goto <bb 7>; [11.00%]

  <bb 3> [local count: 105119324]:
  ivtmp.8_21 = (unsigned long) t_11(D);
  _20 = (unsigned long) n_10(D);
  _26 = _20 * 4;
  _28 = ivtmp.8_21 + _26;

  <bb 4> [local count: 955630225]:
  # LookupFlags_22 = PHI <LookupFlags_4(6), LookupFlags_8(D)(3)>
  # ivtmp.8_6 = PHI <ivtmp.8_5(6), ivtmp.8_21(3)>
  _19 = (void *) ivtmp.8_6;
  v_12 = MEM[(unsigned int *)_19];
  if (v_12 == 0)
    goto <bb 6>; [50.00%]
  else
    goto <bb 5>; [50.00%]

  <bb 5> [local count: 477815112]:
  _16 = v_12 == 1;
  _17 = (unsigned int) _16;
  _18 = _17 | LookupFlags_22;

  <bb 6> [local count: 955630225]:
  # LookupFlags_4 = PHI <v_12(4), _18(5)>
  *p_13(D) = LookupFlags_4;
  ivtmp.8_5 = ivtmp.8_6 + 4;
  if (ivtmp.8_5 != _28)
    goto <bb 4>; [89.00%]
  else
    goto <bb 7>; [11.00%]

  <bb 7> [local count: 118111600]:
  return;

}

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[rguenther at suse dot de]

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

--- Comment #33 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 4 Jul 2023, slyfox at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110228
> 
> --- Comment #32 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #29)
> > I can't reproduce the original failure on trunk or with GCC 13.1, but the
> > testcase from comment#16 is now fixed.  I also cannot reproduce the testcase
> > from comment#21 before the fix.
> > 
> > Can people double-check this is now fixed, maybe also on the branch when this
> > fix is cherry-picked?
> 
> Test suite on llvm-16 still fails the same single 'LLVM ::
> ExecutionEngine/JITLink/X86/MachO_weak_references.s' test for me. 
> 
> gcc was built from 2c12ccf800fc7890925402d30a02f0fa9e2627cc.

Can you try 819285ef10a87d663f8c181c06aa88d1d9f75aed - your small
testcase looks OK now.

[Bug middle-end/110228] [13/14 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[slyfox at gcc dot gnu.org]

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

--- Comment #34 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #33)
> On Tue, 4 Jul 2023, slyfox at gcc dot gnu.org wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110228
> > 
> > --- Comment #32 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
> > (In reply to Richard Biener from comment #29)
> > > I can't reproduce the original failure on trunk or with GCC 13.1, but the
> > > testcase from comment#16 is now fixed.  I also cannot reproduce the testcase
> > > from comment#21 before the fix.
> > > 
> > > Can people double-check this is now fixed, maybe also on the branch when this
> > > fix is cherry-picked?
> > 
> > Test suite on llvm-16 still fails the same single 'LLVM ::
> > ExecutionEngine/JITLink/X86/MachO_weak_references.s' test for me. 
> > 
> > gcc was built from 2c12ccf800fc7890925402d30a02f0fa9e2627cc.
> 
> Can you try 819285ef10a87d663f8c181c06aa88d1d9f75aed - your small
> testcase looks OK now.

Ah, that commit fixes 'LLVM ::
ExecutionEngine/JITLink/X86/MachO_weak_references.s' and the rest of llvm-16
test suite for me. Thank you!

[Bug middle-end/110228] [13 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |13.1.0
      Known to work|                            |14.0
            Summary|[13/14 Regression] llvm-16  |[13 Regression] llvm-16
                   |miscompiled due to an maybe |miscompiled due to an maybe
                   |uninitialized variable      |uninitialized variable

--- Comment #35 from Richard Biener <rguenth at gcc dot gnu.org> ---
While the original issue is fixed the comment#16 testcase at least also affects
the GCC 13 branch and picking the fix solves it there.

[Bug middle-end/110228] [13 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[cvs-commit at gcc dot gnu.org]

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

--- Comment #36 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:79b6a4875f3dcc4bbca9242313b3edc1bad69660

commit r13-7533-g79b6a4875f3dcc4bbca9242313b3edc1bad69660
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Jul 4 10:29:26 2023 +0200

    tree-optimization/110228 - avoid undefs in ifcombine more thoroughly

    The following replaces the simplistic gimple_uses_undefined_value_p
    with the conservative mark_ssa_maybe_undefs approach as already
    used by LIM and IVOPTs.  This is to avoid exposing an unconditional
    uninitialized read on a path from entry by if-combine.

            PR tree-optimization/110228
            * tree-ssa-ifcombine.cc (pass_tree_ifcombine::execute):
            Mark SSA may-undefs.
            (bb_no_side_effects_p): Check stmt uses for undefs.

            * gcc.dg/torture/pr110228.c: New testcase.
            * gcc.dg/uninit-pr101912.c: Un-XFAIL.

    (cherry picked from commit b083203f053f1666e9cc1ded2abdf4e1688d1ec0)

[Bug middle-end/110228] [13 Regression] llvm-16 miscompiled due to an maybe uninitialized variable

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |13.1.1
         Resolution|---                         |FIXED

--- Comment #37 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.