google / benchmark

A microbenchmark support library
Apache License 2.0
8.94k stars 1.62k forks source link

reenable msan sanitizer workflow #1589

Closed dmah42 closed 1 year ago

LebedevRI commented 1 year ago

Yup, i do not see where you custom-build libc++. That is really necessary for MSan. Roughly, you need

$ git clone --depth=1 https://github.com/llvm/llvm-project
$ mkdir <llvm-build> && cd <llvm-build>
$ cmake -GNinja ../llvm-project/runtimes/ -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DLLVM_USE_SANITIZER=MemoryWithOrigins
$ ninja cxx cxxabi unwind
$ cd benchmark-build-dir
$ CC=clang CXX=clang++ CXXFLAGS="-fsanitize=memory -fsanitize-memory-track-origins -O1 -fno-omit-frame-pointer -fno-optimize-sibling-calls -stdlib=libc++ -L<llvm-build>/lib -lc++abi -Isystem<llvm-build>/include -Isystem<llvm-build>/include/c++/v1 -Wl,-rpath,<llvm-build>/lib" cmake <...> ..
dmah42 commented 1 year ago

something like https://github.com/google/benchmark/blob/main/.github/libcxx-setup.sh ?

we used to have this, but with it the sanitizer workflow timed out because the libc++ build is way too slow. maybe it's worth trying just for msan.

LebedevRI commented 1 year ago

something like https://github.com/google/benchmark/blob/main/.github/libcxx-setup.sh ?

Close, but my snippet is the more modern version of that incantation.

we used to have this, but with it the sanitizer workflow timed out because the libc++ build is way too slow. maybe it's worth trying just for msan.

Unless the whole LLVM gets built accidentally, i don't believe that should be slow at all, maybe 10 extra minutes.

dmah42 commented 1 year ago

note: LLVM build is at 30% after 12m.

dmah42 commented 1 year ago

msan debug failure is the same as when we didn't build libc++ with msan (unless i messed up the linking somehow)

LebedevRI commented 1 year ago

msan debug failure is the same as when we didn't build libc++ with msan (unless i messed up the linking somehow)

You have noticed that the libc++ build itself failed, right?

dmah42 commented 1 year ago

msan debug failure is the same as when we didn't build libc++ with msan (unless i messed up the linking somehow)

You have noticed that the libc++ build itself failed, right?

gah, no i didn't. i guess the script should return an error status.

dmah42 commented 1 year ago

ok it builds much faster (yay) but still fails with the same error in sysinfo.

LebedevRI commented 1 year ago

ok it builds much faster (yay) but still fails with the same error in sysinfo.

Nice! I'd suggest to drop the install stuff, and explicitly add -stdlib=libc++ -L<llvm-build>/lib -lc++abi -Isystem<llvm-build>/include -Isystem<llvm-build>/include/c++/v1 -Wl,-rpath,<llvm-build>/lib to CXXFLAGS.

dmah42 commented 1 year ago

welp. msan passes, now asan is failing. it looks like an odr violation in libcxx so i tried suppression but failed. concerned that it is odr violation but coming from something in our code (the output tests maybe?)

LebedevRI commented 1 year ago

Could you try passing -DBUILD_SHARED_LIBS=OFF (!) for sanitizer build jobs?

LebedevRI commented 1 year ago

Ok, final guess: add -fsanitize-address-use-odr-indicator to global CXXFLAGS, when building both the libc++ and benchmark.

LebedevRI commented 1 year ago

@dmah42 sorry for so much back-n-forth here.

dmah42 commented 1 year ago

@dmah42 sorry for so much back-n-forth here.

no problem. I'm at a loss so I'm happy for guidance.

LebedevRI commented 1 year ago

Ok, final guess: add -fsanitize-address-use-odr-indicator to global CXXFLAGS, when building both the libc++ and benchmark.

LebedevRI commented 1 year ago

I see. Do we really need to custom-build libc++ for non-msan? Because that isn't really necessary. Can we just not do that instead?

dmah42 commented 1 year ago

I see. Do we really need to custom-build libc++ for non-msan? Because that isn't really necessary. Can we just not do that instead?

they're all passing in the current state.. maybe just don't use the custom libc++ for asan (as it seems to have the issue)? or find the way to suppress the warning correctly.

LebedevRI commented 1 year ago

I see. Do we really need to custom-build libc++ for non-msan? Because that isn't really necessary. Can we just not do that instead?

they're all passing in the current state.. maybe just don't use the custom libc++ for asan (as it seems to have the issue)? or find the way to suppress the warning correctly.

That's what i'm suggesting, yes.

dmah42 commented 1 year ago

thanks for all the help!