Skip to content

Conversation

fabio-d
Copy link
Contributor

@fabio-d fabio-d commented Sep 26, 2025

Instead of directly instantiating scudo::Allocator, using the test TestAllocator wrapper class ensures that unmapTestOnly is called at the end of the test.

This fixes the issue of QuarantineIterateOverChunks failing on Fuchsia because of a clobbered TLS pointer left by QuarantineEnabled.

Instead of directly instantiating scudo::Allocator, using the
test TestAllocator wrapper class ensures that unmapTestOnly is
called at the end of the test.

This fixes the issue of QuarantineIterateOverChunks failing
on Fuchsia because of a clobbered TLS pointer left by
QuarantineEnabled.
@llvmbot
Copy link
Member

llvmbot commented Sep 26, 2025

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

Author: Fabio D'Urso (fabio-d)

Changes

Instead of directly instantiating scudo::Allocator, using the test TestAllocator wrapper class ensures that unmapTestOnly is called at the end of the test.

This fixes the issue of QuarantineIterateOverChunks failing on Fuchsia because of a clobbered TLS pointer left by QuarantineEnabled.


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

1 Files Affected:

  • (modified) compiler-rt/lib/scudo/standalone/tests/combined_test.cpp (+4-4)
diff --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
index 5b56b973d55f8..5fdfd1e7c55cc 100644
--- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -1043,7 +1043,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepot) {
 // which covers only simple operations and ensure the configuration is able to
 // compile.
 TEST(ScudoCombinedTest, BasicTrustyConfig) {
-  using AllocatorT = scudo::Allocator<scudo::TrustyConfig>;
+  using AllocatorT = TestAllocator<scudo::TrustyConfig>;
   auto Allocator = std::unique_ptr<AllocatorT>(new AllocatorT());
 
   for (scudo::uptr ClassId = 1U;
@@ -1107,7 +1107,7 @@ struct TestQuarantineConfig {
 
 // Verify that the quarantine exists by default.
 TEST(ScudoCombinedTest, QuarantineEnabled) {
-  using AllocatorT = scudo::Allocator<TestQuarantineConfig>;
+  using AllocatorT = TestAllocator<TestQuarantineConfig>;
   auto Allocator = std::unique_ptr<AllocatorT>(new AllocatorT());
 
   const scudo::uptr Size = 1000U;
@@ -1132,7 +1132,7 @@ struct TestQuarantineDisabledConfig : TestQuarantineConfig {
 };
 
 TEST(ScudoCombinedTest, QuarantineDisabled) {
-  using AllocatorT = scudo::Allocator<TestQuarantineDisabledConfig>;
+  using AllocatorT = TestAllocator<TestQuarantineDisabledConfig>;
   auto Allocator = std::unique_ptr<AllocatorT>(new AllocatorT());
 
   const scudo::uptr Size = 1000U;
@@ -1154,7 +1154,7 @@ TEST(ScudoCombinedTest, QuarantineDisabled) {
 
 // Verify that no special quarantine blocks appear in iterateOverChunks.
 TEST(ScudoCombinedTest, QuarantineIterateOverChunks) {
-  using AllocatorT = scudo::Allocator<TestQuarantineConfig>;
+  using AllocatorT = TestAllocator<TestQuarantineConfig>;
   auto Allocator = std::unique_ptr<AllocatorT>(new AllocatorT());
 
   // Do a bunch of allocations and deallocations. At the end there should

Copy link
Contributor

@cferris1000 cferris1000 left a comment

Choose a reason for hiding this comment

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

LGTM. I ran on Android just to make sure no surprise failures.

@fabio-d fabio-d merged commit d0a260b into llvm:main Sep 27, 2025
13 checks passed
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
Instead of directly instantiating scudo::Allocator, using the test
TestAllocator wrapper class ensures that unmapTestOnly is called at the
end of the test.

This fixes the issue of QuarantineIterateOverChunks failing on Fuchsia
because of a clobbered TLS pointer left by QuarantineEnabled.
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