Skip to content

Conversation

@lntue
Copy link
Contributor

@lntue lntue commented Feb 23, 2025

Full build precommit bots were failing due to mis-alignment of atomics in hermetic tests. This PR enforces the alignment for the bump allocator of hermetic test framework.

Fixes #128185.

@llvmbot
Copy link
Member

llvmbot commented Feb 23, 2025

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

Full build precommit bots were failing due to mis-alignment of atomics in hermetic tests. This PR enforces the alignment for the bump allocator of hermetic test framework.

Fixes #128185.


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

1 Files Affected:

  • (modified) libc/test/UnitTest/HermeticTestUtils.cpp (+5-3)
diff --git a/libc/test/UnitTest/HermeticTestUtils.cpp b/libc/test/UnitTest/HermeticTestUtils.cpp
index 47f813b0b7a4e..343b622fd4f30 100644
--- a/libc/test/UnitTest/HermeticTestUtils.cpp
+++ b/libc/test/UnitTest/HermeticTestUtils.cpp
@@ -33,6 +33,10 @@ int atexit(void (*func)(void));
 
 } // namespace LIBC_NAMESPACE_DECL
 
+extern "C" {
+constexpr uint64_t ALIGNMENT = alignof(uintptr_t);
+}
+
 namespace {
 
 // Integration tests cannot use the SCUDO standalone allocator as SCUDO pulls
@@ -42,7 +46,7 @@ namespace {
 // which just hands out continuous blocks from a statically allocated chunk of
 // memory.
 static constexpr uint64_t MEMORY_SIZE = 65336;
-static uint8_t memory[MEMORY_SIZE];
+alignas(ALIGNMENT) static uint8_t memory[MEMORY_SIZE];
 static uint8_t *ptr = memory;
 
 } // anonymous namespace
@@ -74,8 +78,6 @@ void *memset(void *ptr, int value, size_t count) {
 // This is needed if the test was compiled with '-fno-use-cxa-atexit'.
 int atexit(void (*func)(void)) { return LIBC_NAMESPACE::atexit(func); }
 
-constexpr uint64_t ALIGNMENT = alignof(uintptr_t);
-
 void *malloc(size_t s) {
   // Keep the bump pointer aligned on an eight byte boundary.
   s = ((s + ALIGNMENT - 1) / ALIGNMENT) * ALIGNMENT;


} // namespace LIBC_NAMESPACE_DECL

extern "C" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why extern C? Constexpr has no linkage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was inside an extern "C" block before so I kept it the same way when moving up. Is it safe to remove extern "C" then?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be.

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.

@lntue lntue merged commit 8ea6b73 into llvm:main Feb 23, 2025
16 checks passed
@lntue lntue deleted the hermetic branch February 23, 2025 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[libc] Re-enable test/src/__support/File tests.

3 participants