Wrong code at -O1/2/3/s on aarch64-linux-gnu

[junweizeng]

The following code snippet, clang-14 and clang-17 at -O2/3/s produces the wrong code. clang-12 has no problem.

$ cat test.c
int printf(const char *, ...);
int __attribute__((noinline))
f(int *pi, long *pl)
{
  *pi = 1;
  *pl = 0;
  return *(char *)pi;
}

int main()
{
  union { long l; int i; } a;
  if (f (&a.i, &a.l) != 0)
    printf("error\n");
  return 0;
}
$
$ clang-12 test.c -O0; ./a.out
$ clang-12 test.c -O1; ./a.out
$ clang-12 test.c -O2; ./a.out
$ clang-12 test.c -O3; ./a.out
$ clang-12 test.c -Os; ./a.out
$
$ clang-14 test.c -O0; ./a.out
$ clang-14 test.c -O1; ./a.out
error
$ clang-14 test.c -O2; ./a.out
error
$ clang-14 test.c -O3; ./a.out
error
$ clang-14 test.c -Os; ./a.out
error
$
$ clang-17 test.c -O0; ./a.out
$ clang-17 test.c -O1; ./a.out
error
$ clang-17 test.c -O2; ./a.out
error
$ clang-17 test.c -O3; ./a.out
error
$ clang-17 test.c -Os; ./a.out
error
$ clang-12 --version
Ubuntu clang version 12.0.1-++20211102090516+fed41342a82f-1~exp1~20211102211019.11
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
$
$ clang-14 --version
Ubuntu clang version 14.0.6
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
$
$ clang-17 --version
Ubuntu clang version 17.0.6 (++20231208085808+6009708b4367-1~exp1~20231208085905.78)
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

[dtcxzyw]

This is a UB. See also https://en.cppreference.com/w/cpp/language/union. As a workaround, you can pass -fno-strict-aliasing to disable TBAA: https://godbolt.org/z/n1Wo18sYP

[AaronBallman]

I believe this code has well-defined behavior and should work only in C (it has UB in C++ because of the active union member rules).

See C23 6.5.3.4p2 (esp see the footnote), 6.5p7

CC @zygoloid @hubert-reinterpretcast @efriedma-quic for additional opinions

[AaronBallman]

Hmmm, this looks an awful lot like DR236 where the committee eventually claims it's UB because the pointers designate the same region of storage, but I've yet to see anything in the standard to back that up. Investigating more.

[nikic]

@AaronBallman Could avoid that question by replacing the union with a malloc, at which point this is a duplicate of the many existing issues around our incorrect handling of stores changing effective types in C (like https://github.com/llvm/llvm-project/issues/58225 or https://github.com/llvm/llvm-project/issues/54878).

[AaronBallman]

Aha!

3.18 defines object as "region of data storage in the execution environment, the contents of which can represent values"

so f() is being called with both arguments pointing to the same object despite having different declared types within the union. 6.5p7 says "An object shall have its stored value accessed only by an lvalue expression that has one of the following types:"

so one of the assignments within f() is valid and one of the assignments is invalid because the type is not compatible with the effective type of the object. The cast to char * and subsequent dereference are well-defined otherwise. It's not quite clear which assignment is an issue though.

A second set of eyes on the standard would be worthwhile though. If folks agree with my logic, then we can put the invalid and undefined behavior labels on the issue and close it.

[hubert-reinterpretcast]

3.18 defines object as "region of data storage in the execution environment, the contents of which can represent values"

so f() is being called with both arguments pointing to the same object despite having different declared types within the union.

I don't think the relationship between objects and regions of data storage map both ways. A region of data storage can map to more than one object: for example, a structure object without padding containing a single member and that member is the same region of storage (but not the same object). One way to notice that they are different objects is that the values they hold are not the same: when the member has a non-value representation, the value of the structure object is not a non-value representation.

That said, if TBAA is supposed to be useful at all, I wouldn't give much credence to the C union access and effective type rules (and the common initial sequence rules in either C or C++). Realistically, changing the type stored within a union, accessing a union member as a different type, etc. should involve an lvalue that is a member access of the union. Similarly, changing the effective type of an object should be a special operation at the C source level. The last I checked, Clang/LLVM doesn't get even C++ placement new right though.

[hubert-reinterpretcast]

DR 236 seems to say that the program is UB. The clearest part is the comment "union type must be used when changing effective type". I do not believe that the wording of the standard is clear about that intent.

[uecker]

First, DR236 is a historical document and the "the union type must be used when changing the effective type" would not help. C allows changing the effective type for allocated storage, so allowing this for union is not an additional problem. The issue is when reading the non-active union for declared types. Here it would make sense to clarify in the standard that this has to use the union type. So my recommendation for this example is to implement the type-changing stores correctly, because this is needed anyway for C and something where Clang is currently not compliant. Then it would also work for changing the active type in unions via direct pointers.

[hubert-reinterpretcast]

So my recommendation for this example is to implement the type-changing stores correctly

Yes, not even for C++. Once we have the technology to handle the C++ case, the front-end should be able to use it to (conservatively) handle the C cases.