[compiler-rt] [scudo] Add hooks to mark the range of realloc (PR #73883)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 4 09:50:14 PST 2023
https://github.com/ChiaHungDuan updated https://github.com/llvm/llvm-project/pull/73883
>From 5392fc2d55c614cc0b3257fb99c4953161eb00fe Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Wed, 29 Nov 2023 23:53:32 +0000
Subject: [PATCH 1/5] [scudo] Fix realloc hooks behavior
`realloc` may involve both allocation and deallocation. Given that the
reporting the events is not atomic and which may lead the hook user to a
false case that the double-use pattern happens, we always report the old
pointer is released and report the new allocation afterward (even it's
the same pointer).
This also fixes that we didn't report the new size when it doesn't need
to allocate a new space.
---
compiler-rt/lib/scudo/standalone/combined.h | 11 ++++++++--
.../standalone/include/scudo/interface.h | 1 -
.../standalone/tests/wrappers_c_test.cpp | 21 ++++++++++++-------
.../lib/scudo/standalone/wrappers_c.inc | 18 ++++++++++++++--
4 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index b1700e5ecef7f..0a7d6492e1194 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -827,10 +827,15 @@ class Allocator {
// for it, which then forces realloc to copy the usable size of a chunk as
// opposed to its actual size.
uptr getUsableSize(const void *Ptr) {
- initThreadMaybe();
if (UNLIKELY(!Ptr))
return 0;
+ return getAllocSize(Ptr);
+ }
+
+ uptr getAllocSize(const void *Ptr) {
+ initThreadMaybe();
+
#ifdef GWP_ASAN_HOOKS
if (UNLIKELY(GuardedAlloc.pointerIsMine(Ptr)))
return GuardedAlloc.getSize(Ptr);
@@ -839,9 +844,11 @@ class Allocator {
Ptr = getHeaderTaggedPointer(const_cast<void *>(Ptr));
Chunk::UnpackedHeader Header;
Chunk::loadHeader(Cookie, Ptr, &Header);
- // Getting the usable size of a chunk only makes sense if it's allocated.
+
+ // Getting the alloc size of a chunk only makes sense if it's allocated.
if (UNLIKELY(Header.State != Chunk::State::Allocated))
reportInvalidChunkState(AllocatorAction::Sizing, const_cast<void *>(Ptr));
+
return getSize(Ptr, &Header);
}
diff --git a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
index 6c0c521f8d82f..260f1a7bd84bb 100644
--- a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
+++ b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
@@ -17,7 +17,6 @@ extern "C" {
__attribute__((weak)) const char *__scudo_default_options(void);
// Post-allocation & pre-deallocation hooks.
-// They must be thread-safe and not use heap related functions.
__attribute__((weak)) void __scudo_allocate_hook(void *ptr, size_t size);
__attribute__((weak)) void __scudo_deallocate_hook(void *ptr);
diff --git a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
index 623550535b6c8..150688b5b70a5 100644
--- a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
@@ -83,9 +83,12 @@ class ScudoWrappersCTest : public Test {
printf("Hooks are enabled but hooks tests are disabled.\n");
}
- void invalidateAllocHookPtrAs(UNUSED void *Ptr) {
- if (SCUDO_ENABLE_HOOKS_TESTS)
- AC.Ptr = Ptr;
+ void invalidateHookPtrs() {
+ if (SCUDO_ENABLE_HOOKS_TESTS) {
+ void *InvalidPtr = reinterpret_cast<void *>(0xdeadbeef);
+ AC.Ptr = InvalidPtr;
+ DC.Ptr = InvalidPtr;
+ }
}
void verifyAllocHookPtr(UNUSED void *Ptr) {
if (SCUDO_ENABLE_HOOKS_TESTS)
@@ -258,6 +261,7 @@ TEST_F(ScudoWrappersCTest, AlignedAlloc) {
}
TEST_F(ScudoWrappersCDeathTest, Realloc) {
+ invalidateHookPtrs();
// realloc(nullptr, N) is malloc(N)
void *P = realloc(nullptr, Size);
EXPECT_NE(P, nullptr);
@@ -266,6 +270,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
free(P);
verifyDeallocHookPtr(P);
+ invalidateHookPtrs();
P = malloc(Size);
EXPECT_NE(P, nullptr);
// realloc(P, 0U) is free(P) and returns nullptr
@@ -277,7 +282,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
EXPECT_LE(Size, malloc_usable_size(P));
memset(P, 0x42, Size);
- invalidateAllocHookPtrAs(reinterpret_cast<void *>(0xdeadbeef));
+ invalidateHookPtrs();
void *OldP = P;
P = realloc(P, Size * 2U);
EXPECT_NE(P, nullptr);
@@ -285,14 +290,15 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
for (size_t I = 0; I < Size; I++)
EXPECT_EQ(0x42, (reinterpret_cast<uint8_t *>(P))[I]);
if (OldP == P) {
- verifyAllocHookPtr(reinterpret_cast<void *>(0xdeadbeef));
+ verifyDeallocHookPtr(OldP);
+ verifyAllocHookPtr(OldP);
} else {
verifyAllocHookPtr(P);
verifyAllocHookSize(Size * 2U);
verifyDeallocHookPtr(OldP);
}
- invalidateAllocHookPtrAs(reinterpret_cast<void *>(0xdeadbeef));
+ invalidateHookPtrs();
OldP = P;
P = realloc(P, Size / 2U);
EXPECT_NE(P, nullptr);
@@ -300,7 +306,8 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
for (size_t I = 0; I < Size / 2U; I++)
EXPECT_EQ(0x42, (reinterpret_cast<uint8_t *>(P))[I]);
if (OldP == P) {
- verifyAllocHookPtr(reinterpret_cast<void *>(0xdeadbeef));
+ verifyDeallocHookPtr(OldP);
+ verifyAllocHookPtr(OldP);
} else {
verifyAllocHookPtr(P);
verifyAllocHookSize(Size / 2U);
diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c.inc b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
index e213da73393b5..0413ea49eac08 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
@@ -175,10 +175,24 @@ INTERFACE WEAK void *SCUDO_PREFIX(realloc)(void *ptr, size_t size) {
return nullptr;
}
+ // Given that the reporting of deallocation and allocation are not atomic, we
+ // always pretend the old pointer will be released so that the user doesn't
+ // need to worry about the false double-use case from the view of hooks.
+ //
+ // For example, assume that `realloc` releases the old pointer and allocates a
+ // new pointer. Before the reporting of both operations has been done, another
+ // thread may get the old pointer from `malloc`. It may be misinterpreted as
+ // double-use if it's not handled properly on the hook side.
+ reportDeallocation(ptr);
void *NewPtr = SCUDO_ALLOCATOR.reallocate(ptr, size, SCUDO_MALLOC_ALIGNMENT);
- if (NewPtr != ptr) {
+ if (NewPtr != nullptr) {
+ // Note that even if NewPtr == ptr, the size has changed. We still need to
+ // report the new size.
reportAllocation(NewPtr, size);
- reportDeallocation(ptr);
+ } else {
+ // If `realloc` fails, the old pointer is not released. Report the old
+ // pointer as allocated back.
+ reportAllocation(ptr, SCUDO_ALLOCATOR.getAllocSize(ptr));
}
return scudo::setErrnoOnNull(NewPtr);
>From 971a31aed67fd06ecc98e91334a1166bfab46320 Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Wed, 29 Nov 2023 23:53:32 +0000
Subject: [PATCH 2/5] [scudo] Add hooks to mark the range of realloc
`realloc` may involve both allocation and deallocation. Given that the
reporting the events is not atomic and which may lead the hook user to a
false case that the double-use pattern happens. In general, this can be
resolved on the hook side. To alleviate the task of handling it, we add
two new hooks to mark the range and reorder the reporting.
---
.../standalone/include/scudo/interface.h | 6 ++
.../standalone/tests/wrappers_c_test.cpp | 39 +++++++--
.../lib/scudo/standalone/wrappers_c.inc | 79 ++++++++++++-------
3 files changed, 86 insertions(+), 38 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
index 260f1a7bd84bb..e4534fc9ec2f7 100644
--- a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
+++ b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
@@ -20,6 +20,12 @@ __attribute__((weak)) const char *__scudo_default_options(void);
__attribute__((weak)) void __scudo_allocate_hook(void *ptr, size_t size);
__attribute__((weak)) void __scudo_deallocate_hook(void *ptr);
+// These hooks are used to mark the scope of doing realloc(). Note that the
+// allocation/deallocation are still reported through the hooks above, this is
+// only used when you want to group two operations in the realloc().
+__attribute__((weak)) void __scudo_realloc_begin_hook(void *old_ptr);
+__attribute__((weak)) void __scudo_realloc_end_hook(void *old_ptr);
+
void __scudo_print_stats(void);
typedef void (*iterate_callback)(uintptr_t base, size_t size, void *arg);
diff --git a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
index 150688b5b70a5..7b47c872ae88d 100644
--- a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
@@ -61,8 +61,13 @@ struct AllocContext {
struct DeallocContext {
void *Ptr;
};
+struct ReallocContext {
+ void *Begin;
+ void *End;
+};
static AllocContext AC;
static DeallocContext DC;
+static ReallocContext RC;
#if (SCUDO_ENABLE_HOOKS_TESTS == 1)
__attribute__((visibility("default"))) void __scudo_allocate_hook(void *Ptr,
@@ -73,6 +78,14 @@ __attribute__((visibility("default"))) void __scudo_allocate_hook(void *Ptr,
__attribute__((visibility("default"))) void __scudo_deallocate_hook(void *Ptr) {
DC.Ptr = Ptr;
}
+__attribute__((visibility("default"))) void
+__scudo_realloc_begin_hook(void *Ptr) {
+ RC.Begin = Ptr;
+}
+__attribute__((visibility("default"))) void
+__scudo_realloc_end_hook(void *Ptr) {
+ RC.End = Ptr;
+}
#endif // (SCUDO_ENABLE_HOOKS_TESTS == 1)
}
@@ -83,11 +96,11 @@ class ScudoWrappersCTest : public Test {
printf("Hooks are enabled but hooks tests are disabled.\n");
}
- void invalidateHookPtrs() {
+ void invalidateHookPtrsAs(UNUSED void *Ptr) {
if (SCUDO_ENABLE_HOOKS_TESTS) {
- void *InvalidPtr = reinterpret_cast<void *>(0xdeadbeef);
- AC.Ptr = InvalidPtr;
- DC.Ptr = InvalidPtr;
+ AC.Ptr = Ptr;
+ DC.Ptr = Ptr;
+ RC.Begin = RC.End = Ptr;
}
}
void verifyAllocHookPtr(UNUSED void *Ptr) {
@@ -102,6 +115,12 @@ class ScudoWrappersCTest : public Test {
if (SCUDO_ENABLE_HOOKS_TESTS)
EXPECT_EQ(Ptr, DC.Ptr);
}
+ void verifyReallocHooksScope(UNUSED void *Ptr) {
+ if (SCUDO_ENABLE_HOOKS_TESTS) {
+ EXPECT_EQ(Ptr, RC.Begin);
+ EXPECT_EQ(Ptr, RC.End);
+ }
+ }
};
using ScudoWrappersCDeathTest = ScudoWrappersCTest;
@@ -261,28 +280,30 @@ TEST_F(ScudoWrappersCTest, AlignedAlloc) {
}
TEST_F(ScudoWrappersCDeathTest, Realloc) {
- invalidateHookPtrs();
+ invalidateHookPtrsAs(reinterpret_cast<void *>(0xdeadbeef));
// realloc(nullptr, N) is malloc(N)
void *P = realloc(nullptr, Size);
EXPECT_NE(P, nullptr);
verifyAllocHookPtr(P);
verifyAllocHookSize(Size);
+ verifyReallocHooksScope(nullptr);
free(P);
verifyDeallocHookPtr(P);
- invalidateHookPtrs();
+ invalidateHookPtrsAs(reinterpret_cast<void *>(0xdeadbeef));
P = malloc(Size);
EXPECT_NE(P, nullptr);
// realloc(P, 0U) is free(P) and returns nullptr
EXPECT_EQ(realloc(P, 0U), nullptr);
verifyDeallocHookPtr(P);
+ verifyReallocHooksScope(P);
P = malloc(Size);
EXPECT_NE(P, nullptr);
EXPECT_LE(Size, malloc_usable_size(P));
memset(P, 0x42, Size);
- invalidateHookPtrs();
+ invalidateHookPtrsAs(reinterpret_cast<void *>(0xdeadbeef));
void *OldP = P;
P = realloc(P, Size * 2U);
EXPECT_NE(P, nullptr);
@@ -297,8 +318,9 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
verifyAllocHookSize(Size * 2U);
verifyDeallocHookPtr(OldP);
}
+ verifyReallocHooksScope(OldP);
- invalidateHookPtrs();
+ invalidateHookPtrsAs(reinterpret_cast<void *>(0xdeadbeef));
OldP = P;
P = realloc(P, Size / 2U);
EXPECT_NE(P, nullptr);
@@ -312,6 +334,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
verifyAllocHookPtr(P);
verifyAllocHookSize(Size / 2U);
}
+ verifyReallocHooksScope(OldP);
free(P);
EXPECT_DEATH(P = realloc(P, Size), "");
diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c.inc b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
index 0413ea49eac08..fde459bf5bca6 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
@@ -27,6 +27,51 @@ static void reportDeallocation(void *ptr) {
if (__scudo_deallocate_hook)
__scudo_deallocate_hook(ptr);
}
+static void reportReallocBegin(void *old_ptr) {
+ if (SCUDO_ENABLE_HOOKS)
+ if (__scudo_realloc_begin_hook)
+ __scudo_realloc_begin_hook(old_ptr);
+}
+static void reportReallocEnd(void *old_ptr) {
+ if (SCUDO_ENABLE_HOOKS)
+ if (__scudo_realloc_end_hook)
+ __scudo_realloc_end_hook(old_ptr);
+}
+
+static void *reallocImpl(void *ptr, size_t size) {
+ if (!ptr) {
+ void *Ptr = SCUDO_ALLOCATOR.allocate(size, scudo::Chunk::Origin::Malloc,
+ SCUDO_MALLOC_ALIGNMENT);
+ reportAllocation(Ptr, size);
+ return Ptr;
+ }
+ if (size == 0) {
+ reportDeallocation(ptr);
+ SCUDO_ALLOCATOR.deallocate(ptr, scudo::Chunk::Origin::Malloc);
+ return nullptr;
+ }
+
+ // Given that the reporting of deallocation and allocation are not atomic, we
+ // always pretend the old pointer will be released so that the user don't need
+ // to worry about the false double-use case from the view of hooks.
+ // For example, before the reporting all the events in the `realloc` has
+ // finished, another thread may have got the pointer released by the
+ // `realloc` and report it to the hook. It may be misinterpreted as double-use
+ // if it's not handled properly on the hook side.
+ reportDeallocation(ptr);
+ void *NewPtr = SCUDO_ALLOCATOR.reallocate(ptr, size, SCUDO_MALLOC_ALIGNMENT);
+ if (NewPtr != nullptr) {
+ // Note that even if NewPtr == ptr, the size has changed. We still need to
+ // report the new size.
+ reportAllocation(NewPtr, size);
+ } else {
+ // If `realloc` fails, the old pointer is not released. Report the old
+ // pointer as allocated.
+ reportAllocation(ptr, SCUDO_ALLOCATOR.getAllocSize(ptr));
+ }
+
+ return NewPtr;
+}
extern "C" {
@@ -163,37 +208,11 @@ INTERFACE WEAK void *SCUDO_PREFIX(pvalloc)(size_t size) {
}
INTERFACE WEAK void *SCUDO_PREFIX(realloc)(void *ptr, size_t size) {
- if (!ptr) {
- void *Ptr = SCUDO_ALLOCATOR.allocate(size, scudo::Chunk::Origin::Malloc,
- SCUDO_MALLOC_ALIGNMENT);
- reportAllocation(Ptr, size);
- return scudo::setErrnoOnNull(Ptr);
- }
- if (size == 0) {
- reportDeallocation(ptr);
- SCUDO_ALLOCATOR.deallocate(ptr, scudo::Chunk::Origin::Malloc);
- return nullptr;
- }
+ reportReallocBegin(ptr);
- // Given that the reporting of deallocation and allocation are not atomic, we
- // always pretend the old pointer will be released so that the user doesn't
- // need to worry about the false double-use case from the view of hooks.
- //
- // For example, assume that `realloc` releases the old pointer and allocates a
- // new pointer. Before the reporting of both operations has been done, another
- // thread may get the old pointer from `malloc`. It may be misinterpreted as
- // double-use if it's not handled properly on the hook side.
- reportDeallocation(ptr);
- void *NewPtr = SCUDO_ALLOCATOR.reallocate(ptr, size, SCUDO_MALLOC_ALIGNMENT);
- if (NewPtr != nullptr) {
- // Note that even if NewPtr == ptr, the size has changed. We still need to
- // report the new size.
- reportAllocation(NewPtr, size);
- } else {
- // If `realloc` fails, the old pointer is not released. Report the old
- // pointer as allocated back.
- reportAllocation(ptr, SCUDO_ALLOCATOR.getAllocSize(ptr));
- }
+ void *NewPtr = reallocImpl(ptr, size);
+
+ reportReallocEnd(ptr);
return scudo::setErrnoOnNull(NewPtr);
}
>From 78654f10111fc02119db867df11c931465a47d47 Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Thu, 30 Nov 2023 01:07:09 +0000
Subject: [PATCH 3/5] fixup! [scudo] Add hooks to mark the range of realloc
---
.../scudo/standalone/tests/wrappers_c_test.cpp | 17 +++++++++--------
compiler-rt/lib/scudo/standalone/wrappers_c.inc | 2 +-
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
index 7b47c872ae88d..d68871fbf6a58 100644
--- a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
@@ -96,11 +96,12 @@ class ScudoWrappersCTest : public Test {
printf("Hooks are enabled but hooks tests are disabled.\n");
}
- void invalidateHookPtrsAs(UNUSED void *Ptr) {
+ void invalidateHookPtrs() {
if (SCUDO_ENABLE_HOOKS_TESTS) {
- AC.Ptr = Ptr;
- DC.Ptr = Ptr;
- RC.Begin = RC.End = Ptr;
+ void *InvalidPtr = reinterpret_cast<void *>(0xdeadbeef);
+ AC.Ptr = InvalidPtr;
+ DC.Ptr = InvalidPtr;
+ RC.Begin = RC.End = InvalidPtr;
}
}
void verifyAllocHookPtr(UNUSED void *Ptr) {
@@ -280,7 +281,7 @@ TEST_F(ScudoWrappersCTest, AlignedAlloc) {
}
TEST_F(ScudoWrappersCDeathTest, Realloc) {
- invalidateHookPtrsAs(reinterpret_cast<void *>(0xdeadbeef));
+ invalidateHookPtrs();
// realloc(nullptr, N) is malloc(N)
void *P = realloc(nullptr, Size);
EXPECT_NE(P, nullptr);
@@ -290,7 +291,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
free(P);
verifyDeallocHookPtr(P);
- invalidateHookPtrsAs(reinterpret_cast<void *>(0xdeadbeef));
+ invalidateHookPtrs();
P = malloc(Size);
EXPECT_NE(P, nullptr);
// realloc(P, 0U) is free(P) and returns nullptr
@@ -303,7 +304,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
EXPECT_LE(Size, malloc_usable_size(P));
memset(P, 0x42, Size);
- invalidateHookPtrsAs(reinterpret_cast<void *>(0xdeadbeef));
+ invalidateHookPtrs();
void *OldP = P;
P = realloc(P, Size * 2U);
EXPECT_NE(P, nullptr);
@@ -320,7 +321,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
}
verifyReallocHooksScope(OldP);
- invalidateHookPtrsAs(reinterpret_cast<void *>(0xdeadbeef));
+ invalidateHookPtrs();
OldP = P;
P = realloc(P, Size / 2U);
EXPECT_NE(P, nullptr);
diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c.inc b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
index fde459bf5bca6..15d7c740dc4b3 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
@@ -66,7 +66,7 @@ static void *reallocImpl(void *ptr, size_t size) {
reportAllocation(NewPtr, size);
} else {
// If `realloc` fails, the old pointer is not released. Report the old
- // pointer as allocated.
+ // pointer as allocated back.
reportAllocation(ptr, SCUDO_ALLOCATOR.getAllocSize(ptr));
}
>From 94f533fbf1b0d00426118dce4614cbf004bdab96 Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Thu, 30 Nov 2023 04:14:19 +0000
Subject: [PATCH 4/5] fixup! fixup! [scudo] Add hooks to mark the range of
realloc
---
.../lib/scudo/standalone/include/scudo/interface.h | 3 ++-
compiler-rt/lib/scudo/standalone/wrappers_c.inc | 13 +++++++------
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
index e4534fc9ec2f7..9ac900b96533e 100644
--- a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
+++ b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
@@ -22,7 +22,8 @@ __attribute__((weak)) void __scudo_deallocate_hook(void *ptr);
// These hooks are used to mark the scope of doing realloc(). Note that the
// allocation/deallocation are still reported through the hooks above, this is
-// only used when you want to group two operations in the realloc().
+// only used when a hook user wants to know that the allocation/deallocation
+// operations are a single realloc operation.
__attribute__((weak)) void __scudo_realloc_begin_hook(void *old_ptr);
__attribute__((weak)) void __scudo_realloc_end_hook(void *old_ptr);
diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c.inc b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
index 15d7c740dc4b3..37c86f3a15776 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
@@ -52,12 +52,13 @@ static void *reallocImpl(void *ptr, size_t size) {
}
// Given that the reporting of deallocation and allocation are not atomic, we
- // always pretend the old pointer will be released so that the user don't need
- // to worry about the false double-use case from the view of hooks.
- // For example, before the reporting all the events in the `realloc` has
- // finished, another thread may have got the pointer released by the
- // `realloc` and report it to the hook. It may be misinterpreted as double-use
- // if it's not handled properly on the hook side.
+ // always pretend the old pointer will be released so that the user doesn't
+ // need to worry about the false double-use case from the view of hooks.
+ //
+ // For example, assume that `realloc` releases the old pointer and allocates a
+ // new pointer. Before the reporting of both operations has been done, another
+ // thread may get the old pointer from `malloc`. It may be misinterpreted as
+ // double-use if it's not handled properly on the hook side.
reportDeallocation(ptr);
void *NewPtr = SCUDO_ALLOCATOR.reallocate(ptr, size, SCUDO_MALLOC_ALIGNMENT);
if (NewPtr != nullptr) {
>From 3f9a8dc5e3ae63b05409d1b4e4bc0792dafb8795 Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Fri, 1 Dec 2023 21:27:29 +0000
Subject: [PATCH 5/5] fixup! Report realloc range only on
SCUDO_ALLOCATOR.reallocate()
---
.../standalone/tests/wrappers_c_test.cpp | 2 --
.../lib/scudo/standalone/wrappers_c.inc | 24 +++++++++----------
2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
index d68871fbf6a58..c6ad40c8cc2a3 100644
--- a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
@@ -287,7 +287,6 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
EXPECT_NE(P, nullptr);
verifyAllocHookPtr(P);
verifyAllocHookSize(Size);
- verifyReallocHooksScope(nullptr);
free(P);
verifyDeallocHookPtr(P);
@@ -297,7 +296,6 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
// realloc(P, 0U) is free(P) and returns nullptr
EXPECT_EQ(realloc(P, 0U), nullptr);
verifyDeallocHookPtr(P);
- verifyReallocHooksScope(P);
P = malloc(Size);
EXPECT_NE(P, nullptr);
diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c.inc b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
index 37c86f3a15776..c6d0fb1132aa9 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
@@ -39,18 +39,6 @@ static void reportReallocEnd(void *old_ptr) {
}
static void *reallocImpl(void *ptr, size_t size) {
- if (!ptr) {
- void *Ptr = SCUDO_ALLOCATOR.allocate(size, scudo::Chunk::Origin::Malloc,
- SCUDO_MALLOC_ALIGNMENT);
- reportAllocation(Ptr, size);
- return Ptr;
- }
- if (size == 0) {
- reportDeallocation(ptr);
- SCUDO_ALLOCATOR.deallocate(ptr, scudo::Chunk::Origin::Malloc);
- return nullptr;
- }
-
// Given that the reporting of deallocation and allocation are not atomic, we
// always pretend the old pointer will be released so that the user doesn't
// need to worry about the false double-use case from the view of hooks.
@@ -209,6 +197,18 @@ INTERFACE WEAK void *SCUDO_PREFIX(pvalloc)(size_t size) {
}
INTERFACE WEAK void *SCUDO_PREFIX(realloc)(void *ptr, size_t size) {
+ if (!ptr) {
+ void *Ptr = SCUDO_ALLOCATOR.allocate(size, scudo::Chunk::Origin::Malloc,
+ SCUDO_MALLOC_ALIGNMENT);
+ reportAllocation(Ptr, size);
+ return scudo::setErrnoOnNull(Ptr);
+ }
+ if (size == 0) {
+ reportDeallocation(ptr);
+ SCUDO_ALLOCATOR.deallocate(ptr, scudo::Chunk::Origin::Malloc);
+ return nullptr;
+ }
+
reportReallocBegin(ptr);
void *NewPtr = reallocImpl(ptr, size);
More information about the llvm-commits
mailing list