search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28k stars 11.56k forks source link

-fsanitize-coverage=inline-8bit-counters + ThinLTO = lld crash #41079

Closed LebedevRI closed 3 years ago

LebedevRI commented 5 years ago
Bugzilla Link 41734
Resolution FIXED
Resolved on Sep 04, 2021 05:14
Version trunk
OS Linux
CC @zmodem,@MaskRay,@kcc,@morehouse,@hctim,@pcc,@petrhosek

Extended Description

I know that no one will bother to look at the bug if there isn't a simple self-contained reproducer, but i'm not sure how to create it here :S

This bug appears to originate from -fsanitize-coverage=inline-8bit-counters, and only happens if ThinLTO (and lld) is used. I can't tell if this is LLD bug, or instrumentation bug.

The observable effect of the bug:

ld.lld: /build/llvm/include/llvm/MC/MCSymbol.h:267: llvm::MCSection& llvm::MCSymbol::getSection() const: Assertion `isInSection() && "Invalid accessor!"' failed. Stack dump:

  1. Program arguments: /build/llvm-build-GCC-release/bin/ld.lld --hash-style=both --build-id --eh-frame-hdr -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2 -o fuzz/librawspeed/decompressors/VC5DecompressorFuzzer /usr/lib64/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu/crt1.o /usr/lib64/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu/crti.o /usr/lib64/gcc/x86_64-linux-gnu/8/crtbegin.o -L/usr/lib64/gcc/x86_64-linux-gnu/8 -L/usr/lib64/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu -L/usr/lib64/gcc/x86_64-linux-gnu/8/../../../../lib64 -L/lib/x86_64-linux-gnu -L/lib/../lib64 -L/usr/lib/x86_64-linux-gnu -L/usr/lib/../lib64 -L/usr/lib/x86_64-linux-gnu/../../lib64 -L/usr/lib64/gcc/x86_64-linux-gnu/8/../../.. -L/usr/lib/llvm-9/bin/../lib -L/lib -L/usr/lib -plugin /usr/lib/llvm-9/bin/../lib/LLVMgold.so -plugin-opt=mcpu=x86-64 -plugin-opt=O3 -plugin-opt=thinlto --whole-archive /usr/lib/llvm-9/lib/clang/9.0.0/lib/linux/libclang_rt.ubsan_standalone-x86_64.a --no-whole-archive --dynamic-list=/usr/lib/llvm-9/lib/clang/9.0.0/lib/linux/libclang_rt.ubsan_standalone-x86_64.a.syms --whole-archive /usr/lib/llvm-9/lib/clang/9.0.0/lib/linux/libclang_rt.ubsan_standalone_cxx-x86_64.a --no-whole-archive --dynamic-list=/usr/lib/llvm-9/lib/clang/9.0.0/lib/linux/libclang_rt.ubsan_standalone_cxx-x86_64.a.syms --as-needed --thinlto-cache-dir=/home/lebedevri/rawspeed/build/clang-thinlto-cache --thinlto-cache-policy cache_size_bytes=1g fuzz/librawspeed/decompressors/CMakeFiles/VC5DecompressorFuzzer.dir/VC5Decompressor.cpp.o librawspeed.a fuzz/librawspeed_fuzz.a librawspeed.a /usr/lib/x86_64-linux-gnu/libpugixml.so /usr/lib/x86_64-linux-gnu/libjpeg.so /usr/lib/x86_64-linux-gnu/libz.so /usr/lib/llvm-9/lib/libomp.so -lstdc++ -lm --no-as-needed -lpthread -lrt -lm -ldl -lgcc_s -lgcc -lc -lc -lgcc_s -lgcc /usr/lib64/gcc/x86_64-linux-gnu/8/crtend.o /usr/lib64/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu/crtn.o

    ​0 0x00007f2dd1bbaaca llvm::sys::PrintStackTrace(llvm::raw_ostream&) /build/llvm/lib/Support/Unix/Signals.inc:494:22

    ​1 0x00007f2dd1bb88f4 llvm::sys::RunSignalHandlers() /build/llvm/lib/Support/Signals.cpp:68:20

    ​2 0x00007f2dd1bb8a55 SignalHandler(int) /build/llvm/lib/Support/Unix/Signals.inc:357:1

    ​3 0x00007f2dd19b2730 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x12730)

    ​4 0x00007f2dd112f7bb raise (/lib/x86_64-linux-gnu/libc.so.6+0x377bb)

    ​5 0x00007f2dd111a535 abort (/lib/x86_64-linux-gnu/libc.so.6+0x22535)

    ​6 0x00007f2dd111a40f (/lib/x86_64-linux-gnu/libc.so.6+0x2240f)

    ​7 0x00007f2dd1128102 (/lib/x86_64-linux-gnu/libc.so.6+0x30102)

    ​8 0x00007f2dcf8092e0 llvm::MCSymbol::getVariableValue(bool) const /build/llvm/include/llvm/MC/MCSymbol.h:300:12

    ​9 0x00007f2dcf8092e0 llvm::MCSymbol::getFragment(bool) const /build/llvm/include/llvm/MC/MCSymbol.h:387:65

    ​10 0x00007f2dcf8092e0 llvm::MCSymbol::getFragment(bool) const /build/llvm/include/llvm/MC/MCSymbol.h:383:15

    ​11 0x00007f2dcf8092e0 llvm::MCSymbol::isUndefined(bool) const /build/llvm/include/llvm/MC/MCSymbol.h:257:23

    ​12 0x00007f2dcf8092e0 llvm::MCSymbol::isDefined() const /build/llvm/include/llvm/MC/MCSymbol.h:247:47

    ​13 0x00007f2dcf8092e0 llvm::MCSymbol::isInSection() const /build/llvm/include/llvm/MC/MCSymbol.h:252:21

    ​14 0x00007f2dcf8092e0 llvm::MCSymbol::getSection() const /build/llvm/include/llvm/MC/MCSymbol.h:267:5

    ​15 0x00007f2dcf810c36 llvm::isa_impl_cl<llvm::MCSectionELF, llvm::MCSection const>::doit(llvm::MCSection const) /build/llvm/include/llvm/Support/Casting.h:105:5

    ​16 0x00007f2dcf810c36 llvm::isa_impl_wrap<llvm::MCSectionELF, llvm::MCSection const, llvm::MCSection const>::doit(llvm::MCSection const* const&) /build/llvm/include/llvm/Support/Casting.h:132:40

    ​17 0x00007f2dcf810c36 llvm::isa_impl_wrap<llvm::MCSectionELF, llvm::MCSection const, llvm::MCSection const>::doit(llvm::MCSection* const&) /build/llvm/include/llvm/Support/Casting.h:123:60

    ​18 0x00007f2dcf810c36 bool llvm::isa<llvm::MCSectionELF, llvm::MCSection>(llvm::MCSection const&) /build/llvm/include/llvm/Support/Casting.h:143:74

    ​19 0x00007f2dcf810c36 llvm::cast_retty<llvm::MCSectionELF, llvm::MCSection>::ret_type llvm::cast<llvm::MCSectionELF, llvm::MCSection>(llvm::MCSection) /build/llvm/include/llvm/Support/Casting.h:264:3

    ​20 0x00007f2dcf810c36 writeSection /build/llvm/lib/MC/ELFObjectWriter.cpp:1031:49

    ​21 0x00007f2dcf810c36 writeSectionHeader /build/llvm/lib/MC/ELFObjectWriter.cpp:1067:17

    ​22 0x00007f2dcf810c36 (anonymous namespace)::ELFWriter::writeObject(llvm::MCAssembler&, llvm::MCAsmLayout const&) (.constprop.465) /build/llvm/lib/MC/ELFObjectWriter.cpp:1224:21

    ​23 0x00007f2dcf811b37 std::vector<llvm::MCSectionELF const, std::allocator<llvm::MCSectionELF const> >::~vector() /usr/include/c++/8/bits/stl_vector.h:567:15

    ​24 0x00007f2dcf811b37 ~ELFWriter /build/llvm/lib/MC/ELFObjectWriter.cpp:101:8

Reproduction is easy and hard at the same time. I'm not sure how to write a minimal reproducer by hand, and i'm not sure how to reduce the existing "reproducer". Just to star somewhere, i'll state the whole reproduction process: $ git clone https://github.com/darktable-org/rawspeed.git $ cd rawspeed && mkdir build && cd build $ CC=clang CXX=clang++ CFLAGS="-fsanitize-coverage=inline-8bit-counters" CXXFLAGS="-fsanitize-coverage=inline-8bit-counters" cmake -DCMAKE_BUILD_TYPE=Release ../ -DRAWSPEED_ENABLE_LTO=ON -GNinja && ninja VC5DecompressorFuzzer $ # you may want to pass some cmake options to disable some (or all!) deps, although there is nothing heavy. See https://github.com/google/oss-fuzz/blob/ae9398deefdf485c50bb625a20fd44a2def49418/projects/librawspeed/build.sh#L32-L38 $ [80/80] Linking CXX executable fuzz/librawspeed/decompressors/VC5DecompressorFuzzer FAILED: fuzz/librawspeed/decompressors/VC5DecompressorFuzzer : && /usr/local/bin/clang++ -fsanitize-coverage=inline-8bit-counters -std=c++14 -flto=thin -fforce-emit-vtables -fwhole-program-vtables -fstrict-vtable-pointers -Wall -Wextra -Weverything -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-conversion -Wno-covered-switch-default -Wno-deprecated -Wno-double-promotion -Wno-exit-time-destructors -Wno-global-constructors -Wno-gnu-zero-variadic-macro-arguments -Wno-old-style-cast -Wno-padded -Wno-switch-enum -Wno-unused-macros -Wno-unused-parameter -Wno-weak-vtables -Wno-zero-as-null-pointer-constant -Wextra-semi -Wframe-larger-than=4096 -Wlarger-than=32768 -O3 -DNDEBUG -O3 -Wl,--as-needed -flto=thin -fuse-ld="/usr/local/bin/ld.lld" -Wl,--thinlto-cache-dir="/home/lebedevri/rawspeed/build/clang-thinlto-cache" -Wl,--thinlto-cache-policy,cache_size_bytes=1g fuzz/librawspeed/decompressors/CMakeFiles/VC5DecompressorFuzzer.dir/VC5Decompressor.cpp.o -o fuzz/librawspeed/decompressors/VC5DecompressorFuzzer librawspeed.a fuzz/librawspeed_fuzz.a librawspeed.a /usr/lib/x86_64-linux-gnu/libpugixml.so /usr/lib/x86_64-linux-gnu/libjpeg.so /usr/lib/x86_64-linux-gnu/libz.so /usr/lib/llvm-9/lib/libomp.so && : ld.lld: error: lto.tmp: invalid sh_link index: 0 clang: error: linker command failed with exit code 1 (use -v to see invocation) ninja: build stopped: subcommand failed.

LebedevRI commented 3 years ago

I can no longer reproduce. I believe this has been either fixed or masked. Thanks!

zmodem commented 4 years ago

Okay, thanks for checking.

LebedevRI commented 4 years ago

Roman, did you have a chance to test this? It would be nice to close the bug if the commits above fixed this.

I can't comment whether or not those helped, but i'm still seeing compiler crashes with that config, so i'm not sure.

zmodem commented 4 years ago

Roman, did you have a chance to test this? It would be nice to close the bug if the commits above fixed this.

LebedevRI commented 4 years ago

D72899 [MC] Set sh_link to 0 if the associated symbol is undefined and D72904 [ELF] Allow SHF_LINK_ORDER sections to have sh_link=0

have been pushed.

Thank you! I will check if that resolves the original issue.

MaskRay commented 4 years ago

D72899 [MC] Set sh_link to 0 if the associated symbol is undefined and D72904 [ELF] Allow SHF_LINK_ORDER sections to have sh_link=0

have been pushed.

MaskRay commented 4 years ago

I don't think it is a suitable blocker for LLVM 10. This is a longstanding issue since !associated was created.

See https://reviews.llvm.org/D29110 : people had concerns with the proposed semantics of !associated

It breaks many people's belief: "an optimization can drop a metadata it does not understand".

zmodem commented 4 years ago

This does not seem to be getting fixed for clang 10, and I don't think we can block on it.

LebedevRI commented 4 years ago

I cannot reproduce the original failure.

Please can you try adding the flags i recommended in the original comment: -DWITH_PUGIXML=OFF -DUSE_XMLLINT=OFF -DWITH_JPEG=OFF -DWITH_ZLIB=OFF

MaskRay commented 4 years ago

I cannot reproduce the original failure.

% rm -r Release; PATH=~/llvm/Release/bin:$PATH cmake -DCMAKE_BUILD_TYPE=Release -GNinja -H. -BRelease -DCMAKE_C_COMPILER=~/llvm/Release/bin/clang -DCMAKE_CXX_COMPILER=~/llvm/Release/bin/clang++ -DCMAKE_C_FLAGS=-fsanitize-coverage=inline-8bit-counters -DCMAKE_CXX_FLAGS=-fsanitize-coverage=inline-8bit-counters -DBUILD_TESTING=off -DUSE_BUNDLED_PUGIXML=ON -DALLOW_DOWNLOADING_PUGIXML=on -DWITH_OPENMP=off; ninja -C Release # succeeded

% rm -r Release; PATH=~/llvm/Release/bin:$PATH cmake -DCMAKE_BUILD_TYPE=Release -GNinja -H. -BRelease -DCMAKE_C_COMPILER=~/llvm/Release/bin/clang -DCMAKE_CXX_COMPILER=~/llvm/Release/bin/clang++ -DCMAKE_C_FLAGS=-fsanitize-coverage=inline-8bit-counters -DCMAKE_CXX_FLAGS=-fsanitize-coverage=inline-8bit-counters -DBUILD_TESTING=off -DUSE_BUNDLED_PUGIXML=ON -DALLOW_DOWNLOADING_PUGIXML=on -DWITH_OPENMP=off -DRAWSPEED_ENABLE_LTO=on ... -- Checking prototype uncompress for HAVE_ZLIB_UNCOMPRESS_PROTOTYPE - False [60/1631] CMake Error at cmake/Modules/CheckZLIB.cmake:37 (message): Found unexpected prototype for uncompress() in Call Stack (most recent call first): cmake/src-dependencies.cmake:172 (include) CMakeLists.txt:226 (include)

-- Checking prototype zError for HAVE_ZLIB_ZERROR_PROTOTYPE - False CMake Error at cmake/Modules/CheckZLIB.cmake:46 (message): Found unexpected prototype for zError() in Call Stack (most recent call first): cmake/src-dependencies.cmake:172 (include) CMakeLists.txt:226 (include)

I have seen the ZLIB error a few months ago. Maybe a build system problem.

Reuse

% rm -r Release; PATH=~/llvm/Release/bin:$PATH cmake -DCMAKE_BUILD_TYPE=Release -GNinja -H. -BRelease -DCMAKE_C_COMPILER=~/llvm/Release/bin/clang -DCMAKE_CXX_COMPILER=~/llvm/Release/bin/clang++ -DCMAKE_C_FLAGS=-fsanitize-coverage=inline-8bit-counters -DCMAKE_CXX_FLAGS=-fsanitize-coverage=inline-8bit-counters -DBUILD_TESTING=off -DUSE_BUNDLED_PUGIXML=ON -DALLOW_DOWNLOADING_PUGIXML=on -DWITH_OPENMP=off % edit Release/CMakeCache.txt # change RAWSPEED_ENABLE_LTO to ON ... [188/291] Linking CXX executable fuzz/librawspeed/decoders/TiffDecoders/TiffDecoderFuzzer-ThreefrDecoder ... ld.lld: error: duplicate symbol: rawspeed::TiffParser::Map

defined in librawspeed.a(TiffParser.cpp.o) defined in librawspeed.a(TiffParser.cpp.o)

ld.lld: error: duplicate symbol: typeinfo name for rawspeed::TiffParser

defined in librawspeed.a(TiffParser.cpp.o) defined in librawspeed.a(TiffParser.cpp.o)

MaskRay commented 5 years ago

Was there any progress here?

No, but saw another related issue:) llvm/llvm-bugzilla-archive#43147

LebedevRI commented 5 years ago

Was there any progress here?

pcc commented 5 years ago

I would favour the solution of creating a dummy section to associate using SHF_LINK_ORDER if the associated metadata is unresolvable. It seems like this would need to be a non-GCable (e.g. .init_array) section in order to ensure that the associated section is always kept since we don't really know whether it is valid in general to drop the section.

MaskRay commented 5 years ago

-fsanitize-coverage-inline-8bbit-counters creates a @​sancovgen and a section sancov_cntrs for each instrumented function.

@&#8203;__sancov_gen_ = private global [3 x i8] zeroinitializer, section "__sancov_cntrs", comdat($foo), align 1, !associated !&#8203;0
!&#8203;0 = !{i32 (i8*, i8*)* @&#8203;foo}
; __sancov_gen_ is in llvm.compiler.used

!associated lowers to SHF_LINK_ORDER on ELF platforms. The created __sancov_cntrs section has its sh_link referencing .text.foo (the function section). With some LTO optimizations (deadargelim,inline,...), the function foo may be optimized out, the metadata will become null and the sh_link field will be 0. Linkers reject SHF_LINK_ORDER sections with a 0 sh_link.

!&#8203;0 = !{null}

A sh_link!=0 check is missed somewhere. I don't know what we should do if sh_link is 0.

One solution I am thinking about is:

I'll take a closer look.