[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