c23 aggregate initialization

[uecker]

It seems clang recently started to miscompile aggregate initialization.

union foo {
    char x;
    int  y;
    long z;
} x = { 0 };

would only initialize x, which is OK (although a questionable decision). But my understanding is that
for C23 x = { } zero-filling is required, and this also does not happen anymore.

https://godbolt.org/z/68q9bY5cT

[shafik]

This looks like a potential regression from clang-17: https://godbolt.org/z/nPYo1ejWr

CC @AaronBallman

[AaronBallman]

Something odd is going on here as it seems to relate to use of [[gnu::noinline]] of all things: https://godbolt.org/z/ze5YMP7bG and the issue does not reproduce at -O0.

[AaronBallman]

FWIW, the changes in Clang 17 related to aggregate initialization for C2x are in https://reviews.llvm.org/D147349. But those changes were in Clang 17 and so I don't think relate to the issue (Clang 17 doesn't demonstrate the bug).

[uecker]

Am Dienstag, dem 16.01.2024 um 15:22 +0000 schrieb Aaron Ballman:

Something odd is going on here as it seems to relate to use of [[gnu::noinline]] of all things: https://godbolt.org/z/ze5YMP7bG and the issue does not reproduce at -O0.

The noinline is only required so that the test with memcpy is not optimized and correctly also compares the padding. Martin

…

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: ***@***.***>

[AaronBallman]

We seem to be initializing the union based on the first element of the union rather than the largest element of the union. Switching element order so that the largest is first causes the issue to go away.

https://godbolt.org/z/cvojEdaMc

store i8 0, ptr %a, align 8 is the problematic bit. If you change the order of the union elements, then it becomes store i64 0, ptr %a, align 8 as it was in Clang 17.

CC @rjmccall @efriedma-quic

[AaronBallman]

I think this is an LLVM optimization pass issue, the IR looks correct at O0 with no LLVM passes being run:

define dso_local i32 @main() {
entry:
  %retval = alloca i32, align 4
  %a = alloca %union.foo, align 8
  store i32 0, ptr %retval, align 4
  call void @llvm.memset.p0.i64(ptr align 8 %a, i8 0, i64 8, i1 false)
  %call = call i32 @c(ptr noundef %a)
  ret i32 %call
}

That call to memset should be sufficient to zero initialize the local, but for some reason LLVM seems to think it doesn't need to zero initialize the inactive members (maybe because it thinks they're padding and thus unspecified?).

[efriedma-quic]

Looks like the codegen changed... but it's semantically the same: we initialize the first element, the rest is undef. https://godbolt.org/z/Wxb5esWhY . I think clang 17 just got lucky with LLVM optimizations for this particular example.

As far as I can tell, the issue is just that our original extension for "empty initialization" didn't initialize padding. And the corresponding C++ rule doesn't require initializing padding. And nobody realized the C23 has different semantics from the extension we originally implemented. (Probably a consequence of the fact that the rules are sort of weird.)

Some examples:

union X { char a; long b; } x = {};
struct Y {char a; __attribute((packed)) long b; int c;} y = {};
void g(void*);
void f() {
  union X x = {}; g(&x);
  struct Y y = {}; g(&y);
  struct Z {long a; union X x; } z = { (long)&f, {}}; g(&z);
}

On a side-note, it's probably worth amending LLVM LangRef to explicitly specify what happens to padding in global variables of struct type; I don't think it's noted anywhere.

[efriedma-quic]

Actually, regarding the C++ rule, I'm not sure it allows initializing the padding even if we wanted to. (For simple variables, probably it's impossible for user code to tell, but list initialization shows up in other places.)