[rtsan] Remove std::variant from rtsan diagnostics

[davidtrevelyan]

Following issue #109529 and PR #109715, this PR removes the std::variant in rtsan's diagnostics code, in favour of a solution by enum without the C++ runtime. Many thanks to @zeroomega for the report and investigations, also to @vitalybuka for the holistic insight and suggestion for this solution.

[llvmbot]

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

Author: None (davidtrevelyan)

Changes

Following issue #109529 and PR #109715, this PR removes the std::variant in rtsan's diagnostics code, in favour of a solution by enum without the C++ runtime. Many thanks to @zeroomega for the report and investigations, also to @vitalybuka for the holistic insight and suggestion for this solution.

---

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

3 Files Affected:

• (modified) compiler-rt/lib/rtsan/rtsan.cpp (+4-2)
• (modified) compiler-rt/lib/rtsan/rtsan_diagnostics.cpp (+28-30)
• (modified) compiler-rt/lib/rtsan/rtsan_diagnostics.h (+5-14)

diff --git a/compiler-rt/lib/rtsan/rtsan.cpp b/compiler-rt/lib/rtsan/rtsan.cpp
index 2afdf3c76696e7..b288da64ffbe25 100644
--- a/compiler-rt/lib/rtsan/rtsan.cpp
+++ b/compiler-rt/lib/rtsan/rtsan.cpp
@@ -87,7 +87,8 @@ __rtsan_notify_intercepted_call(const char *func_name) {
   GET_CALLER_PC_BP;
   ExpectNotRealtime(
       GetContextForThisThread(),
-      PrintDiagnosticsAndDieAction({InterceptedCallInfo{func_name}, pc, bp}));
+      PrintDiagnosticsAndDieAction(
+          {DiagnosticsInfoType::InterceptedCall, func_name, pc, bp}));
 }
 
 SANITIZER_INTERFACE_ATTRIBUTE void
@@ -96,7 +97,8 @@ __rtsan_notify_blocking_call(const char *func_name) {
   GET_CALLER_PC_BP;
   ExpectNotRealtime(
       GetContextForThisThread(),
-      PrintDiagnosticsAndDieAction({BlockingCallInfo{func_name}, pc, bp}));
+      PrintDiagnosticsAndDieAction(
+          {DiagnosticsInfoType::BlockingCall, func_name, pc, bp}));
 }
 
 } // extern "C"
diff --git a/compiler-rt/lib/rtsan/rtsan_diagnostics.cpp b/compiler-rt/lib/rtsan/rtsan_diagnostics.cpp
index ac13b0743be069..afd314f5324bcb 100644
--- a/compiler-rt/lib/rtsan/rtsan_diagnostics.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_diagnostics.cpp
@@ -37,12 +37,6 @@ class Decorator : public __sanitizer::SanitizerCommonDecorator {
   const char *FunctionName() const { return Green(); }
   const char *Reason() const { return Blue(); }
 };
-
-template <class... Ts> struct Overloaded : Ts... {
-  using Ts::operator()...;
-};
-// TODO: Remove below when c++20
-template <class... Ts> Overloaded(Ts...) -> Overloaded<Ts...>;
 } // namespace
 
 static void PrintStackTrace(uptr pc, uptr bp) {
@@ -53,35 +47,39 @@ static void PrintStackTrace(uptr pc, uptr bp) {
 }
 
 static void PrintError(const Decorator &decorator,
-                       const DiagnosticsCallerInfo &info) {
-  const char *violation_type = std::visit(
-      Overloaded{
-          [](const InterceptedCallInfo &) { return "unsafe-library-call"; },
-          [](const BlockingCallInfo &) { return "blocking-call"; }},
-      info);
+                       const DiagnosticsInfo &info) {
+  const auto violation_type_str = [&info]() -> const char * {
+    switch (info.type) {
+    case DiagnosticsInfoType::InterceptedCall:
+      return "unsafe-library-call";
+    case DiagnosticsInfoType::BlockingCall:
+      return "blocking-call";
+    }
+    return "unrecognised-caller-type";
+  };
 
   Printf("%s", decorator.Error());
-  Report("ERROR: RealtimeSanitizer: %s\n", violation_type);
+  Report("ERROR: RealtimeSanitizer: %s\n", violation_type_str());
 }
 
 static void PrintReason(const Decorator &decorator,
-                        const DiagnosticsCallerInfo &info) {
+                        const DiagnosticsInfo &info) {
   Printf("%s", decorator.Reason());
 
-  std::visit(
-      Overloaded{[decorator](const InterceptedCallInfo &call) {
-                   Printf("Intercepted call to real-time unsafe function "
-                          "`%s%s%s` in real-time context!",
-                          decorator.FunctionName(),
-                          call.intercepted_function_name_, decorator.Reason());
-                 },
-                 [decorator](const BlockingCallInfo &arg) {
-                   Printf("Call to blocking function "
-                          "`%s%s%s` in real-time context!",
-                          decorator.FunctionName(), arg.blocking_function_name_,
-                          decorator.Reason());
-                 }},
-      info);
+  switch (info.type) {
+  case DiagnosticsInfoType::InterceptedCall: {
+    Printf("Intercepted call to real-time unsafe function "
+           "`%s%s%s` in real-time context!",
+           decorator.FunctionName(), info.func_name, decorator.Reason());
+    break;
+  }
+  case DiagnosticsInfoType::BlockingCall: {
+    Printf("Call to blocking function "
+           "`%s%s%s` in real-time context!",
+           decorator.FunctionName(), info.func_name, decorator.Reason());
+    break;
+  }
+  }
 
   Printf("\n");
 }
@@ -90,8 +88,8 @@ void __rtsan::PrintDiagnostics(const DiagnosticsInfo &info) {
   ScopedErrorReportLock l;
 
   Decorator d;
-  PrintError(d, info.call_info);
-  PrintReason(d, info.call_info);
+  PrintError(d, info);
+  PrintReason(d, info);
   Printf("%s", d.Default());
   PrintStackTrace(info.pc, info.bp);
 }
diff --git a/compiler-rt/lib/rtsan/rtsan_diagnostics.h b/compiler-rt/lib/rtsan/rtsan_diagnostics.h
index 8aec512584b309..f8a6b8a954a24a 100644
--- a/compiler-rt/lib/rtsan/rtsan_diagnostics.h
+++ b/compiler-rt/lib/rtsan/rtsan_diagnostics.h
@@ -15,25 +15,16 @@
 #include "sanitizer_common/sanitizer_common.h"
 #include "sanitizer_common/sanitizer_internal_defs.h"
 
-#include <variant>
-
 namespace __rtsan {
 
-struct InterceptedCallInfo {
-  const char *intercepted_function_name_;
-};
-
-struct BlockingCallInfo {
-public:
-  const char *blocking_function_name_;
+enum class DiagnosticsInfoType {
+  InterceptedCall,
+  BlockingCall,
 };
 
-using DiagnosticsCallerInfo =
-    std::variant<InterceptedCallInfo, BlockingCallInfo>;
-
 struct DiagnosticsInfo {
-  DiagnosticsCallerInfo call_info;
-
+  DiagnosticsInfoType type;
+  const char *func_name;
   __sanitizer::uptr pc;
   __sanitizer::uptr bp;
 };

[davidtrevelyan]

@cjappl for review :)

[cjappl]

Pre-merge build failure is unrelated:

/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-dqhhn-1/llvm-project/github-pull-requests/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:9796:12: error: no matching function for call to 'has_single_bit'
    assert(llvm::has_single_bit(Mask) && "Invalid mask.");

[cjappl]

Strange styling here, but I presume it is what clang-format spit out. (the white space being not indented here

[davidtrevelyan]

Yup, this is clang-format's IndentCaseLabels option at work - I've seen it as false more often than not on the codebases I've worked on, but I get it's a bit odd if you're used to indented cases

[cjappl]

I will merge this once CI passes - should be soon

EDIT: same unrelated failure as before, merging