Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sanitizer] Allow *___lcxx_override symbolse in symbolizer #79904

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Jan 29, 2024

We don't intercept them, and they are not called and used only as
markers anyway.

These symbols introduced with #69498.

Created using spr 1.3.4
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 29, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Vitaly Buka (vitalybuka)

Changes

We don't intercept them, and they are not called and used only as
markers anyway.


Full diff: https://github.com/llvm/llvm-project/pull/79904.diff

2 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh (+4-2)
  • (modified) compiler-rt/lib/sanitizer_common/symbolizer/scripts/global_symbols.txt (+5)
diff --git a/compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh b/compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
index f24d42cc84e4176..7f4082a6cd190e2 100755
--- a/compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
+++ b/compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
@@ -191,8 +191,10 @@ $OPT -passes=internalize -internalize-public-api-list=${SYMBOLIZER_API_LIST} all
 $CC $FLAGS -fno-lto -c opt.bc -o symbolizer.o
 
 echo "Checking undefined symbols..."
-nm -f posix -g symbolizer.o | cut -f 1,2 -d \  | LC_COLLATE=C sort -u > undefined.new
-(diff -u $SCRIPT_DIR/global_symbols.txt undefined.new | grep -E "^\+[^+]") && \
+export LC_ALL=C
+nm -f posix -g symbolizer.o | cut -f 1,2 -d \  | sort -u > undefined.new
+grep -Ev "^#|^$" $SCRIPT_DIR/global_symbols.txt | sort -u > expected.new
+(diff -u expected.new undefined.new | grep -E "^\+[^+]") && \
   (echo "Failed: unexpected symbols"; exit 1)
 
 cp -f symbolizer.o $OUTPUT
diff --git a/compiler-rt/lib/sanitizer_common/symbolizer/scripts/global_symbols.txt b/compiler-rt/lib/sanitizer_common/symbolizer/scripts/global_symbols.txt
index 0a4bc6989a0d72a..ef522976a2c2da3 100644
--- a/compiler-rt/lib/sanitizer_common/symbolizer/scripts/global_symbols.txt
+++ b/compiler-rt/lib/sanitizer_common/symbolizer/scripts/global_symbols.txt
@@ -1,3 +1,6 @@
+# This file is used to control symbols used by internal symbolizer. We want to
+# avoid unexpected dependency on function intercepted by sanitizers.
+
 _GLOBAL_OFFSET_TABLE_ U
 _ZN11__sanitizer13internal_mmapEPvjiiiy U
 _ZN11__sanitizer13internal_mmapEPvmiiiy U
@@ -62,6 +65,8 @@ __sanitizer_symbolize_flush T
 __sanitizer_symbolize_frame T
 __sanitizer_symbolize_set_demangle T
 __sanitizer_symbolize_set_inline_frames T
+__start___lcxx_override U
+__stop___lcxx_override U
 __strdup U
 __udivdi3 U
 __umoddi3 U

@vitalybuka
Copy link
Collaborator Author

Please don't merge yet, testing

And I don't know why bots are inconsistent, I would expect that all fail without this patch.

@thurstond
Copy link
Contributor

@kstoimenov (build watch): FYI: these symbols appear in the alternating buildbot failures

@vitalybuka vitalybuka merged commit 8dbedf4 into main Jan 29, 2024
5 of 6 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/sanitizer-allow-___lcxx_override-symbolse-in-symbolizer branch January 29, 2024 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants