[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 22:31:17 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/10] [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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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) {
More information about the llvm-commits
mailing list