From 1e55c8156bae97d6847865459141df8488a0953b Mon Sep 17 00:00:00 2001 From: Fabio D'Urso Date: Thu, 23 May 2024 17:43:00 +0200 Subject: [PATCH 1/3] [scudo] Apply filling option when realloc grows a block in-place too --- compiler-rt/lib/scudo/standalone/combined.h | 13 ++++++++++++ .../scudo/standalone/tests/combined_test.cpp | 21 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h index 15a199ae0349b..60c7fae1d57d2 100644 --- a/compiler-rt/lib/scudo/standalone/combined.h +++ b/compiler-rt/lib/scudo/standalone/combined.h @@ -565,6 +565,19 @@ class Allocator { storeSecondaryAllocationStackMaybe(Options, OldPtr, NewSize); } } + + // If we've increased the size, fill the extra bytes. + if (NewSize > OldSize) { + const FillContentsMode FillContents = + TSDRegistry.getDisableMemInit() ? NoFill + : Options.getFillContentsMode(); + if (FillContents != NoFill) { + memset(reinterpret_cast(OldTaggedPtr) + OldSize, + FillContents == ZeroFill ? 0 : PatternFillByte, + NewSize - OldSize); + } + } + return OldTaggedPtr; } } diff --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp index 1a36155bcd423..4af0d44493b2a 100644 --- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp @@ -347,7 +347,17 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, ZeroFill) { EXPECT_NE(P, nullptr); for (scudo::uptr I = 0; I < Size; I++) ASSERT_EQ((reinterpret_cast(P))[I], '\0'); + + // Fill with a non-zero pattern. memset(P, 0xaa, Size); + + // Shrink and then grow by one byte, verifying that it gets re-filled in + // the process. We assume that changing the size by just 1 is done in + // place. + ASSERT_EQ(Allocator->reallocate(P, Size - 1), P); + ASSERT_EQ(Allocator->reallocate(P, Size), P); + EXPECT_EQ((reinterpret_cast(P))[Size - 1], '\0'); + Allocator->deallocate(P, Origin, Size); } } @@ -374,7 +384,18 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, PatternOrZeroFill) { else ASSERT_TRUE(V == scudo::PatternFillByte || V == 0); } + + // Fill with a known pattern different from PatternFillByte. memset(P, 0xaa, Size); + + // Shrink and then grow by one byte, verifying that it gets re-filled in + // the process. We assume that changing the size by just 1 is done in + // place. + ASSERT_EQ(Allocator->reallocate(P, Size - 1), P); + ASSERT_EQ(Allocator->reallocate(P, Size), P); + EXPECT_EQ((reinterpret_cast(P))[Size - 1], + scudo::PatternFillByte); + Allocator->deallocate(P, Origin, Size); } } From 17625ca946e870cb5d6963f6bfe6724dbfe37ad4 Mon Sep 17 00:00:00 2001 From: Fabio D'Urso Date: Tue, 4 Jun 2024 18:09:05 +0200 Subject: [PATCH 2/3] Moved test to ScudoCombinedDeathTest::ReallocateSame --- .../scudo/standalone/tests/combined_test.cpp | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp index 4af0d44493b2a..75fe740386e8b 100644 --- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp @@ -347,17 +347,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, ZeroFill) { EXPECT_NE(P, nullptr); for (scudo::uptr I = 0; I < Size; I++) ASSERT_EQ((reinterpret_cast(P))[I], '\0'); - - // Fill with a non-zero pattern. memset(P, 0xaa, Size); - - // Shrink and then grow by one byte, verifying that it gets re-filled in - // the process. We assume that changing the size by just 1 is done in - // place. - ASSERT_EQ(Allocator->reallocate(P, Size - 1), P); - ASSERT_EQ(Allocator->reallocate(P, Size), P); - EXPECT_EQ((reinterpret_cast(P))[Size - 1], '\0'); - Allocator->deallocate(P, Origin, Size); } } @@ -384,18 +374,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, PatternOrZeroFill) { else ASSERT_TRUE(V == scudo::PatternFillByte || V == 0); } - - // Fill with a known pattern different from PatternFillByte. memset(P, 0xaa, Size); - - // Shrink and then grow by one byte, verifying that it gets re-filled in - // the process. We assume that changing the size by just 1 is done in - // place. - ASSERT_EQ(Allocator->reallocate(P, Size - 1), P); - ASSERT_EQ(Allocator->reallocate(P, Size), P); - EXPECT_EQ((reinterpret_cast(P))[Size - 1], - scudo::PatternFillByte); - Allocator->deallocate(P, Origin, Size); } } @@ -468,19 +447,32 @@ SCUDO_TYPED_TEST(ScudoCombinedDeathTest, ReallocateSame) { // returns the same chunk. This requires that all the sizes we iterate on use // the same block size, but that should be the case for MaxSize - 64 with our // default class size maps. - constexpr scudo::uptr ReallocSize = + constexpr scudo::uptr InitialSize = TypeParam::Primary::SizeClassMap::MaxSize - 64; - void *P = Allocator->allocate(ReallocSize, Origin); const char Marker = 'A'; - memset(P, Marker, ReallocSize); + Allocator->setFillContents(scudo::PatternOrZeroFill); + + void *P = Allocator->allocate(InitialSize, Origin); + scudo::uptr CurrentSize = InitialSize; for (scudo::sptr Delta = -32; Delta < 32; Delta += 8) { + memset(P, Marker, CurrentSize); const scudo::uptr NewSize = - static_cast(static_cast(ReallocSize) + Delta); + static_cast(static_cast(InitialSize) + Delta); void *NewP = Allocator->reallocate(P, NewSize); EXPECT_EQ(NewP, P); - for (scudo::uptr I = 0; I < ReallocSize - 32; I++) + + // Verify that existing contents have been preserved. + for (scudo::uptr I = 0; I < CurrentSize; I++) EXPECT_EQ((reinterpret_cast(NewP))[I], Marker); + + // Verify that, if we have grown the allocation, new bytes have been set + // according to FillContentsMode. + for (scudo::uptr I = CurrentSize; I < NewSize; I++) + EXPECT_EQ((reinterpret_cast(NewP))[I], + scudo::PatternFillByte); + checkMemoryTaggingMaybe(Allocator, NewP, NewSize, 0); + CurrentSize = NewSize; } Allocator->deallocate(P, Origin); } From 4826711d0522771d12edfd3b215b305f0b876fce Mon Sep 17 00:00:00 2001 From: Fabio D'Urso Date: Wed, 5 Jun 2024 14:42:23 +0200 Subject: [PATCH 3/3] Apply fill when shrank instead of when grown --- compiler-rt/lib/scudo/standalone/combined.h | 9 +++++---- compiler-rt/lib/scudo/standalone/tests/combined_test.cpp | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h index 60c7fae1d57d2..9283e11aa3b7f 100644 --- a/compiler-rt/lib/scudo/standalone/combined.h +++ b/compiler-rt/lib/scudo/standalone/combined.h @@ -566,15 +566,16 @@ class Allocator { } } - // If we've increased the size, fill the extra bytes. - if (NewSize > OldSize) { + // If we have reduced the size, set the extra bytes to the fill value + // so that we are ready to grow it again in the future. + if (NewSize < OldSize) { const FillContentsMode FillContents = TSDRegistry.getDisableMemInit() ? NoFill : Options.getFillContentsMode(); if (FillContents != NoFill) { - memset(reinterpret_cast(OldTaggedPtr) + OldSize, + memset(reinterpret_cast(OldTaggedPtr) + NewSize, FillContents == ZeroFill ? 0 : PatternFillByte, - NewSize - OldSize); + OldSize - NewSize); } } diff --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp index 75fe740386e8b..655dc87cbac64 100644 --- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp @@ -462,14 +462,14 @@ SCUDO_TYPED_TEST(ScudoCombinedDeathTest, ReallocateSame) { EXPECT_EQ(NewP, P); // Verify that existing contents have been preserved. - for (scudo::uptr I = 0; I < CurrentSize; I++) + for (scudo::uptr I = 0; I < scudo::Min(CurrentSize, NewSize); I++) EXPECT_EQ((reinterpret_cast(NewP))[I], Marker); - // Verify that, if we have grown the allocation, new bytes have been set - // according to FillContentsMode. - for (scudo::uptr I = CurrentSize; I < NewSize; I++) + // Verify that new bytes are set according to FillContentsMode. + for (scudo::uptr I = CurrentSize; I < NewSize; I++) { EXPECT_EQ((reinterpret_cast(NewP))[I], scudo::PatternFillByte); + } checkMemoryTaggingMaybe(Allocator, NewP, NewSize, 0); CurrentSize = NewSize;