[compiler-rt] [scudo] Add hooks to mark the range of realloc (PR #74353)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 7 11:41:46 PST 2023
https://github.com/ChiaHungDuan updated https://github.com/llvm/llvm-project/pull/74353
>From dbb117b46d7fa3db646301fd67c8c53efc29eae9 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/4] [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 so that the hook user can combine those
calls together.
---
.../standalone/include/scudo/interface.h | 7 +++
.../standalone/tests/wrappers_c_test.cpp | 22 +++++++
.../lib/scudo/standalone/wrappers_c.inc | 58 +++++++++++++------
3 files changed, 68 insertions(+), 19 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..9ac900b96533e 100644
--- a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
+++ b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
@@ -20,6 +20,13 @@ __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 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);
+
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..c6ad40c8cc2a3 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)
}
@@ -88,6 +101,7 @@ class ScudoWrappersCTest : public Test {
void *InvalidPtr = reinterpret_cast<void *>(0xdeadbeef);
AC.Ptr = InvalidPtr;
DC.Ptr = InvalidPtr;
+ RC.Begin = RC.End = InvalidPtr;
}
}
void verifyAllocHookPtr(UNUSED void *Ptr) {
@@ -102,6 +116,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;
@@ -297,6 +317,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
verifyAllocHookSize(Size * 2U);
verifyDeallocHookPtr(OldP);
}
+ verifyReallocHooksScope(OldP);
invalidateHookPtrs();
OldP = P;
@@ -312,6 +333,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..c6d0fb1132aa9 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
@@ -27,6 +27,40 @@ 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) {
+ // 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));
+ }
+
+ return NewPtr;
+}
extern "C" {
@@ -175,25 +209,11 @@ 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 != 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));
- }
+ reportReallocBegin(ptr);
+
+ void *NewPtr = reallocImpl(ptr, size);
+
+ reportReallocEnd(ptr);
return scudo::setErrnoOnNull(NewPtr);
}
>From ee27083f5aa510a7ff2be33fa1e95a2b0d82494f Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Mon, 4 Dec 2023 19:39:31 +0000
Subject: [PATCH 2/4] fixup! Inline `reallocImpl`
---
.../lib/scudo/standalone/wrappers_c.inc | 46 +++++++++----------
1 file changed, 21 insertions(+), 25 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c.inc b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
index c6d0fb1132aa9..bad3751c4fc02 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
@@ -38,30 +38,6 @@ static void reportReallocEnd(void *old_ptr) {
__scudo_realloc_end_hook(old_ptr);
}
-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 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));
- }
-
- return NewPtr;
-}
-
extern "C" {
INTERFACE WEAK void *SCUDO_PREFIX(calloc)(size_t nmemb, size_t size) {
@@ -209,9 +185,29 @@ INTERFACE WEAK void *SCUDO_PREFIX(realloc)(void *ptr, size_t size) {
return nullptr;
}
+ // Mark the range of `SCUDO_ALLOCATOR.reallocate()` so that the hook user
+ // knows the deallocation/allocation are paired.
reportReallocBegin(ptr);
- void *NewPtr = reallocImpl(ptr, 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 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));
+ }
reportReallocEnd(ptr);
>From a4a18e2a6a55bcb12ec0df3e65e8a6fefd62747f Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Thu, 7 Dec 2023 01:00:44 +0000
Subject: [PATCH 3/4] fixup! Change the way of using realloc hooks
---
.../standalone/include/scudo/interface.h | 19 ++++++---
.../standalone/tests/wrappers_c_test.cpp | 40 +++++++++++++------
.../lib/scudo/standalone/wrappers_c.inc | 37 ++++++++---------
3 files changed, 60 insertions(+), 36 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
index 9ac900b96533e..cce035f468a78 100644
--- a/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
+++ b/compiler-rt/lib/scudo/standalone/include/scudo/interface.h
@@ -20,12 +20,19 @@ __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 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);
+// `realloc` involves both deallocation and allocation but they are not reported
+// atomically. In one specific case which may keep taking a snapshot right in
+// the middle of `realloc` reporting the deallocation and allocation, it may
+// confuse the user by the missing memory from `realloc`. To alleviate that
+// case, define the two `realloc` hooks to get the knowledge of the bundled hook
+// calls. This hooks are optional and only used when you are pretty likely to
+// hit that specific case. Otherwise, the two general hooks above are pretty
+// sufficient for the most cases.
+//
+// See more details in the comment of `realloc` in wrapper_c.inc.
+__attribute__((weak)) void
+__scudo_realloc_allocate_hook(void *old_ptr, void *new_ptr, size_t size);
+__attribute__((weak)) void __scudo_realloc_deallocate_hook(void *old_ptr);
void __scudo_print_stats(void);
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 c6ad40c8cc2a3..aefadfc274bd0 100644
--- a/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/wrappers_c_test.cpp
@@ -62,8 +62,9 @@ struct DeallocContext {
void *Ptr;
};
struct ReallocContext {
- void *Begin;
- void *End;
+ void *AllocPtr;
+ void *DeallocPtr;
+ size_t Size;
};
static AllocContext AC;
static DeallocContext DC;
@@ -79,12 +80,26 @@ __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;
+__scudo_realloc_allocate_hook(void *OldPtr, void *NewPtr, size_t Size) {
+ // Verify that __scudo_realloc_deallocate_hook is called first and set the
+ // right pointer.
+ EXPECT_EQ(OldPtr, RC.DeallocPtr);
+ RC.AllocPtr = NewPtr;
+ RC.Size = Size;
+
+ // Note that this is only used for testing. In general, only one pair of hooks
+ // will be invoked in `realloc`. if __scudo_realloc_*_hook are not defined,
+ // it'll call the general hooks only. To make the test easier, we call the
+ // general one here so that either case (wether __scudo_realloc_*_hook are
+ // defined) will be verified without separating them into different tests.
+ __scudo_allocate_hook(NewPtr, Size);
}
__attribute__((visibility("default"))) void
-__scudo_realloc_end_hook(void *Ptr) {
- RC.End = Ptr;
+__scudo_realloc_deallocate_hook(void *Ptr) {
+ RC.DeallocPtr = Ptr;
+
+ // See the comment in the __scudo_realloc_allocate_hook above.
+ __scudo_deallocate_hook(Ptr);
}
#endif // (SCUDO_ENABLE_HOOKS_TESTS == 1)
}
@@ -101,7 +116,7 @@ class ScudoWrappersCTest : public Test {
void *InvalidPtr = reinterpret_cast<void *>(0xdeadbeef);
AC.Ptr = InvalidPtr;
DC.Ptr = InvalidPtr;
- RC.Begin = RC.End = InvalidPtr;
+ RC.AllocPtr = RC.DeallocPtr = InvalidPtr;
}
}
void verifyAllocHookPtr(UNUSED void *Ptr) {
@@ -116,10 +131,11 @@ class ScudoWrappersCTest : public Test {
if (SCUDO_ENABLE_HOOKS_TESTS)
EXPECT_EQ(Ptr, DC.Ptr);
}
- void verifyReallocHooksScope(UNUSED void *Ptr) {
+ void verifyReallocHookPtrs(UNUSED void *OldPtr, void *NewPtr, size_t Size) {
if (SCUDO_ENABLE_HOOKS_TESTS) {
- EXPECT_EQ(Ptr, RC.Begin);
- EXPECT_EQ(Ptr, RC.End);
+ EXPECT_EQ(OldPtr, RC.DeallocPtr);
+ EXPECT_EQ(NewPtr, RC.AllocPtr);
+ EXPECT_EQ(Size, RC.Size);
}
}
};
@@ -317,7 +333,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
verifyAllocHookSize(Size * 2U);
verifyDeallocHookPtr(OldP);
}
- verifyReallocHooksScope(OldP);
+ verifyReallocHookPtrs(OldP, P, Size * 2U);
invalidateHookPtrs();
OldP = P;
@@ -333,7 +349,7 @@ TEST_F(ScudoWrappersCDeathTest, Realloc) {
verifyAllocHookPtr(P);
verifyAllocHookSize(Size / 2U);
}
- verifyReallocHooksScope(OldP);
+ verifyReallocHookPtrs(OldP, P, Size / 2U);
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 bad3751c4fc02..4df141bacf05d 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
@@ -27,15 +27,21 @@ 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 reportReallocAllocation(void *old_ptr, void *new_ptr, size_t size) {
+ if (SCUDO_ENABLE_HOOKS) {
+ if (__scudo_realloc_allocate_hook)
+ __scudo_realloc_allocate_hook(old_ptr, new_ptr, size);
+ else
+ __scudo_allocate_hook(new_ptr, size);
+ }
}
-static void reportReallocEnd(void *old_ptr) {
- if (SCUDO_ENABLE_HOOKS)
- if (__scudo_realloc_end_hook)
- __scudo_realloc_end_hook(old_ptr);
+static void reportReallocDeallocation(void *old_ptr) {
+ if (SCUDO_ENABLE_HOOKS) {
+ if (__scudo_realloc_deallocate_hook)
+ __scudo_realloc_deallocate_hook(old_ptr);
+ else
+ __scudo_deallocate_hook(old_ptr);
+ }
}
extern "C" {
@@ -185,10 +191,6 @@ INTERFACE WEAK void *SCUDO_PREFIX(realloc)(void *ptr, size_t size) {
return nullptr;
}
- // Mark the range of `SCUDO_ALLOCATOR.reallocate()` so that the hook user
- // knows the deallocation/allocation are paired.
- 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.
@@ -197,20 +199,19 @@ INTERFACE WEAK void *SCUDO_PREFIX(realloc)(void *ptr, size_t size) {
// 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);
+ reportReallocDeallocation(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);
+ reportReallocAllocation(/*OldPtr=*/ptr, 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));
+ // pointer as allocated again.
+ reportReallocAllocation(/*OldPtr=*/ptr, /*NewPtr=*/ptr,
+ SCUDO_ALLOCATOR.getAllocSize(ptr));
}
- reportReallocEnd(ptr);
-
return scudo::setErrnoOnNull(NewPtr);
}
>From cc66d7538cc867d36252599c261c2b27c4a9f0ba Mon Sep 17 00:00:00 2001
From: Chia-hung Duan <chiahungduan at google.com>
Date: Thu, 7 Dec 2023 19:40:28 +0000
Subject: [PATCH 4/4] fixup! Check if the general hooks are defined in realloc
path
---
compiler-rt/lib/scudo/standalone/wrappers_c.inc | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/wrappers_c.inc b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
index 4df141bacf05d..56d8ef20156e2 100644
--- a/compiler-rt/lib/scudo/standalone/wrappers_c.inc
+++ b/compiler-rt/lib/scudo/standalone/wrappers_c.inc
@@ -28,10 +28,12 @@ static void reportDeallocation(void *ptr) {
__scudo_deallocate_hook(ptr);
}
static void reportReallocAllocation(void *old_ptr, void *new_ptr, size_t size) {
+ DCHECK_NE(new_ptr, nullptr);
+
if (SCUDO_ENABLE_HOOKS) {
if (__scudo_realloc_allocate_hook)
__scudo_realloc_allocate_hook(old_ptr, new_ptr, size);
- else
+ else if (__scudo_allocate_hook)
__scudo_allocate_hook(new_ptr, size);
}
}
@@ -39,7 +41,7 @@ static void reportReallocDeallocation(void *old_ptr) {
if (SCUDO_ENABLE_HOOKS) {
if (__scudo_realloc_deallocate_hook)
__scudo_realloc_deallocate_hook(old_ptr);
- else
+ else if (__scudo_deallocate_hook)
__scudo_deallocate_hook(old_ptr);
}
}
More information about the llvm-commits
mailing list