Skip to content

[symbolizer] Don't treat symbolizer API as optional #76103

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

Conversation

vitalybuka
Copy link
Collaborator

There is an assumption that we dont need to to mix sanitizer with
symbolizer from different LLVM revison. If so we can detect it by
__sanitizer_symbolize_code and assume that the rest is present.

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Dec 20, 2023

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

Author: Vitaly Buka (vitalybuka)

Changes

There is an assumption that we dont need to to mix sanitizer with
symbolizer from different LLVM revison. If so we can detect it by
__sanitizer_symbolize_code and assume that the rest is present.


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

1 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp (+9-14)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
index 28f11352a6b5bb..3b61eb42993ec9 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp
@@ -341,15 +341,14 @@ __sanitizer_symbolize_set_inline_frames(bool InlineFrames);
 class InternalSymbolizer final : public SymbolizerTool {
  public:
   static InternalSymbolizer *get(LowLevelAllocator *alloc) {
-    if (&__sanitizer_symbolize_set_demangle)
-      CHECK(__sanitizer_symbolize_set_demangle(common_flags()->demangle));
-    if (&__sanitizer_symbolize_set_inline_frames)
-      CHECK(__sanitizer_symbolize_set_inline_frames(
+    // These one is the most used one, so we will use it to detect a presense of
+    // internal symbolizer.
+    if (&__sanitizer_symbolize_code == nullptr)
+      return nullptr;
+    CHECK(__sanitizer_symbolize_set_demangle(common_flags()->demangle));
+    CHECK(__sanitizer_symbolize_set_inline_frames(
           common_flags()->symbolize_inline_frames));
-    // These are essential, we don't have InternalSymbolizer without them.
-    if (&__sanitizer_symbolize_code && &__sanitizer_symbolize_data)
-      return new (*alloc) InternalSymbolizer();
-    return 0;
+    return new (*alloc) InternalSymbolizer();
   }
 
   bool SymbolizePC(uptr addr, SymbolizedStack *stack) override {
@@ -371,8 +370,6 @@ class InternalSymbolizer final : public SymbolizerTool {
   }
 
   bool SymbolizeFrame(uptr addr, FrameInfo *info) override {
-    if (&__sanitizer_symbolize_frame == nullptr)
-      return false;
     bool result = __sanitizer_symbolize_frame(info->module, info->module_offset,
                                               buffer_, sizeof(buffer_));
     if (result)
@@ -381,13 +378,11 @@ class InternalSymbolizer final : public SymbolizerTool {
   }
 
   void Flush() override {
-    if (&__sanitizer_symbolize_flush)
-      __sanitizer_symbolize_flush();
+    __sanitizer_symbolize_flush();
   }
 
   const char *Demangle(const char *name) override {
-    if (&__sanitizer_symbolize_demangle &&
-        __sanitizer_symbolize_demangle(name, buffer_, sizeof(buffer_))) {
+    if (__sanitizer_symbolize_demangle(name, buffer_, sizeof(buffer_))) {
       char *res_buff = nullptr;
       ExtractToken(buffer_, "", &res_buff);
       return res_buff;

Copy link

github-actions bot commented Dec 20, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@thurstond
Copy link
Contributor

Nit for PR title: "Don't treat symbolizer"

CHECK(__sanitizer_symbolize_set_demangle(common_flags()->demangle));
if (&__sanitizer_symbolize_set_inline_frames)
CHECK(__sanitizer_symbolize_set_inline_frames(
// These one is the most used one, so we will use it to detect a presense of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: "presence"

Created using spr 1.3.4
Created using spr 1.3.4
@vitalybuka vitalybuka merged commit 3dca63a into main Dec 20, 2023
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/symbolizer-dont-threat-symbolizer-api-as-optional-2 branch December 20, 2023 23:38
@vitalybuka vitalybuka changed the title [symbolizer] Don't threat symbolizer API as optional [symbolizer] Don't treat symbolizer API as optional Dec 27, 2023
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.

3 participants