[sanitizer] Support "alloc_dealloc_mismatch" suppressions #124197

andrewjcg · 2025-01-23T21:43:09Z

This adds a stack-based suppression for alloc-dealloc-mismatch violations, using the function name to match.

llvmbot · 2025-01-23T21:45:28Z

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

Author: None (andrewjcg)

Changes

This adds a stack-based suppression for alloc-dealloc-mismatch violations, using the function name to match.

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

3 Files Affected:

(modified) compiler-rt/lib/asan/asan_allocator.cpp (+3-1)
(modified) compiler-rt/lib/asan/asan_suppressions.cpp (+20-15)
(added) compiler-rt/test/asan/TestCases/suppressions-alloc-dealloc-mismatch.cpp (+35)

diff --git a/compiler-rt/lib/asan/asan_allocator.cpp b/compiler-rt/lib/asan/asan_allocator.cpp
index 9e66f77217ec6b..a5f939b4c2f8aa 100644
--- a/compiler-rt/lib/asan/asan_allocator.cpp
+++ b/compiler-rt/lib/asan/asan_allocator.cpp
@@ -21,6 +21,7 @@
 #include "asan_poisoning.h"
 #include "asan_report.h"
 #include "asan_stack.h"
+#include "asan_suppressions.h"
 #include "asan_thread.h"
 #include "lsan/lsan_common.h"
 #include "sanitizer_common/sanitizer_allocator_checks.h"
@@ -732,7 +733,8 @@ struct Allocator {
     if (!AtomicallySetQuarantineFlagIfAllocated(m, ptr, stack)) return;
 
     if (m->alloc_type != alloc_type) {
-      if (atomic_load(&alloc_dealloc_mismatch, memory_order_acquire)) {
+      if (atomic_load(&alloc_dealloc_mismatch, memory_order_acquire) &&
+          !IsStackTraceSuppressed(stack)) {
         ReportAllocTypeMismatch((uptr)ptr, stack, (AllocType)m->alloc_type,
                                 (AllocType)alloc_type);
       }
diff --git a/compiler-rt/lib/asan/asan_suppressions.cpp b/compiler-rt/lib/asan/asan_suppressions.cpp
index 94289d14d7e780..f45b5cf611b59b 100644
--- a/compiler-rt/lib/asan/asan_suppressions.cpp
+++ b/compiler-rt/lib/asan/asan_suppressions.cpp
@@ -26,9 +26,10 @@ static const char kInterceptorName[] = "interceptor_name";
 static const char kInterceptorViaFunction[] = "interceptor_via_fun";
 static const char kInterceptorViaLibrary[] = "interceptor_via_lib";
 static const char kODRViolation[] = "odr_violation";
+static const char kAllocDeallocMismatch[] = "alloc_dealloc_mismatch";
 static const char *kSuppressionTypes[] = {
     kInterceptorName, kInterceptorViaFunction, kInterceptorViaLibrary,
-    kODRViolation};
+    kODRViolation, kAllocDeallocMismatch};
 
 SANITIZER_INTERFACE_WEAK_DEF(const char *, __asan_default_suppressions, void) {
   return "";
@@ -52,7 +53,8 @@ bool IsInterceptorSuppressed(const char *interceptor_name) {
 bool HaveStackTraceBasedSuppressions() {
   CHECK(suppression_ctx);
   return suppression_ctx->HasSuppressionType(kInterceptorViaFunction) ||
-         suppression_ctx->HasSuppressionType(kInterceptorViaLibrary);
+         suppression_ctx->HasSuppressionType(kInterceptorViaLibrary) ||
+         suppression_ctx->HasSuppressionType(kAllocDeallocMismatch);
 }
 
 bool IsODRViolationSuppressed(const char *global_var_name) {
@@ -79,19 +81,22 @@ bool IsStackTraceSuppressed(const StackTrace *stack) {
           return true;
     }
 
-    if (suppression_ctx->HasSuppressionType(kInterceptorViaFunction)) {
-      SymbolizedStackHolder symbolized_stack(symbolizer->SymbolizePC(addr));
-      const SymbolizedStack *frames = symbolized_stack.get();
-      CHECK(frames);
-      for (const SymbolizedStack *cur = frames; cur; cur = cur->next) {
-        const char *function_name = cur->info.function;
-        if (!function_name) {
-          continue;
-        }
-        // Match "interceptor_via_fun" suppressions.
-        if (suppression_ctx->Match(function_name, kInterceptorViaFunction,
-                                   &s)) {
-          return true;
+    const char *suppressions[] = {kInterceptorViaFunction,
+                                  kAllocDeallocMismatch};
+    for (const char *suppression : suppressions) {
+      if (suppression_ctx->HasSuppressionType(suppression)) {
+        SymbolizedStackHolder symbolized_stack(symbolizer->SymbolizePC(addr));
+        const SymbolizedStack *frames = symbolized_stack.get();
+        CHECK(frames);
+        for (const SymbolizedStack *cur = frames; cur; cur = cur->next) {
+          const char *function_name = cur->info.function;
+          if (!function_name) {
+            continue;
+          }
+          // Match suppressions.
+          if (suppression_ctx->Match(function_name, suppression, &s)) {
+            return true;
+          }
         }
       }
     }
diff --git a/compiler-rt/test/asan/TestCases/suppressions-alloc-dealloc-mismatch.cpp b/compiler-rt/test/asan/TestCases/suppressions-alloc-dealloc-mismatch.cpp
new file mode 100644
index 00000000000000..47b99a090beddd
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/suppressions-alloc-dealloc-mismatch.cpp
@@ -0,0 +1,35 @@
+// Check that without suppressions, we catch the issue.
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck --check-prefix=CHECK-CRASH %s
+
+// RUN: echo "alloc_dealloc_mismatch:function" > %t.supp
+// RUN: %clangxx_asan -O0 %s -o %t && %env_asan_opts=suppressions='"%t.supp"' %run %t 2>&1 | FileCheck --check-prefix=CHECK-IGNORE %s
+// RUN: %clangxx_asan -O3 %s -o %t && %env_asan_opts=suppressions='"%t.supp"' %run %t 2>&1 | FileCheck --check-prefix=CHECK-IGNORE %s
+
+// XFAIL: android
+// UNSUPPORTED: ios
+
+// FIXME: atos does not work for inlined functions, yet llvm-symbolizer
+// does not always work with debug info on Darwin.
+// UNSUPPORTED: darwin
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+void function() {
+  char *a = (char *)malloc(6);
+  a[0] = '\0';
+  size_t len = strlen(a);
+  delete a; // BOOM
+  fprintf(stderr, "strlen ignored, len = %zu\n", len);
+}
+
+int main() {
+  function();
+}
+
+// CHECK-CRASH: AddressSanitizer: alloc-dealloc-mismatch
+// CHECK-CRASH-NOT: strlen ignored
+// CHECK-IGNORE-NOT: AddressSanitizer: alloc-dealloc-mismatch
+// CHECK-IGNORE: strlen ignored

github-actions · 2025-01-23T21:52:14Z

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

bcardosolopes

The PR looks simple enough to me, solid testcase and there has been some experience using it in production (at Meta). You should wait a for a stamp and actual review though from @vitalybuka, but LGTM.

bcardosolopes · 2025-01-24T21:36:00Z

compiler-rt/test/asan/TestCases/suppressions-alloc-dealloc-mismatch.cpp

+// RUN: %clangxx_asan -O0 %s -o %t && %env_asan_opts=suppressions='"%t.supp"' %run %t 2>&1 | FileCheck --check-prefix=CHECK-IGNORE %s
+// RUN: %clangxx_asan -O3 %s -o %t && %env_asan_opts=suppressions='"%t.supp"' %run %t 2>&1 | FileCheck --check-prefix=CHECK-IGNORE %s
+
+// XFAIL: android


Any comment worth on why it's XFAILd for android?

Ah, this is copied from another test case, which I used to start this one. Will clean up.

Probably Android test harness does not copy suppression onto device.
I assume it will fail here too, comment here will be helpful.

Should I just remove this? Or keep w/ a comment about assuming a failure would happen? I'm not sure how to test on android.

Ahh, I did remove it in the most recent update -- not sure if this was the right thing to do.

compiler-rt/lib/asan/asan_suppressions.cpp

vitalybuka · 2025-01-26T22:50:24Z

compiler-rt/lib/asan/asan_allocator.cpp

@@ -732,7 +733,8 @@ struct Allocator {
    if (!AtomicallySetQuarantineFlagIfAllocated(m, ptr, stack)) return;

    if (m->alloc_type != alloc_type) {
-      if (atomic_load(&alloc_dealloc_mismatch, memory_order_acquire)) {
+      if (atomic_load(&alloc_dealloc_mismatch, memory_order_acquire) &&
+          !IsStackTraceSuppressed(stack)) {


This looks very random.
Why ReportAllocTypeMismatch and not ReportNewDeleteTypeMismatch?

I propose to move IsStackTraceSuppressed into Report* functions.
WDYT?

Do do so, some refactoring is needed, some Report* functions don't accept stack,
but they generate it internally anyway. We need to change that, and than move existing IsStackTraceSuppressed into Report* function.

I don't have preference if we do that before, or after the current PR.

Why ReportAllocTypeMismatch and not ReportNewDeleteTypeMismatch?

Yeah, I'm not super clear on the distinction between the two.

I actually think there's a bug w/ my change here -- I think I need a separate suppression check function, otherwise all the other stack-trace suppressions would match this, which isn't what we want. Fixing now.

This adds a stack-based suppressions for alloc-dealloc-mismatch violations, using the function name to match.

benlangmuir · 2025-01-29T18:29:46Z

This is failing on Darwin: https://green.lab.llvm.org/job/llvm.org/job/clang-stage1-cmake-RA-expensive/3243/testReport/junit/AddressSanitizer-x86_64-darwin/TestCases/suppressions_alloc_dealloc_mismatch_cpp/

suppressions-alloc-dealloc-mismatch.cpp:23:17: error: CHECK-CRASH: expected string not found in input
// CHECK-CRASH: AddressSanitizer: alloc-dealloc-mismatch
                ^
<stdin>:1:1: note: scanning from here
strlen ignored, len = 0
^

andrewjcg · 2025-01-29T18:51:21Z

Ah apologies. Looking into this now.

benlangmuir · 2025-01-29T19:12:32Z

Maybe just needs %env_asan_opts=alloc_dealloc_mismatch=1?

benlangmuir · 2025-01-29T21:13:28Z

Here's my attempt to fix it: #124987

Android is missing suppression file on device. Follow up to #124197.

Android is missing suppression file on device. Follow up to llvm#124197. (cherry picked from commit 751ae26)

llvmbot added compiler-rt compiler-rt:asan Address sanitizer compiler-rt:sanitizer labels Jan 23, 2025

andrewjcg force-pushed the alloc-dealloc-suppress branch from 9a8c15e to b036c4d Compare January 23, 2025 21:43

andrewjcg force-pushed the alloc-dealloc-suppress branch from b036c4d to 0309388 Compare January 23, 2025 21:54

bcardosolopes reviewed Jan 24, 2025

View reviewed changes

andrewjcg force-pushed the alloc-dealloc-suppress branch from 0309388 to 43d999a Compare January 24, 2025 22:02

WenleiHe requested a review from vitalybuka January 26, 2025 02:04

vitalybuka reviewed Jan 26, 2025

View reviewed changes

vitalybuka approved these changes Jan 26, 2025

View reviewed changes

andrewjcg force-pushed the alloc-dealloc-suppress branch 2 times, most recently from 0f29a8f to b84a13e Compare January 27, 2025 18:51

[sanitizer] Support "alloc_dealloc_mismatch" suppressions

f99aacb

This adds a stack-based suppressions for alloc-dealloc-mismatch violations, using the function name to match.

andrewjcg force-pushed the alloc-dealloc-suppress branch from b84a13e to f99aacb Compare January 27, 2025 20:19

bcardosolopes merged commit 6b654a0 into llvm:main Jan 29, 2025
7 checks passed

vitalybuka added a commit that referenced this pull request Jan 30, 2025

[asan][android] XFAIL suppressions-alloc-dealloc-mismatch

751ae26

Android is missing suppression file on device. Follow up to #124197.

andrewjcg deleted the alloc-dealloc-suppress branch January 30, 2025 02:40

swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 5, 2025

[asan][android] XFAIL suppressions-alloc-dealloc-mismatch

2d1f7e0

Android is missing suppression file on device. Follow up to llvm#124197. (cherry picked from commit 751ae26)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[sanitizer] Support "alloc_dealloc_mismatch" suppressions #124197

[sanitizer] Support "alloc_dealloc_mismatch" suppressions #124197

Uh oh!

andrewjcg commented Jan 23, 2025

Uh oh!

llvmbot commented Jan 23, 2025

Uh oh!

github-actions bot commented Jan 23, 2025 •

edited

Loading

Uh oh!

bcardosolopes left a comment

Uh oh!

bcardosolopes Jan 24, 2025

Uh oh!

andrewjcg Jan 24, 2025

Uh oh!

vitalybuka Jan 26, 2025

Uh oh!

andrewjcg Jan 27, 2025

Uh oh!

andrewjcg Jan 27, 2025

Uh oh!

Uh oh!

vitalybuka Jan 26, 2025

Uh oh!

andrewjcg Jan 27, 2025

Uh oh!

Uh oh!

benlangmuir commented Jan 29, 2025

Uh oh!

andrewjcg commented Jan 29, 2025

Uh oh!

benlangmuir commented Jan 29, 2025

Uh oh!

benlangmuir commented Jan 29, 2025

Uh oh!

Uh oh!

[sanitizer] Support "alloc_dealloc_mismatch" suppressions #124197

[sanitizer] Support "alloc_dealloc_mismatch" suppressions #124197

Uh oh!

Conversation

andrewjcg commented Jan 23, 2025

Uh oh!

llvmbot commented Jan 23, 2025

Uh oh!

github-actions bot commented Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bcardosolopes left a comment

Choose a reason for hiding this comment

Uh oh!

bcardosolopes Jan 24, 2025

Choose a reason for hiding this comment

Uh oh!

andrewjcg Jan 24, 2025

Choose a reason for hiding this comment

Uh oh!

vitalybuka Jan 26, 2025

Choose a reason for hiding this comment

Uh oh!

andrewjcg Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

andrewjcg Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vitalybuka Jan 26, 2025

Choose a reason for hiding this comment

Uh oh!

andrewjcg Jan 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

benlangmuir commented Jan 29, 2025

Uh oh!

andrewjcg commented Jan 29, 2025

Uh oh!

benlangmuir commented Jan 29, 2025

Uh oh!

benlangmuir commented Jan 29, 2025

Uh oh!

Uh oh!

github-actions bot commented Jan 23, 2025 •

edited

Loading