From 4e3dac6f0a4c419ee0cd575407e95f3833a01a2e Mon Sep 17 00:00:00 2001 From: Guillaume Chelfi Date: Tue, 14 Feb 2023 18:44:37 +0000 Subject: [PATCH] [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 --- .../lib/scudo/standalone/allocator_config.h | 2 +- compiler-rt/lib/scudo/standalone/combined.h | 2 + .../lib/scudo/standalone/tests/CMakeLists.txt | 8 ++ .../scudo/standalone/tests/scudo_hooks_test.cpp | 114 +++++++++++++++++++++ 4 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 compiler-rt/lib/scudo/standalone/tests/scudo_hooks_test.cpp diff --git a/compiler-rt/lib/scudo/standalone/allocator_config.h b/compiler-rt/lib/scudo/standalone/allocator_config.h index 63eb325..6430606 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 826e2a0..f41254b 100644 --- a/compiler-rt/lib/scudo/standalone/combined.h +++ b/compiler-rt/lib/scudo/standalone/combined.h @@ -687,6 +687,8 @@ public: 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 8200cd2..474c7517 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 0000000..cf69f37 --- /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 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; + } + } +} -- 2.7.4