[compiler-rt] [asan] Add experimental 'track_poison' flag (PR #133175)
Thurston Dang via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 27 09:28:38 PDT 2025
https://github.com/thurstond updated https://github.com/llvm/llvm-project/pull/133175
>From 4cffa15d2349de86fc3ee33d8070360fc2eedfb0 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 26 Mar 2025 02:43:38 +0000
Subject: [PATCH 1/5] [asan] Add experimental 'track_poison' flag
This adds an experimental flag that will keep track of where the manual
memory poisoning (__asan_poison_memory_region) is called from, and print
the stack trace if the poisoned region is accessed. (Currently, ASan
will tell you what code accessed a poisoned region, but not which code
set the poison.)
This implementation performs best-effort record keeping using ring buffers, as suggested by Vitaly. The size of each ring buffer is set by the track_poison flag value.
Some records may be lost in multi-threaded programs.
---
compiler-rt/lib/asan/asan_descriptions.h | 4 +
compiler-rt/lib/asan/asan_errors.cpp | 66 ++++++++++++++++-
compiler-rt/lib/asan/asan_flags.inc | 3 +
compiler-rt/lib/asan/asan_poisoning.cpp | 74 ++++++++++++++++++-
compiler-rt/lib/asan/asan_poisoning.h | 49 ++++++++++++
compiler-rt/lib/asan/asan_rtl.cpp | 2 +
.../TestCases/use-after-poison-tracked.cpp | 47 ++++++++++++
7 files changed, 240 insertions(+), 5 deletions(-)
create mode 100644 compiler-rt/test/asan/TestCases/use-after-poison-tracked.cpp
diff --git a/compiler-rt/lib/asan/asan_descriptions.h b/compiler-rt/lib/asan/asan_descriptions.h
index a614f47d461bb..6e23555b35d28 100644
--- a/compiler-rt/lib/asan/asan_descriptions.h
+++ b/compiler-rt/lib/asan/asan_descriptions.h
@@ -15,6 +15,7 @@
#define ASAN_DESCRIPTIONS_H
#include "asan_allocator.h"
+#include "asan_poisoning.h"
#include "asan_thread.h"
#include "sanitizer_common/sanitizer_common.h"
#include "sanitizer_common/sanitizer_report_decorator.h"
@@ -46,6 +47,9 @@ class Decorator : public __sanitizer::SanitizerCommonDecorator {
const char *Allocation() { return Magenta(); }
const char *ShadowByte(u8 byte) {
+ if (IsPoisonTrackingMagic(byte))
+ return Blue();
+
switch (byte) {
case kAsanHeapLeftRedzoneMagic:
case kAsanArrayCookieMagic:
diff --git a/compiler-rt/lib/asan/asan_errors.cpp b/compiler-rt/lib/asan/asan_errors.cpp
index 4f112cc5d1bca..d0be3d48172da 100644
--- a/compiler-rt/lib/asan/asan_errors.cpp
+++ b/compiler-rt/lib/asan/asan_errors.cpp
@@ -12,8 +12,10 @@
//===----------------------------------------------------------------------===//
#include "asan_errors.h"
+
#include "asan_descriptions.h"
#include "asan_mapping.h"
+#include "asan_poisoning.h"
#include "asan_report.h"
#include "asan_stack.h"
#include "sanitizer_common/sanitizer_stackdepot.h"
@@ -505,6 +507,19 @@ ErrorGeneric::ErrorGeneric(u32 tid, uptr pc_, uptr bp_, uptr sp_, uptr addr,
far_from_bounds = AdjacentShadowValuesAreFullyPoisoned(shadow_addr);
break;
}
+
+ if (flags()->track_poison > 0 && IsPoisonTrackingMagic(shadow_val)) {
+ if (internal_strcmp(bug_descr, "unknown-crash") != 0) {
+ Printf(
+ "ERROR: use-after-poison tracking magic values overlap with "
+ "other constants.\n");
+ Printf("Please file a bug.\n");
+ } else {
+ bug_descr = "use-after-poison";
+ bug_type_score = 20;
+ }
+ }
+
scariness.Scare(bug_type_score + read_after_free_bonus, bug_descr);
if (far_from_bounds) scariness.Scare(10, "far-from-bounds");
}
@@ -550,8 +565,12 @@ static void PrintLegend(InternalScopedString *str) {
PrintShadowByte(str, " Global redzone: ", kAsanGlobalRedzoneMagic);
PrintShadowByte(str, " Global init order: ",
kAsanInitializationOrderMagic);
- PrintShadowByte(str, " Poisoned by user: ",
- kAsanUserPoisonedMemoryMagic);
+ // TODO: sync description with PoisonTrackingMagicValues
+ PrintShadowByte(
+ str, " Poisoned by user: ", kAsanUserPoisonedMemoryMagic,
+ flags()->track_poison > 0 ? " with detailed tracking using {0x80-0x8F, "
+ "0x90-0x9F, 0xD0-0xDF, 0xE0-0xEF}\n"
+ : "\n");
PrintShadowByte(str, " Container overflow: ",
kAsanContiguousContainerOOBMagic);
PrintShadowByte(str, " Array cookie: ",
@@ -600,6 +619,44 @@ static void PrintShadowMemoryForAddress(uptr addr) {
Printf("%s", str.data());
}
+static void CheckPoisonRecords(uptr addr) {
+ if (!AddrIsInMem(addr))
+ return;
+ uptr shadow_addr = MemToShadow(addr);
+ unsigned char poison_magic = *(reinterpret_cast<u8 *>(shadow_addr));
+ int poison_index = PoisonTrackingMagicToIndex[poison_magic];
+
+ if (poison_index < 0 || poison_index >= NumPoisonTrackingMagicValues)
+ return;
+
+ PoisonRecordRingBuffer *PoisonRecord =
+ reinterpret_cast<PoisonRecordRingBuffer *>(PoisonRecords[poison_index]);
+ if (PoisonRecord) {
+ bool FoundMatch = false;
+
+ for (unsigned int i = 0; i < PoisonRecord->size(); i++) {
+ struct PoisonRecord Record = (*PoisonRecord)[i];
+ if (Record.begin <= addr && addr <= Record.end) {
+ FoundMatch = true;
+
+ StackTrace poison_stack = StackDepotGet(Record.stack_id);
+
+ Printf("\n");
+ Printf("Memory was manually poisoned by thread T%u:\n",
+ Record.thread_id);
+ poison_stack.Print();
+
+ break;
+ }
+ }
+
+ if (!FoundMatch) {
+ Printf("ERROR: no matching poison tracking record found.\n");
+ Printf("Try setting a larger track_poison value.\n");
+ }
+ }
+}
+
void ErrorGeneric::Print() {
Decorator d;
Printf("%s", d.Error());
@@ -623,6 +680,11 @@ void ErrorGeneric::Print() {
PrintContainerOverflowHint();
ReportErrorSummary(bug_descr, &stack);
PrintShadowMemoryForAddress(addr);
+
+ // This uses a range of shadow values, hence it is not convenient to make a
+ // specific error handler.
+ if (flags()->track_poison > 0)
+ CheckPoisonRecords(addr);
}
} // namespace __asan
diff --git a/compiler-rt/lib/asan/asan_flags.inc b/compiler-rt/lib/asan/asan_flags.inc
index fad1577d912a5..2e3373f55c887 100644
--- a/compiler-rt/lib/asan/asan_flags.inc
+++ b/compiler-rt/lib/asan/asan_flags.inc
@@ -116,6 +116,9 @@ ASAN_FLAG(bool, poison_partial, true,
"stack buffers.")
ASAN_FLAG(bool, poison_array_cookie, true,
"Poison (or not) the array cookie after operator new[].")
+ASAN_FLAG(int, track_poison, 0,
+ "[EXPERIMENTAL] If non-zero, record the stack trace of manual "
+ "memory poisoning calls.")
// Turn off alloc/dealloc mismatch checker on Mac and Windows for now.
// https://github.com/google/sanitizers/issues/131
diff --git a/compiler-rt/lib/asan/asan_poisoning.cpp b/compiler-rt/lib/asan/asan_poisoning.cpp
index 762670632f4e0..b65d21fe2ce4c 100644
--- a/compiler-rt/lib/asan/asan_poisoning.cpp
+++ b/compiler-rt/lib/asan/asan_poisoning.cpp
@@ -20,11 +20,48 @@
#include "sanitizer_common/sanitizer_flags.h"
#include "sanitizer_common/sanitizer_interface_internal.h"
#include "sanitizer_common/sanitizer_libc.h"
+#include "sanitizer_common/sanitizer_stackdepot.h"
namespace __asan {
static atomic_uint8_t can_poison_memory;
+PoisonRecordRingBuffer *PoisonRecords[NumPoisonTrackingMagicValues] = {0};
+int PoisonTrackingMagicToIndex[256] = {-1};
+
+void InitializePoisonTracking() {
+ if (flags()->track_poison <= 0)
+ return;
+
+ for (unsigned int i = 0; i < sizeof(PoisonTrackingMagicToIndex) / sizeof(int);
+ i++) {
+ PoisonTrackingMagicToIndex[i] = -1;
+ }
+
+ for (unsigned int i = 0; i < NumPoisonTrackingMagicValues; i++) {
+ int magic = PoisonTrackingIndexToMagic[i];
+ CHECK(magic > 0);
+ CHECK((unsigned int)magic <
+ sizeof(PoisonTrackingMagicToIndex) / sizeof(int));
+
+ // Necessary for AddressIsPoisoned calculations
+ CHECK((char)magic < 0);
+
+ PoisonTrackingMagicToIndex[magic] = i;
+
+ PoisonRecords[i] = PoisonRecordRingBuffer::New(flags()->track_poison);
+ }
+}
+
+bool IsPoisonTrackingMagic(int byte) {
+ return (byte >= 0 &&
+ (unsigned long)byte <
+ (sizeof(PoisonTrackingMagicToIndex) / sizeof(int)) &&
+ PoisonTrackingMagicToIndex[byte] >= 0 &&
+ PoisonTrackingMagicToIndex[byte] < NumPoisonTrackingMagicValues &&
+ PoisonTrackingIndexToMagic[PoisonTrackingMagicToIndex[byte]] == byte);
+}
+
void SetCanPoisonMemory(bool value) {
atomic_store(&can_poison_memory, value, memory_order_release);
}
@@ -107,6 +144,31 @@ void __asan_poison_memory_region(void const volatile *addr, uptr size) {
uptr end_addr = beg_addr + size;
VPrintf(3, "Trying to poison memory region [%p, %p)\n", (void *)beg_addr,
(void *)end_addr);
+
+ u32 poison_magic = kAsanUserPoisonedMemoryMagic;
+
+ GET_CALLER_PC_BP;
+ GET_STORE_STACK_TRACE_PC_BP(pc, bp);
+ // TODO: garbage collect stacks once they fall off the ring buffer?
+ // StackDepot doesn't currently have a way to delete stacks.
+ u32 stack_id = StackDepotPut(stack);
+
+ if (flags()->track_poison > 0) {
+ u32 current_tid = GetCurrentTidOrInvalid();
+ u32 poison_index = ((stack_id * 151157) ^ (current_tid * 733123)) %
+ NumPoisonTrackingMagicValues;
+ poison_magic = PoisonTrackingIndexToMagic[poison_index];
+ PoisonRecord record{.stack_id = stack_id,
+ .thread_id = current_tid,
+ .begin = beg_addr,
+ .end = end_addr};
+ // This is racy: with concurrent writes, some records may be lost,
+ // but it's a sacrifice I am willing to make for speed.
+ // The sharding across PoisonRecords reduces the likelihood of
+ // concurrent writes.
+ PoisonRecords[poison_index]->push(record);
+ }
+
ShadowSegmentEndpoint beg(beg_addr);
ShadowSegmentEndpoint end(end_addr);
if (beg.chunk == end.chunk) {
@@ -119,7 +181,7 @@ void __asan_poison_memory_region(void const volatile *addr, uptr size) {
if (beg.offset > 0) {
*beg.chunk = Min(value, beg.offset);
} else {
- *beg.chunk = kAsanUserPoisonedMemoryMagic;
+ *beg.chunk = poison_magic;
}
}
return;
@@ -134,10 +196,11 @@ void __asan_poison_memory_region(void const volatile *addr, uptr size) {
}
beg.chunk++;
}
- REAL(memset)(beg.chunk, kAsanUserPoisonedMemoryMagic, end.chunk - beg.chunk);
+
+ REAL(memset)(beg.chunk, poison_magic, end.chunk - beg.chunk);
// Poison if byte in end.offset is unaddressable.
if (end.value > 0 && end.value <= end.offset) {
- *end.chunk = kAsanUserPoisonedMemoryMagic;
+ *end.chunk = poison_magic;
}
}
@@ -147,6 +210,11 @@ void __asan_unpoison_memory_region(void const volatile *addr, uptr size) {
uptr end_addr = beg_addr + size;
VPrintf(3, "Trying to unpoison memory region [%p, %p)\n", (void *)beg_addr,
(void *)end_addr);
+
+ // Note: we don't need to update the poison tracking here. Since the shadow
+ // memory will be unpoisoned, the poison tracking ring buffer entries will be
+ // ignored.
+
ShadowSegmentEndpoint beg(beg_addr);
ShadowSegmentEndpoint end(end_addr);
if (beg.chunk == end.chunk) {
diff --git a/compiler-rt/lib/asan/asan_poisoning.h b/compiler-rt/lib/asan/asan_poisoning.h
index b68af1086e17d..9c8c5e3a9b72a 100644
--- a/compiler-rt/lib/asan/asan_poisoning.h
+++ b/compiler-rt/lib/asan/asan_poisoning.h
@@ -10,6 +10,8 @@
//
// Shadow memory poisoning by ASan RTL and by user application.
//===----------------------------------------------------------------------===//
+#ifndef ASAN_POISONING_H
+#define ASAN_POISONING_H
#ifndef ASAN_POISONING_H
#define ASAN_POISONING_H
@@ -19,9 +21,56 @@
#include "asan_mapping.h"
#include "sanitizer_common/sanitizer_flags.h"
#include "sanitizer_common/sanitizer_platform.h"
+#include "sanitizer_common/sanitizer_ring_buffer.h"
+
+// For platforms which support slow unwinder only, we restrict the store context
+// size to 1, basically only storing the current pc. We do this because the slow
+// unwinder which is based on libunwind is not async signal safe and causes
+// random freezes in forking applications as well as in signal handlers.
+#define GET_STORE_STACK_TRACE_PC_BP(pc, bp) \
+ UNINITIALIZED BufferedStackTrace stack; \
+ int max_stack = 16; \
+ if (!SANITIZER_CAN_FAST_UNWIND) \
+ max_stack = Min(max_stack, 1); \
+ stack.Unwind(pc, bp, nullptr, common_flags()->fast_unwind_on_malloc, \
+ max_stack);
+
+#define GET_STORE_STACK_TRACE \
+ GET_STORE_STACK_TRACE_PC_BP(StackTrace::GetCurrentPc(), GET_CURRENT_FRAME())
namespace __asan {
+// These need to be negative chars (i.e., in the range [0x80 .. 0xff]) for
+// AddressIsPoisoned calculations.
+static const int PoisonTrackingIndexToMagic[] = {
+ 0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87, 0x88, 0x89, 0x8a,
+ 0x8b, 0x8c, 0x8d, 0x8e, 0x8f, 0x90, 0x91, 0x92, 0x93, 0x94, 0x95,
+ 0x96, 0x97, 0x98, 0x99, 0x9a, 0x9b, 0x9c, 0x9d, 0x9e, 0x9f, 0xd0,
+ 0xd1, 0xd2, 0xd3, 0xd4, 0xd5, 0xd6, 0xd7, 0xd8, 0xd9, 0xda, 0xdb,
+ 0xdc, 0xdd, 0xde, 0xdf, 0xe0, 0xe1, 0xe2, 0xe3, 0xe4, 0xe5, 0xe6,
+ 0xe7, 0xe8, 0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef,
+};
+static const int NumPoisonTrackingMagicValues =
+ sizeof(PoisonTrackingIndexToMagic) / sizeof(int);
+
+extern int PoisonTrackingMagicToIndex[256];
+
+struct PoisonRecord {
+ unsigned int stack_id;
+ unsigned int thread_id;
+ uptr begin;
+ uptr end;
+};
+
+typedef RingBuffer<struct PoisonRecord> PoisonRecordRingBuffer;
+extern PoisonRecordRingBuffer* PoisonRecords[NumPoisonTrackingMagicValues];
+
+// Set up data structures for track_poison.
+void InitializePoisonTracking();
+
+// Is this number a magic value used for poison tracking?
+bool IsPoisonTrackingMagic(int byte);
+
// Enable/disable memory poisoning.
void SetCanPoisonMemory(bool value);
bool CanPoisonMemory();
diff --git a/compiler-rt/lib/asan/asan_rtl.cpp b/compiler-rt/lib/asan/asan_rtl.cpp
index b3f6677a99cfb..44cdc2a08c461 100644
--- a/compiler-rt/lib/asan/asan_rtl.cpp
+++ b/compiler-rt/lib/asan/asan_rtl.cpp
@@ -472,6 +472,8 @@ static bool AsanInitInternal() {
allocator_options.SetFrom(flags(), common_flags());
InitializeAllocator(allocator_options);
+ InitializePoisonTracking();
+
if (SANITIZER_START_BACKGROUND_THREAD_IN_ASAN_INTERNAL)
MaybeStartBackgroudThread();
diff --git a/compiler-rt/test/asan/TestCases/use-after-poison-tracked.cpp b/compiler-rt/test/asan/TestCases/use-after-poison-tracked.cpp
new file mode 100644
index 0000000000000..c4167279f5164
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/use-after-poison-tracked.cpp
@@ -0,0 +1,47 @@
+// Check that __asan_poison_memory_region and ASAN_OPTIONS=track_poison work.
+//
+// RUN: %clangxx_asan -O0 %s -o %t && env ASAN_OPTIONS=track_poison=1000 not %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK-AC,CHECK-A
+// RUN: %clangxx_asan -O0 %s -o %t && env ASAN_OPTIONS=track_poison=1000 %run %t 20 2>&1 | FileCheck %s --check-prefixes=CHECK-B
+// RUN: %clangxx_asan -O0 %s -o %t && env ASAN_OPTIONS=track_poison=1000 not %run %t 30 30 2>&1 | FileCheck %s --check-prefixes=CHECK-AC,CHECK-C
+
+#include <stdio.h>
+#include <stdlib.h>
+
+extern "C" void __asan_poison_memory_region(void *, size_t);
+extern "C" void __asan_unpoison_memory_region(void *, size_t);
+
+void novichok(char *x) {
+ __asan_poison_memory_region(x, 64); // A
+ __asan_unpoison_memory_region(x + 16, 8); // B
+ __asan_poison_memory_region(x + 24, 16); // C
+}
+
+void fsb(char *x) { novichok(x); }
+
+int main(int argc, char **argv) {
+ char *x = new char[64];
+ x[10] = 0;
+ fsb(x);
+ // Bytes [ 0, 15]: poisoned by A
+ // Bytes [16, 23]: unpoisoned by B
+ // Bytes [24, 63]: poisoned by C
+
+ int res = x[argc * 10]; // BOOOM
+ // CHECK-AC: ERROR: AddressSanitizer: use-after-poison
+ // CHECK-AC: main{{.*}}use-after-poison-tracked.cpp:[[@LINE-2]]
+ // CHECK-B-NOT: ERROR: AddressSanitizer: use-after-poison
+
+ // CHECK-AC: Memory was manually poisoned by thread T0:
+ // CHECK-A: novichok{{.*}}use-after-poison-tracked.cpp:[[@LINE-21]]
+ // CHECK-C: novichok{{.*}}use-after-poison-tracked.cpp:[[@LINE-20]]
+ // CHECK-AC: fsb{{.*}}use-after-poison-tracked.cpp:[[@LINE-18]]
+ // CHECK-AC: main{{.*}}use-after-poison-tracked.cpp:[[@LINE-14]]
+ // CHECK-B-NOT: Memory was manually poisoned by thread T0:
+
+ delete[] x;
+
+ printf("End of program reached\n");
+ // CHECK-B: End of program reached
+
+ return 0;
+}
>From 1c42b53184d53a48718d22dbb63fd59a14da35f9 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Wed, 26 Mar 2025 23:14:06 +0000
Subject: [PATCH 2/5] Use GET_STACK_TRACE (and only use it if track_poison is
enabled)
---
compiler-rt/lib/asan/asan_poisoning.cpp | 13 ++++++-------
compiler-rt/lib/asan/asan_poisoning.h | 15 ---------------
2 files changed, 6 insertions(+), 22 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_poisoning.cpp b/compiler-rt/lib/asan/asan_poisoning.cpp
index b65d21fe2ce4c..ef2640fa0f59a 100644
--- a/compiler-rt/lib/asan/asan_poisoning.cpp
+++ b/compiler-rt/lib/asan/asan_poisoning.cpp
@@ -147,13 +147,12 @@ void __asan_poison_memory_region(void const volatile *addr, uptr size) {
u32 poison_magic = kAsanUserPoisonedMemoryMagic;
- GET_CALLER_PC_BP;
- GET_STORE_STACK_TRACE_PC_BP(pc, bp);
- // TODO: garbage collect stacks once they fall off the ring buffer?
- // StackDepot doesn't currently have a way to delete stacks.
- u32 stack_id = StackDepotPut(stack);
-
if (flags()->track_poison > 0) {
+ GET_STACK_TRACE(/*max_size=*/ 16, /*fast=*/ false);
+ // TODO: garbage collect stacks once they fall off the ring buffer?
+ // StackDepot doesn't currently have a way to delete stacks.
+ u32 stack_id = StackDepotPut(stack);
+
u32 current_tid = GetCurrentTidOrInvalid();
u32 poison_index = ((stack_id * 151157) ^ (current_tid * 733123)) %
NumPoisonTrackingMagicValues;
@@ -162,7 +161,7 @@ void __asan_poison_memory_region(void const volatile *addr, uptr size) {
.thread_id = current_tid,
.begin = beg_addr,
.end = end_addr};
- // This is racy: with concurrent writes, some records may be lost,
+ // This is a data race: with concurrent writes, some records may be lost,
// but it's a sacrifice I am willing to make for speed.
// The sharding across PoisonRecords reduces the likelihood of
// concurrent writes.
diff --git a/compiler-rt/lib/asan/asan_poisoning.h b/compiler-rt/lib/asan/asan_poisoning.h
index 9c8c5e3a9b72a..310d6a578f8d7 100644
--- a/compiler-rt/lib/asan/asan_poisoning.h
+++ b/compiler-rt/lib/asan/asan_poisoning.h
@@ -23,21 +23,6 @@
#include "sanitizer_common/sanitizer_platform.h"
#include "sanitizer_common/sanitizer_ring_buffer.h"
-// For platforms which support slow unwinder only, we restrict the store context
-// size to 1, basically only storing the current pc. We do this because the slow
-// unwinder which is based on libunwind is not async signal safe and causes
-// random freezes in forking applications as well as in signal handlers.
-#define GET_STORE_STACK_TRACE_PC_BP(pc, bp) \
- UNINITIALIZED BufferedStackTrace stack; \
- int max_stack = 16; \
- if (!SANITIZER_CAN_FAST_UNWIND) \
- max_stack = Min(max_stack, 1); \
- stack.Unwind(pc, bp, nullptr, common_flags()->fast_unwind_on_malloc, \
- max_stack);
-
-#define GET_STORE_STACK_TRACE \
- GET_STORE_STACK_TRACE_PC_BP(StackTrace::GetCurrentPc(), GET_CURRENT_FRAME())
-
namespace __asan {
// These need to be negative chars (i.e., in the range [0x80 .. 0xff]) for
>From 32a8dda11179fb56929b28d0a022613b434db9e7 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Thu, 27 Mar 2025 16:11:58 +0000
Subject: [PATCH 3/5] Single poison value
---
compiler-rt/lib/asan/asan_descriptions.h | 4 ---
compiler-rt/lib/asan/asan_errors.cpp | 26 +++-----------
compiler-rt/lib/asan/asan_poisoning.cpp | 44 +++++-------------------
compiler-rt/lib/asan/asan_poisoning.h | 21 ++---------
4 files changed, 14 insertions(+), 81 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_descriptions.h b/compiler-rt/lib/asan/asan_descriptions.h
index 6e23555b35d28..a614f47d461bb 100644
--- a/compiler-rt/lib/asan/asan_descriptions.h
+++ b/compiler-rt/lib/asan/asan_descriptions.h
@@ -15,7 +15,6 @@
#define ASAN_DESCRIPTIONS_H
#include "asan_allocator.h"
-#include "asan_poisoning.h"
#include "asan_thread.h"
#include "sanitizer_common/sanitizer_common.h"
#include "sanitizer_common/sanitizer_report_decorator.h"
@@ -47,9 +46,6 @@ class Decorator : public __sanitizer::SanitizerCommonDecorator {
const char *Allocation() { return Magenta(); }
const char *ShadowByte(u8 byte) {
- if (IsPoisonTrackingMagic(byte))
- return Blue();
-
switch (byte) {
case kAsanHeapLeftRedzoneMagic:
case kAsanArrayCookieMagic:
diff --git a/compiler-rt/lib/asan/asan_errors.cpp b/compiler-rt/lib/asan/asan_errors.cpp
index d0be3d48172da..fbc1c4bd035ae 100644
--- a/compiler-rt/lib/asan/asan_errors.cpp
+++ b/compiler-rt/lib/asan/asan_errors.cpp
@@ -508,18 +508,6 @@ ErrorGeneric::ErrorGeneric(u32 tid, uptr pc_, uptr bp_, uptr sp_, uptr addr,
break;
}
- if (flags()->track_poison > 0 && IsPoisonTrackingMagic(shadow_val)) {
- if (internal_strcmp(bug_descr, "unknown-crash") != 0) {
- Printf(
- "ERROR: use-after-poison tracking magic values overlap with "
- "other constants.\n");
- Printf("Please file a bug.\n");
- } else {
- bug_descr = "use-after-poison";
- bug_type_score = 20;
- }
- }
-
scariness.Scare(bug_type_score + read_after_free_bonus, bug_descr);
if (far_from_bounds) scariness.Scare(10, "far-from-bounds");
}
@@ -565,12 +553,8 @@ static void PrintLegend(InternalScopedString *str) {
PrintShadowByte(str, " Global redzone: ", kAsanGlobalRedzoneMagic);
PrintShadowByte(str, " Global init order: ",
kAsanInitializationOrderMagic);
- // TODO: sync description with PoisonTrackingMagicValues
- PrintShadowByte(
- str, " Poisoned by user: ", kAsanUserPoisonedMemoryMagic,
- flags()->track_poison > 0 ? " with detailed tracking using {0x80-0x8F, "
- "0x90-0x9F, 0xD0-0xDF, 0xE0-0xEF}\n"
- : "\n");
+ PrintShadowByte(str, " Poisoned by user: ",
+ kAsanUserPoisonedMemoryMagic);
PrintShadowByte(str, " Container overflow: ",
kAsanContiguousContainerOOBMagic);
PrintShadowByte(str, " Array cookie: ",
@@ -624,13 +608,11 @@ static void CheckPoisonRecords(uptr addr) {
return;
uptr shadow_addr = MemToShadow(addr);
unsigned char poison_magic = *(reinterpret_cast<u8 *>(shadow_addr));
- int poison_index = PoisonTrackingMagicToIndex[poison_magic];
- if (poison_index < 0 || poison_index >= NumPoisonTrackingMagicValues)
+ if (poison_magic != kAsanUserPoisonedMemoryMagic)
return;
- PoisonRecordRingBuffer *PoisonRecord =
- reinterpret_cast<PoisonRecordRingBuffer *>(PoisonRecords[poison_index]);
+ PoisonRecordRingBuffer *PoisonRecord = GetPoisonRecord();
if (PoisonRecord) {
bool FoundMatch = false;
diff --git a/compiler-rt/lib/asan/asan_poisoning.cpp b/compiler-rt/lib/asan/asan_poisoning.cpp
index ef2640fa0f59a..f5e69c84c5c7b 100644
--- a/compiler-rt/lib/asan/asan_poisoning.cpp
+++ b/compiler-rt/lib/asan/asan_poisoning.cpp
@@ -26,40 +26,17 @@ namespace __asan {
static atomic_uint8_t can_poison_memory;
-PoisonRecordRingBuffer *PoisonRecords[NumPoisonTrackingMagicValues] = {0};
-int PoisonTrackingMagicToIndex[256] = {-1};
+static PoisonRecordRingBuffer *PoisonRecords = nullptr;
void InitializePoisonTracking() {
if (flags()->track_poison <= 0)
return;
- for (unsigned int i = 0; i < sizeof(PoisonTrackingMagicToIndex) / sizeof(int);
- i++) {
- PoisonTrackingMagicToIndex[i] = -1;
- }
-
- for (unsigned int i = 0; i < NumPoisonTrackingMagicValues; i++) {
- int magic = PoisonTrackingIndexToMagic[i];
- CHECK(magic > 0);
- CHECK((unsigned int)magic <
- sizeof(PoisonTrackingMagicToIndex) / sizeof(int));
-
- // Necessary for AddressIsPoisoned calculations
- CHECK((char)magic < 0);
-
- PoisonTrackingMagicToIndex[magic] = i;
-
- PoisonRecords[i] = PoisonRecordRingBuffer::New(flags()->track_poison);
- }
+ PoisonRecords = PoisonRecordRingBuffer::New(flags()->track_poison);
}
-bool IsPoisonTrackingMagic(int byte) {
- return (byte >= 0 &&
- (unsigned long)byte <
- (sizeof(PoisonTrackingMagicToIndex) / sizeof(int)) &&
- PoisonTrackingMagicToIndex[byte] >= 0 &&
- PoisonTrackingMagicToIndex[byte] < NumPoisonTrackingMagicValues &&
- PoisonTrackingIndexToMagic[PoisonTrackingMagicToIndex[byte]] == byte);
+PoisonRecordRingBuffer* GetPoisonRecord() {
+ return PoisonRecords;
}
void SetCanPoisonMemory(bool value) {
@@ -149,23 +126,18 @@ void __asan_poison_memory_region(void const volatile *addr, uptr size) {
if (flags()->track_poison > 0) {
GET_STACK_TRACE(/*max_size=*/ 16, /*fast=*/ false);
+ u32 current_tid = GetCurrentTidOrInvalid();
+
// TODO: garbage collect stacks once they fall off the ring buffer?
// StackDepot doesn't currently have a way to delete stacks.
u32 stack_id = StackDepotPut(stack);
- u32 current_tid = GetCurrentTidOrInvalid();
- u32 poison_index = ((stack_id * 151157) ^ (current_tid * 733123)) %
- NumPoisonTrackingMagicValues;
- poison_magic = PoisonTrackingIndexToMagic[poison_index];
PoisonRecord record{.stack_id = stack_id,
.thread_id = current_tid,
.begin = beg_addr,
.end = end_addr};
- // This is a data race: with concurrent writes, some records may be lost,
- // but it's a sacrifice I am willing to make for speed.
- // The sharding across PoisonRecords reduces the likelihood of
- // concurrent writes.
- PoisonRecords[poison_index]->push(record);
+ // TODO: mutex
+ GetPoisonRecord()->push(record);
}
ShadowSegmentEndpoint beg(beg_addr);
diff --git a/compiler-rt/lib/asan/asan_poisoning.h b/compiler-rt/lib/asan/asan_poisoning.h
index 310d6a578f8d7..bc968a1f8a38b 100644
--- a/compiler-rt/lib/asan/asan_poisoning.h
+++ b/compiler-rt/lib/asan/asan_poisoning.h
@@ -25,21 +25,6 @@
namespace __asan {
-// These need to be negative chars (i.e., in the range [0x80 .. 0xff]) for
-// AddressIsPoisoned calculations.
-static const int PoisonTrackingIndexToMagic[] = {
- 0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87, 0x88, 0x89, 0x8a,
- 0x8b, 0x8c, 0x8d, 0x8e, 0x8f, 0x90, 0x91, 0x92, 0x93, 0x94, 0x95,
- 0x96, 0x97, 0x98, 0x99, 0x9a, 0x9b, 0x9c, 0x9d, 0x9e, 0x9f, 0xd0,
- 0xd1, 0xd2, 0xd3, 0xd4, 0xd5, 0xd6, 0xd7, 0xd8, 0xd9, 0xda, 0xdb,
- 0xdc, 0xdd, 0xde, 0xdf, 0xe0, 0xe1, 0xe2, 0xe3, 0xe4, 0xe5, 0xe6,
- 0xe7, 0xe8, 0xe9, 0xea, 0xeb, 0xec, 0xed, 0xee, 0xef,
-};
-static const int NumPoisonTrackingMagicValues =
- sizeof(PoisonTrackingIndexToMagic) / sizeof(int);
-
-extern int PoisonTrackingMagicToIndex[256];
-
struct PoisonRecord {
unsigned int stack_id;
unsigned int thread_id;
@@ -47,14 +32,12 @@ struct PoisonRecord {
uptr end;
};
-typedef RingBuffer<struct PoisonRecord> PoisonRecordRingBuffer;
-extern PoisonRecordRingBuffer* PoisonRecords[NumPoisonTrackingMagicValues];
+using PoisonRecordRingBuffer = RingBuffer<struct PoisonRecord>;
// Set up data structures for track_poison.
void InitializePoisonTracking();
-// Is this number a magic value used for poison tracking?
-bool IsPoisonTrackingMagic(int byte);
+PoisonRecordRingBuffer* GetPoisonRecord();
// Enable/disable memory poisoning.
void SetCanPoisonMemory(bool value);
>From f0800219e61d248a31b6867e01f6cd004201b39a Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Thu, 27 Mar 2025 16:26:56 +0000
Subject: [PATCH 4/5] Add mutex
---
compiler-rt/lib/asan/asan_errors.cpp | 3 ++-
compiler-rt/lib/asan/asan_poisoning.cpp | 13 ++++++++++---
compiler-rt/lib/asan/asan_poisoning.h | 4 +++-
3 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_errors.cpp b/compiler-rt/lib/asan/asan_errors.cpp
index fbc1c4bd035ae..44856a659f249 100644
--- a/compiler-rt/lib/asan/asan_errors.cpp
+++ b/compiler-rt/lib/asan/asan_errors.cpp
@@ -612,7 +612,7 @@ static void CheckPoisonRecords(uptr addr) {
if (poison_magic != kAsanUserPoisonedMemoryMagic)
return;
- PoisonRecordRingBuffer *PoisonRecord = GetPoisonRecord();
+ PoisonRecordRingBuffer *PoisonRecord = AcquirePoisonRecords();
if (PoisonRecord) {
bool FoundMatch = false;
@@ -637,6 +637,7 @@ static void CheckPoisonRecords(uptr addr) {
Printf("Try setting a larger track_poison value.\n");
}
}
+ ReleasePoisonRecords();
}
void ErrorGeneric::Print() {
diff --git a/compiler-rt/lib/asan/asan_poisoning.cpp b/compiler-rt/lib/asan/asan_poisoning.cpp
index f5e69c84c5c7b..9c2e08303a989 100644
--- a/compiler-rt/lib/asan/asan_poisoning.cpp
+++ b/compiler-rt/lib/asan/asan_poisoning.cpp
@@ -26,6 +26,7 @@ namespace __asan {
static atomic_uint8_t can_poison_memory;
+static Mutex PoisonRecordsMutex;
static PoisonRecordRingBuffer *PoisonRecords = nullptr;
void InitializePoisonTracking() {
@@ -35,10 +36,16 @@ void InitializePoisonTracking() {
PoisonRecords = PoisonRecordRingBuffer::New(flags()->track_poison);
}
-PoisonRecordRingBuffer* GetPoisonRecord() {
+PoisonRecordRingBuffer* SANITIZER_ACQUIRE(PoisonRecordsMutex) AcquirePoisonRecords() {
+ PoisonRecordsMutex.Lock();
+
return PoisonRecords;
}
+void SANITIZER_RELEASE(PoisonRecordsMutex) ReleasePoisonRecords() {
+ PoisonRecordsMutex.Unlock();
+}
+
void SetCanPoisonMemory(bool value) {
atomic_store(&can_poison_memory, value, memory_order_release);
}
@@ -136,8 +143,8 @@ void __asan_poison_memory_region(void const volatile *addr, uptr size) {
.thread_id = current_tid,
.begin = beg_addr,
.end = end_addr};
- // TODO: mutex
- GetPoisonRecord()->push(record);
+ AcquirePoisonRecords()->push(record);
+ ReleasePoisonRecords();
}
ShadowSegmentEndpoint beg(beg_addr);
diff --git a/compiler-rt/lib/asan/asan_poisoning.h b/compiler-rt/lib/asan/asan_poisoning.h
index bc968a1f8a38b..ae941fa8b43c7 100644
--- a/compiler-rt/lib/asan/asan_poisoning.h
+++ b/compiler-rt/lib/asan/asan_poisoning.h
@@ -10,6 +10,7 @@
//
// Shadow memory poisoning by ASan RTL and by user application.
//===----------------------------------------------------------------------===//
+
#ifndef ASAN_POISONING_H
#define ASAN_POISONING_H
@@ -37,7 +38,8 @@ using PoisonRecordRingBuffer = RingBuffer<struct PoisonRecord>;
// Set up data structures for track_poison.
void InitializePoisonTracking();
-PoisonRecordRingBuffer* GetPoisonRecord();
+PoisonRecordRingBuffer* AcquirePoisonRecords();
+void ReleasePoisonRecords();
// Enable/disable memory poisoning.
void SetCanPoisonMemory(bool value);
>From 7882b797fe0c8bac4e3674266f2f3627714b0441 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Thu, 27 Mar 2025 16:28:12 +0000
Subject: [PATCH 5/5] clang-format
---
compiler-rt/lib/asan/asan_errors.cpp | 4 ++--
compiler-rt/lib/asan/asan_poisoning.cpp | 3 ++-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_errors.cpp b/compiler-rt/lib/asan/asan_errors.cpp
index 44856a659f249..5a99eabe1716d 100644
--- a/compiler-rt/lib/asan/asan_errors.cpp
+++ b/compiler-rt/lib/asan/asan_errors.cpp
@@ -553,8 +553,8 @@ static void PrintLegend(InternalScopedString *str) {
PrintShadowByte(str, " Global redzone: ", kAsanGlobalRedzoneMagic);
PrintShadowByte(str, " Global init order: ",
kAsanInitializationOrderMagic);
- PrintShadowByte(str, " Poisoned by user: ",
- kAsanUserPoisonedMemoryMagic);
+ PrintShadowByte(str,
+ " Poisoned by user: ", kAsanUserPoisonedMemoryMagic);
PrintShadowByte(str, " Container overflow: ",
kAsanContiguousContainerOOBMagic);
PrintShadowByte(str, " Array cookie: ",
diff --git a/compiler-rt/lib/asan/asan_poisoning.cpp b/compiler-rt/lib/asan/asan_poisoning.cpp
index 9c2e08303a989..a7f8473432635 100644
--- a/compiler-rt/lib/asan/asan_poisoning.cpp
+++ b/compiler-rt/lib/asan/asan_poisoning.cpp
@@ -36,7 +36,8 @@ void InitializePoisonTracking() {
PoisonRecords = PoisonRecordRingBuffer::New(flags()->track_poison);
}
-PoisonRecordRingBuffer* SANITIZER_ACQUIRE(PoisonRecordsMutex) AcquirePoisonRecords() {
+PoisonRecordRingBuffer *SANITIZER_ACQUIRE(PoisonRecordsMutex)
+ AcquirePoisonRecords() {
PoisonRecordsMutex.Lock();
return PoisonRecords;
More information about the llvm-commits
mailing list