eb0e1978df7b9e7 caused msan false positive in vectorized crc code

[nico]

Bugzilla Link	44652
Resolution	FIXED
Resolved on	Jan 27, 2020 06:19
Version	trunk
OS	Linux
Blocks	#43900
Attachments	crc_folding.c standalone repro
CC	@topperc,@eugenis,@zmodem,@RKSimon,@rotateright

Extended Description

This is very similar to issue 42982, but in slightly different vectorized crc code in zlib.

As far as I can tell, no uninitialized reads happen here, and things are happy before eb0e197

bin/clang crc_folding.c -msse4.2 -mpclmul -g -O2 -fsanitize=memory && ./a.out
==135688==WARNING: MemorySanitizer: use-of-uninitialized-value
#​0 0x499549 in crc_fold_512to32 /usr/local/google/home/thakis/src/chrome/src/crc_folding.c:479:16
#​1 0x499549 in main /usr/local/google/home/thakis/src/chrome/src/crc_folding.c:514:10
#​2 0x7f706ab6f52a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2352a)
#​3 0x41f269 in _start (/usr/local/google/home/thakis/src/chrome/src/a.out+0x41f269)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /usr/local/google/home/thakis/src/chrome/src/crc_folding.c:479:16 in crc_fold_512to32

(reduced from https://crbug.com/1045291)

[nico]

assigned to @eugenis

[RKSimon]

Do you have the ir output before/after eb0e197 please?

[nico]

ir at c2443155a0fb245c8f17f2c1c72b6ea391e86e81
I have it at efb130f and c244315 (which are from late Nov 2019 and from trunk as of yesterday), if that's useful.

[nico]

ir at efb130fc93059bdf02d3a83950ddabc5d119f9de

[RKSimon]

Ideally we'd compare eb0e197 and 1737cc7 (the commit directly before)

[nico]

Well, the repro steps are in comment 0. It's a standalone repro.

If it takes you a while to investigate, can we revert in the meantime?

[nico]

Actually, one of my linux boxes isn't doing anything right now, let me try and build at / right before that rev. Should have ll before/after files in about 15 min.

[nico]

ir at eb0e1978df7b9e7

[nico]

ir at eb0e1978df7b9e7^ == 1737cc750c4

[nico]

Hm, that IR looks pretty similar. I produced it with

../../llvm-build-pr/bin/clang crc_folding.c -msse4.2 -mpclmul -g -O2 -fsanitize=memory -S -emit-llvm -o ir_before.ll

I did check that the problem repros in the ir_after.ll case but doesn't in the ir_before.ll case, and the version lines in the .ll files looks right, so I don't think I got -o flags confused.

Should I use a different command to get ll contents?

Maybe the msan runtime is instrumented differently?

[nico]

Maybe the msan runtime is instrumented differently?

It's not. I'm using ninja clang msan compiler-rt-headers to build, and after moving forward across the change again, these are the only build steps that run:

llvm-build-pr$ time ninja clang msan compiler-rt-headers -j1000 | cat
[1/15] Generating VCSRevision.h
-- Found Git: /usr/bin/git (found version "2.25.0.341.g760bfbb309-goog")
[2/15] Generating VCSVersion.inc
-- Found Git: /usr/bin/git (found version "2.25.0.341.g760bfbb309-goog")
[3/15] Building CXX object lib/Object/CMakeFiles/LLVMObject.dir/IRSymtab.cpp.o
[4/15] Linking CXX static library lib/libLLVMObject.a
[5/15] Building CXX object tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Version.cpp.o
[6/15] Linking CXX static library lib/libclangBasic.a
[7/15] Linking CXX executable bin/clang-offload-bundler
[8/15] Linking CXX executable bin/clang-offload-wrapper
[9/15] Building CXX object lib/LTO/CMakeFiles/LLVMLTO.dir/ThinLTOCodeGenerator.cpp.o
[10/15] Building CXX object lib/LTO/CMakeFiles/LLVMLTO.dir/LTO.cpp.o
[11/15] Linking CXX static library lib/libLLVMLTO.a
[12/15] Building CXX object lib/CodeGen/SelectionDAG/CMakeFiles/LLVMSelectionDAG.dir/TargetLowering.cpp.o
[13/15] Linking CXX static library lib/libLLVMSelectionDAG.a
[14/15] Linking CXX executable bin/clang-10
[15/15] Creating executable symlink bin/clang

So the msan runtime doesn't change.

The IR produced by clang is identical. But the change is in selectiondag which I think runs after IR (?), so I probably need to run a different command to dump what you want. Please let me know what you need :)

[RKSimon]

Cheers Nico

[RKSimon]

I haven't worked out what's caused this yet but I'm seeing this IR:

%_msprop122 = shufflevector <16 x i8> <i8 undef, i8 undef, i8 undef, i8 undef, i8 undef, i8 undef, i8 undef, i8 undef, i8 undef, i8 undef, i8 undef, i8 undef, i8 0, i8 0, i8 0, i8 0>, <16 x i8> %_msprop120, <16 x i32> <i32 12, i32 13, i32 14, i32 15, i32 16, i32 17, i32 18, i32 19, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef, i32 undef>, !dbg !​497

%52 = bitcast <16 x i8> %_msprop122 to i128, !dbg !​498

%_mscmp135 = icmp eq i128 %52, 0, !dbg !​498

So we're creating %_msprop122 with the upper half elements set to undef, this gets bitcast to a i128 (so an entire HALF of the bits is undef) and then compared against zero, this can't be good......

x86_64 lowering legalizes i128 to <2 x i64> and the extract of the upper i64 just returns an undef scalar after which the compare is completely broken.

SimplifyMultipleUseDemandedBits is doing what its supposed to - so it looks like whatever has created that shuffle to blame?

[eugenis]

It looks like MSan needs special handling of pclmul intrinsics.

W/o knowing about pclmul MSan does the default thing: checks that all arguments are fully initialized.

[RKSimon]

If we can't get msan to work, disabling the clmul cases in InstCombiner::visitCallInst avoids this issue.