Update compiler-rt to LLVM 18.1.2 #21663
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is the list of files using 80-bit long double: https://github.com/llvm/llvm-project/blob/6009708b4367171ccdbf4b5905cb6a803753fe18/compiler-rt/lib/builtins/CMakeLists.txt#L279-L294 (This file is from LLVM 17.0.6, which is currently our compiler-rt version) We don't have 80-bit long doubles so it looks we can exclude them.
This looks somehow deleted during the conflict resolution.
Many lines that call `InternalScopedString::AppendF`, including the below, https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp#L31 are format-checked by https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/compiler-rt/lib/sanitizer_common/sanitizer_common.h#L650 which checks if the formats of arguments conform to that of functions like `printf`. But this errors out because the format strings currently do not match their arguments. llvm/llvm-project@9c8f888#diff-5bf0494d1b61b99e55aefb25e59e41b678f20a392c2c6d77e5fbdc97c2ca4c3f started the effort to check `__attribute__((format))` in the codebase, but apparently there were too many violating instances, it made the code build with `-Wno-format` (presumably as an temporary solution): https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/compiler-rt/lib/sanitizer_common/CMakeLists.txt#L223-L224 So this adds `-Wno-format` to our cflags as well. The reason it was fine without it until now is, we didn't build the whole file thanks to this line: https://github.com/emscripten-core/emscripten/blob/a9b347bfcabda59a5edff60ee18b8a0ab70aa9dc/system/lib/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp#L15 But recent refactoring efforts including llvm/llvm-project#73032, llvm/llvm-project#73192, and llvm/llvm-project#73193 moved many parts out of that file to be built by a specific platform (e.g. fushia) and made the file build unconditionally. I tried to add back `#if SANITIZER_SYMBOLIZER_MARKUP` to that file but then this line (https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace_printer.cpp#L67) breaks with an undefined referenc error. So just build the library with `-Wno-format`, which is also what the upstream is effectively doing, seems a simpler solution.
This deals with the following upstream changes: llvm/llvm-project@4e1b55a llvm/llvm-project@fd16d46 by adding `ThreadStartParams` class as `asan_win.cpp` did.
sbc100
approved these changes
Apr 7, 2024
| @@ -2059,7 +2062,7 @@ class libsanitizer_common_rt(CompilerRTLibrary, MTLibrary): | |||
| 'system/lib/compiler-rt/lib', | |||
| 'system/lib/libc'] | |||
| never_force = True | |||
There was a problem hiding this comment.
Add a comment/TODO here with link the upstream issue?
There was a problem hiding this comment.
Not sure if there is a specific upstream issue for this. Will add a todo saying we can remove this after upstream format problems are resolved.
There was a problem hiding this comment.
sbc100
approved these changes
Apr 7, 2024
impact-maker
pushed a commit
to impact-maker/emscripten
that referenced
this pull request
Apr 11, 2024
This updates compiler-rt to LLVM 18.1.2. Things to note: - llvm/llvm-project#73573 reformatted https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp, so our diff got somewhat bigger because adding a preprocessor like `ifdef` somewhere means changing the indentation of all inner directives. - In https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp, ```diff - #elif SANITIZER_USE_GETAUXVAL - return getauxval(AT_PAGESZ); ``` These lines were somehow deleted in emscripten-core#11662 for no apparent reason. (It looks this is not used by us) Restored it to reduce the diff between us and the upstream. - Build libsanitizer_common_rt with `-Wno-format`: emscripten-core@67108b2 Some code that got newly compiled in this version has violations to `__attribute__ ((format))` that crashes compilation without `-Wno-format`. Detailed description of the problem is in emscripten-core@67108b2. The simple temporary fix of adding `-Wno-format` is basically what the upstream code currently does with `CMakeLists.txt`. - `ThreadStart`-related changes in https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/asan/asan_emscripten.cpp: emscripten-core@36ce3d3 This deals with the following upstream changes: llvm/llvm-project@4e1b55a llvm/llvm-project@fd16d46 by adding `ThreadStartParams` class as https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/asan/asan_win.cpp did. Each fix is described in its own commit, except for the fixes required to resolve conflicts when merging our changes, which I wasn't able to commit separately.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates compiler-rt to LLVM 18.1.2.
Things to note:
[sanitizer][nfc] Reformat sanitizer_linux sources llvm/llvm-project#73573 reformatted https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp, so our diff got somewhat bigger because adding a preprocessor like
ifdefsomewhere means changing the indentation of all inner directives.In https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp,
These lines were somehow deleted in Update compiler-rt (and sanitizers) to llvm 10.0.0 #11662 for no apparent reason. (It looks this is not used by us) Restored it to reduce the diff between us and the upstream.
Build libsanitizer_common_rt with
-Wno-format: 67108b2Some code that got newly compiled in this version has violations to
__attribute__ ((format))that crashes compilation without-Wno-format. Detailed description of the problem is in 67108b2. The simple temporary fix of adding-Wno-formatis basically what the upstream code currently does withCMakeLists.txt.ThreadStart-related changes in https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/asan/asan_emscripten.cpp: 36ce3d3This deals with the following upstream changes:
llvm/llvm-project@4e1b55a
llvm/llvm-project@fd16d46
by adding
ThreadStartParamsclass as https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/asan/asan_win.cpp did.Each fix is described in its own commit, except for the fixes required to resolve conflicts when merging our changes, which I wasn't able to commit separately.