[compiler-rt] 4e3dac6 - [scudo] Call __scudo_deallocate_hook on reallocations.
Chia-hung Duan via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 14 11:04:30 PST 2023
Author: Guillaume Chelfi
Date: 2023-02-14T18:44:38Z
New Revision: 4e3dac6f0a4c419ee0cd575407e95f3833a01a2e
URL: https://github.com/llvm/llvm-project/commit/4e3dac6f0a4c419ee0cd575407e95f3833a01a2e
DIFF: https://github.com/llvm/llvm-project/commit/4e3dac6f0a4c419ee0cd575407e95f3833a01a2e.diff
LOG: [scudo] Call __scudo_deallocate_hook on reallocations.
Scudo is expected to call __scudo_allocate_hook on allocations, and
__scudo_deallocate_hook on deallocations, but it's behavior is not
clear on reallocations. Currently, non-trivial reallocations call
__scudo_allocate_hook but never __scudo_deallocate_hook. We should
prefer either calling both, none, or a dedicated
hook (__scudo_reallocate_hook, for instance).
This patch implements the former, and adds a unit test to enforce
those expectations.
Reviewed By: Chia-hungDuan
Differential Revision: https://reviews.llvm.org/D141407
Added:
compiler-rt/lib/scudo/standalone/tests/scudo_hooks_test.cpp
Modified:
compiler-rt/lib/scudo/standalone/allocator_config.h
compiler-rt/lib/scudo/standalone/combined.h
compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
Removed:
################################################################################
diff --git a/compiler-rt/lib/scudo/standalone/allocator_config.h b/compiler-rt/lib/scudo/standalone/allocator_config.h
index 63eb325c9b876..64306066123e0 100644
--- a/compiler-rt/lib/scudo/standalone/allocator_config.h
+++ b/compiler-rt/lib/scudo/standalone/allocator_config.h
@@ -26,7 +26,7 @@ namespace scudo {
// allocator.
//
// struct ExampleConfig {
-// // SizeClasMmap to use with the Primary.
+// // SizeClassMap to use with the Primary.
// using SizeClassMap = DefaultSizeClassMap;
// // Indicates possible support for Memory Tagging.
// static const bool MaySupportMemoryTagging = false;
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index 826e2a0806e00..f41254bef3268 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -687,6 +687,8 @@ class Allocator {
void *NewPtr = allocate(NewSize, Chunk::Origin::Malloc, Alignment);
if (LIKELY(NewPtr)) {
memcpy(NewPtr, OldTaggedPtr, Min(NewSize, OldSize));
+ if (UNLIKELY(&__scudo_deallocate_hook))
+ __scudo_deallocate_hook(OldTaggedPtr);
quarantineOrDeallocateChunk(Options, OldTaggedPtr, &OldHeader, OldSize);
}
return NewPtr;
diff --git a/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt b/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
index 8200cd2588b35..474c75173d9f3 100644
--- a/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
+++ b/compiler-rt/lib/scudo/standalone/tests/CMakeLists.txt
@@ -127,3 +127,11 @@ set(SCUDO_CXX_UNIT_TEST_SOURCES
add_scudo_unittest(ScudoCxxUnitTest
SOURCES ${SCUDO_CXX_UNIT_TEST_SOURCES}
ADDITIONAL_RTOBJECTS RTScudoStandaloneCWrappers RTScudoStandaloneCxxWrappers)
+
+set(SCUDO_HOOKS_UNIT_TEST_SOURCES
+ scudo_hooks_test.cpp
+ scudo_unit_test_main.cpp
+ )
+
+add_scudo_unittest(ScudoHooksUnitTest
+ SOURCES ${SCUDO_HOOKS_UNIT_TEST_SOURCES})
diff --git a/compiler-rt/lib/scudo/standalone/tests/scudo_hooks_test.cpp b/compiler-rt/lib/scudo/standalone/tests/scudo_hooks_test.cpp
new file mode 100644
index 0000000000000..cf69f376ac446
--- /dev/null
+++ b/compiler-rt/lib/scudo/standalone/tests/scudo_hooks_test.cpp
@@ -0,0 +1,114 @@
+//===-- scudo_hooks_test.cpp ------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "tests/scudo_unit_test.h"
+
+#include "allocator_config.h"
+#include "combined.h"
+
+namespace {
+void *LastAllocatedPtr = nullptr;
+size_t LastRequestSize = 0;
+void *LastDeallocatedPtr = nullptr;
+} // namespace
+
+// Scudo defines weak symbols that can be defined by a client binary
+// to register callbacks at key points in the allocation timeline. In
+// order to enforce those invariants, we provide definitions that
+// update some global state every time they are called, so that tests
+// can inspect their effects. An unfortunate side effect of this
+// setup is that because those symbols are part of the binary, they
+// can't be selectively enabled; that means that they will get called
+// on unrelated tests in the same compilation unit. To mitigate this
+// issue, we insulate those tests in a separate compilation unit.
+extern "C" {
+__attribute__((visibility("default"))) void __scudo_allocate_hook(void *Ptr,
+ size_t Size) {
+ LastAllocatedPtr = Ptr;
+ LastRequestSize = Size;
+}
+__attribute__((visibility("default"))) void __scudo_deallocate_hook(void *Ptr) {
+ LastDeallocatedPtr = Ptr;
+}
+}
+
+// Simple check that allocation callbacks, when registered, are called:
+// 1) __scudo_allocate_hook is called when allocating.
+// 2) __scudo_deallocate_hook is called when deallocating.
+// 3) Both hooks are called when reallocating.
+// 4) Neither are called for a no-op reallocation.
+TEST(ScudoHooksTest, AllocateHooks) {
+ scudo::Allocator<scudo::DefaultConfig> Allocator;
+ constexpr scudo::uptr DefaultSize = 16U;
+ constexpr scudo::Chunk::Origin Origin = scudo::Chunk::Origin::Malloc;
+
+ // Simple allocation and deallocation.
+ {
+ LastAllocatedPtr = nullptr;
+ LastRequestSize = 0;
+
+ void *Ptr = Allocator.allocate(DefaultSize, Origin);
+
+ EXPECT_EQ(Ptr, LastAllocatedPtr);
+ EXPECT_EQ(DefaultSize, LastRequestSize);
+
+ LastDeallocatedPtr = nullptr;
+
+ Allocator.deallocate(Ptr, Origin);
+
+ EXPECT_EQ(Ptr, LastDeallocatedPtr);
+ }
+
+ // Simple no-op, same size reallocation.
+ {
+ void *Ptr = Allocator.allocate(DefaultSize, Origin);
+
+ LastAllocatedPtr = nullptr;
+ LastRequestSize = 0;
+ LastDeallocatedPtr = nullptr;
+
+ void *NewPtr = Allocator.reallocate(Ptr, DefaultSize);
+
+ EXPECT_EQ(Ptr, NewPtr);
+ EXPECT_EQ(nullptr, LastAllocatedPtr);
+ EXPECT_EQ(0, LastRequestSize);
+ EXPECT_EQ(nullptr, LastDeallocatedPtr);
+ }
+
+ // Reallocation in increasing size classes. This ensures that at
+ // least one of the reallocations will be meaningful.
+ {
+ void *Ptr = Allocator.allocate(0, Origin);
+
+ for (scudo::uptr ClassId = 1U;
+ ClassId <= scudo::DefaultConfig::Primary::SizeClassMap::LargestClassId;
+ ++ClassId) {
+ const scudo::uptr Size =
+ scudo::DefaultConfig::Primary::SizeClassMap::getSizeByClassId(
+ ClassId);
+
+ LastAllocatedPtr = nullptr;
+ LastRequestSize = 0;
+ LastDeallocatedPtr = nullptr;
+
+ void *NewPtr = Allocator.reallocate(Ptr, Size);
+
+ if (NewPtr != Ptr) {
+ EXPECT_EQ(NewPtr, LastAllocatedPtr);
+ EXPECT_EQ(Size, LastRequestSize);
+ EXPECT_EQ(Ptr, LastDeallocatedPtr);
+ } else {
+ EXPECT_EQ(nullptr, LastAllocatedPtr);
+ EXPECT_EQ(0, LastRequestSize);
+ EXPECT_EQ(nullptr, LastDeallocatedPtr);
+ }
+
+ Ptr = NewPtr;
+ }
+ }
+}
More information about the llvm-commits
mailing list