[compiler-rt] [llvm] [asan] Fix misalignment of variables in fake stack frames (PR #152819)
Thurston Dang via llvm-commits
llvm-commits at lists.llvm.org
Sat Aug 9 20:57:20 PDT 2025
https://github.com/thurstond updated https://github.com/llvm/llvm-project/pull/152819
>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 01/24] [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) {
>From fdfcc01b20a1e82d85203d56ce450986b5c0711a Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 03:55:24 +0000
Subject: [PATCH 02/24] Update SizeRequiredForFlags
---
compiler-rt/lib/asan/asan_fake_stack.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index 5255f1bf443f7..3eb2aaf6c731a 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -75,7 +75,8 @@ class FakeStack {
static uptr SizeRequiredForFlags(uptr stack_size_log) {
// Padding is needed to protect alignment in GetFrame().
uptr size = ((uptr)1) << (stack_size_log + 1 - kMinStackFrameSizeLog);
- return RoundUpTo(size + 1, 1 << kMaxStackFrameSizeLog) - kFlagsOffset;
+ return RoundUpTo(size + kFlagsOffset, 1 << kMaxStackFrameSizeLog) -
+ kFlagsOffset;
}
// Each size class occupies stack_size bytes.
>From 0b69b3bf0b74a9402e8b93e4ea93492837e90ed3 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 04:41:54 +0000
Subject: [PATCH 03/24] Proof of alignment invariant in GetFrame()
---
compiler-rt/lib/asan/asan_fake_stack.h | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index 3eb2aaf6c731a..16209592ee21b 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -48,8 +48,8 @@ struct FakeFrame {
//
// 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.
+// unaligned; see also GetFrame()), 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.
@@ -118,6 +118,27 @@ class FakeStack {
}
// Get frame by class_id and pos.
+ //
+ // ASanStackFrameLayout::ComputeASanStackFrameLayout() will align variables
+ // correctly if GetFrame() returns addresses aligned to
+ // BytesInSizeClass(class_id).
+ //
+ // In steps 1-3, we prove an even stronger property: GetFrame() returns
+ // addresses aligned to 1<<kMaxStackFrameSizeLog (aka
+ // BytesInSizeClass(max_class_id)), which implies alignment to
+ // BytesInSizeClass for any class_id, since they are increasing powers of 2.
+ //
+ // 1) 'this' is aligned to 1<<kMaxStackFrameSizeLog (see FakeStack::Create)
+ // 2) (kFlagsOffset + SizeRequiredForFlags()) is aligned to
+ // 1<<kMaxStackFrameSizeLog (see SizeRequiredForFlags())
+ // 3) We know that stack_size_log >= kMaxStackFrameSizeLog (otherwise you
+ // couldn't store a single frame in the entire stack)
+ // hence (1 << stack_size_log) is aligned to 1<<kMaxStackFrameSizeLog
+ // and ((1 << stack_size_log) * class_id) is aligned to
+ // 1<<kMaxStackFrameSizeLog
+ // 4) BytesInSizeClass(class_id) * pos is aligned to
+ // BytesInSizeClass(class_id);
+ // The sum of these is aligned to BytesInSizeClass(max_class_id).
u8 *GetFrame(uptr stack_size_log, uptr class_id, uptr pos) {
return reinterpret_cast<u8 *>(this) + kFlagsOffset +
SizeRequiredForFlags(stack_size_log) +
>From fe98da49729a3e74c5fd198fb211c539f91dd52f Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 04:45:06 +0000
Subject: [PATCH 04/24] GetFrame() wording
---
compiler-rt/lib/asan/asan_fake_stack.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index 16209592ee21b..dd98026da67e6 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -123,10 +123,9 @@ class FakeStack {
// correctly if GetFrame() returns addresses aligned to
// BytesInSizeClass(class_id).
//
- // In steps 1-3, we prove an even stronger property: GetFrame() returns
- // addresses aligned to 1<<kMaxStackFrameSizeLog (aka
- // BytesInSizeClass(max_class_id)), which implies alignment to
- // BytesInSizeClass for any class_id, since they are increasing powers of 2.
+ // Note that alignment to 1<<kMaxStackFrameSizeLog (aka
+ // BytesInSizeClass(max_class_id)) implies alignment to BytesInSizeClass for
+ // any class_id, since the class sizes are increasing powers of 2.
//
// 1) 'this' is aligned to 1<<kMaxStackFrameSizeLog (see FakeStack::Create)
// 2) (kFlagsOffset + SizeRequiredForFlags()) is aligned to
>From 58e18411aab407e9adf2ab669f7a94d7a8643f69 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 04:46:34 +0000
Subject: [PATCH 05/24] Punctuation
---
compiler-rt/lib/asan/asan_fake_stack.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index dd98026da67e6..c28395374ab8b 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -124,8 +124,8 @@ class FakeStack {
// BytesInSizeClass(class_id).
//
// Note that alignment to 1<<kMaxStackFrameSizeLog (aka
- // BytesInSizeClass(max_class_id)) implies alignment to BytesInSizeClass for
- // any class_id, since the class sizes are increasing powers of 2.
+ // BytesInSizeClass(max_class_id)) implies alignment to BytesInSizeClass()
+ // for any class_id, since the class sizes are increasing powers of 2.
//
// 1) 'this' is aligned to 1<<kMaxStackFrameSizeLog (see FakeStack::Create)
// 2) (kFlagsOffset + SizeRequiredForFlags()) is aligned to
>From 222c89f99a974e9bbae1d9845daf5ca78b2ec633 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 04:47:20 +0000
Subject: [PATCH 06/24] Wording
---
compiler-rt/lib/asan/asan_fake_stack.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index c28395374ab8b..72385f6f3c6be 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -131,7 +131,7 @@ class FakeStack {
// 2) (kFlagsOffset + SizeRequiredForFlags()) is aligned to
// 1<<kMaxStackFrameSizeLog (see SizeRequiredForFlags())
// 3) We know that stack_size_log >= kMaxStackFrameSizeLog (otherwise you
- // couldn't store a single frame in the entire stack)
+ // couldn't store a single frame of that size in the entire stack)
// hence (1 << stack_size_log) is aligned to 1<<kMaxStackFrameSizeLog
// and ((1 << stack_size_log) * class_id) is aligned to
// 1<<kMaxStackFrameSizeLog
>From ccb538f08ebc18ba10768be7a590b0f6e65755f3 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 04:48:36 +0000
Subject: [PATCH 07/24] Fix claim
---
compiler-rt/lib/asan/asan_fake_stack.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index 72385f6f3c6be..13dc4fc9371bf 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -136,8 +136,8 @@ class FakeStack {
// and ((1 << stack_size_log) * class_id) is aligned to
// 1<<kMaxStackFrameSizeLog
// 4) BytesInSizeClass(class_id) * pos is aligned to
- // BytesInSizeClass(class_id);
- // The sum of these is aligned to BytesInSizeClass(max_class_id).
+ // BytesInSizeClass(class_id)
+ // The sum of these is aligned to BytesInSizeClass(class_id).
u8 *GetFrame(uptr stack_size_log, uptr class_id, uptr pos) {
return reinterpret_cast<u8 *>(this) + kFlagsOffset +
SizeRequiredForFlags(stack_size_log) +
>From 48bf1aa891d7233e668768f2cf61c5eb62ee083f Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 05:08:49 +0000
Subject: [PATCH 08/24] Comment on near-optimality
---
compiler-rt/lib/asan/asan_fake_stack.h | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index 13dc4fc9371bf..8b2b9b93d0dd3 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -125,7 +125,7 @@ class FakeStack {
//
// Note that alignment to 1<<kMaxStackFrameSizeLog (aka
// BytesInSizeClass(max_class_id)) implies alignment to BytesInSizeClass()
- // for any class_id, since the class sizes are increasing powers of 2.
+ // for any class_id, since the class sizes are increasing powers of 2. [*]
//
// 1) 'this' is aligned to 1<<kMaxStackFrameSizeLog (see FakeStack::Create)
// 2) (kFlagsOffset + SizeRequiredForFlags()) is aligned to
@@ -138,6 +138,12 @@ class FakeStack {
// 4) BytesInSizeClass(class_id) * pos is aligned to
// BytesInSizeClass(class_id)
// The sum of these is aligned to BytesInSizeClass(class_id).
+ //
+ // This is nearly-optimal: we must ensure that GetFrame() is aligned to
+ // BytesInSizeClass(max_class_id)) when called with max_class_id, and
+ // alignment implies a cost of up to (1<<kMaxStackFrameSizeLog) bytes. Our
+ // alignment in steps 1) and 2) incur nearly double this cost.
+ // Corollary of [*]: we pay the cost of aligning to
u8 *GetFrame(uptr stack_size_log, uptr class_id, uptr pos) {
return reinterpret_cast<u8 *>(this) + kFlagsOffset +
SizeRequiredForFlags(stack_size_log) +
>From bbecef46313c51f624c3c2f49908198b1b8a2994 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 05:09:24 +0000
Subject: [PATCH 09/24] Wording
---
compiler-rt/lib/asan/asan_fake_stack.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index 8b2b9b93d0dd3..566a1dc1d7528 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -142,8 +142,7 @@ class FakeStack {
// This is nearly-optimal: we must ensure that GetFrame() is aligned to
// BytesInSizeClass(max_class_id)) when called with max_class_id, and
// alignment implies a cost of up to (1<<kMaxStackFrameSizeLog) bytes. Our
- // alignment in steps 1) and 2) incur nearly double this cost.
- // Corollary of [*]: we pay the cost of aligning to
+ // alignment in steps 1) and 2) incurs nearly double this cost.
u8 *GetFrame(uptr stack_size_log, uptr class_id, uptr pos) {
return reinterpret_cast<u8 *>(this) + kFlagsOffset +
SizeRequiredForFlags(stack_size_log) +
>From 0e5b51405e56321b7078847860b94f1e6e460230 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 05:30:52 +0000
Subject: [PATCH 10/24] Simplify alignment step to FakeStack::Create() only
---
compiler-rt/lib/asan/asan_fake_stack.cpp | 15 +++++++-----
compiler-rt/lib/asan/asan_fake_stack.h | 23 ++++++-------------
.../lib/asan/tests/asan_fake_stack_test.cpp | 16 ++++++-------
3 files changed, 24 insertions(+), 30 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.cpp b/compiler-rt/lib/asan/asan_fake_stack.cpp
index baef8069a485b..badc07d176b24 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.cpp
+++ b/compiler-rt/lib/asan/asan_fake_stack.cpp
@@ -56,23 +56,26 @@ FakeStack *FakeStack::Create(uptr stack_size_log) {
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"));
+ // GetFrame() requires the property that
+ // (res + kFlagsOffset + SizeRequiredForFlags(stack_size_log)) is aligned to
+ // (1 << kMaxStackFrameSizeLog)
+ // We didn't use MmapAlignedOrDieOnFatalError, because it requires that the
+ // *size* is a power of 2, which is an overly strong condition.
FakeStack *res = reinterpret_cast<FakeStack *>(
- RoundUpTo((uptr)true_res, 1 << kMaxStackFrameSizeLog));
+ RoundUpTo((uptr)true_res + kFlagsOffset + SizeRequiredForFlags(stack_size_log), 1 << kMaxStackFrameSizeLog)
+ - kFlagsOffset - SizeRequiredForFlags(stack_size_log));
res->true_start = true_res;
res->stack_size_log_ = stack_size_log;
u8 *p = reinterpret_cast<u8 *>(res);
VReport(1,
"T%d: FakeStack created: %p -- %p stack_size_log: %zd; "
- "mmapped %zdK, noreserve=%d \n",
+ "mmapped %zdK, noreserve=%d, true_start: %p\n",
GetCurrentTidOrInvalid(), (void *)p,
(void *)(p + FakeStack::RequiredSize(stack_size_log)), stack_size_log,
- size >> 10, flags()->uar_noreserve);
+ size >> 10, flags()->uar_noreserve, res->true_start);
return res;
}
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index 566a1dc1d7528..80863d788cd73 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -73,10 +73,7 @@ class FakeStack {
// stack_size_log is at least 15 (stack_size >= 32K).
static uptr SizeRequiredForFlags(uptr stack_size_log) {
- // Padding is needed to protect alignment in GetFrame().
- uptr size = ((uptr)1) << (stack_size_log + 1 - kMinStackFrameSizeLog);
- return RoundUpTo(size + kFlagsOffset, 1 << kMaxStackFrameSizeLog) -
- kFlagsOffset;
+ return ((uptr)1) << (stack_size_log + 1 - kMinStackFrameSizeLog);
}
// Each size class occupies stack_size bytes.
@@ -120,29 +117,23 @@ class FakeStack {
// Get frame by class_id and pos.
//
// ASanStackFrameLayout::ComputeASanStackFrameLayout() will align variables
- // correctly if GetFrame() returns addresses aligned to
+ // correctly if GetFrame(..., class_id, ...) returns addresses aligned to
// BytesInSizeClass(class_id).
//
// Note that alignment to 1<<kMaxStackFrameSizeLog (aka
// BytesInSizeClass(max_class_id)) implies alignment to BytesInSizeClass()
- // for any class_id, since the class sizes are increasing powers of 2. [*]
+ // for any class_id, since the class sizes are increasing powers of 2.
//
- // 1) 'this' is aligned to 1<<kMaxStackFrameSizeLog (see FakeStack::Create)
- // 2) (kFlagsOffset + SizeRequiredForFlags()) is aligned to
- // 1<<kMaxStackFrameSizeLog (see SizeRequiredForFlags())
- // 3) We know that stack_size_log >= kMaxStackFrameSizeLog (otherwise you
+ // 1) ('this' + kFlagsOffset + SizeRequiredForFlags())) is aligned to
+ // 1<<kMaxStackFrameSizeLog (see FakeStack::Create)
+ // 2) We know that stack_size_log >= kMaxStackFrameSizeLog (otherwise you
// couldn't store a single frame of that size in the entire stack)
// hence (1 << stack_size_log) is aligned to 1<<kMaxStackFrameSizeLog
// and ((1 << stack_size_log) * class_id) is aligned to
// 1<<kMaxStackFrameSizeLog
- // 4) BytesInSizeClass(class_id) * pos is aligned to
+ // 3) BytesInSizeClass(class_id) * pos is aligned to
// BytesInSizeClass(class_id)
// The sum of these is aligned to BytesInSizeClass(class_id).
- //
- // This is nearly-optimal: we must ensure that GetFrame() is aligned to
- // BytesInSizeClass(max_class_id)) when called with max_class_id, and
- // alignment implies a cost of up to (1<<kMaxStackFrameSizeLog) bytes. Our
- // alignment in steps 1) and 2) incurs nearly double this cost.
u8 *GetFrame(uptr stack_size_log, uptr class_id, uptr pos) {
return reinterpret_cast<u8 *>(this) + kFlagsOffset +
SizeRequiredForFlags(stack_size_log) +
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 d48a10532685a..17b6cc1bd80f8 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 << 16) - 4096);
- EXPECT_EQ(FakeStack::SizeRequiredForFlags(11), (1U << 16) - 4096);
- EXPECT_EQ(FakeStack::SizeRequiredForFlags(20), (1U << 16) - 4096);
+ EXPECT_EQ(FakeStack::SizeRequiredForFlags(10), 1U << 16);
+ EXPECT_EQ(FakeStack::SizeRequiredForFlags(11), 1U << 16);
+ EXPECT_EQ(FakeStack::SizeRequiredForFlags(20), 1U << 16);
}
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), 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);
+ EXPECT_EQ(FakeStack::RequiredSize(15), 430080U);
+ EXPECT_EQ(FakeStack::RequiredSize(16), 790528U);
+ EXPECT_EQ(FakeStack::RequiredSize(17), 1511424U);
+ EXPECT_EQ(FakeStack::RequiredSize(18), 2953216U);
+ EXPECT_EQ(FakeStack::RequiredSize(19), 5836800U);
}
TEST(FakeStack, FlagsOffset) {
>From 570f8cf2cb12787227a3deda2b91f98b261d9a69 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 05:34:19 +0000
Subject: [PATCH 11/24] Revert test
---
.../lib/asan/tests/asan_fake_stack_test.cpp | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
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 17b6cc1bd80f8..504b0aaf30143 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 << 16);
- EXPECT_EQ(FakeStack::SizeRequiredForFlags(11), 1U << 16);
- EXPECT_EQ(FakeStack::SizeRequiredForFlags(20), 1U << 16);
+ EXPECT_EQ(FakeStack::SizeRequiredForFlags(10), 1U << 5);
+ EXPECT_EQ(FakeStack::SizeRequiredForFlags(11), 1U << 6);
+ EXPECT_EQ(FakeStack::SizeRequiredForFlags(20), 1U << 15);
}
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), 430080U);
- EXPECT_EQ(FakeStack::RequiredSize(16), 790528U);
- EXPECT_EQ(FakeStack::RequiredSize(17), 1511424U);
- EXPECT_EQ(FakeStack::RequiredSize(18), 2953216U);
- EXPECT_EQ(FakeStack::RequiredSize(19), 5836800U);
+ 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);
}
TEST(FakeStack, FlagsOffset) {
>From d4441e4c9befe2841c123d21b13927f32fe86bcf Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 05:37:35 +0000
Subject: [PATCH 12/24] Update FakeStack comment
---
compiler-rt/lib/asan/asan_fake_stack.h | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index 80863d788cd73..f3f72d1347250 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -32,29 +32,26 @@ struct FakeFrame {
// is not popped but remains there for quite some time until gets used again.
// So, we poison the objects on the fake stack when function returns.
// It helps us find use-after-return bugs.
-//
// The FakeStack objects is allocated by a single mmap call and has no other
// pointers. The size of the fake stack depends on the actual thread stack size
// and thus can not be a constant.
// stack_size is a power of two greater or equal to the thread's stack size;
// we store it as its logarithm (stack_size_log).
+// FakeStack is padded such that GetFrame() is aligned to BytesInSizeClass().
// FakeStack has kNumberOfSizeClasses (11) size classes, each size class
// 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,
-// padded to 2**kMaxStackFrameSizeLog (to prevent frames from becoming
-// unaligned; see also GetFrame()), 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
+// 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.
+// Allocate() flips the appropriate allocation flag atomically, thus achieving
+// async-signal safety.
// This allocator does not have quarantine per se, but it tries to allocate the
// frames in round robin fashion to maximize the delay between a deallocation
// and the next allocation.
>From 7b314b51664914de57ddfb71886e3b71bc16535f Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 05:38:37 +0000
Subject: [PATCH 13/24] Remove duplicated comment
---
compiler-rt/lib/asan/asan_fake_stack.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index f3f72d1347250..9f39fa04c5fba 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -50,8 +50,6 @@ struct FakeFrame {
// any memory accesses solely based on 'this' and stack_size_log.
// Allocate() flips the appropriate allocation flag atomically, thus achieving
// async-signal safety.
-// Allocate() flips the appropriate allocation flag atomically, thus achieving
-// async-signal safety.
// This allocator does not have quarantine per se, but it tries to allocate the
// frames in round robin fashion to maximize the delay between a deallocation
// and the next allocation.
>From 6cef8c51954af3b667d20171c5d20a9915e23fd8 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 05:40:17 +0000
Subject: [PATCH 14/24] Update comment
---
compiler-rt/lib/asan/asan_fake_stack.h | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index 9f39fa04c5fba..c6990f98e29d5 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -110,10 +110,9 @@ class FakeStack {
}
// Get frame by class_id and pos.
- //
- // ASanStackFrameLayout::ComputeASanStackFrameLayout() will align variables
- // correctly if GetFrame(..., class_id, ...) returns addresses aligned to
- // BytesInSizeClass(class_id).
+ // Return values are guaranteed to be aligned to BytesInSizeClass(class_id),
+ // which is useful in combination with
+ // ASanStackFrameLayout::ComputeASanStackFrameLayout().
//
// Note that alignment to 1<<kMaxStackFrameSizeLog (aka
// BytesInSizeClass(max_class_id)) implies alignment to BytesInSizeClass()
>From 46e7c1e84801245eddf4a32676f707bb2a715103 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 05:41:26 +0000
Subject: [PATCH 15/24] Formatting
---
compiler-rt/lib/asan/asan_fake_stack.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index c6990f98e29d5..2f983bf66834a 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -118,12 +118,12 @@ class FakeStack {
// BytesInSizeClass(max_class_id)) implies alignment to BytesInSizeClass()
// for any class_id, since the class sizes are increasing powers of 2.
//
- // 1) ('this' + kFlagsOffset + SizeRequiredForFlags())) is aligned to
+ // 1) (this + kFlagsOffset + SizeRequiredForFlags())) is aligned to
// 1<<kMaxStackFrameSizeLog (see FakeStack::Create)
// 2) We know that stack_size_log >= kMaxStackFrameSizeLog (otherwise you
// couldn't store a single frame of that size in the entire stack)
- // hence (1 << stack_size_log) is aligned to 1<<kMaxStackFrameSizeLog
- // and ((1 << stack_size_log) * class_id) is aligned to
+ // hence (1<<stack_size_log) is aligned to 1<<kMaxStackFrameSizeLog
+ // and ((1<<stack_size_log) * class_id) is aligned to
// 1<<kMaxStackFrameSizeLog
// 3) BytesInSizeClass(class_id) * pos is aligned to
// BytesInSizeClass(class_id)
>From 07ee0bb88fd6f6f83adcf6554a3c7fb6756a5c56 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 05:57:06 +0000
Subject: [PATCH 16/24] Add alignment check to test
---
compiler-rt/lib/asan/tests/asan_fake_stack_test.cpp | 1 +
1 file changed, 1 insertion(+)
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..c60e2eadc35db 100644
--- a/compiler-rt/lib/asan/tests/asan_fake_stack_test.cpp
+++ b/compiler-rt/lib/asan/tests/asan_fake_stack_test.cpp
@@ -113,6 +113,7 @@ TEST(FakeStack, Allocate) {
uptr bytes_in_class = FakeStack::BytesInSizeClass(cid);
for (uptr j = 0; j < n; j++) {
FakeFrame *ff = fs->Allocate(stack_size_log, cid, 0);
+ EXPECT_EQ(reinterpret_cast<uptr>(ff) % bytes_in_class, 0U);
uptr x = reinterpret_cast<uptr>(ff);
EXPECT_TRUE(s.insert(std::make_pair(ff, cid)).second);
EXPECT_EQ(x, fs->AddrIsInFakeStack(x));
>From 8282b9bc337f87d84d7bcd513a09c40f1109c0fe Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 06:02:11 +0000
Subject: [PATCH 17/24] clang-format
---
compiler-rt/lib/asan/asan_fake_stack.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.cpp b/compiler-rt/lib/asan/asan_fake_stack.cpp
index badc07d176b24..e9079e9369d5b 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.cpp
+++ b/compiler-rt/lib/asan/asan_fake_stack.cpp
@@ -65,8 +65,10 @@ FakeStack *FakeStack::Create(uptr stack_size_log) {
// We didn't use MmapAlignedOrDieOnFatalError, because it requires that the
// *size* is a power of 2, which is an overly strong condition.
FakeStack *res = reinterpret_cast<FakeStack *>(
- RoundUpTo((uptr)true_res + kFlagsOffset + SizeRequiredForFlags(stack_size_log), 1 << kMaxStackFrameSizeLog)
- - kFlagsOffset - SizeRequiredForFlags(stack_size_log));
+ RoundUpTo(
+ (uptr)true_res + kFlagsOffset + SizeRequiredForFlags(stack_size_log),
+ 1 << kMaxStackFrameSizeLog) -
+ kFlagsOffset - SizeRequiredForFlags(stack_size_log));
res->true_start = true_res;
res->stack_size_log_ = stack_size_log;
u8 *p = reinterpret_cast<u8 *>(res);
>From a3788762c83de58de712b8d6d72349cbe6becfca Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 06:33:19 +0000
Subject: [PATCH 18/24] More logging
---
compiler-rt/lib/asan/asan_fake_stack.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.cpp b/compiler-rt/lib/asan/asan_fake_stack.cpp
index e9079e9369d5b..e8b950e59b0ec 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.cpp
+++ b/compiler-rt/lib/asan/asan_fake_stack.cpp
@@ -74,10 +74,12 @@ FakeStack *FakeStack::Create(uptr stack_size_log) {
u8 *p = reinterpret_cast<u8 *>(res);
VReport(1,
"T%d: FakeStack created: %p -- %p stack_size_log: %zd; "
- "mmapped %zdK, noreserve=%d, true_start: %p\n",
+ "mmapped %zdK, noreserve=%d, true_start: %p, start of first frame: "
+ "0x%zx\n",
GetCurrentTidOrInvalid(), (void *)p,
(void *)(p + FakeStack::RequiredSize(stack_size_log)), stack_size_log,
- size >> 10, flags()->uar_noreserve, res->true_start);
+ size >> 10, flags()->uar_noreserve, res->true_start,
+ res->GetFrame(stack_size_log, /*class_id*/ 0, /*pos*/ 0));
return res;
}
>From a1859f02a5a3dfe902928aaf3265cfb0ae741322 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 07:32:29 +0000
Subject: [PATCH 19/24] Fix edge case of frame with small variables that need
heavy alignment
---
.../Instrumentation/AddressSanitizer.cpp | 22 +++++++++++--------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 50258af5e26c3..236562cc52884 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3530,9 +3530,13 @@ void FunctionStackPoisoner::processStaticAllocas() {
auto DescriptionString = ComputeASanStackFrameDescription(SVD);
LLVM_DEBUG(dbgs() << DescriptionString << " --- " << L.FrameSize << "\n");
uint64_t LocalStackSize = L.FrameSize;
+ // In rare cases, a frame may have small objects with significant alignment
+ // needs. Placing the frame into a small fake stack frame would not meet
+ // the alignment needs.
+ uint64_t AlignedLocalStackSize = std::max(L.FrameSize, L.FrameAlignment);
bool DoStackMalloc =
ASan.UseAfterReturn != AsanDetectStackUseAfterReturnMode::Never &&
- !ASan.CompileKernel && LocalStackSize <= kMaxStackMallocSize;
+ !ASan.CompileKernel && AlignedLocalStackSize <= kMaxStackMallocSize;
bool DoDynamicAlloca = ClDynamicAllocaStack;
// Don't do dynamic alloca or stack malloc if:
// 1) There is inline asm: too often it makes assumptions on which registers
@@ -3568,11 +3572,11 @@ void FunctionStackPoisoner::processStaticAllocas() {
Instruction *Term =
SplitBlockAndInsertIfThen(UseAfterReturnIsEnabled, InsBefore, false);
IRBuilder<> IRBIf(Term);
- StackMallocIdx = StackMallocSizeClass(LocalStackSize);
+ StackMallocIdx = StackMallocSizeClass(AlignedLocalStackSize);
assert(StackMallocIdx <= kMaxAsanStackMallocSizeClass);
- Value *FakeStackValue =
- RTCI.createRuntimeCall(IRBIf, AsanStackMallocFunc[StackMallocIdx],
- ConstantInt::get(IntptrTy, LocalStackSize));
+ Value *FakeStackValue = RTCI.createRuntimeCall(
+ IRBIf, AsanStackMallocFunc[StackMallocIdx],
+ ConstantInt::get(IntptrTy, AlignedLocalStackSize));
IRB.SetInsertPoint(InsBefore);
FakeStack = createPHI(IRB, UseAfterReturnIsEnabled, FakeStackValue, Term,
ConstantInt::get(IntptrTy, 0));
@@ -3581,10 +3585,10 @@ void FunctionStackPoisoner::processStaticAllocas() {
// void *FakeStack = __asan_stack_malloc_N(LocalStackSize);
// void *LocalStackBase = (FakeStack) ? FakeStack :
// alloca(LocalStackSize);
- StackMallocIdx = StackMallocSizeClass(LocalStackSize);
- FakeStack =
- RTCI.createRuntimeCall(IRB, AsanStackMallocFunc[StackMallocIdx],
- ConstantInt::get(IntptrTy, LocalStackSize));
+ StackMallocIdx = StackMallocSizeClass(AlignedLocalStackSize);
+ FakeStack = RTCI.createRuntimeCall(
+ IRB, AsanStackMallocFunc[StackMallocIdx],
+ ConstantInt::get(IntptrTy, AlignedLocalStackSize));
}
Value *NoFakeStack =
IRB.CreateICmpEQ(FakeStack, Constant::getNullValue(IntptrTy));
>From d8dab9ffef4fd6d1558b33d7e8f0c9048f648966 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 07:37:03 +0000
Subject: [PATCH 20/24] Reword comment
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 236562cc52884..ca5fed191a06a 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3530,9 +3530,10 @@ void FunctionStackPoisoner::processStaticAllocas() {
auto DescriptionString = ComputeASanStackFrameDescription(SVD);
LLVM_DEBUG(dbgs() << DescriptionString << " --- " << L.FrameSize << "\n");
uint64_t LocalStackSize = L.FrameSize;
- // In rare cases, a frame may have small objects with significant alignment
- // needs. Placing the frame into a small fake stack frame would not meet
- // the alignment needs.
+ // Fake stack frames only guarantee alignment of the frame size.
+ // In rare cases, where a frame has small objects with significant alignment
+ // needs, we need to place it in a frame that is at least as large as the
+ // required frame alignment.
uint64_t AlignedLocalStackSize = std::max(L.FrameSize, L.FrameAlignment);
bool DoStackMalloc =
ASan.UseAfterReturn != AsanDetectStackUseAfterReturnMode::Never &&
>From 8529f76bbf5fe40adb790141acd7a9cd14fa8895 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 15:32:17 +0000
Subject: [PATCH 21/24] Partly revert aligned local stack size change, to
maximize protection
---
.../Instrumentation/AddressSanitizer.cpp | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index ca5fed191a06a..edd9685957385 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3563,6 +3563,10 @@ void FunctionStackPoisoner::processStaticAllocas() {
// void *FakeStack = __asan_option_detect_stack_use_after_return
// ? __asan_stack_malloc_N(LocalStackSize)
// : nullptr;
+ // where _N is the size class of AlignedLocalStackSize
+ // (we still only allocate LocalStackSize, not AlignedLocalStackSize, to
+ // maximize poisoning)
+ //
// void *LocalStackBase = (FakeStack) ? FakeStack :
// alloca(LocalStackSize);
Constant *OptionDetectUseAfterReturn = F.getParent()->getOrInsertGlobal(
@@ -3575,9 +3579,9 @@ void FunctionStackPoisoner::processStaticAllocas() {
IRBuilder<> IRBIf(Term);
StackMallocIdx = StackMallocSizeClass(AlignedLocalStackSize);
assert(StackMallocIdx <= kMaxAsanStackMallocSizeClass);
- Value *FakeStackValue = RTCI.createRuntimeCall(
- IRBIf, AsanStackMallocFunc[StackMallocIdx],
- ConstantInt::get(IntptrTy, AlignedLocalStackSize));
+ Value *FakeStackValue =
+ RTCI.createRuntimeCall(IRBIf, AsanStackMallocFunc[StackMallocIdx],
+ ConstantInt::get(IntptrTy, LocalStackSize));
IRB.SetInsertPoint(InsBefore);
FakeStack = createPHI(IRB, UseAfterReturnIsEnabled, FakeStackValue, Term,
ConstantInt::get(IntptrTy, 0));
@@ -3587,9 +3591,9 @@ void FunctionStackPoisoner::processStaticAllocas() {
// void *LocalStackBase = (FakeStack) ? FakeStack :
// alloca(LocalStackSize);
StackMallocIdx = StackMallocSizeClass(AlignedLocalStackSize);
- FakeStack = RTCI.createRuntimeCall(
- IRB, AsanStackMallocFunc[StackMallocIdx],
- ConstantInt::get(IntptrTy, AlignedLocalStackSize));
+ FakeStack =
+ RTCI.createRuntimeCall(IRB, AsanStackMallocFunc[StackMallocIdx],
+ ConstantInt::get(IntptrTy, LocalStackSize));
}
Value *NoFakeStack =
IRB.CreateICmpEQ(FakeStack, Constant::getNullValue(IntptrTy));
>From 8df0f3936c94b9b8b094a428b0207a315837ce50 Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 15:43:02 +0000
Subject: [PATCH 22/24] Statically assert that FakeStack as a whole is aligned
---
compiler-rt/lib/asan/asan_fake_stack.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.cpp b/compiler-rt/lib/asan/asan_fake_stack.cpp
index e8b950e59b0ec..4a42ab4f7f5ac 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.cpp
+++ b/compiler-rt/lib/asan/asan_fake_stack.cpp
@@ -61,9 +61,10 @@ FakeStack *FakeStack::Create(uptr stack_size_log) {
: MmapOrDie(padded_size, "FakeStack"));
// GetFrame() requires the property that
// (res + kFlagsOffset + SizeRequiredForFlags(stack_size_log)) is aligned to
- // (1 << kMaxStackFrameSizeLog)
+ // (1 << kMaxStackFrameSizeLog).
// We didn't use MmapAlignedOrDieOnFatalError, because it requires that the
// *size* is a power of 2, which is an overly strong condition.
+ static_assert(alignof(FakeStack) <= (1 << kMaxStackFrameSizeLog));
FakeStack *res = reinterpret_cast<FakeStack *>(
RoundUpTo(
(uptr)true_res + kFlagsOffset + SizeRequiredForFlags(stack_size_log),
>From cc040e1d321da33c98db734035547692eeb27c1c Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sat, 9 Aug 2025 21:37:01 +0000
Subject: [PATCH 23/24] Drive-by fix: remove deprecated comment
---
compiler-rt/lib/asan/asan_fake_stack.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/compiler-rt/lib/asan/asan_fake_stack.h b/compiler-rt/lib/asan/asan_fake_stack.h
index 2f983bf66834a..eaf15fc5b18d3 100644
--- a/compiler-rt/lib/asan/asan_fake_stack.h
+++ b/compiler-rt/lib/asan/asan_fake_stack.h
@@ -181,7 +181,6 @@ class FakeStack {
uptr hint_position_[kNumberOfSizeClasses];
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
>From e19176ece684bbd9941fe7d36fc9f37a2b0bbe8f Mon Sep 17 00:00:00 2001
From: Thurston Dang <thurston at google.com>
Date: Sun, 10 Aug 2025 03:56:39 +0000
Subject: [PATCH 24/24] Add note that most modern compilers have sizeof(type)
>= alignof(type)
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index edd9685957385..3aeaf6147b4c7 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3533,7 +3533,8 @@ void FunctionStackPoisoner::processStaticAllocas() {
// Fake stack frames only guarantee alignment of the frame size.
// In rare cases, where a frame has small objects with significant alignment
// needs, we need to place it in a frame that is at least as large as the
- // required frame alignment.
+ // required frame alignment. (In most modern compilers, sizeof(type)
+ // >= alignof(type), making this impossible.)
uint64_t AlignedLocalStackSize = std::max(L.FrameSize, L.FrameAlignment);
bool DoStackMalloc =
ASan.UseAfterReturn != AsanDetectStackUseAfterReturnMode::Never &&
More information about the llvm-commits
mailing list