[PATCH] D135202: [IR] Add a target extension type to LLVM.

[Mitch Phillips via Phabricator via llvm-commits]

hctim added a comment.

Hey, looks like this patch may have introduced a memory leak, as reported below by ASan.

This is caught on my local machine, which doesn't use an asan-instrumented libcxx (like our bots do). This does result in some false negatives, so an instrumented libcxx is always a better option, but this is a nice fast build for me to reproduce failures and bisect faster.

The stack trace points to a std::string heap leak, and so my hunch was that the bots didn't pick up this failure due to the different point at which libcxx and libstdc++ stop inlining the string and push it off to the heap. That turned out to be correct, changing the test like so even makes it reproduce using an instrumented libcxx (and thus the bots upstream):

  diff --git a/llvm/test/Assembler/target-types.ll b/llvm/test/Assembler/target-types.ll
  index 741a1a159678..7497fa8806df 100644
  --- a/llvm/test/Assembler/target-types.ll
  +++ b/llvm/test/Assembler/target-types.ll
  @@ -1,7 +1,7 @@
   ; RUN: llvm-as < %s | llvm-dis | FileCheck %s
   ; Check support for basic target extension type usage
  
  - at global = global target("spirv.DeviceEvent") zeroinitializer
  + at global = global target("spirv.DeviceEventXXXXXX") zeroinitializer
  
   define target("spirv.Sampler") @foo(target("spirv.Sampler") %a) {
     ret target("spirv.Sampler") %a
  @@ -13,7 +13,7 @@ define target("spirv.Event") @func2() {
     ret target("spirv.Event") poison
   }
  
  -; CHECK: @global = global target("spirv.DeviceEvent") zeroinitializer
  +; CHECK: @global = global target("spirv.DeviceEventXXXXXX") zeroinitializer
   ; CHECK: define target("spirv.Sampler") @foo(target("spirv.Sampler") %a) {
   ; CHECK:   ret target("spirv.Sampler") %a
   ; CHECK: }

My invocation for the "fast asan build" and the breaking unit test which uses libstdc++ is below. This fails at e6b02214c68df2c9f826e02310c9352ac652e456 <https://reviews.llvm.org/rGe6b02214c68df2c9f826e02310c9352ac652e456> (this commit) but passes the commit before.

  $ cmake \
  -DLLVM_ENABLE_ASSERTIONS=ON \
  -DCMAKE_C_COMPILER=clang \
  -DCMAKE_CXX_COMPILER=clang++ \
  -DLLVM_USE_LINKER=lld \
  -GNinja \
  -DCMAKE_BUILD_TYPE=Release \
  -DCMAKE_C_FLAGS="-fsanitize=address" \
  -DCMAKE_CXX_FLAGS="-fsanitize=address" \
  -DLLVM_ENABLE_PROJECTS="'clang;lld;clang-tools-extra;mlir'" \
  -DLLVM_LIBC_ENABLE_LINTING=OFF \
  -DLLVM_USE_SANITIZER=Address \
  /llvm
  $ LIT_OPTS='--filter=Assembler/target-types.ll' ninja check-llvm
  Direct leak of 18 byte(s) in 1 object(s) allocated from:
      #0 0x557fdbae956d in operator new(unsigned long) /compiler-rt/lib/asan/asan_new_delete.cpp:95:3
      #1 0x557fdbeb4f33 in _M_construct<const char *> /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.tcc:225:14
      #2 0x557fdbeb4f33 in basic_string /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:620:2
      #3 0x557fdbeb4f33 in basic_string /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:188:9
      #4 0x557fdbeb4f33 in basic_string<llvm::StringRef, void> /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/basic_string.h:787:4
      #5 0x557fdbeb4f33 in llvm::TargetExtType::TargetExtType(llvm::LLVMContext&, llvm::StringRef, llvm::ArrayRef<llvm::Type*>, llvm::ArrayRef<unsigned int>) /llvm/lib/IR/Type.cpp:796:31
      #6 0x557fdbeb6104 in llvm::TargetExtType::get(llvm::LLVMContext&, llvm::StringRef, llvm::ArrayRef<llvm::Type*>, llvm::ArrayRef<unsigned int>) /llvm/lib/IR/Type.cpp:830:14
      #7 0x557fdbb8ce03 in parseTypeTableBody /llvm/lib/Bitcode/Reader/BitcodeReader.cpp:2514:18
      #8 0x557fdbb8ce03 in parseTypeTable /llvm/lib/Bitcode/Reader/BitcodeReader.cpp:2252:10
      #9 0x557fdbb8ce03 in (anonymous namespace)::BitcodeReader::parseModule(unsigned long, bool, llvm::function_ref<std::optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>> (llvm::StringRef)>) /llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4245:25
      #10 0x557fdbb056e3 in parseBitcodeInto /llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4476:10
      #11 0x557fdbb056e3 in llvm::BitcodeModule::getModuleImpl(llvm::LLVMContext&, bool, bool, bool, llvm::function_ref<std::optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>> (llvm::StringRef)>) /llvm/lib/Bitcode/Reader/BitcodeReader.cpp:7927:22
      #12 0x557fdbb09212 in llvm::BitcodeModule::getLazyModule(llvm::LLVMContext&, bool, bool) /llvm/lib/Bitcode/Reader/BitcodeReader.cpp:7946:10
      #13 0x557fdbaf1144 in main /llvm/tools/llvm-dis/llvm-dis.cpp:205:16
      #14 0x7f137e229209 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
  
  SUMMARY: AddressSanitizer: 18 byte(s) leaked in 1 allocation(s).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135202/new/

https://reviews.llvm.org/D135202