[compiler-rt] [asan] Fix misalignment of variables in fake stack frames (PR #152819)
Thurston Dang via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 8 18:55:36 PDT 2025
https://github.com/thurstond created https://github.com/llvm/llvm-project/pull/152819
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.
>From 45198b2da0575bc36aa6746d76103f5a153bc87d Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 01:24:22 +0000
Subject: [PATCH] [asan] Fix misalignment of variables in fake stack frames
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 memory used to
back the fake stacks, and also padding the space used for flags, to
maintain an alignment invariant.
This costs approximately 128KB of additional memory per fake stack.
---
compiler-rt/lib/asan/asan_fake_stack.cpp | 18 ++++++++++++----
compiler-rt/lib/asan/asan_fake_stack.h | 21 ++++++++++++++-----
.../lib/asan/tests/asan_fake_stack_test.cpp | 16 +++++++-------
3 files changed, 38 insertions(+), 17 deletions(-)
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) {
More information about the llvm-commits
mailing list