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

[NFC, sanitizer_symbolizer] Split Fuchsia and Markup. #72305

Merged
merged 3 commits into from
Nov 15, 2023

Conversation

avillega
Copy link
Contributor

This PR separates parts of the symbolizer markup
implementation that are Fuchsia OS specific. This
is in preparation of enabling symbolizer markup
in other OSs.

This PR separated parts of the symbolizer markup
implementation that are Fuchsia OS specific. This
is in preparation of enabling symbolizer markup
in other OSs.
@llvmbot llvmbot added clang Clang issues not falling into any other category compiler-rt xray compiler-rt:sanitizer labels Nov 14, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 14, 2023

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

@llvm/pr-subscribers-clang

Author: Andres Villegas (avillega)

Changes

This PR separates parts of the symbolizer markup
implementation that are Fuchsia OS specific. This
is in preparation of enabling symbolizer markup
in other OSs.


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

10 Files Affected:

  • (modified) clang/docs/tools/clang-formatted-files.txt (+1-1)
  • (modified) compiler-rt/CMakeLists.txt (+1)
  • (modified) compiler-rt/lib/sanitizer_common/CMakeLists.txt (+3-1)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_coverage_fuchsia.cpp (+1-1)
  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp (+5-52)
  • (renamed) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup_constants.h (+5-5)
  • (added) compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report_fuchsia.cpp (+33)
  • (added) compiler-rt/lib/sanitizer_common/sanitizer_unwind_fuchsia.cpp (+66)
  • (modified) compiler-rt/lib/xray/xray_utils.cpp (+1-1)
  • (modified) llvm/utils/gn/secondary/compiler-rt/lib/sanitizer_common/BUILD.gn (+1-1)
diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt
index 48cd800bffd0046..18512b1a7bf6b58 100644
--- a/clang/docs/tools/clang-formatted-files.txt
+++ b/clang/docs/tools/clang-formatted-files.txt
@@ -1837,7 +1837,7 @@ compiler-rt/lib/sanitizer_common/sanitizer_stack_store.cpp
 compiler-rt/lib/sanitizer_common/sanitizer_stack_store.h
 compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_fuchsia.h
 compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
-compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_fuchsia.h
+compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup_constants.h
 compiler-rt/lib/sanitizer_common/sanitizer_thread_safety.h
 compiler-rt/lib/sanitizer_common/sanitizer_tls_get_addr.h
 compiler-rt/lib/sanitizer_common/sanitizer_type_traits.cpp
diff --git a/compiler-rt/CMakeLists.txt b/compiler-rt/CMakeLists.txt
index 1a46f5b33480694..361dc72c0b97444 100644
--- a/compiler-rt/CMakeLists.txt
+++ b/compiler-rt/CMakeLists.txt
@@ -337,6 +337,7 @@ if(NOT COMPILER_RT_HAS_FVISIBILITY_HIDDEN_FLAG)
   append_list_if(COMPILER_RT_HAS_FVISIBILITY_INLINES_HIDDEN_FLAG -fvisibility-inlines-hidden SANITIZER_COMMON_CFLAGS)
 endif()
 append_list_if(COMPILER_RT_HAS_FNO_LTO_FLAG -fno-lto SANITIZER_COMMON_CFLAGS)
+append_list_if(COMPILER_RT_ENABLE_SYMBOLIZER_MARKUP -DENABLE_SANITIZER_SYMBOLIZER_MARKUP SANITIZER_COMMON_CFLAGS)
 
 # By default do not instrument or use profdata for compiler-rt.
 if(NOT COMPILER_RT_ENABLE_PGO)
diff --git a/compiler-rt/lib/sanitizer_common/CMakeLists.txt b/compiler-rt/lib/sanitizer_common/CMakeLists.txt
index ce6d4cf80919b25..61e832a30eb3767 100644
--- a/compiler-rt/lib/sanitizer_common/CMakeLists.txt
+++ b/compiler-rt/lib/sanitizer_common/CMakeLists.txt
@@ -90,8 +90,10 @@ set(SANITIZER_SYMBOLIZER_SOURCES
   sanitizer_symbolizer_markup.cpp
   sanitizer_symbolizer_posix_libcdep.cpp
   sanitizer_symbolizer_report.cpp
+  sanitizer_symbolizer_report_fuchsia.cpp
   sanitizer_symbolizer_win.cpp
   sanitizer_unwind_linux_libcdep.cpp
+  sanitizer_unwind_fuchsia.cpp
   sanitizer_unwind_win.cpp
   )
 
@@ -189,7 +191,7 @@ set(SANITIZER_IMPL_HEADERS
   sanitizer_stoptheworld.h
   sanitizer_suppressions.h
   sanitizer_symbolizer.h
-  sanitizer_symbolizer_fuchsia.h
+  sanitizer_symbolizer_markup_constants.h
   sanitizer_symbolizer_internal.h
   sanitizer_symbolizer_libbacktrace.h
   sanitizer_symbolizer_mac.h
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_coverage_fuchsia.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_coverage_fuchsia.cpp
index 35c325359148ab7..a7f9273cf4a32dc 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_coverage_fuchsia.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_coverage_fuchsia.cpp
@@ -35,7 +35,7 @@
 #include "sanitizer_common.h"
 #include "sanitizer_interface_internal.h"
 #include "sanitizer_internal_defs.h"
-#include "sanitizer_symbolizer_fuchsia.h"
+#include "sanitizer_symbolizer_markup_constants.h"
 
 using namespace __sanitizer;
 
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
index d1b0f46004efaff..1b2da1ae5e1a469 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp
@@ -12,18 +12,14 @@
 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_platform.h"
-#if SANITIZER_SYMBOLIZER_MARKUP
 
-#if SANITIZER_FUCHSIA
-#include "sanitizer_symbolizer_fuchsia.h"
-#  endif
+#if SANITIZER_SYMBOLIZER_MARKUP
 
-#  include <limits.h>
-#  include <unwind.h>
 
-#  include "sanitizer_stacktrace.h"
-#  include "sanitizer_stacktrace_printer.h"
-#  include "sanitizer_symbolizer.h"
+#include "sanitizer_common.h"
+#include "sanitizer_stacktrace_printer.h"
+#include "sanitizer_symbolizer.h"
+#include "sanitizer_symbolizer_markup_constants.h"
 
 namespace __sanitizer {
 
@@ -110,49 +106,6 @@ Symbolizer *Symbolizer::PlatformInit() {
 
 void Symbolizer::LateInitialize() { Symbolizer::GetOrInit(); }
 
-void StartReportDeadlySignal() {}
-void ReportDeadlySignal(const SignalContext &sig, u32 tid,
-                        UnwindSignalStackCallbackType unwind,
-                        const void *unwind_context) {}
-
-#if SANITIZER_CAN_SLOW_UNWIND
-struct UnwindTraceArg {
-  BufferedStackTrace *stack;
-  u32 max_depth;
-};
-
-_Unwind_Reason_Code Unwind_Trace(struct _Unwind_Context *ctx, void *param) {
-  UnwindTraceArg *arg = static_cast<UnwindTraceArg *>(param);
-  CHECK_LT(arg->stack->size, arg->max_depth);
-  uptr pc = _Unwind_GetIP(ctx);
-  if (pc < PAGE_SIZE) return _URC_NORMAL_STOP;
-  arg->stack->trace_buffer[arg->stack->size++] = pc;
-  return (arg->stack->size == arg->max_depth ? _URC_NORMAL_STOP
-                                             : _URC_NO_REASON);
-}
-
-void BufferedStackTrace::UnwindSlow(uptr pc, u32 max_depth) {
-  CHECK_GE(max_depth, 2);
-  size = 0;
-  UnwindTraceArg arg = {this, Min(max_depth + 1, kStackTraceMax)};
-  _Unwind_Backtrace(Unwind_Trace, &arg);
-  CHECK_GT(size, 0);
-  // We need to pop a few frames so that pc is on top.
-  uptr to_pop = LocatePcInTrace(pc);
-  // trace_buffer[0] belongs to the current function so we always pop it,
-  // unless there is only 1 frame in the stack trace (1 frame is always better
-  // than 0!).
-  PopStackFrames(Min(to_pop, static_cast<uptr>(1)));
-  trace_buffer[0] = pc;
-}
-
-void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) {
-  CHECK(context);
-  CHECK_GE(max_depth, 2);
-  UNREACHABLE("signal context doesn't exist");
-}
-#endif  // SANITIZER_CAN_SLOW_UNWIND
-
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_SYMBOLIZER_MARKUP
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_fuchsia.h b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup_constants.h
similarity index 81%
rename from compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_fuchsia.h
rename to compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup_constants.h
index c4061e38c6a47cf..4280b92b7c23a59 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_fuchsia.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup_constants.h
@@ -1,4 +1,4 @@
-//===-- sanitizer_symbolizer_fuchsia.h -----------------------------------===//
+//===-- sanitizer_symbolizer_markup_constants.h -----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -8,10 +8,10 @@
 //
 // This file is shared between various sanitizers' runtime libraries.
 //
-// Define Fuchsia's string formats and limits for the markup symbolizer.
+// Define string formats and limits for the markup symbolizer.
 //===----------------------------------------------------------------------===//
-#ifndef SANITIZER_SYMBOLIZER_FUCHSIA_H
-#define SANITIZER_SYMBOLIZER_FUCHSIA_H
+#ifndef SANITIZER_SYMBOLIZER_MARKUP_CONSTANTS_H
+#define SANITIZER_SYMBOLIZER_MARKUP_CONSTANTS_H
 
 #include "sanitizer_internal_defs.h"
 
@@ -39,4 +39,4 @@ constexpr const char *kFormatFrame = "{{{bt:%u:%p}}}";
 
 }  // namespace __sanitizer
 
-#endif  // SANITIZER_SYMBOLIZER_FUCHSIA_H
+#endif  // SANITIZER_SYMBOLIZER_MARKUP_CONSTANTS_H
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report_fuchsia.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report_fuchsia.cpp
new file mode 100644
index 000000000000000..527f12e7f002226
--- /dev/null
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_report_fuchsia.cpp
@@ -0,0 +1,33 @@
+//===-- sanitizer_symbolizer_report_fuchsia.cpp -----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Implementation of the report functions for fuchsia.
+//
+//===----------------------------------------------------------------------===//
+
+#include "sanitizer_platform.h"
+
+#if SANITIZER_SYMBOLIZER_MARKUP
+
+
+#include "sanitizer_common.h"
+
+namespace __sanitizer {
+void StartReportDeadlySignal() {}
+
+void ReportDeadlySignal(const SignalContext &sig, u32 tid,
+                        UnwindSignalStackCallbackType unwind,
+                        const void *unwind_context) {}
+
+void HandleDeadlySignal(void *siginfo, void *context, u32 tid,
+                        UnwindSignalStackCallbackType unwind,
+                        const void *unwind_context) {}
+
+} // namespace __sanitizer
+
+#endif  // SANITIZER_SYMBOLIZER_MARKUP
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_unwind_fuchsia.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_unwind_fuchsia.cpp
new file mode 100644
index 000000000000000..f3eb8591dcbc485
--- /dev/null
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_unwind_fuchsia.cpp
@@ -0,0 +1,66 @@
+//===------------------ sanitizer_unwind_fuchsia.cpp
+//---------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+/// Sanitizer unwind Fuchsia specific functions.
+//
+//===----------------------------------------------------------------------===//
+
+#include "sanitizer_platform.h"
+#if SANITIZER_FUCHSIA
+
+#  include <limits.h>
+#  include <unwind.h>
+
+#  include "sanitizer_common.h"
+#  include "sanitizer_stacktrace.h"
+
+namespace __sanitizer {
+
+#  if SANITIZER_CAN_SLOW_UNWIND
+struct UnwindTraceArg {
+  BufferedStackTrace *stack;
+  u32 max_depth;
+};
+
+_Unwind_Reason_Code Unwind_Trace(struct _Unwind_Context *ctx, void *param) {
+  UnwindTraceArg *arg = static_cast<UnwindTraceArg *>(param);
+  CHECK_LT(arg->stack->size, arg->max_depth);
+  uptr pc = _Unwind_GetIP(ctx);
+  if (pc < GetPageSizeCached())
+    return _URC_NORMAL_STOP;
+  arg->stack->trace_buffer[arg->stack->size++] = pc;
+  return (arg->stack->size == arg->max_depth ? _URC_NORMAL_STOP
+                                             : _URC_NO_REASON);
+}
+
+void BufferedStackTrace::UnwindSlow(uptr pc, u32 max_depth) {
+  CHECK_GE(max_depth, 2);
+  size = 0;
+  UnwindTraceArg arg = {this, Min(max_depth + 1, kStackTraceMax)};
+  _Unwind_Backtrace(Unwind_Trace, &arg);
+  CHECK_GT(size, 0);
+  // We need to pop a few frames so that pc is on top.
+  uptr to_pop = LocatePcInTrace(pc);
+  // trace_buffer[0] belongs to the current function so we always pop it,
+  // unless there is only 1 frame in the stack trace (1 frame is always better
+  // than 0!).
+  PopStackFrames(Min(to_pop, static_cast<uptr>(1)));
+  trace_buffer[0] = pc;
+}
+
+void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) {
+  CHECK(context);
+  CHECK_GE(max_depth, 2);
+  UNREACHABLE("signal context doesn't exist");
+}
+#  endif  //  SANITIZER_CAN_SLOW_UNWIND
+
+}  // namespace __sanitizer
+
+#endif  // SANITIZER_FUCHSIA
diff --git a/compiler-rt/lib/xray/xray_utils.cpp b/compiler-rt/lib/xray/xray_utils.cpp
index befbabfe453229e..5d51df9937c2cad 100644
--- a/compiler-rt/lib/xray/xray_utils.cpp
+++ b/compiler-rt/lib/xray/xray_utils.cpp
@@ -28,7 +28,7 @@
 #include <utility>
 
 #if SANITIZER_FUCHSIA
-#include "sanitizer_common/sanitizer_symbolizer_fuchsia.h"
+#include "sanitizer_common/sanitizer_symbolizer_markup_constants.h"
 
 #include <inttypes.h>
 #include <zircon/process.h>
diff --git a/llvm/utils/gn/secondary/compiler-rt/lib/sanitizer_common/BUILD.gn b/llvm/utils/gn/secondary/compiler-rt/lib/sanitizer_common/BUILD.gn
index 5a4a6f17cf311a6..08b7df340b7475d 100644
--- a/llvm/utils/gn/secondary/compiler-rt/lib/sanitizer_common/BUILD.gn
+++ b/llvm/utils/gn/secondary/compiler-rt/lib/sanitizer_common/BUILD.gn
@@ -143,7 +143,7 @@ source_set("sources") {
     "sanitizer_suppressions.h",
     "sanitizer_symbolizer.cpp",
     "sanitizer_symbolizer.h",
-    "sanitizer_symbolizer_fuchsia.h",
+    "sanitizer_symbolizer_markup_constants.h",
     "sanitizer_symbolizer_internal.h",
     "sanitizer_symbolizer_libbacktrace.cpp",
     "sanitizer_symbolizer_libbacktrace.h",

Copy link

github-actions bot commented Nov 14, 2023

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

Copy link
Contributor

@PiJoules PiJoules left a comment

Choose a reason for hiding this comment

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

LGTM

@avillega avillega merged commit 3dc098d into llvm:main Nov 15, 2023
4 checks passed
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
This PR separates parts of the symbolizer markup
implementation that are Fuchsia OS specific. This
is in preparation of enabling symbolizer markup
in other OSs.
@vitalybuka
Copy link
Collaborator

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category compiler-rt:sanitizer compiler-rt xray
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants