wasm-ld crash when LTO enabled: __cxx_global_var_init.1 present in "init functions" but not defined
#62243
Comments
|
@llvm/issue-subscribers-backend-webassembly |
EugeneZelenko
added
lld:wasm
LTO
|
@llvm/issue-subscribers-lld-wasm |
|
Pasting this here as well for visibility from emscripten-core/emscripten#19774 . A potential repro is this: CMakeLists.txt: cmake_minimum_required(VERSION 3.13)
set(CMAKE_EXECUTABLE_SUFFIX_CXX .html)
set(CMAKE_CXX_STANDARD 17)
add_executable(lto_bug dummy.cpp)
add_library(main
main.cpp
)
add_library(lib1
lib1.cpp
)
target_link_libraries(lto_bug
PRIVATE
main
lib1
)
main.cpp int DoIt()
{
return 0;
}
inline int Unused0 = DoIt();
int GetUnused();
int main(int, char **)
{
return GetUnused();
}lib1.cpp: int DoIt();
inline int Unused0 = DoIt();
int GetUnused()
{
return Unused0;
}dummy.cpp: // Dummy file for cmake's add_executableCompilation commands: |
|
hi @EugeneZelenko, it's great that there is now a simple reproducer for this issue! cc @MaskRay |
This was referenced Nov 6, 2023
Closed
|
Here's another reproducer: cat <<EOT > foo.h
int foo();
inline int unused = foo(); // FIXME: Remove to fix the build
EOT
cat <<EOT > foo.cc
#include "foo.h"
int foo() {
return 42;
}
EOT
cat <<EOT > main.cc
#include "foo.h"
int main() {
return foo();
}
EOT
em++ -c -flto foo.cc -o foo.o
em++ -c -flto main.cc -o main.o
emar rcsT libfoo.a foo.o
emar rcsT libmain.a main.o
em++ libmain.a libfoo.a |
aheejin
added a commit
to aheejin/llvm-project
that referenced
this issue
Nov 22, 2023
tl;dr: When doing LTO on multiple archives, the order with which bitcodes are linked to the LTO module is hard to control, given that processing undefined symbols can lead to parsing of an object file, which in turn lead to parsing of another object file before finishing parsing of the previous file. This can result in encountering a non-prevailing comdat first when linking, which can make the the symbol undefined, and the real definition is added later with an additional prefix to avoid duplication (e.g. `__cxx_global_var_init` and `__cxx_global_var_init.2`) So this one-line fix ensures we compile bitcodes in the order that we process comdats, so that when multiple archived bitcode files have the same variable with the same comdat, we make sure that the prevailing comdat will be linked first in the LTO. Fixes llvm#62243. --- - Long version, feel free to skip: When linking (non-archived) bitcode files directly in LTO, we add files in `bitcodeFiles` vector in the order they are given in the command line: https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/Driver.cpp#L1201-L1204 -> https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/SymbolTable.cpp#L54 The order they are added in `bitcodeFiles` is the order they are compiled and linked into the LTO module later. While parsing these bitcode files, we also add symbols to the symbol table. Depending on their status, they are added as defined symbols or undefined symbols. --- On the other hand, when linking archives that contain bitcode files, the order bitcode files are added to `bitcodeFiles` vector is hard to predict. When parsing archives, firstly, symbols are parsed as lazy symbols: https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/InputFiles.cpp#L734-L739 And then we handle undefined symbols here: https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/Driver.cpp#L1208-L1210 where we try to fetch lazy symbols' original objects: https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/Driver.cpp#L708 -> https://github.com/llvm/llvm-project/blob/907ed77ad1d7a154317e5f8398d17d441711dc38/lld/wasm/Symbols.cpp#L428 which causes the corresponding object file to be added to the symbol table: https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/InputFiles.cpp#L763 and the object file is parsed: https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/SymbolTable.cpp#L53 But this `parse` call can lead to the parsing of other files before it is added to `bitcodeFiles` in the next line. For example, we have two bitcode files `a.o` and `b.o`, each of which is archived as `liba.a` and `libb.a`. Both files have a variable with the same name and the same comdat. When handling undefined symbols, suppose we start from a symbol from `a.o`. We arrive here and all comdats in `a.o` are added to `keptComdats`: https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/InputFiles.cpp#L844-L845 And we call `createBitCodeSymbol` on all symbols in `a.o`: https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/InputFiles.cpp#L847-L848 But this can cause the loading of another object in case some of those symbols are undefined functions or data, like here: https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/InputFiles.cpp#L791-L792 -> https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/SymbolTable.cpp#L534 So effectively, in the middle of parsing `a.o`, we proceed to parse `b.o`. And when parsing `b.o`, we encounter the comdat variable. But because we already added that comdat in `keptComdats` when parsing `a.o`, so `b.o`'s comdat variable ends up being excluded, so it is added as an undefined data, whereas `a.o`'s comdat variable will be added as a defined data: https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/InputFiles.cpp#L786-L798 For a lazy symbol, `addUndefinedData` does not replace the symbol unless it is the first time being inserted, but `addDefinedData` does: https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/SymbolTable.cpp#L382-L385 And because of this `a.o`'s comdat is to be the prevailing one, because the symbol's `getFile()` will return `a.o`: https://github.com/llvm/llvm-project/blob/907ed77ad1d7a154317e5f8398d17d441711dc38/lld/wasm/LTO.cpp#L104 But because `b.o`'s parsing started in the middle of `a.o`s parsing, it ends first and gets added to `bitcodeFiles` first. https://github.com/llvm/llvm-project/blob/fb57f4e0e0b302ec1b3181e952a4bd4b3c57a286/lld/wasm/SymbolTable.cpp#L54 So to sum up, `bitcodeFiles` now contains [`b.o`, `a.o`] but `a.o` has the prevailing comdat. This does not happen when directly linking bitcodes (without archives) because symbols are added in one-pass in the order specified in the command line. This also does not happen in non-LTO (even with archives), because object parsing uses `ObjFile::parse` which is a separate process from this bitcode processing. --- I'm not sure if there is a rule that the prevailing comdat has to exist in the first file having that comdat within `bitcodeFiles`, but my observation says this is likely the case. When adding bitcodes to the LTO, if a symbol's comdat is not a prevailing one, this code demotes its linkage to `available_externally`: https://github.com/llvm/llvm-project/blob/23c84fb362849865990c0e160158b19f54742147/llvm/lib/LTO/LTO.cpp#L869-L895 and subsequently all other variables that are associated with the same comdat in that object file are demoted in the same way. https://github.com/llvm/llvm-project/blob/23c84fb362849865990c0e160158b19f54742147/llvm/lib/LTO/LTO.cpp#L788-L792 In out case, because of the orders of `bitcodeFiles`, we start from `b.o`, and the comdat symbols there become `available_externally`. This is how `__cxx_global_var_init` in `global_var_init2.ll` in the attached test becomes `available_externally`. When linking those bitcodes in the LTO, if we traverse symbols in the order of prevailing->non-prevailing, we end up keeping only one global variable, but if we traverse in the other way around (i.e., non-prevailing first), we end up adding two symbols, because it lets us add a `available_externally` symbol when we don't already have a definition for it: https://github.com/llvm/llvm-project/blob/23c84fb362849865990c0e160158b19f54742147/llvm/lib/LTO/LTO.cpp#L974-L986 Also when it is an `available_externally` symbol, it becomes a declaration, not a definition. So we end up with a module like this: ```ll declare dso_local void @__cxx_global_var_init() define internal void @__cxx_global_var_init.2() comdat($unused) { ... } ``` --- So this one-line fix ensures we compile bitcodes in the order that we process comdats, so that when multiple archived bitcode files have the same variable with the same comdat, we make sure that the prevailing comdat will be linked first in the LTO. --- This crash is said to be happening after llvm@12050a3 but even reverting this is not really a solution for us because we end up with two definitions if we do that: ```ll define internal void @__cxx_global_var_init() comdat($unused) { ... } define internal void @__cxx_global_var_init.2() comdat($unused) { ... } ``` And the wasm's `__wasm_call_ctors` will be like ```wast (func $__wasm_call_ctors (call $emscripten_stack_init) (call $__cxx_global_var_init) (call $__cxx_global_var_init.2) ) ```
aheejin
added a commit
that referenced
this issue
Nov 29, 2023
When doing LTO on multiple archives, the order with which bitcodes are linked to the LTO module is hard to control, given that processing undefined symbols can lead to parsing of an object file, which in turn lead to parsing of another object file before finishing parsing of the previous file. This can result in encountering a non-prevailing comdat first when linking, which can make the the symbol undefined, and the real definition is added later with an additional prefix to avoid duplication (e.g. `__cxx_global_var_init` and `__cxx_global_var_init.2`) So this one-line fix ensures we compile bitcodes in the order that we process comdats, so that when multiple archived bitcode files have the same variable with the same comdat, we make sure that the prevailing comdat will be linked first in the LTO. Fixes #62243.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We've seen certain situations where in LTO builds wasm-ld will crash during
Writer::createCallCtorsFunctionbecause one of the init function in the generted LTO object file (__cxx_global_var_init.1) in undefined and does not get assigned a function index (so crashes insym->getFunctionIndex()).I'm still working on reducing the repro case as more.
This seems to have started with https://reviews.llvm.org/D135427, which I guess makes sense since that change explicitly calls out
__cxx_global_var_initas being discarded.The text was updated successfully, but these errors were encountered: