[compiler-rt] [sanitizer_common][lsan] Keep the original allocation alive when realloc fails (PR #222849)
Bojun Seo via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 10 23:27:29 PDT 2026
https://github.com/Bojun-Seo updated https://github.com/llvm/llvm-project/pull/222849
>From ad3b879220a608612dbb88fbb8f3eeaa0271ca6c Mon Sep 17 00:00:00 2001
From: "bojun.seo" <bojun.seo at lge.com>
Date: Wed, 26 Aug 2026 11:15:39 +0900
Subject: [PATCH 1/2] [sanitizer_common] Keep the original allocation alive
when Reallocate fails
CombinedAllocator::Reallocate() deallocated the original chunk even when the
replacement allocation could not be satisfied, so a failing realloc() released
memory that the caller still owned. Callers that correctly retry or fall back
on failure were left with a dangling pointer.
Return early instead, which matches realloc() semantics: on failure the
original allocation is untouched and remains owned by the caller.
Stand-alone LeakSanitizer is the in-tree caller that can observe this, and it
does so in the default configuration: its realloc() reaches this function
directly, so the allocator_may_return_null check in __lsan::Allocate() is
bypassed and null is returned to the program after the chunk has already gone
back on the free list. __sanitizer_get_ownership() still reports that address
as live while a later malloc() hands the very same chunk out again. This commit
stops the release; the LSan side is fixed in the next one.
AddressSanitizer and MemProfiler are unaffected: both implement their own
Reallocate(), which releases the original only once the replacement has been
allocated. InternalRealloc() is layered on this function, but it turns a null
result into a fatal ReportInternalAllocatorOutOfMemory(), so the dangling
pointer never reaches its caller there.
Assisted-by: Claude Opus 5
---
.../sanitizer_allocator_combined.h | 6 ++-
.../tests/sanitizer_allocator_test.cpp | 52 +++++++++++++++++++
2 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_combined.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_combined.h
index 49940d9b5d505..938bb4c173764 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_combined.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_combined.h
@@ -106,8 +106,10 @@ class CombinedAllocator {
uptr old_size = GetActuallyAllocatedSize(p);
uptr memcpy_size = Min(new_size, old_size);
void *new_p = Allocate(cache, new_size, alignment);
- if (new_p)
- internal_memcpy(new_p, p, memcpy_size);
+ // On failure the caller still owns p, as realloc() requires.
+ if (!new_p)
+ return nullptr;
+ internal_memcpy(new_p, p, memcpy_size);
Deallocate(cache, p);
return new_p;
}
diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp
index 601897a64f051..77753b75fe526 100644
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp
+++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp
@@ -750,6 +750,58 @@ void TestCombinedAllocator(uptr premapped_heap = 0) {
allocated.clear();
a->SwallowCache(&cache);
}
+
+ // A failing Reallocate() must keep p allocated. (uptr)-1 overflows the
+ // size-plus-alignment check inside Allocate(), which logs a warning.
+ {
+ const uptr kSize = 128;
+ char* p = reinterpret_cast<char*>(a->Allocate(&cache, kSize, 1));
+ ASSERT_NE(p, nullptr);
+ uptr* meta = reinterpret_cast<uptr*>(a->GetMetaData(p));
+ *meta = kSize;
+ internal_memset(p, 'x', kSize);
+
+ EXPECT_EQ(a->Reallocate(&cache, p, (uptr)-1, 1), nullptr);
+
+ // These hold even for a released chunk; the loop below is what detects it.
+ EXPECT_EQ(*reinterpret_cast<uptr*>(a->GetMetaData(p)), kSize);
+ EXPECT_EQ(p[0], 'x');
+ EXPECT_EQ(p[kSize - 1], 'x');
+
+ void* others[8];
+ for (uptr i = 0; i < ARRAY_SIZE(others); i++) {
+ others[i] = a->Allocate(&cache, kSize, 1);
+ EXPECT_NE(others[i], p);
+ }
+ for (uptr i = 0; i < ARRAY_SIZE(others); i++)
+ a->Deallocate(&cache, others[i]);
+
+ *meta = 0;
+ a->Deallocate(&cache, p);
+ a->SwallowCache(&cache);
+ }
+
+ // Same for the secondary, where a release unmaps the chunk. A regression
+ // leaves p unmapped, so assert ownership before reading through it.
+ {
+ const uptr kLarge = 1 << 20;
+ void* p = a->Allocate(&cache, kLarge, 1);
+ ASSERT_NE(p, nullptr);
+ if (!a->FromPrimary(p)) {
+ uptr* meta = reinterpret_cast<uptr*>(a->GetMetaData(p));
+ *meta = kLarge;
+
+ EXPECT_EQ(a->Reallocate(&cache, p, (uptr)-1, 1), nullptr);
+
+ ASSERT_TRUE(a->PointerIsMine(p));
+ EXPECT_EQ(a->GetBlockBegin(p), p);
+ EXPECT_EQ(*reinterpret_cast<uptr*>(a->GetMetaData(p)), kLarge);
+ *meta = 0;
+ }
+ a->Deallocate(&cache, p);
+ a->SwallowCache(&cache);
+ }
+
a->DestroyCache(&cache);
a->TestOnlyUnmap();
}
>From a14201b2900284937b79ea1defe1a32808b8545f Mon Sep 17 00:00:00 2001
From: "bojun.seo" <bojun.seo at lge.com>
Date: Wed, 9 Sep 2026 01:20:49 +0000
Subject: [PATCH 2/2] [lsan] Do not release the original allocation when
realloc fails
Reallocate() unregistered p before asking the allocator for a replacement, so a
failing realloc() reported a free hook for a pointer the caller still owned and
then re-registered that pointer with the *new* size and the realloc stack.
Hook consumers saw a free with no matching malloc, and
__sanitizer_get_allocated_size() reported a size that was never allocated.
Reorder the operation the way AddressSanitizer already does it: allocate the
replacement first and release the original only once that succeeded. On failure
p is now left completely untouched, so no hook fires and its metadata still
describes the original allocation.
The replacement now comes from __lsan::Allocate() rather than from
CombinedAllocator::Reallocate(), so realloc() also honors
allocator_may_return_null like every other allocation entry point: an
unsatisfiable request previously returned null whatever that flag said, and now
reports out-of-memory unless the flag allows null.
The same routing changes realloc(NULL, 0), which is malloc(0) by definition but
was registered with a requested size of zero: __sanitizer_get_ownership()
reported the returned pointer as not owned and __sanitizer_get_allocated_size()
reported zero for it, while malloc(0) reported one byte for the same thing. It is
now tracked exactly like malloc(0), and realloc_zero.c is extended to cover that
so it cannot silently regress.
LSan was the only sanitizer front-end still calling
CombinedAllocator::Reallocate(); the function keeps its other in-tree caller,
InternalRealloc(). Its CHECK(PointerIsMine(p)) moves here along with the rest:
without it the copy length below would come from whatever GetMetaData() reads
next to a pointer the allocator does not own.
Assisted-by: Claude Opus 5
---
compiler-rt/lib/lsan/lsan_allocator.cpp | 23 ++++--
.../Linux/realloc_failure_keeps_original.cpp | 75 +++++++++++++++++++
.../test/lsan/TestCases/realloc_zero.c | 14 ++++
3 files changed, 105 insertions(+), 7 deletions(-)
create mode 100644 compiler-rt/test/lsan/TestCases/Linux/realloc_failure_keeps_original.cpp
diff --git a/compiler-rt/lib/lsan/lsan_allocator.cpp b/compiler-rt/lib/lsan/lsan_allocator.cpp
index 110c1cfc3bf92..16a27658a468b 100644
--- a/compiler-rt/lib/lsan/lsan_allocator.cpp
+++ b/compiler-rt/lib/lsan/lsan_allocator.cpp
@@ -133,13 +133,22 @@ void *Reallocate(const StackTrace &stack, void *p, uptr new_size,
ReportAllocationSizeTooBig(new_size, stack);
return nullptr;
}
- RegisterDeallocation(p);
- void *new_p =
- allocator.Reallocate(GetAllocatorCache(), p, new_size, alignment);
- if (new_p)
- RegisterAllocation(stack, new_p, new_size);
- else if (new_size != 0)
- RegisterAllocation(stack, p, new_size);
+ if (!p)
+ return Allocate(stack, new_size, alignment, false);
+ if (!new_size) {
+ Deallocate(p);
+ return nullptr;
+ }
+ CHECK(allocator.PointerIsMine(p));
+ ChunkMetadata* m = Metadata(p);
+ CHECK(m);
+ const uptr old_size = m->requested_size;
+ // Allocate first: on failure p must be left untouched.
+ void* new_p = Allocate(stack, new_size, alignment, false);
+ if (!new_p)
+ return nullptr;
+ internal_memcpy(new_p, p, Min(new_size, old_size));
+ Deallocate(p);
return new_p;
}
diff --git a/compiler-rt/test/lsan/TestCases/Linux/realloc_failure_keeps_original.cpp b/compiler-rt/test/lsan/TestCases/Linux/realloc_failure_keeps_original.cpp
new file mode 100644
index 0000000000000..35482a6fbaf9a
--- /dev/null
+++ b/compiler-rt/test/lsan/TestCases/Linux/realloc_failure_keeps_original.cpp
@@ -0,0 +1,75 @@
+// Verifies that a failing realloc() leaves the original allocation untouched
+// and reports no free hook for it.
+//
+// RUN: %clangxx_lsan %s -o %t
+// RUN: %env_lsan_opts=detect_leaks=0:allocator_may_return_null=1 %run %t 2>&1 | FileCheck %s
+// REQUIRES: lsan-standalone
+
+#include <sanitizer/allocator_interface.h>
+
+#include <cassert>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <sys/resource.h>
+#include <unistd.h>
+
+static void *g_freed;
+static void *g_alloced;
+
+static void OnMalloc(const volatile void *ptr, size_t) {
+ g_alloced = (void *)ptr;
+}
+
+static void OnFree(const volatile void *ptr) { g_freed = (void *)ptr; }
+
+// Caps the address space just above the current usage, so that the large
+// mmap below fails inside the allocator rather than being rejected up front
+// by max_allocation_size_mb.
+static void CapAddressSpace() {
+ FILE *f = fopen("/proc/self/statm", "r");
+ assert(f);
+ unsigned long vsz_pages = 0;
+ int scanned = fscanf(f, "%lu", &vsz_pages);
+ fclose(f);
+ assert(scanned == 1);
+
+ rlimit rl;
+ int res = getrlimit(RLIMIT_AS, &rl);
+ assert(res == 0);
+ rl.rlim_cur = (rlim_t)vsz_pages * getpagesize() + (64UL << 20);
+ res = setrlimit(RLIMIT_AS, &rl);
+ assert(res == 0);
+}
+
+int main() {
+ const size_t kSize = 100;
+ char *p = (char *)malloc(kSize);
+ assert(p);
+ memset(p, 'a', kSize);
+
+ // Install the hooks only after CapAddressSpace(), which itself allocates.
+ CapAddressSpace();
+ int installed = __sanitizer_install_malloc_and_free_hooks(OnMalloc, OnFree);
+ assert(installed);
+
+ void *q = realloc(p, 512UL << 20);
+ assert(q == NULL);
+
+ assert(g_freed == nullptr);
+ assert(g_alloced == nullptr);
+
+ assert(__sanitizer_get_ownership(p));
+ assert(__sanitizer_get_allocated_size(p) == kSize);
+ for (size_t i = 0; i < kSize; ++i)
+ assert(p[i] == 'a');
+
+ fprintf(stderr, "original allocation survived realloc failure\n");
+ free(p);
+ assert(g_freed == p);
+ fprintf(stderr, "freed once\n");
+ return 0;
+}
+
+// CHECK: original allocation survived realloc failure
+// CHECK: freed once
diff --git a/compiler-rt/test/lsan/TestCases/realloc_zero.c b/compiler-rt/test/lsan/TestCases/realloc_zero.c
index d4ce4754d9bdf..84ec830455065 100644
--- a/compiler-rt/test/lsan/TestCases/realloc_zero.c
+++ b/compiler-rt/test/lsan/TestCases/realloc_zero.c
@@ -4,10 +4,24 @@
#include <assert.h>
#include <stdlib.h>
+#if __has_feature(leak_sanitizer)
+# include <sanitizer/allocator_interface.h>
+#endif
+
int main() {
char *p = malloc(1);
// The behavior of realloc(p, 0) is implementation-defined.
// We free the allocation.
assert(realloc(p, 0) == NULL);
+
+ // realloc(NULL, 0) allocates instead, and must be tracked like malloc(0).
+ void *q = realloc(NULL, 0);
+ assert(q != NULL);
+#if __has_feature(leak_sanitizer)
+ assert(__sanitizer_get_ownership(q));
+ assert(__sanitizer_get_allocated_size(q) == 1);
+#endif
+ free(q);
+
p = 0;
}
More information about the llvm-commits
mailing list