Skip to content

[asan] Add test case for alignment of FakeStack frames #152889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Aug 11, 2025

Conversation

thurstond
Copy link
Contributor

@thurstond thurstond commented Aug 10, 2025

This test case demonstrates that ASan does not currently align FakeStack frames correctly:

  • for 4KB objects on a 64KB stack, alignment is deterministically incorrect
  • for objects larger than 4KB, even with large stack sizes, alignment is not guaranteed

#152819 will fix it.

This test case demonstrates that ASan does not currently align FakeStack frames correctly.
(llvm#152819 will fix it.)
@llvmbot
Copy link
Member

llvmbot commented Aug 10, 2025

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

Author: Thurston Dang (thurstond)

Changes

This test case demonstrates that ASan does not currently align FakeStack frames correctly. (#152819 will fix it.)


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

1 Files Affected:

  • (added) compiler-rt/test/asan/TestCases/fakestack_alignment.cpp (+74)
diff --git a/compiler-rt/test/asan/TestCases/fakestack_alignment.cpp b/compiler-rt/test/asan/TestCases/fakestack_alignment.cpp
new file mode 100644
index 0000000000000..597a486e26534
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/fakestack_alignment.cpp
@@ -0,0 +1,74 @@
+// RUN: %clangxx_asan -fsanitize-address-use-after-return=always -O0 %s -o %t && %run %t 2>&1
+// XFAIL: *
+
+#include <assert.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+struct alignas(4096) page {
+    int x;
+};
+
+struct alignas(16384) larry {
+    int x;
+};
+
+bool misaligned = false;
+
+void grandchild(void) {
+  larry l2;
+  uint alignment = (unsigned long)&l2 % alignof(larry);
+  if (alignment != 0)
+    misaligned = true;
+
+  printf ("Grandchild: address modulo alignment %u\n", alignment);
+}
+
+// Even if the FakeStack frame is aligned by chance to 16384, we can use an
+// intervening stack frame to knock it out of alignment.
+void child(void) {
+  page p1;
+  uint alignment = (unsigned long)&p1 % alignof(page);
+  printf ("Child: address modulo alignment is %u\n", alignment);
+  if (alignment != 0)
+    misaligned = true;
+
+  grandchild();
+}
+
+// Check whether the FakeStack frame is sufficiently aligned. Alignment can
+// happen by chance, so try this on many threads if you don't want
+void *Thread(void *unused)  {
+  larry l1;
+  uint alignment = (unsigned long)&l1 % alignof(larry);
+  printf ("Thread: address modulo alignment is %u\n", alignment);
+  if (alignment != 0)
+    misaligned = true;
+
+  child();
+
+  return NULL;
+}
+
+int main(int argc, char **argv) {
+  pthread_attr_t attr;
+  pthread_attr_init(&attr);
+
+  pthread_t t[10];
+  for (int i = 0; i < 10; i++) {
+     pthread_create(&t[i], &attr, Thread, 0);
+  }
+  pthread_attr_destroy(&attr);
+  for (int i = 0; i < 10; i++) {
+     pthread_join(t[i], 0);
+  }
+
+  if (misaligned) {
+    printf ("Test failed: not perfectly aligned\n");
+    exit(1);
+  }
+
+  return 0;
+}

Copy link

github-actions bot commented Aug 10, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@thurstond thurstond changed the title [asan] Add test case for alignment of FakeStack frames [asan] Add test case for alignment of FakeStack frames for >4KB objects Aug 10, 2025
thurstond added a commit to thurstond/llvm-project that referenced this pull request Aug 10, 2025
This test case demonstrates that ASan does not currently align FakeStack frames correctly for 4KB objects.

It deliberately uses a smaller thread stack size (64KB), which forces
the FakeStack frames to no longer be 4KB aligned.

This differs from llvm#152889,
which is a test case for objects >4KB, which relies on the fact that the
default 4KB alignment for fake stack sizes >64KB is insufficient.

llvm#152819 will fix both issues.
pthread_attr_init(&attr);

pthread_t t[32];
for (int i = 0; i < 32; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

braces

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

void *Thread(void *unused) {
larry l1;
uint alignment = (unsigned long)&l1 % alignof(larry);
printf("Thread: address modulo alignment is %u\n", alignment);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to print this 32 times?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

#include <stdlib.h>
#include <string.h>

struct alignas(4096) page {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unused? Can we merge this with the other test, and make the alignas / thread count a define instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged and #define'd

@thurstond thurstond changed the title [asan] Add test case for alignment of FakeStack frames for >4KB objects [asan] Add test case for alignment of FakeStack frames Aug 11, 2025
@thurstond thurstond requested a review from fmayer August 11, 2025 21:34
if (alignment != 0)
misaligned = true;

return NULL;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is C++, use nullptr

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

#endif

pthread_t t[THREAD_COUNT];
for (int i = 0; i < THREAD_COUNT; i++)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optionally, if you think it's an improvement: we can use range based for loops for all of these.

for (phtread_t& t : threads) {
  pthread_create(&t, ...)

etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@thurstond thurstond requested a review from fmayer August 11, 2025 22:29
@thurstond thurstond merged commit e36bd61 into llvm:main Aug 11, 2025
9 checks passed
thurstond added a commit that referenced this pull request Aug 12, 2025
ASan's instrumentation pass uses `ASanStackFrameLayout::ComputeASanStackFrameLayout()` to calculate the offset of variables, taking into account alignment. However, the fake stack frames returned by the runtime's `GetFrame()` are not guaranteed to be sufficiently aligned (and in some cases, even guaranteed to be misaligned), hence the offset addresses may sometimes be misaligned.

This change fixes the misalignment issue by padding the FakeStack. Every fake stack frame is guaranteed to be aligned to the size of the frame.

The memory overhead is low: 64KB per FakeStack, compared to the FakeStack size of ~700KB (min) to 11MB (max).

Updates the test case from #152889.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Aug 12, 2025
…(#152819)

ASan's instrumentation pass uses `ASanStackFrameLayout::ComputeASanStackFrameLayout()` to calculate the offset of variables, taking into account alignment. However, the fake stack frames returned by the runtime's `GetFrame()` are not guaranteed to be sufficiently aligned (and in some cases, even guaranteed to be misaligned), hence the offset addresses may sometimes be misaligned.

This change fixes the misalignment issue by padding the FakeStack. Every fake stack frame is guaranteed to be aligned to the size of the frame.

The memory overhead is low: 64KB per FakeStack, compared to the FakeStack size of ~700KB (min) to 11MB (max).

Updates the test case from llvm/llvm-project#152889.
thurstond added a commit to thurstond/llvm-project that referenced this pull request Aug 12, 2025
…lvm#153139)

This reverts commit 29ad073 i.e.,
relands 927e19f.

It was reverted because of buildbot breakages. This reland adds
"-pthread" and also moves the test to Posix-only.

Original commit message:
    [asan] Fix misalignment of variables in fake stack frames (llvm#152819)

    ASan's instrumentation pass uses `ASanStackFrameLayout::ComputeASanStackFrameLayout()` to calculate the offset of variables, taking into account alignment. However, the fake stack frames returned by the runtime's `GetFrame()` are not guaranteed to be sufficiently aligned (and in some cases, even guaranteed to be misaligned), hence the offset addresses may sometimes be misaligned.

    This change fixes the misalignment issue by padding the FakeStack. Every fake stack frame is guaranteed to be aligned to the size of the frame.

    The memory overhead is low: 64KB per FakeStack, compared to the FakeStack size of ~700KB (min) to 11MB (max).

    Updates the test case from llvm#152889.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants