From 8ebc69eb0aeb13dd920ade5cc0a66e60ce2fba9e Mon Sep 17 00:00:00 2001 From: gf712 Date: Sun, 5 Jul 2026 10:13:09 +0100 Subject: [PATCH 1/3] gc: scrub popped-frame stack residue in GC completeness tests --- src/memory/GarbageCollector_tests.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/memory/GarbageCollector_tests.cpp b/src/memory/GarbageCollector_tests.cpp index f5591ba3..4c2572e9 100644 --- a/src/memory/GarbageCollector_tests.cpp +++ b/src/memory/GarbageCollector_tests.cpp @@ -1,6 +1,9 @@ #include "GarbageCollector.hpp" #include "Heap_test.hpp" +#include +#include + namespace { static int64_t g_counter = 0; @@ -49,6 +52,23 @@ static_assert(false, "compiler not supported"); ASSERT_EQ(ptr4->foo, 4); ASSERT_EQ(ptr5->foo, 5); } + +// Overwrites the stack region just vacated by a popped frame. Unoptimized +// builds give every temporary its own slot and the collector's call chain +// does not reliably overwrite all of them before the conservative scan runs, +// so a stale copy of a dead GC pointer can be picked up as a root. Zeroing +// the dead region makes collection of unreachable objects deterministic. +// no_stack_protector: the scrub zeroes everything between `buffer` and the +// frame header (alignment padding can hold a word of residue), which would +// destroy a stack-protector canary placed in that range. +__attribute__((noinline, no_stack_protector)) void scrub_dead_stack() +{ + uint8_t buffer[16 * 1024]; + auto *frame_top = static_cast(__builtin_frame_address(0)); + std::memset(buffer, 0, static_cast(frame_top - buffer)); + // keep the memset from being eliminated as a dead store + asm volatile("" ::"r"(buffer) : "memory"); +} }// namespace TEST_F(TestHeap, GarbageCollectorDoesNotDeallocateGCPointersOnTheStack) @@ -74,6 +94,7 @@ TEST_F(TestHeap, GarbageCollectorDeallocatesGCPointersWhenStackFrameIsPopped) new_stack_frame_function(*m_heap); + scrub_dead_stack(); m_heap->collect_garbage(); ASSERT_EQ(g_counter, 5); @@ -122,6 +143,7 @@ TEST_F(TestHeap, MutuallyReferencingObjectsAreCollected) allocate_cycle_in_popped_frame(*m_heap, counter); + scrub_dead_stack(); m_heap->collect_garbage(); ASSERT_EQ(counter, 2); From 2d2b6d173566c598a65315a0c260e9e72bc27b1b Mon Sep 17 00:00:00 2001 From: gf712 Date: Mon, 6 Jul 2026 16:38:45 +0100 Subject: [PATCH 2/3] gc: keep test stack scrub within the buffer's bounds The scrub memset ran from the buffer up to the frame header, past the end of the array: an out-of-bounds write that aborts under ASan with stack-buffer-overflow (verified empirically). Bound the memset by the array itself and route the allocating helpers through a padded-frame trampoline so their stale-pointer residue lands inside the span the scrub can legally zero, instead of in the last few words below the frame header that the buffer cannot reach. Addresses the review comment on PR #33. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017NJ25FGK2EUWCLz2PaUwbT --- src/memory/GarbageCollector_tests.cpp | 28 +++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/memory/GarbageCollector_tests.cpp b/src/memory/GarbageCollector_tests.cpp index 4c2572e9..53c91fe3 100644 --- a/src/memory/GarbageCollector_tests.cpp +++ b/src/memory/GarbageCollector_tests.cpp @@ -58,17 +58,29 @@ static_assert(false, "compiler not supported"); // does not reliably overwrite all of them before the conservative scan runs, // so a stale copy of a dead GC pointer can be picked up as a root. Zeroing // the dead region makes collection of unreachable objects deterministic. -// no_stack_protector: the scrub zeroes everything between `buffer` and the -// frame header (alignment padding can hold a word of residue), which would -// destroy a stack-protector canary placed in that range. -__attribute__((noinline, no_stack_protector)) void scrub_dead_stack() +// The scrub stays within `buffer`, which cannot reach the last few words +// below this function's frame header (alignment padding and compiler-placed +// slots sit there); callers must run the allocating helper through +// call_in_padded_frame so its residue lands below that blind spot. +__attribute__((noinline)) void scrub_dead_stack() { uint8_t buffer[16 * 1024]; - auto *frame_top = static_cast(__builtin_frame_address(0)); - std::memset(buffer, 0, static_cast(frame_top - buffer)); + std::memset(buffer, 0, sizeof(buffer)); // keep the memset from being eliminated as a dead store asm volatile("" ::"r"(buffer) : "memory"); } + +// Runs fn with its stack frame pushed at least sizeof(pad) bytes deeper than +// the caller's, so every slot fn writes lies inside the span that +// scrub_dead_stack can zero without stepping past its buffer's bounds. +template __attribute__((noinline)) void call_in_padded_frame(Fn &&fn) +{ + volatile uint8_t pad[256] = {}; + fn(); + // volatile read keeps pad live across the call, preventing a tail call + // that would collapse this frame into fn's + (void)pad[0]; +} }// namespace TEST_F(TestHeap, GarbageCollectorDoesNotDeallocateGCPointersOnTheStack) @@ -92,7 +104,7 @@ TEST_F(TestHeap, GarbageCollectorDeallocatesGCPointersWhenStackFrameIsPopped) ASSERT_EQ(g_counter, 0); m_heap->collect_garbage(); - new_stack_frame_function(*m_heap); + call_in_padded_frame([this] { new_stack_frame_function(*m_heap); }); scrub_dead_stack(); m_heap->collect_garbage(); @@ -141,7 +153,7 @@ TEST_F(TestHeap, MutuallyReferencingObjectsAreCollected) int64_t counter = 0; m_heap->garbage_collector().set_frequency(1); - allocate_cycle_in_popped_frame(*m_heap, counter); + call_in_padded_frame([&] { allocate_cycle_in_popped_frame(*m_heap, counter); }); scrub_dead_stack(); m_heap->collect_garbage(); From 1402e4feae07cc03381ec43318197bda0ec71357 Mon Sep 17 00:00:00 2001 From: gf712 Date: Mon, 6 Jul 2026 16:39:45 +0100 Subject: [PATCH 3/3] testing: disable ASan use-after-return detection in the test binary The garbage collector finds roots by conservatively scanning the machine stack. ASan's use-after-return fake stack relocates locals to heap memory the scan never sees, so the GC completeness tests fail under an ASan build even with a correct stack scrub. Bake detect_stack_use_after_return=0 into the test binary via __asan_default_options so ASan builds work out of the box; the rest of ASan's checks are unaffected. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017NJ25FGK2EUWCLz2PaUwbT --- src/testing/main.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/testing/main.cpp b/src/testing/main.cpp index 84215585..4574d47d 100644 --- a/src/testing/main.cpp +++ b/src/testing/main.cpp @@ -5,6 +5,22 @@ #include +#if defined(__SANITIZE_ADDRESS__) +#define PYTHON_ASAN_ENABLED +#elif defined(__has_feature) +#if __has_feature(address_sanitizer) +#define PYTHON_ASAN_ENABLED +#endif +#endif + +#ifdef PYTHON_ASAN_ENABLED +// The garbage collector finds roots by conservatively scanning the machine +// stack. ASan's use-after-return detection relocates locals to a heap-backed +// fake stack that the scan never sees, so live GC pointers go unnoticed and +// stale ones linger, breaking the collector's reachability tests. +extern "C" const char *__asan_default_options() { return "detect_stack_use_after_return=0"; } +#endif + class PythonVMEnvironment : public ::testing::Environment { char **m_argv;