[compiler-rt] [asan] Fix misalignment of variables in fake stack frames (PR #152819)

via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 8 18:56:11 PDT 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Thurston Dang (thurstond)

<details>
<summary>Changes</summary>

ASan's instrumentation pass uses ASanStackFrameLayout::ComputeASanStackFrameLayout() to calculate the offset of variables, taking into account alignment. However, the fake stacks (specifically, GetFrame()) are not sufficiently aligned, hence the offset addresses are not guaranteed to be aligned.

This change fixes the misalignment issue by aligning the fake stacks and padding the space used for flags, to maintain the alignment invariant.

This costs approximately 60KB of additional memory per fake stack.

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


3 Files Affected:

- (modified) compiler-rt/lib/asan/asan_fake_stack.cpp (+14-4) 
- (modified) compiler-rt/lib/asan/asan_fake_stack.h (+16-5) 
- (modified) compiler-rt/lib/asan/tests/asan_fake_stack_test.cpp (+8-8) 


``````````diff
diff --git a/compiler-rt/lib/asan/asan_fake_stack.cpp b/compiler-rt/lib/asan/asan_fake_stack.cpp
index 0f696075fb78d..baef8069a485b 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.cpp
+++ b/compiler-rt/lib/asan/asan_fake_stack.cpp
@@ -55,9 +55,16 @@ FakeStack *FakeStack::Create(uptr stack_size_log) {
   if (stack_size_log > kMaxStackSizeLog)
     stack_size_log = kMaxStackSizeLog;
   uptr size = RequiredSize(stack_size_log);
+  uptr padded_size = size + (1 << kMaxStackFrameSizeLog);
+  // Alignment here is needed to protect the alignment invariant in GetFrame()
+  // MmapAlignedOrDieOnFatalError requires that the *size* is a power of 2,
+  // which is an overly strong condition.
+  void *true_res = reinterpret_cast<void *>(
+      flags()->uar_noreserve ? MmapNoReserveOrDie(padded_size, "FakeStack")
+                             : MmapOrDie(padded_size, "FakeStack"));
   FakeStack *res = reinterpret_cast<FakeStack *>(
-      flags()->uar_noreserve ? MmapNoReserveOrDie(size, "FakeStack")
-                             : MmapOrDie(size, "FakeStack"));
+      RoundUpTo((uptr)true_res, 1 << kMaxStackFrameSizeLog));
+  res->true_start = true_res;
   res->stack_size_log_ = stack_size_log;
   u8 *p = reinterpret_cast<u8 *>(res);
   VReport(1,
@@ -79,8 +86,11 @@ void FakeStack::Destroy(int tid) {
     Report("T%d: FakeStack destroyed: %s\n", tid, str.data());
   }
   uptr size = RequiredSize(stack_size_log_);
-  FlushUnneededASanShadowMemory(reinterpret_cast<uptr>(this), size);
-  UnmapOrDie(this, size);
+  uptr padded_size = size + (1 << kMaxStackFrameSizeLog);
+  void *true_start = this->true_start;
+  FlushUnneededASanShadowMemory(reinterpret_cast<uptr>(true_start),
+                                padded_size);
+  UnmapOrDie(true_start, padded_size);
 }
 
 void FakeStack::PoisonAll(u8 magic) {
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index 270a19816d6e2..5255f1bf443f7 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -42,11 +42,16 @@ struct FakeFrame {
 // is a power of two, starting from 64 bytes. Each size class occupies
 // stack_size bytes and thus can allocate
 // NumberOfFrames=(stack_size/BytesInSizeClass) fake frames (also a power of 2).
+//
 // For each size class we have NumberOfFrames allocation flags,
 // each flag indicates whether the given frame is currently allocated.
-// All flags for size classes 0 .. 10 are stored in a single contiguous region
-// followed by another contiguous region which contains the actual memory for
-// size classes. The addresses are computed by GetFlags and GetFrame without
+//
+// All flags for size classes 0 .. 10 are stored in a single contiguous region,
+// padded to 2**kMaxStackFrameSizeLog (to prevent frames from becoming
+// unaligned), followed by another contiguous region which contains the actual
+// memory for size classes.
+//
+// The addresses are computed by GetFlags and GetFrame without
 // any memory accesses solely based on 'this' and stack_size_log.
 // Allocate() flips the appropriate allocation flag atomically, thus achieving
 // async-signal safety.
@@ -68,7 +73,9 @@ class FakeStack {
 
   // stack_size_log is at least 15 (stack_size >= 32K).
   static uptr SizeRequiredForFlags(uptr stack_size_log) {
-    return ((uptr)1) << (stack_size_log + 1 - kMinStackFrameSizeLog);
+    // Padding is needed to protect alignment in GetFrame().
+    uptr size = ((uptr)1) << (stack_size_log + 1 - kMinStackFrameSizeLog);
+    return RoundUpTo(size + 1, 1 << kMaxStackFrameSizeLog) - kFlagsOffset;
   }
 
   // Each size class occupies stack_size bytes.
@@ -156,7 +163,7 @@ class FakeStack {
 
  private:
   FakeStack() { }
-  static const uptr kFlagsOffset = 4096;  // This is were the flags begin.
+  static const uptr kFlagsOffset = 4096;  // This is where the flags begin.
   // Must match the number of uses of DEFINE_STACK_MALLOC_FREE_WITH_CLASS_ID
   COMPILER_CHECK(kNumberOfSizeClasses == 11);
   static const uptr kMaxStackMallocSize = ((uptr)1) << kMaxStackFrameSizeLog;
@@ -165,6 +172,10 @@ class FakeStack {
   uptr stack_size_log_;
   // a bit is set if something was allocated from the corresponding size class.
   bool needs_gc_;
+  // We allocated more memory than needed to ensure the FakeStack (and, by
+  // extension, each of the fake stack frames) is aligned. We keep track of the
+  // true start so that we can unmap it.
+  void *true_start;
 };
 
 FakeStack *GetTLSFakeStack();
diff --git a/compiler-rt/lib/asan/tests/asan_fake_stack_test.cpp b/compiler-rt/lib/asan/tests/asan_fake_stack_test.cpp
index 504b0aaf30143..d48a10532685a 100644
--- a/compiler-rt/lib/asan/tests/asan_fake_stack_test.cpp
+++ b/compiler-rt/lib/asan/tests/asan_fake_stack_test.cpp
@@ -25,9 +25,9 @@
 namespace __asan {
 
 TEST(FakeStack, FlagsSize) {
-  EXPECT_EQ(FakeStack::SizeRequiredForFlags(10), 1U << 5);
-  EXPECT_EQ(FakeStack::SizeRequiredForFlags(11), 1U << 6);
-  EXPECT_EQ(FakeStack::SizeRequiredForFlags(20), 1U << 15);
+  EXPECT_EQ(FakeStack::SizeRequiredForFlags(10), (1U << 16) - 4096);
+  EXPECT_EQ(FakeStack::SizeRequiredForFlags(11), (1U << 16) - 4096);
+  EXPECT_EQ(FakeStack::SizeRequiredForFlags(20), (1U << 16) - 4096);
 }
 
 TEST(FakeStack, RequiredSize) {
@@ -35,11 +35,11 @@ TEST(FakeStack, RequiredSize) {
   //  uptr alloc_size = FakeStack::RequiredSize(i);
   //  printf("%zdK ==> %zd\n", 1 << (i - 10), alloc_size);
   // }
-  EXPECT_EQ(FakeStack::RequiredSize(15), 365568U);
-  EXPECT_EQ(FakeStack::RequiredSize(16), 727040U);
-  EXPECT_EQ(FakeStack::RequiredSize(17), 1449984U);
-  EXPECT_EQ(FakeStack::RequiredSize(18), 2895872U);
-  EXPECT_EQ(FakeStack::RequiredSize(19), 5787648U);
+  EXPECT_EQ(FakeStack::RequiredSize(15), 425984U);
+  EXPECT_EQ(FakeStack::RequiredSize(16), 786432U);
+  EXPECT_EQ(FakeStack::RequiredSize(17), 1507328U);
+  EXPECT_EQ(FakeStack::RequiredSize(18), 2949120U);
+  EXPECT_EQ(FakeStack::RequiredSize(19), 5832704U);
 }
 
 TEST(FakeStack, FlagsOffset) {

``````````

</details>


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


More information about the llvm-commits mailing list