[compiler-rt] compiler-rt: ubsan: match suppressions against inlined frames (PR #206735)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: maflcko

<details>
<summary>Changes</summary>

When a function is inlined, it may not be possible to name it in a UBSan suppressions file if only the top stack frame is checked.

Match function names against every inlined stack frame, while keeping file names matched only against the top stack frame as before.

Also add a test for each case.

Fixes https://github.com/llvm/llvm-project/issues/132533

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


4 Files Affected:

- (modified) compiler-rt/lib/ubsan/ubsan_diag.cpp (+8-4) 
- (added) compiler-rt/test/ubsan/TestCases/Integer/Inputs/suppressions-inline-origin-file.h (+1) 
- (added) compiler-rt/test/ubsan/TestCases/Integer/suppressions-inline-origin-file.c (+22) 
- (added) compiler-rt/test/ubsan/TestCases/Integer/suppressions-nested-inline-function.c (+20) 


``````````diff
diff --git a/compiler-rt/lib/ubsan/ubsan_diag.cpp b/compiler-rt/lib/ubsan/ubsan_diag.cpp
index 2146ed3c27287..543e358651d6d 100644
--- a/compiler-rt/lib/ubsan/ubsan_diag.cpp
+++ b/compiler-rt/lib/ubsan/ubsan_diag.cpp
@@ -429,6 +429,7 @@ bool __ubsan::IsVptrCheckSuppressed(const char *TypeName) {
 bool __ubsan::IsPCSuppressed(ErrorType ET, uptr PC, const char *Filename) {
   InitAsStandaloneIfNecessary();
   CHECK(suppression_ctx);
+  PC = StackTrace::GetPreviousInstructionPc(PC);
   const char *SuppType = ConvertTypeToFlagName(ET);
   // Fast path: don't symbolize PC if there is no suppressions for given UB
   // type.
@@ -443,11 +444,14 @@ bool __ubsan::IsPCSuppressed(ErrorType ET, uptr PC, const char *Filename) {
     if (suppression_ctx->Match(Module, SuppType, &s))
       return true;
   }
-  // Suppress by function or source file name from debug info.
+  // Suppress by function name from any inlined frame or by source file name
+  // from the top frame.
   SymbolizedStackHolder Stack(Symbolizer::GetOrInit()->SymbolizePC(PC));
-  const AddressInfo &AI = Stack.get()->info;
-  return suppression_ctx->Match(AI.function, SuppType, &s) ||
-         suppression_ctx->Match(AI.file, SuppType, &s);
+  for (const SymbolizedStack *Frame = Stack.get(); Frame; Frame = Frame->next) {
+    if (suppression_ctx->Match(Frame->info.function, SuppType, &s))
+      return true;
+  }
+  return suppression_ctx->Match(Stack.get()->info.file, SuppType, &s);
 }
 
 #endif  // CAN_SANITIZE_UB
diff --git a/compiler-rt/test/ubsan/TestCases/Integer/Inputs/suppressions-inline-origin-file.h b/compiler-rt/test/ubsan/TestCases/Integer/Inputs/suppressions-inline-origin-file.h
new file mode 100644
index 0000000000000..4fed13ac4ce24
--- /dev/null
+++ b/compiler-rt/test/ubsan/TestCases/Integer/Inputs/suppressions-inline-origin-file.h
@@ -0,0 +1 @@
+__attribute__((always_inline)) static inline int fun(unsigned a) { return a; }
diff --git a/compiler-rt/test/ubsan/TestCases/Integer/suppressions-inline-origin-file.c b/compiler-rt/test/ubsan/TestCases/Integer/suppressions-inline-origin-file.c
new file mode 100644
index 0000000000000..b9dfa1cc0dbb7
--- /dev/null
+++ b/compiler-rt/test/ubsan/TestCases/Integer/suppressions-inline-origin-file.c
@@ -0,0 +1,22 @@
+// REQUIRES: can-symbolize
+// UNSUPPORTED: android
+
+// RUN: %clang -fsanitize=integer -O1 -g %s -o %t
+// RUN: echo "implicit-integer-sign-change:%s" > %t.main.supp
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1:report_error_type=1:suppressions='"%t.main.supp"' \
+// RUN:   not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-MAIN-FILE
+// RUN: echo "implicit-integer-sign-change:%p/Inputs/suppressions-inline-origin-file.h" > %t.header.supp
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1:report_error_type=1:suppressions='"%t.header.supp"' \
+// RUN:   %run %t 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-HEADER-FILE
+
+#include "Inputs/suppressions-inline-origin-file.h"
+
+int main(void) { (void)fun(4222111000U); }
+
+// Suppressing the caller file must not suppress a UB originating in the
+// inlined header.
+// CHECK-MAIN-FILE: runtime error: implicit conversion
+// CHECK-MAIN-FILE: {{.*}} in fun
+// CHECK-MAIN-FILE: {{.*}} in main
+
+// CHECK-HEADER-FILE-NOT: runtime error:
diff --git a/compiler-rt/test/ubsan/TestCases/Integer/suppressions-nested-inline-function.c b/compiler-rt/test/ubsan/TestCases/Integer/suppressions-nested-inline-function.c
new file mode 100644
index 0000000000000..e691b65c57a1f
--- /dev/null
+++ b/compiler-rt/test/ubsan/TestCases/Integer/suppressions-nested-inline-function.c
@@ -0,0 +1,20 @@
+// REQUIRES: can-symbolize
+// UNSUPPORTED: android
+
+// RUN: %clang -fsanitize=integer -O1 -g %s -o %t
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1:report_error_type=1 \
+// RUN:   not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-NOSUP
+// RUN: echo "implicit-integer-sign-change:mid" > %t.supp
+// RUN: %env_ubsan_opts=halt_on_error=1:print_stacktrace=1:report_error_type=1:suppressions='"%t.supp"' \
+// RUN:   %run %t 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-SUP
+
+inline int leaf(unsigned a) { return a; }
+inline int mid(unsigned a) { return leaf(a); }
+
+int main(void) { (void)mid(4222111000U); }
+
+// CHECK-NOSUP: runtime error: implicit conversion
+// CHECK-NOSUP: {{.*}} in leaf
+// CHECK-NOSUP: {{.*}} in mid
+
+// CHECK-SUP-NOT: runtime error:

``````````

</details>


https://github.com/llvm/llvm-project/pull/206735