Incorrect code generation with -02

[Quuxplusone]

Bugzilla Link	PR40401
Status	NEW
Importance	P normal
Reported by	Petr Azmanov (petr.azmanov@wartsila.com)
Reported on	2019-01-22 04:46:34 -0800
Last modified on	2019-01-24 05:58:50 -0800
Version	7.0
Hardware	PC Linux
CC	blitzrakete@gmail.com, dgregor@apple.com, erik.pilkington@gmail.com, ignat.loskutov@gmail.com, llvm-bugs@lists.llvm.org, llvm-dev@redking.me.uk, richard-llvm@metafoo.co.uk
Fixed by commit(s)
Attachments	testfail.cpp (1691 bytes, text/x-c++src) run.sh (160 bytes, application/x-shellscript)
Blocks
Blocked by
See also

Created attachment 21362
Test program source

Overview:

Attached is a reduced serialization code snippet. The program compares instance
of struct test_t with the result of deserialization of the same struct from
binary data and outputs the result of this comparison.

Program returns incorrect result. Doesn't happen when -O1 or lower is used. The
test code is probably specific to x86_64. AFAIK the code doesn't contain UB.

Steps to Reproduce:

Run the following script (see attachments):

#!/bin/bash
clang++ -O2 -std=c++11 testfail.cpp
echo -ne "clang: "
./a.out || echo -ne OK
echo -ne "\ngcc: "
g++ -O2 -std=c++11 testfail.cpp
./a.out || echo -ne OK

Actual Results:

Output:
clang:
gcc: OK

Expected Results:

Expected output:
clang: OK
gcc: OK

Build Date & Hardware:

clang version 7.0.1 (tags/RELEASE_701/final)
x86_64
Linux archlinux 4.20.0-arch1-1-ARCH

Additional Builds and Platforms:

Checked with clang version 7.0.0 with the same result.

Additional Info:

Checked with -std=c++14 and -std=c++17 with the same result.

Test sample contains commented code with slightly different comparisons. Only
one of them results in correct code generation.

godbolt.org outputs trivial main returning constant for versions 5, 6, 7 and
trunk.

[Quuxplusone]

Attached testfail.cpp (1691 bytes, text/x-c++src): Test program source

[Quuxplusone]

Attached run.sh (160 bytes, application/x-shellscript): Build & run script reproducing problem

[Quuxplusone]

Adding -fno-strict-aliasing seems to lead to the behavior you expect. Is it legal to alias a char array with a pointer of type mapped_data_t?

[Quuxplusone]

(In reply to Ignat Loskutov from comment #2)
> Adding -fno-strict-aliasing seems to lead to the behavior you expect. Is it
> legal to alias a char array with a pointer of type mapped_data_t?

Yes test works as expected with -fno-strict-aliasing.

After further investigation and discussion with colleagues I admit that
strictly speaking test code contains UB (even several UBs of different types,
see http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0593r2.html).

But code patterns similar to type punning in test code is a common practice
when working with mapped data. Libraries like FlatBuffers doesn't require -fno-
strict-aliasing (https://stackoverflow.com/questions/24330925/does-flatbuffers-
avoid-strict-aliasing-somehow).

Exploiting UB in this case is something that I wouldn't expect from compiler,
because I have no language means to barrier optimizer. I think it's wrong to
force usage of -fno-strict-aliasing on all code when only fraction of it is
liable for UB.