[compiler-rt] Reland "[scudo] Apply filling when realloc shrinks and re-grows a block in-place" (PR #95838)
Fabio D'Urso via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 17 13:27:44 PDT 2024
https://github.com/fabio-d updated https://github.com/llvm/llvm-project/pull/95838
>From e3c9ce6602142062103432d43175d1d7e40159ff Mon Sep 17 00:00:00 2001
From: Fabio D'Urso <fdurso at google.com>
Date: Mon, 17 Jun 2024 17:04:27 +0200
Subject: [PATCH 1/4] Reland "[scudo] Apply filling when realloc shrinks and
re-grows a block in-place"
Reland of #93212, which has been reverted in
commit bddd8eae17df6511aee789744ccdc158de817081.
---
compiler-rt/lib/scudo/standalone/combined.h | 14 +++++++++++
.../scudo/standalone/tests/combined_test.cpp | 23 +++++++++++++++----
2 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index f9ed36581f8d3..73da686287747 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -565,6 +565,20 @@ class Allocator {
storeSecondaryAllocationStackMaybe(Options, OldPtr, NewSize);
}
}
+
+ // 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<char *>(OldTaggedPtr) + NewSize,
+ FillContents == ZeroFill ? 0 : PatternFillByte,
+ OldSize - NewSize);
+ }
+ }
+
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..655dc87cbac64 100644
--- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -447,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<scudo::uptr>(static_cast<scudo::sptr>(ReallocSize) + Delta);
+ static_cast<scudo::uptr>(static_cast<scudo::sptr>(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 < scudo::Min(CurrentSize, NewSize); I++)
EXPECT_EQ((reinterpret_cast<char *>(NewP))[I], Marker);
+
+ // Verify that new bytes are set according to FillContentsMode.
+ for (scudo::uptr I = CurrentSize; I < NewSize; I++) {
+ EXPECT_EQ((reinterpret_cast<unsigned char *>(NewP))[I],
+ scudo::PatternFillByte);
+ }
+
checkMemoryTaggingMaybe(Allocator, NewP, NewSize, 0);
+ CurrentSize = NewSize;
}
Allocator->deallocate(P, Origin);
}
>From 70c6365916f19264cd400138e84ea5b48c26de46 Mon Sep 17 00:00:00 2001
From: Fabio D'Urso <fdurso at google.com>
Date: Mon, 17 Jun 2024 22:01:50 +0200
Subject: [PATCH 2/4] Fill with pattern *before* MTE tags are changed + relax
expectation in test
---
compiler-rt/lib/scudo/standalone/combined.h | 26 +++++++++----------
.../scudo/standalone/tests/combined_test.cpp | 4 +--
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index 73da686287747..eb7665bcfb62e 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -549,6 +549,19 @@ class Allocator {
// header to reflect the size change.
if (reinterpret_cast<uptr>(OldTaggedPtr) + NewSize <= BlockEnd) {
if (NewSize > OldSize || (OldSize - NewSize) < getPageSizeCached()) {
+ // 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<char *>(OldTaggedPtr) + NewSize,
+ FillContents == ZeroFill ? 0 : PatternFillByte,
+ OldSize - NewSize);
+ }
+ }
+
Header.SizeOrUnusedBytes =
(ClassId ? NewSize
: BlockEnd -
@@ -566,19 +579,6 @@ class Allocator {
}
}
- // 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<char *>(OldTaggedPtr) + NewSize,
- FillContents == ZeroFill ? 0 : PatternFillByte,
- OldSize - NewSize);
- }
- }
-
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 655dc87cbac64..8ad0ad2d381fc 100644
--- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -467,8 +467,8 @@ SCUDO_TYPED_TEST(ScudoCombinedDeathTest, ReallocateSame) {
// Verify that new bytes are set according to FillContentsMode.
for (scudo::uptr I = CurrentSize; I < NewSize; I++) {
- EXPECT_EQ((reinterpret_cast<unsigned char *>(NewP))[I],
- scudo::PatternFillByte);
+ unsigned char V = (reinterpret_cast<unsigned char *>(NewP))[I];
+ ASSERT_TRUE(V == scudo::PatternFillByte || V == 0);
}
checkMemoryTaggingMaybe(Allocator, NewP, NewSize, 0);
>From 29f40b3fcd19fc1a89575e2ca97caed98a0036c9 Mon Sep 17 00:00:00 2001
From: Fabio D'Urso <fdurso at google.com>
Date: Mon, 17 Jun 2024 22:15:10 +0200
Subject: [PATCH 3/4] Remove unnecessary whitespace change
---
compiler-rt/lib/scudo/standalone/combined.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index eb7665bcfb62e..fcf65652c5fce 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -578,7 +578,6 @@ class Allocator {
storeSecondaryAllocationStackMaybe(Options, OldPtr, NewSize);
}
}
-
return OldTaggedPtr;
}
}
>From a025f896b5ebbed1b2cb3173e7a69feb68a08157 Mon Sep 17 00:00:00 2001
From: Fabio D'Urso <fabiodurso at hotmail.it>
Date: Mon, 17 Jun 2024 22:27:36 +0200
Subject: [PATCH 4/4] Replace ASSERT_TRUE with EXPECT_TRUE
Co-authored-by: ChiaHungDuan <f103119 at gmail.com>
---
compiler-rt/lib/scudo/standalone/tests/combined_test.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
index 8ad0ad2d381fc..16b19e807e11b 100644
--- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -468,7 +468,7 @@ SCUDO_TYPED_TEST(ScudoCombinedDeathTest, ReallocateSame) {
// Verify that new bytes are set according to FillContentsMode.
for (scudo::uptr I = CurrentSize; I < NewSize; I++) {
unsigned char V = (reinterpret_cast<unsigned char *>(NewP))[I];
- ASSERT_TRUE(V == scudo::PatternFillByte || V == 0);
+ EXPECT_TRUE(V == scudo::PatternFillByte || V == 0);
}
checkMemoryTaggingMaybe(Allocator, NewP, NewSize, 0);
More information about the llvm-commits
mailing list