Release mode clang crashes with llvm-ir input

[dobbelaj-snps]

Bugzilla Link	44896
Resolution	FIXED
Resolved on	Feb 28, 2020 05:11
Version	10.0
OS	Windows NT
Blocks	#43900
CC	@DimitryAndric,@zmodem

Extended Description

A release build of clang (top-of-tree; llvm-10) crashes with following llvm-ir input:

---

define linkonce i8* @​b(i8* %a)
{
ret i8* %a
}

and a command line like: clang -O3 S -emit-llvm test.ll

Also see: https://www.godbolt.org/z/7NjLKP

Note: clang-9 was fine.

[dobbelaj-snps]

The problem is related to '-fdiscard-value-names' and '-fno-discard-value-names'.

It seems that by default, a Release build enables '-fdiscard-value-names'.
This results in the following behavior:

Function::~Function():

• first calls 'clearArguments'. This sets the names for the arguments to ""

• the Value::setNameImpl(...)' has a shortcut on 'getContext().shouldDiscardValueNames()' and does nothing

• later on, the destructor of Function calls the destructor of its member variable:
  Function::SymTab
  which expects to be empty. But because of the shortcut, it still contains the Value's of the arguments, and later on results in a crash.

[dobbelaj-snps]

The culprit is

---

60d3947 is the first bad commit
commit 60d3947
Author: Rong Xu xur@google.com
Date: Tue Jan 14 15:33:15 2020 -0800

[remark][diagnostics] Using clang diagnostic handler for IR input files

[...]

Differential Revision: https://reviews.llvm.org/D72523

---

[llvmbot]

I cannot reproduce this. I built release compiles with and without -fdiscard-value-names. I used both compilers to run the test and it passed without any problem.

Please list the detailed instructions to reproduce the issue.

[dobbelaj-snps]

I get this behavior in the following way:

OS: Linux (Red Hat Enterprise 6.6)
tested compiler : gcc-7.3.0 and gcc-9.1.0
cmake-3.13.2 configuration:
mkdir BUILD-gcc9
cd BUILD-gcc9
cmake ../llvm -G Ninja -DCLANG_ENABLE_ARCMT=OFF -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON -DCMAKE_EXE_LINKER_FLAGS=-Wl,-z,origin,--disable-new-dtags,--rpath,$ORIGIN -DLLVM_ENABLE_TERMINFO=OFF -DLLVM_ENABLE_PROJECTS="clang" -DLLVM_ENABLE_ASSERTIONS:BOOL=OFF -DCMAKE_CXX_COMPILER=g++ -DCMAKE_C_COMPILER=gcc -DCMAKE_INSTALL_PREFIX:PATH=$PWD/../install -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD=all

Build command:
ninja clang

How to verify:
--- test.ll ---
define linkonce i8* @​b(i8* %a) {
ret i8* %a
}

../BUILD-gcc9/bin/clang -O3 -emit-llvm -S test.ll -o -

output:
warning: overriding the module target triple with x86_64-unknown-linux-gnu [-Woverride-module]
*** glibc detected *** ../BUILD-gcc9/bin/clang: double free or corruption (fasttop): 0x0000000006c13c00 ***
[.....]
Stack dump:
0. Program arguments: ../BUILD-gcc9/bin/clang -O3 -emit-llvm -S test.ll -o -

1.  Per-module optimization passes
   

2.  Running pass 'Global Variable Optimizer' on module 'test.ll'.
   

../BUILD-gcc9/bin/clang(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamE+0x1a)[0x255146a]
../BUILD-gcc9/bin/clang(_ZN4llvm3sys17RunSignalHandlersEv+0x34)[0x254f214]
../BUILD-gcc9/bin/clang(_ZN4llvm3sys15CleanupOnSignalEm+0xa5)[0x254f455]
../BUILD-gcc9/bin/clang[0x24d0968]
/lib64/libpthread.so.0(+0xf7e0)[0x7ffff7bce7e0]
/lib64/libc.so.6(gsignal+0x35)[0x7ffff69b3495]
/lib64/libc.so.6(abort+0x175)[0x7ffff69b4c75]
/lib64/libc.so.6(+0x703a7)[0x7ffff69f13a7]
/lib64/libc.so.6(+0x75dee)[0x7ffff69f6dee]
/lib64/libc.so.6(+0x78c3d)[0x7ffff69f9c3d]
../BUILD-gcc9/bin/clang(_ZN4llvm16ValueSymbolTableD1Ev+0x3c)[0x1f2d35c]
../BUILD-gcc9/bin/clang(_ZN4llvm8FunctionD1Ev+0x38)[0x1e9e378]
../BUILD-gcc9/bin/clang(_ZN4llvm8Function15eraseFromParentEv+0x47)[0x1e9e457]

Using valgrind:

valgrind ../BUILD-gcc9/bin/clang -O3 -emit-llvm -S test.ll -o -

output:
==35799== Memcheck, a memory error detector
==35799== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==35799== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==35799== Command: ../BUILD-gcc9/bin/clang -O3 -emit-llvm -S test.ll -o -
==35799==
warning: overriding the module target triple with x86_64-unknown-linux-gnu [-Woverride-module]
==35799== Invalid free() / delete / delete[] / realloc()
==35799== at 0x77C2430: free (vg_replace_malloc.c:446)
==35799== by 0x1F2D35B: llvm::ValueSymbolTable::~ValueSymbolTable() (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1E9E377: llvm::Function::~Function() (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1E9E456: llvm::Function::eraseFromParent() (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1FDAEC6: deleteIfDead(llvm::GlobalValue&, llvm::SmallPtrSetImpl<llvm::Comdat const*>&) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1FE2D14: optimizeGlobalsInModule(llvm::Module&, llvm::DataLayout const&, llvm::function_ref<llvm::TargetLibraryInfo& (llvm::Function&)>, llvm::function_ref<llvm::TargetTransformInfo& (llvm::Function&)>, llvm::function_ref<llvm::BlockFrequencyInfo& (llvm::Function&)>, llvm::function_ref<llvm::DominatorTree& (llvm::Function&)>) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1FE50E8: (anonymous namespace)::GlobalOptLegacyPass::runOnModule(llvm::Module&) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1EDF6C0: llvm::legacy::PassManagerImpl::run(llvm::Module&) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x278AE6B: (anonymous namespace)::EmitAssemblyHelper::EmitAssembly(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_deletellvm::raw_pwrite_stream >) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x278C8A6: clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayout const&, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_deletellvm::raw_pwrite_stream >) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x3242DE7: clang::CodeGenAction::ExecuteAction() [clone .part.320] (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x2CB24E0: clang::FrontendAction::Execute() (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== Address 0x8faf4d0 is 0 bytes inside a block of size 18 free'd
==35799== at 0x77C2430: free (vg_replace_malloc.c:446)
==35799== by 0x1F2AE25: llvm::Value::destroyValueName() (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1E96CC8: llvm::Function::clearArguments() (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1E9E35E: llvm::Function::~Function() (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1E9E456: llvm::Function::eraseFromParent() (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1FDAEC6: deleteIfDead(llvm::GlobalValue&, llvm::SmallPtrSetImpl<llvm::Comdat const*>&) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1FE2D14: optimizeGlobalsInModule(llvm::Module&, llvm::DataLayout const&, llvm::function_ref<llvm::TargetLibraryInfo& (llvm::Function&)>, llvm::function_ref<llvm::TargetTransformInfo& (llvm::Function&)>, llvm::function_ref<llvm::BlockFrequencyInfo& (llvm::Function&)>, llvm::function_ref<llvm::DominatorTree& (llvm::Function&)>) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1FE50E8: (anonymous namespace)::GlobalOptLegacyPass::runOnModule(llvm::Module&) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x1EDF6C0: llvm::legacy::PassManagerImpl::run(llvm::Module&) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x278AE6B: (anonymous namespace)::EmitAssemblyHelper::EmitAssembly(clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_deletellvm::raw_pwrite_stream >) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x278C8A6: clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayout const&, llvm::Module*, clang::BackendAction, std::unique_ptr<llvm::raw_pwrite_stream, std::default_deletellvm::raw_pwrite_stream >) (in XXXX/BUILD-gcc9/bin/clang-11)
==35799== by 0x3242DE7: clang::CodeGenAction::ExecuteAction() [clone .part.320] (in XXXX/BUILD-gcc9/bin/clang-11)
==35799==
; ModuleID = 'test.ll'
source_filename = "test.ll"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
1 warning generated.
==35799==
==35799== HEAP SUMMARY:
==35799== in use at exit: 614,635 bytes in 567 blocks
==35799== total heap usage: 10,710 allocs, 10,144 frees, 1,822,489 bytes allocated
==35799==
==35799== LEAK SUMMARY:
==35799== definitely lost: 69,152 bytes in 4 blocks
==35799== indirectly lost: 304,856 bytes in 554 blocks
==35799== possibly lost: 0 bytes in 0 blocks
==35799== still reachable: 240,627 bytes in 9 blocks
==35799== suppressed: 0 bytes in 0 blocks
==35799== Rerun with --leak-check=full to see details of leaked memory
==35799==
==35799== For counts of detected and suppressed errors, rerun with: -v
==35799== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 8 from 6)

I verified that the problem is still there with following version of top-of-tree:
commit 0dbc465
Author: Craig Topper craig.topper@gmail.com
Date: Mon Feb 17 23:21:48 2020 -0800

[llvmbot]

Got it. I can reproduce it now. Working on it.

[llvmbot]

Here is what happens:
(1) there is an assumption in Value::setNameImpl(...): if -fdiscard-value-names is enabled. Value names will not be created. This is the shortcut Jeroen mentioned in #​2.
(2) For IR input files, this option is effectively ignored (before my patch). We need named values in loadModule().
(3) In my patch, this option (-fdiscard-value-names) is set in LLVMContext in
CodeGeneratorImpl() when creating BackendConsumer (in clang/lib/CodeGen/ModuleBuilder.cpp:76). However, the names values have already created in loadModule() -- this option was not set before this points and setName() has already be called a number of times. This breaks the assumption and resulted in a segfault.
(4) If I set this option before loadModule(). I got the following error:
test.ll:1:1: error: Can't read textual IR with a Context that discards named Values

The fix I plan to have is to add setDiscardValueNames(false) to IR input files after creating the fake BackendConsumer().

[llvmbot]

https://reviews.llvm.org/D74878

is out for review.

[zmodem]

I believe this is also the cause of test-suite bitcode tests failing during release testing.

[zmodem]

*** Bug llvm/llvm-bugzilla-archive#44763 has been marked as a duplicate of this bug. ***

[zmodem]

https://reviews.llvm.org/D74878 landed on master as 11857d4 and was pushed to 10.x as 6f4f4f2.