search

llvm / llvm-project

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

LLVM CommandLine category registration seems to have initialization order issues (triggers ASAN initialization-order-fiasco) #43568

Open stellaraccident opened 4 years ago

stellaraccident commented 4 years ago
Bugzilla Link 44223
Version unspecified
OS Linux
CC @Benvanik,@dwblaikie,@RKSimon,@stephenneuendorffer

Extended Description

When attempting to add an OptionCategory to https://github.com/google/iree (which uses MLIR and LLVM), we encountered an ASAN initialization-order-fiasco error on the assert in registerCategory(OptionCategory*). Our temporary fix was to remove the category, and I have not yet rolled any kind of repro.

The failure point is in the registerCategory function here: https://llvm.org/doxygen/CommandLine_8cpp_source.html

Looking at this code, I think the assert is not legal to be doing in a static initializer in this way, and it suffers from the initialization order issue when option categories are declared in multiple modules. It looks like other functions (like registerSubCommand()) could have a similar issue. Due to the specific way that StringRef equality is implemented, it seems like, while this is accessing uninitialized memory sometimes, that is just being interpreted as a false negative (versus a crash). It seems odd that ASAN hasn't triggered on this before, and I can't explain that. Also, since this is an assert, it only affects debug builds.

I think that a more appropriate way to do this would be to do the duplicate check on option parse (or equiv) versus static init, although I am not an expert on LLVM option initialization logic.

================================================================= ==7911==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x55d4407a2920 at pc 0x55d43e64a6b5 bp 0x7ffd24ae1c90 sp 0x7ffd24ae1c88 READ of size 8 at 0x55d4407a2920 thread T0

​0 0x55d43e64a6b4 in llvm::cl::OptionCategory::getName() const third_party/llvm/llvm-project/llvm/include/llvm/Support/CommandLine.h:202:38

#​1 0x55d43e63b2d2 in (anonymous namespace)::CommandLineParser::registerCategory(llvm::cl::OptionCategory*)::'lambda'(llvm::cl::OptionCategory const*)::operator()(llvm::cl::OptionCategory const*) const third_party/llvm/llvm-project/llvm/lib/Support/CommandLine.cpp:349:5
#&#8203;2 0x55d43e63b1e9 in std::__u::iterator_traits<llvm::SmallPtrSetIterator<llvm::cl::OptionCategory*> >::difference_type std::__u::count_if<llvm::SmallPtrSetIterator<llvm::cl::OptionCategory*>, (anonymous namespace)::CommandLineParser::registerCategory(llvm::cl::OptionCategory*)::'lambda'(llvm::cl::OptionCategory const*)>(llvm::SmallPtrSetIterator<llvm::cl::OptionCategory*>, llvm::SmallPtrSetIterator<llvm::cl::OptionCategory*>, (anonymous namespace)::CommandLineParser::registerCategory(llvm::cl::OptionCategory*)::'lambda'(llvm::cl::OptionCategory const*)) third_party/crosstool/v18/stable/toolchain/bin/../include/c++/v1/algorithm:1194:13
#&#8203;3 0x55d43e62ac24 in (anonymous namespace)::CommandLineParser::registerCategory(llvm::cl::OptionCategory*) third_party/llvm/llvm-project/llvm/lib/Support/CommandLine.cpp:349:5
#&#8203;4 0x55d438003a98 in __cxx_global_var_init third_party/iree/iree/compiler/Dialect/VM/Target/Bytecode/TranslationFlags.cpp:24:33
#&#8203;5 0x55d438004944 in _GLOBAL__sub_I_TranslationFlags.cpp third_party/iree/iree/compiler/Dialect/VM/Target/Bytecode/TranslationFlags.cpp
#&#8203;6 0x55d43eb7d305 in __do_global_ctors_aux (/build/cas/f31/f31220af7d2ad8992dd9fd55e959be23b8a18c8bdf24a983a8c3ccab24a13e12_020014762630+0xc0a1305)

0x55d4407a2920 is located 0 bytes inside of global variable 'mlir::iree_compiler::IREE::HAL::halTargetOptionsCategory' defined in 'third_party/iree/iree/compiler/Dialect/HAL/Target/ExecutableTarget.cpp:24:33' (0x55d4407a2920) of size 32 registered at:

​0 0x55d437e47ebd in __asan_register_globals third_party/llvm/llvm-project/compiler-rt/lib/asan/asan_globals.cpp:360:3

#&#8203;1 0x55d43db6b76b in asan.module_ctor (/build/cas/f31/f31220af7d2ad8992dd9fd55e959be23b8a18c8bdf24a983a8c3ccab24a13e12_020014762630+0xb08f76b)
#&#8203;2 0x55d43eb7ef56 in _init (/build/cas/f31/f31220af7d2ad8992dd9fd55e959be23b8a18c8bdf24a983a8c3ccab24a13e12_020014762630+0xc0a2f56)
#&#8203;3 0x7fe81dd32b4a in __libc_start_main (/usr/grte/v4/lib64/libc.so.6+0x38b4a)
#&#8203;4 0x55d437e1a228 in _start /usr/grte/v4/debug-src/src/csu/../sysdeps/x86_64/start.S:108

SUMMARY: AddressSanitizer: initialization-order-fiasco third_party/llvm/llvm-project/llvm/include/llvm/Support/CommandLine.h:202:38 in llvm::cl::OptionCategory::getName() const

850d4ab7-4143-43fb-923e-59c6acbb2120 commented 4 years ago

Correct; this has nothing to do with MLIR static dialect registration and is specifically about LLVM's CommandLineParser::registerCategory (which is using static initialization).

dwblaikie commented 4 years ago

Obsolete, since MLIR doesn't use static initialization anymore.

It's not an MLIR bug though (wasn't to begin with, I don't think) the claim seems to be that it's a general bug with the way LLVM's command line parsing/handling works.

Could/should we reopen this?

stephenneuendorffer commented 4 years ago

Obsolete, since MLIR doesn't use static initialization anymore.