From f03a232b19b414dc29100b2c0386f96bb8d2bc1e Mon Sep 17 00:00:00 2001 From: Mitch Phillips <31459023+hctim@users.noreply.github.com> Date: Fri, 14 May 2021 13:45:29 -0700 Subject: [Scudo] Delete unused flag 'rss_limit_mb'. EOM. Reviewed By: cryptoad Differential Revision: https://reviews.llvm.org/D102529 GitOrigin-RevId: 6c913b2f37388ab699b62ae8f0c0622279b2fe9a Change-Id: I32e0e4ba49d89b740c1994bec6565efdfc78a110 --- standalone/flags.inc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/standalone/flags.inc b/standalone/flags.inc index b5cab473416..690d889b8ce 100644 --- a/standalone/flags.inc +++ b/standalone/flags.inc @@ -37,12 +37,6 @@ SCUDO_FLAG(bool, zero_contents, false, "Zero chunk contents on allocation.") SCUDO_FLAG(bool, pattern_fill_contents, false, "Pattern fill chunk contents on allocation.") -SCUDO_FLAG(int, rss_limit_mb, -1, - "Enforce an upper limit (in megabytes) to the process RSS. The " - "allocator will terminate or return NULL when allocations are " - "attempted past that limit (depending on may_return_null). Negative " - "values disable the feature.") - SCUDO_FLAG(bool, may_return_null, true, "Indicate whether the allocator should terminate instead of " "returning NULL in otherwise non-fatal error scenarios, eg: OOM, " -- cgit v1.2.3 From cf336e9c29265c0826222b9180a2ddd5f37811f4 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Tue, 18 May 2021 00:11:20 -0700 Subject: [NFC][scudo] Reduce test region size on MIPS32 GitOrigin-RevId: 2e92f1a9bcd5550761be936ac9c0848e6d9fcb40 Change-Id: If6e955c9bc9e2a3b7a247dc5e51aee7484816e72 --- standalone/tests/primary_test.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/standalone/tests/primary_test.cpp b/standalone/tests/primary_test.cpp index e7aa6f795b6..3574c28c6c0 100644 --- a/standalone/tests/primary_test.cpp +++ b/standalone/tests/primary_test.cpp @@ -32,7 +32,12 @@ struct TestConfig1 { }; struct TestConfig2 { +#if defined(__mips__) + // Unable to allocate greater size on QEMU-user. + static const scudo::uptr PrimaryRegionSizeLog = 23U; +#else static const scudo::uptr PrimaryRegionSizeLog = 24U; +#endif static const scudo::s32 PrimaryMinReleaseToOsIntervalMs = INT32_MIN; static const scudo::s32 PrimaryMaxReleaseToOsIntervalMs = INT32_MAX; static const bool MaySupportMemoryTagging = false; @@ -41,7 +46,12 @@ struct TestConfig2 { }; struct TestConfig3 { +#if defined(__mips__) + // Unable to allocate greater size on QEMU-user. + static const scudo::uptr PrimaryRegionSizeLog = 23U; +#else static const scudo::uptr PrimaryRegionSizeLog = 24U; +#endif static const scudo::s32 PrimaryMinReleaseToOsIntervalMs = INT32_MIN; static const scudo::s32 PrimaryMaxReleaseToOsIntervalMs = INT32_MAX; static const bool MaySupportMemoryTagging = true; -- cgit v1.2.3 From 81a10ce97048bc67abca6fb1ead6bb512bcb65b8 Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Tue, 18 May 2021 12:57:19 -0700 Subject: scudo: Test realloc on increasing size buffers. While developing a change to the allocator I ended up breaking realloc on secondary allocations with increasing sizes. That didn't cause any of the unit tests to fail, which indicated that we're missing some test coverage here. Add a unit test for that case. Differential Revision: https://reviews.llvm.org/D102716 GitOrigin-RevId: 8e93d10633d751a3e9169bf9fa68326925ffa097 Change-Id: I2a5fe9715118d564d38ef9f509351746fc7e1270 --- standalone/tests/combined_test.cpp | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index 5db249d0a85..50602efcf64 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -253,7 +253,28 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, BlockReuse) { EXPECT_TRUE(Found); } -SCUDO_TYPED_TEST(ScudoCombinedTest, ReallocateLarge) { +SCUDO_TYPED_TEST(ScudoCombinedTest, ReallocateLargeIncreasing) { + auto *Allocator = this->Allocator.get(); + + // Reallocate a chunk all the way up to a secondary allocation, verifying that + // we preserve the data in the process. + scudo::uptr Size = 16; + void *P = Allocator->allocate(Size, Origin); + const char Marker = 0xab; + memset(P, Marker, Size); + while (Size < TypeParam::Primary::SizeClassMap::MaxSize * 4) { + void *NewP = Allocator->reallocate(P, Size * 2); + EXPECT_NE(NewP, nullptr); + for (scudo::uptr J = 0; J < Size; J++) + EXPECT_EQ((reinterpret_cast(NewP))[J], Marker); + memset(reinterpret_cast(NewP) + Size, Marker, Size); + Size *= 2U; + P = NewP; + } + Allocator->deallocate(P, Origin); +} + +SCUDO_TYPED_TEST(ScudoCombinedTest, ReallocateLargeDecreasing) { auto *Allocator = this->Allocator.get(); // Reallocate a large chunk all the way down to a byte, verifying that we -- cgit v1.2.3 From a99b779f2dbe2a6a3c3f2271dd0e8a3c71f55dfe Mon Sep 17 00:00:00 2001 From: Mitch Phillips <31459023+hctim@users.noreply.github.com> Date: Thu, 20 May 2021 10:56:47 -0700 Subject: [scudo] Disable secondary cache-unmap tests on arm32. Looks like secondary pointers don't get unmapped on one of the arm32 bots. In the interests of landing some dependent patches, disable this test on arm32 so that it can be tested in isolation later. Reviewed By: cryptoad, vitalybuka Split from differential patchset (1/2): https://reviews.llvm.org/D102648 GitOrigin-RevId: 577a80bff8bdf9b26c0f4ff6d1807e43da66ec6a Change-Id: I7a4adc7dc00f0c837f105ef44f83e97203b95543 --- standalone/tests/secondary_test.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/standalone/tests/secondary_test.cpp b/standalone/tests/secondary_test.cpp index a55704297de..9dcf52d909a 100644 --- a/standalone/tests/secondary_test.cpp +++ b/standalone/tests/secondary_test.cpp @@ -32,9 +32,16 @@ template static void testSecondaryBasic(void) { memset(P, 'A', Size); EXPECT_GE(SecondaryT::getBlockSize(P), Size); L->deallocate(scudo::Options{}, P); + +// TODO(hctim): Looks like the secondary pointer doesn't get unmapped on arm32. +// It's not clear whether this is a kernel issue, or something in EXPECT_DEATH() +// is mmap-ing something that uses the same vaddr space. For now, just disable +// the test on arm32 until we can debug it further. +#ifndef __arm__ // If the Secondary can't cache that pointer, it will be unmapped. if (!L->canCache(Size)) EXPECT_DEATH(memset(P, 'A', Size), ""); +#endif // __arm__ const scudo::uptr Align = 1U << 16; P = L->allocate(scudo::Options{}, Size + Align, Align); -- cgit v1.2.3 From 5cb963698c892aadf81c2bf0725b2e01f6889132 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 20 May 2021 16:17:25 -0700 Subject: [scudo] Fix EXPECT_DEATH tests Put allocate/deallocate next to memory access inside EXPECT_DEATH block. This way we reduce probability that memory is not mapped by unrelated code. It's still not absolutely guaranty that mmap does not happen so we repeat it few times to be sure. Reviewed By: cryptoad Differential Revision: https://reviews.llvm.org/D102886 GitOrigin-RevId: 96b760607f8e82e563dc8cac67b2d2dfb76d5d33 Change-Id: Iac44fbf332b1174dae081eec5730720a996c86fd --- standalone/tests/map_test.cpp | 18 +++++++++++++----- standalone/tests/secondary_test.cpp | 15 +++++++++++++-- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/standalone/tests/map_test.cpp b/standalone/tests/map_test.cpp index 7c40b73ff25..149a704e4dd 100644 --- a/standalone/tests/map_test.cpp +++ b/standalone/tests/map_test.cpp @@ -31,11 +31,19 @@ TEST(ScudoMapTest, MapNoAccessUnmap) { TEST(ScudoMapTest, MapUnmap) { const scudo::uptr Size = 4 * scudo::getPageSizeCached(); - void *P = scudo::map(nullptr, Size, MappingName, 0, nullptr); - EXPECT_NE(P, nullptr); - memset(P, 0xaa, Size); - scudo::unmap(P, Size, 0, nullptr); - EXPECT_DEATH(memset(P, 0xbb, Size), ""); + EXPECT_DEATH( + { + // Repeat few time to avoid missing crash if it's mmaped by unrelated + // code. + for (int i = 0; i < 10; ++i) { + void *P = scudo::map(nullptr, Size, MappingName, 0, nullptr); + if (!P) + continue; + scudo::unmap(P, Size, 0, nullptr); + memset(P, 0xbb, Size); + } + }, + ""); } TEST(ScudoMapTest, MapWithGuardUnmap) { diff --git a/standalone/tests/secondary_test.cpp b/standalone/tests/secondary_test.cpp index 9dcf52d909a..d5d15995be1 100644 --- a/standalone/tests/secondary_test.cpp +++ b/standalone/tests/secondary_test.cpp @@ -39,9 +39,20 @@ template static void testSecondaryBasic(void) { // the test on arm32 until we can debug it further. #ifndef __arm__ // If the Secondary can't cache that pointer, it will be unmapped. - if (!L->canCache(Size)) - EXPECT_DEATH(memset(P, 'A', Size), ""); + if (!L->canCache(Size)) { + EXPECT_DEATH( + { + // Repeat few time to avoid missing crash if it's mmaped by unrelated + // code. + for (int i = 0; i < 10; ++i) { + P = L->allocate(scudo::Options{}, Size); + L->deallocate(scudo::Options{}, P); + memset(P, 'A', Size); + } + }, + ""); #endif // __arm__ + } const scudo::uptr Align = 1U << 16; P = L->allocate(scudo::Options{}, Size + Align, Align); -- cgit v1.2.3 From 4b58de1b69eeb61f5d0a738255965b7109f58374 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 20 May 2021 17:16:27 -0700 Subject: [NFC][scudo] Let disableMemoryTagChecksTestOnly to fail If this happend we can't run corresponding test. GitOrigin-RevId: 2c212db4ea42fbbc0e83647da4f62261f775388b Change-Id: I5af0df762e74ae631dd9d33a6e8e9a7ef5b4bf64 --- standalone/memtag.h | 5 +++-- standalone/tests/combined_test.cpp | 25 ++++++++++++------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/standalone/memtag.h b/standalone/memtag.h index 4bdce16faea..cf1ec3bcc12 100644 --- a/standalone/memtag.h +++ b/standalone/memtag.h @@ -92,12 +92,13 @@ inline bool systemDetectsMemoryTagFaultsTestOnly() { return false; } #endif // SCUDO_LINUX -inline void disableMemoryTagChecksTestOnly() { +inline bool disableMemoryTagChecksTestOnly() { __asm__ __volatile__( R"( .arch_extension memtag msr tco, #1 )"); + return true; } inline void enableMemoryTagChecksTestOnly() { @@ -244,7 +245,7 @@ inline bool systemDetectsMemoryTagFaultsTestOnly() { UNREACHABLE("memory tagging not supported"); } -inline void disableMemoryTagChecksTestOnly() { +inline bool disableMemoryTagChecksTestOnly() { UNREACHABLE("memory tagging not supported"); } diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index 50602efcf64..619a94c24eb 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -380,21 +380,20 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, DisableMemoryTagging) { // Check that disabling memory tagging works correctly. void *P = Allocator->allocate(2048, Origin); EXPECT_DEATH(reinterpret_cast(P)[2048] = 0xaa, ""); - scudo::disableMemoryTagChecksTestOnly(); - Allocator->disableMemoryTagging(); - reinterpret_cast(P)[2048] = 0xaa; - Allocator->deallocate(P, Origin); - - P = Allocator->allocate(2048, Origin); - EXPECT_EQ(scudo::untagPointer(P), P); - reinterpret_cast(P)[2048] = 0xaa; - Allocator->deallocate(P, Origin); + if (scudo::disableMemoryTagChecksTestOnly()) { + Allocator->disableMemoryTagging(); + reinterpret_cast(P)[2048] = 0xaa; + Allocator->deallocate(P, Origin); - Allocator->releaseToOS(); + P = Allocator->allocate(2048, Origin); + EXPECT_EQ(scudo::untagPointer(P), P); + reinterpret_cast(P)[2048] = 0xaa; + Allocator->deallocate(P, Origin); - // Disabling memory tag checks may interfere with subsequent tests. - // Re-enable them now. - scudo::enableMemoryTagChecksTestOnly(); + // Disabling memory tag checks may interfere with subsequent tests. + // Re-enable them now. + scudo::enableMemoryTagChecksTestOnly(); + } } } -- cgit v1.2.3 From f00eda635eb6cfa4b876c44d997d3ea7a70b646f Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 20 May 2021 19:36:49 -0700 Subject: [NFC][scudo] Fix typo in comment GitOrigin-RevId: 51fe7ddce2beb8444a0dccc638b2a9d8dd63c791 Change-Id: Ib860a12c664b14b92c67083c03db4477652614cd --- standalone/tests/combined_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index 619a94c24eb..7711637175b 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -193,7 +193,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, ZeroContents) { SCUDO_TYPED_TEST(ScudoCombinedTest, ZeroFill) { auto *Allocator = this->Allocator.get(); - // Ensure that specifying ZeroContents returns a zero'd out block. + // Ensure that specifying ZeroFill returns a zero'd out block. Allocator->setFillContents(scudo::ZeroFill); for (scudo::uptr SizeLog = 0U; SizeLog <= 20U; SizeLog++) { for (scudo::uptr Delta = 0U; Delta <= 4U; Delta++) { -- cgit v1.2.3 From a6286164050eb1c89ba71824ef3557c6901d0e9a Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 20 May 2021 22:09:43 -0700 Subject: [scudo] Fix compilation after D102886 GitOrigin-RevId: 384a460e59bc67d4225b0c7e528dcb0c84668d49 Change-Id: I317efee23ef1f14bba11c4425ce690e9b6e92098 --- standalone/tests/secondary_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/standalone/tests/secondary_test.cpp b/standalone/tests/secondary_test.cpp index d5d15995be1..608dfd8bc73 100644 --- a/standalone/tests/secondary_test.cpp +++ b/standalone/tests/secondary_test.cpp @@ -51,8 +51,8 @@ template static void testSecondaryBasic(void) { } }, ""); -#endif // __arm__ } +#endif // __arm__ const scudo::uptr Align = 1U << 16; P = L->allocate(scudo::Options{}, Size + Align, Align); -- cgit v1.2.3 From 4990d3d4b8416dc2f293c25905606d52d33cf7e9 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 20 May 2021 17:06:53 -0700 Subject: [scudo][NFC] Split BasicTest further It's still the slowest test under our emulator. GitOrigin-RevId: 53ec41a49c2b32bab74bb1ddf435bea4bf4b8a3a Change-Id: I3993b036f4bbc076adff50e9e22602d39071d9cc --- standalone/tests/combined_test.cpp | 65 ++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 23 deletions(-) diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index 7711637175b..31c0e366d93 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -97,7 +97,7 @@ template struct ScudoCombinedTest : public Test { void RunTest(); - void BasicTest(scudo::uptr SizeLogMin, scudo::uptr SizeLogMax); + void BasicTest(scudo::uptr SizeLog); using AllocatorT = TestAllocator; std::unique_ptr Allocator; @@ -141,37 +141,56 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, IsOwned) { } template -void ScudoCombinedTest::BasicTest(scudo::uptr SizeLogMin, - scudo::uptr SizeLogMax) { +void ScudoCombinedTest::BasicTest(scudo::uptr SizeLog) { auto *Allocator = this->Allocator.get(); // This allocates and deallocates a bunch of chunks, with a wide range of // sizes and alignments, with a focus on sizes that could trigger weird // behaviors (plus or minus a small delta of a power of two for example). - for (scudo::uptr SizeLog = SizeLogMin; SizeLog <= SizeLogMax; SizeLog++) { - for (scudo::uptr AlignLog = MinAlignLog; AlignLog <= 16U; AlignLog++) { - const scudo::uptr Align = 1U << AlignLog; - for (scudo::sptr Delta = -32; Delta <= 32; Delta++) { - if (static_cast(1U << SizeLog) + Delta <= 0) - continue; - const scudo::uptr Size = (1U << SizeLog) + Delta; - void *P = Allocator->allocate(Size, Origin, Align); - EXPECT_NE(P, nullptr); - EXPECT_TRUE(Allocator->isOwned(P)); - EXPECT_TRUE(scudo::isAligned(reinterpret_cast(P), Align)); - EXPECT_LE(Size, Allocator->getUsableSize(P)); - memset(P, 0xaa, Size); - checkMemoryTaggingMaybe(Allocator, P, Size, Align); - Allocator->deallocate(P, Origin, Size); - } + for (scudo::uptr AlignLog = MinAlignLog; AlignLog <= 16U; AlignLog++) { + const scudo::uptr Align = 1U << AlignLog; + for (scudo::sptr Delta = -32; Delta <= 32; Delta++) { + if (static_cast(1U << SizeLog) + Delta <= 0) + continue; + const scudo::uptr Size = (1U << SizeLog) + Delta; + void *P = Allocator->allocate(Size, Origin, Align); + EXPECT_NE(P, nullptr); + EXPECT_TRUE(Allocator->isOwned(P)); + EXPECT_TRUE(scudo::isAligned(reinterpret_cast(P), Align)); + EXPECT_LE(Size, Allocator->getUsableSize(P)); + memset(P, 0xaa, Size); + checkMemoryTaggingMaybe(Allocator, P, Size, Align); + Allocator->deallocate(P, Origin, Size); } } } -SCUDO_TYPED_TEST(ScudoCombinedTest, BasicCombined0) { this->BasicTest(0, 16); } -SCUDO_TYPED_TEST(ScudoCombinedTest, BasicCombined1) { this->BasicTest(17, 18); } -SCUDO_TYPED_TEST(ScudoCombinedTest, BasicCombined2) { this->BasicTest(19, 19); } -SCUDO_TYPED_TEST(ScudoCombinedTest, BasicCombined3) { this->BasicTest(20, 20); } +#define SCUDO_MAKE_BASIC_TEST(SizeLog) \ + SCUDO_TYPED_TEST(ScudoCombinedTest, BasicCombined##SizeLog) { \ + this->BasicTest(SizeLog); \ + } + +SCUDO_MAKE_BASIC_TEST(0) +SCUDO_MAKE_BASIC_TEST(1) +SCUDO_MAKE_BASIC_TEST(2) +SCUDO_MAKE_BASIC_TEST(3) +SCUDO_MAKE_BASIC_TEST(4) +SCUDO_MAKE_BASIC_TEST(5) +SCUDO_MAKE_BASIC_TEST(6) +SCUDO_MAKE_BASIC_TEST(7) +SCUDO_MAKE_BASIC_TEST(8) +SCUDO_MAKE_BASIC_TEST(9) +SCUDO_MAKE_BASIC_TEST(10) +SCUDO_MAKE_BASIC_TEST(11) +SCUDO_MAKE_BASIC_TEST(12) +SCUDO_MAKE_BASIC_TEST(13) +SCUDO_MAKE_BASIC_TEST(14) +SCUDO_MAKE_BASIC_TEST(15) +SCUDO_MAKE_BASIC_TEST(16) +SCUDO_MAKE_BASIC_TEST(17) +SCUDO_MAKE_BASIC_TEST(18) +SCUDO_MAKE_BASIC_TEST(19) +SCUDO_MAKE_BASIC_TEST(20) SCUDO_TYPED_TEST(ScudoCombinedTest, ZeroContents) { auto *Allocator = this->Allocator.get(); -- cgit v1.2.3 From eca04fb0397e8029d5d2dff990beeab6574270a8 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 20 May 2021 16:32:09 -0700 Subject: [scudo] Try to re-enabled the test on arm It's probably fixed by D102886. Builder to watch https://lab.llvm.org/buildbot/#/builders/clang-cmake-armv7-full Reviewed By: hctim, cryptoad Differential Revision: https://reviews.llvm.org/D102887 GitOrigin-RevId: 1c6272a481fda287f9b506a83ed21d74131742af Change-Id: I3ce4fe85d105c3713582ebb80b77d4f99082b190 --- standalone/tests/secondary_test.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/standalone/tests/secondary_test.cpp b/standalone/tests/secondary_test.cpp index 608dfd8bc73..c50101a6d53 100644 --- a/standalone/tests/secondary_test.cpp +++ b/standalone/tests/secondary_test.cpp @@ -33,11 +33,6 @@ template static void testSecondaryBasic(void) { EXPECT_GE(SecondaryT::getBlockSize(P), Size); L->deallocate(scudo::Options{}, P); -// TODO(hctim): Looks like the secondary pointer doesn't get unmapped on arm32. -// It's not clear whether this is a kernel issue, or something in EXPECT_DEATH() -// is mmap-ing something that uses the same vaddr space. For now, just disable -// the test on arm32 until we can debug it further. -#ifndef __arm__ // If the Secondary can't cache that pointer, it will be unmapped. if (!L->canCache(Size)) { EXPECT_DEATH( @@ -52,7 +47,6 @@ template static void testSecondaryBasic(void) { }, ""); } -#endif // __arm__ const scudo::uptr Align = 1U << 16; P = L->allocate(scudo::Options{}, Size + Align, Align); -- cgit v1.2.3 From d5ef724fd421dc966f7463e99d4cecf7157ed03e Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Sat, 22 May 2021 22:28:54 -0700 Subject: [NFC][scudo] Move SKIP_ON_FUCHSIA to common header GitOrigin-RevId: 6994bf7dadf575234a46e831194a00d01b22b2a8 Change-Id: I745db93c23dc4969ead15cc4bb036875e6c94ff3 --- standalone/tests/combined_test.cpp | 6 ------ standalone/tests/scudo_unit_test.h | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index 31c0e366d93..59841ac424b 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -491,12 +491,6 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, ThreadedCombined) { Allocator->releaseToOS(); } -#if SCUDO_FUCHSIA -#define SKIP_ON_FUCHSIA(T) DISABLED_##T -#else -#define SKIP_ON_FUCHSIA(T) T -#endif - // Test that multiple instantiations of the allocator have not messed up the // process's signal handlers (GWP-ASan used to do this). TEST(ScudoCombinedTest, SKIP_ON_FUCHSIA(testSEGV)) { diff --git a/standalone/tests/scudo_unit_test.h b/standalone/tests/scudo_unit_test.h index 555a935254c..7b8218a1b28 100644 --- a/standalone/tests/scudo_unit_test.h +++ b/standalone/tests/scudo_unit_test.h @@ -33,4 +33,10 @@ using Test = ::testing::Test; #define EXPECT_STREQ(X, Y) EXPECT_EQ(strcmp(X, Y), 0) #endif +#if SCUDO_FUCHSIA +#define SKIP_ON_FUCHSIA(T) DISABLED_##T +#else +#define SKIP_ON_FUCHSIA(T) T +#endif + extern bool UseQuarantine; -- cgit v1.2.3 From 459bccaa80ae3115cc5aafe8b9d9631873080905 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Sat, 22 May 2021 22:30:03 -0700 Subject: [NFC][scudo] Add releasePagesToOS test GitOrigin-RevId: 0bccdf82f7057da9bf59f94338e6809284d068ec Change-Id: Ic9621ed02d47f5a2539a07ef59e57372a3e58445 --- standalone/tests/common_test.cpp | 58 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 standalone/tests/common_test.cpp diff --git a/standalone/tests/common_test.cpp b/standalone/tests/common_test.cpp new file mode 100644 index 00000000000..7b35b2494ed --- /dev/null +++ b/standalone/tests/common_test.cpp @@ -0,0 +1,58 @@ +//===-- common_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 "internal_defs.h" +#include "tests/scudo_unit_test.h" + +#include "common.h" +#include +#include + +namespace scudo { + +static uptr getResidentMemorySize() { + uptr Size; + uptr Resident; + std::ifstream IFS("/proc/self/statm"); + IFS >> Size; + IFS >> Resident; + return Resident * getPageSizeCached(); +} + +// Fuchsia needs getResidentMemorySize implementation. +TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { + uptr OnStart = getResidentMemorySize(); + EXPECT_GT(OnStart, 0); + + const uptr Size = 1ull << 30; + const uptr Threshold = Size >> 3; + + MapPlatformData Data; + uptr *P = reinterpret_cast( + map(nullptr, Size, "ResidentMemorySize", 0, &Data)); + const uptr N = Size / sizeof(*P); + ASSERT_NE(nullptr, P); + EXPECT_EQ(std::count(P, P + N, 0), N); + EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); + + memset(P, 1, Size); + EXPECT_EQ(std::count(P, P + N, 0), 0); + EXPECT_LT(getResidentMemorySize() - Size, Threshold); + + releasePagesToOS((uptr)P, 0, Size, &Data); + EXPECT_EQ(std::count(P, P + N, 0), N); + // FIXME: does not work with QEMU-user. + // EXPECT_LT(getResidentMemorySize() - OnStart, Threshold) << OnStart << " " + // << getResidentMemorySize(); + + memset(P, 1, Size); + EXPECT_EQ(std::count(P, P + N, 0), 0); + EXPECT_LT(getResidentMemorySize() - Size, Threshold); +} + +} // namespace scudo \ No newline at end of file -- cgit v1.2.3 From ac28f8f601d06b0c3f6ab8152459266037005259 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Sat, 22 May 2021 22:55:53 -0700 Subject: [NFC][scudo] Replace size_t with uptr GitOrigin-RevId: 887dda5dcdcf4a273f9c2885d147066343efd5b0 Change-Id: Ifb982e2535d6a074f7584b8522fc81641dc7e717 --- standalone/memtag.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/standalone/memtag.h b/standalone/memtag.h index cf1ec3bcc12..ec4b69834af 100644 --- a/standalone/memtag.h +++ b/standalone/memtag.h @@ -110,7 +110,7 @@ inline void enableMemoryTagChecksTestOnly() { } class ScopedDisableMemoryTagChecks { - size_t PrevTCO; + uptr PrevTCO; public: ScopedDisableMemoryTagChecks() { -- cgit v1.2.3 From 5f94ce34bb23ebdd9123c398ce2c9c1a990b4ce6 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Sun, 23 May 2021 14:12:49 -0700 Subject: [NFC][scudo] Enforce header size alignment As-is it should not change struct size, but it will help to keep correct size if more fields added. GitOrigin-RevId: 93d1e5822ed64abd777eb94ea9899e96c4c39fbe Change-Id: Ifbda3e522b784bbafd8c83828f7fb75af74d4c10 --- standalone/secondary.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/standalone/secondary.h b/standalone/secondary.h index ea5d6808aec..77db873375a 100644 --- a/standalone/secondary.h +++ b/standalone/secondary.h @@ -28,7 +28,10 @@ namespace scudo { namespace LargeBlock { -struct Header { +struct alignas(Max(archSupportsMemoryTagging() + ? archMemoryTagGranuleSize() + : 1, + 1U << SCUDO_MIN_ALIGNMENT_LOG)) Header { LargeBlock::Header *Prev; LargeBlock::Header *Next; uptr CommitBase; @@ -38,9 +41,12 @@ struct Header { [[no_unique_address]] MapPlatformData Data; }; -constexpr uptr getHeaderSize() { - return roundUpTo(sizeof(Header), 1U << SCUDO_MIN_ALIGNMENT_LOG); -} +static_assert(sizeof(Header) % (1U << SCUDO_MIN_ALIGNMENT_LOG) == 0, ""); +static_assert(!archSupportsMemoryTagging() || + sizeof(Header) % archMemoryTagGranuleSize() == 0, + ""); + +constexpr uptr getHeaderSize() { return sizeof(Header); } template static uptr addHeaderTag(uptr Ptr) { if (allocatorSupportsMemoryTagging()) @@ -49,8 +55,7 @@ template static uptr addHeaderTag(uptr Ptr) { } template static Header *getHeader(uptr Ptr) { - return reinterpret_cast
(addHeaderTag(Ptr) - - getHeaderSize()); + return reinterpret_cast
(addHeaderTag(Ptr)) - 1; } template static Header *getHeader(const void *Ptr) { -- cgit v1.2.3 From 27c58d47a0a18255417c2f3e741274a7a2bff490 Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Wed, 19 May 2021 09:10:30 -0700 Subject: [scudo] Separate Fuchsia & Default SizeClassMap The Fuchsia allocator config was using the default size class map. This CL gives Fuchsia its own size class map and changes a couple of things in the default one: - make `SizeDelta` configurable in `Config` for a fixed size class map as it currently is for a table size class map; - switch `SizeDelta` to 0 for the default config, it allows for size classes that allow for power of 2s, and overall better wrt pages filling; - increase the max number of caches pointers to 14 in the default, this makes the transfer batch 64/128 bytes on 32/64-bit platforms, which is cache-line friendly (previous size was 48/96 bytes). The Fuchsia size class map remains untouched for now, this doesn't impact Android which uses the table size class map. Differential Revision: https://reviews.llvm.org/D102783 GitOrigin-RevId: 20c1f94220d938936867cb047fb6444a187626ba Change-Id: Ifcf9b0b6b7545c990015a92bf2e946b93b48d19c --- standalone/allocator_config.h | 2 +- standalone/size_class_map.h | 33 +++++++++++++++++++++----------- standalone/tests/combined_test.cpp | 1 + standalone/tests/size_class_map_test.cpp | 2 ++ 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/standalone/allocator_config.h b/standalone/allocator_config.h index 8e103f28b1a..36c160637b5 100644 --- a/standalone/allocator_config.h +++ b/standalone/allocator_config.h @@ -140,7 +140,7 @@ struct AndroidSvelteConfig { #if SCUDO_CAN_USE_PRIMARY64 struct FuchsiaConfig { - using SizeClassMap = DefaultSizeClassMap; + using SizeClassMap = FuchsiaSizeClassMap; static const bool MaySupportMemoryTagging = false; typedef SizeClassAllocator64 Primary; diff --git a/standalone/size_class_map.h b/standalone/size_class_map.h index 1948802df0b..2736e995dba 100644 --- a/standalone/size_class_map.h +++ b/standalone/size_class_map.h @@ -64,12 +64,10 @@ class FixedSizeClassMap : public SizeClassMapBase { static const u8 S = Config::NumBits - 1; static const uptr M = (1UL << S) - 1; - static const uptr SizeDelta = Chunk::getHeaderSize(); - public: static const u32 MaxNumCachedHint = Config::MaxNumCachedHint; - static const uptr MaxSize = (1UL << Config::MaxSizeLog) + SizeDelta; + static const uptr MaxSize = (1UL << Config::MaxSizeLog) + Config::SizeDelta; static const uptr NumClasses = MidClass + ((Config::MaxSizeLog - Config::MidSizeLog) << S) + 1; static_assert(NumClasses <= 256, ""); @@ -79,24 +77,22 @@ public: static uptr getSizeByClassId(uptr ClassId) { DCHECK_NE(ClassId, BatchClassId); if (ClassId <= MidClass) - return (ClassId << Config::MinSizeLog) + SizeDelta; + return (ClassId << Config::MinSizeLog) + Config::SizeDelta; ClassId -= MidClass; const uptr T = MidSize << (ClassId >> S); - return T + (T >> S) * (ClassId & M) + SizeDelta; + return T + (T >> S) * (ClassId & M) + Config::SizeDelta; } static u8 getSizeLSBByClassId(uptr ClassId) { return u8(getLeastSignificantSetBitIndex(getSizeByClassId(ClassId))); } - static constexpr bool usesCompressedLSBFormat() { - return false; - } + static constexpr bool usesCompressedLSBFormat() { return false; } static uptr getClassIdBySize(uptr Size) { - if (Size <= SizeDelta + (1 << Config::MinSizeLog)) + if (Size <= Config::SizeDelta + (1 << Config::MinSizeLog)) return 1; - Size -= SizeDelta; + Size -= Config::SizeDelta; DCHECK_LE(Size, MaxSize); if (Size <= MidSize) return (Size + MinSize - 1) >> Config::MinSizeLog; @@ -227,12 +223,25 @@ struct DefaultSizeClassConfig { static const uptr MinSizeLog = 5; static const uptr MidSizeLog = 8; static const uptr MaxSizeLog = 17; - static const u32 MaxNumCachedHint = 10; + static const u32 MaxNumCachedHint = 14; static const uptr MaxBytesCachedLog = 10; + static const uptr SizeDelta = 0; }; typedef FixedSizeClassMap DefaultSizeClassMap; +struct FuchsiaSizeClassConfig { + static const uptr NumBits = 3; + static const uptr MinSizeLog = 5; + static const uptr MidSizeLog = 8; + static const uptr MaxSizeLog = 17; + static const u32 MaxNumCachedHint = 10; + static const uptr MaxBytesCachedLog = 10; + static const uptr SizeDelta = Chunk::getHeaderSize(); +}; + +typedef FixedSizeClassMap FuchsiaSizeClassMap; + struct AndroidSizeClassConfig { #if SCUDO_WORDSIZE == 64U static const uptr NumBits = 7; @@ -285,6 +294,7 @@ struct SvelteSizeClassConfig { static const uptr MaxSizeLog = 14; static const u32 MaxNumCachedHint = 13; static const uptr MaxBytesCachedLog = 10; + static const uptr SizeDelta = Chunk::getHeaderSize(); #else static const uptr NumBits = 4; static const uptr MinSizeLog = 3; @@ -292,6 +302,7 @@ struct SvelteSizeClassConfig { static const uptr MaxSizeLog = 14; static const u32 MaxNumCachedHint = 14; static const uptr MaxBytesCachedLog = 10; + static const uptr SizeDelta = Chunk::getHeaderSize(); #endif }; diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index 59841ac424b..908c6354eab 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -509,6 +509,7 @@ struct DeathSizeClassConfig { static const scudo::uptr MaxSizeLog = 13; static const scudo::u32 MaxNumCachedHint = 4; static const scudo::uptr MaxBytesCachedLog = 12; + static const scudo::uptr SizeDelta = 0; }; static const scudo::uptr DeathRegionSizeLog = 20U; diff --git a/standalone/tests/size_class_map_test.cpp b/standalone/tests/size_class_map_test.cpp index 88859ded5b2..076f36f86be 100644 --- a/standalone/tests/size_class_map_test.cpp +++ b/standalone/tests/size_class_map_test.cpp @@ -35,6 +35,7 @@ struct OneClassSizeClassConfig { static const scudo::uptr MaxSizeLog = 5; static const scudo::u32 MaxNumCachedHint = 0; static const scudo::uptr MaxBytesCachedLog = 0; + static const scudo::uptr SizeDelta = 0; }; TEST(ScudoSizeClassMapTest, OneClassSizeClassMap) { @@ -49,6 +50,7 @@ struct LargeMaxSizeClassConfig { static const scudo::uptr MaxSizeLog = 63; static const scudo::u32 MaxNumCachedHint = 128; static const scudo::uptr MaxBytesCachedLog = 16; + static const scudo::uptr SizeDelta = 0; }; TEST(ScudoSizeClassMapTest, LargeMaxSizeClassMap) { -- cgit v1.2.3 From 2da068ae53e39a5f8417845116808b936d2043d6 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Sun, 23 May 2021 15:49:43 -0700 Subject: [NFC][scudo] Small test cleanup Fixing issues raised on D102979 review. Reviewed By: cryptoad Differential Revision: https://reviews.llvm.org/D102994 GitOrigin-RevId: 6435ca4e2b9b4dbdec0b1be65fcacbc0bbdb1d22 Change-Id: I7658834b014b2c969259ad7e089988523b6b0b69 --- standalone/tests/common_test.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/standalone/tests/common_test.cpp b/standalone/tests/common_test.cpp index 7b35b2494ed..b67acd38618 100644 --- a/standalone/tests/common_test.cpp +++ b/standalone/tests/common_test.cpp @@ -1,4 +1,4 @@ -//===-- common_test.cpp ---------------------------------------*- C++ -*-===// +//===-- common_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. @@ -16,6 +16,8 @@ namespace scudo { static uptr getResidentMemorySize() { + if (!SCUDO_LINUX) + UNREACHABLE("Not implemented!"); uptr Size; uptr Resident; std::ifstream IFS("/proc/self/statm"); @@ -32,7 +34,7 @@ TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { const uptr Size = 1ull << 30; const uptr Threshold = Size >> 3; - MapPlatformData Data; + MapPlatformData Data = {}; uptr *P = reinterpret_cast( map(nullptr, Size, "ResidentMemorySize", 0, &Data)); const uptr N = Size / sizeof(*P); @@ -47,12 +49,11 @@ TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { releasePagesToOS((uptr)P, 0, Size, &Data); EXPECT_EQ(std::count(P, P + N, 0), N); // FIXME: does not work with QEMU-user. - // EXPECT_LT(getResidentMemorySize() - OnStart, Threshold) << OnStart << " " - // << getResidentMemorySize(); + // EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); memset(P, 1, Size); EXPECT_EQ(std::count(P, P + N, 0), 0); EXPECT_LT(getResidentMemorySize() - Size, Threshold); } -} // namespace scudo \ No newline at end of file +} // namespace scudo -- cgit v1.2.3 From c08c2d413891832fb263d7feb1421517d6f0a62a Mon Sep 17 00:00:00 2001 From: Jinsong Ji Date: Mon, 24 May 2021 21:32:50 +0000 Subject: [compiler-rt][scudo] Fix sign-compare warnings Fix buildbot failure https://lab.llvm.org/buildbot/#/builders/57/builds/6542/steps/6/logs/stdio /llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:1629:28: error: comparison of integers of different signs: 'const unsigned long' and 'const int' [-Werror,-Wsign-compare] GTEST_IMPL_CMP_HELPER_(GT, >); ~~~~~~~~~~~~~~~~~~~~~~~~~~^~ /llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:1609:12: note: expanded from macro 'GTEST_IMPL_CMP_HELPER_' if (val1 op val2) {\ ~~~~ ^ ~~~~ /llvm-project/compiler-rt/lib/scudo/standalone/tests/common_test.cpp:30:3: note: in instantiation of function template specialization 'testing::internal::CmpHelperGT' requested here EXPECT_GT(OnStart, 0); ^ Reviewed By: vitalybuka Differential Revision: https://reviews.llvm.org/D103029 GitOrigin-RevId: bec6b0225211ac2686e329744882cb87f01d73a5 Change-Id: I93c1054acf86fbf836944dede0eece0f23395403 --- standalone/tests/common_test.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/standalone/tests/common_test.cpp b/standalone/tests/common_test.cpp index b67acd38618..f8c64b29a18 100644 --- a/standalone/tests/common_test.cpp +++ b/standalone/tests/common_test.cpp @@ -29,7 +29,7 @@ static uptr getResidentMemorySize() { // Fuchsia needs getResidentMemorySize implementation. TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { uptr OnStart = getResidentMemorySize(); - EXPECT_GT(OnStart, 0); + EXPECT_GT(OnStart, 0UL); const uptr Size = 1ull << 30; const uptr Threshold = Size >> 3; @@ -37,9 +37,9 @@ TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { MapPlatformData Data = {}; uptr *P = reinterpret_cast( map(nullptr, Size, "ResidentMemorySize", 0, &Data)); - const uptr N = Size / sizeof(*P); + const size_t N = Size / sizeof(*P); ASSERT_NE(nullptr, P); - EXPECT_EQ(std::count(P, P + N, 0), N); + EXPECT_EQ((size_t)std::count(P, P + N, 0), N); EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); memset(P, 1, Size); @@ -47,7 +47,7 @@ TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { EXPECT_LT(getResidentMemorySize() - Size, Threshold); releasePagesToOS((uptr)P, 0, Size, &Data); - EXPECT_EQ(std::count(P, P + N, 0), N); + EXPECT_EQ((size_t)std::count(P, P + N, 0), N); // FIXME: does not work with QEMU-user. // EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); -- cgit v1.2.3 From da86a54d0f92c9938ee7897058cdb1f480793603 Mon Sep 17 00:00:00 2001 From: Mitch Phillips <31459023+hctim@users.noreply.github.com> Date: Mon, 24 May 2021 16:08:57 -0700 Subject: [scudo] Add unmapTestOnly() to secondary. When trying to track down a vaddr-poisoning bug, I found that that the secondary cache isn't emptied on test teardown. We should probably do that to make the tests hermetic. Otherwise, repeating the tests lots of times using --gtest_repeat fails after the mmap vaddr space is exhausted. To repro: $ ninja check-scudo_standalone # build $ ./projects/compiler-rt/lib/scudo/standalone/tests/ScudoUnitTest-x86_64-Test \ --gtest_filter=ScudoSecondaryTest.*:-ScudoSecondaryTest.SecondaryCombinations \ --gtest_repeat=10000 Reviewed By: cryptoad Differential Revision: https://reviews.llvm.org/D102874 GitOrigin-RevId: 1fb6a0307240b0c543ec5babb35e39db2c39052b Change-Id: I7a08441fb14e914f8361ff784a13bf5c3436c757 --- standalone/combined.h | 1 + standalone/secondary.h | 5 +++++ standalone/tests/secondary_test.cpp | 5 +++++ 3 files changed, 11 insertions(+) diff --git a/standalone/combined.h b/standalone/combined.h index 8080d677d7b..f17c371af82 100644 --- a/standalone/combined.h +++ b/standalone/combined.h @@ -215,6 +215,7 @@ public: void unmapTestOnly() { TSDRegistry.unmapTestOnly(); Primary.unmapTestOnly(); + Secondary.unmapTestOnly(); #ifdef GWP_ASAN_HOOKS if (getFlags()->GWP_ASAN_InstallSignalHandlers) gwp_asan::segv_handler::uninstallSignalHandlers(); diff --git a/standalone/secondary.h b/standalone/secondary.h index 77db873375a..3894b01c0a4 100644 --- a/standalone/secondary.h +++ b/standalone/secondary.h @@ -83,6 +83,7 @@ public: void enable() {} void releaseToOS() {} void disableMemoryTagging() {} + void unmapTestOnly() {} bool setOption(Option O, UNUSED sptr Value) { if (O == Option::ReleaseInterval || O == Option::MaxCacheEntriesCount || O == Option::MaxCacheEntrySize) @@ -326,6 +327,8 @@ public: void enable() { Mutex.unlock(); } + void unmapTestOnly() { empty(); } + private: void empty() { struct { @@ -456,6 +459,8 @@ public: void disableMemoryTagging() { Cache.disableMemoryTagging(); } + void unmapTestOnly() { Cache.unmapTestOnly(); } + private: typename Config::SecondaryCache Cache; diff --git a/standalone/tests/secondary_test.cpp b/standalone/tests/secondary_test.cpp index c50101a6d53..2320a4d546a 100644 --- a/standalone/tests/secondary_test.cpp +++ b/standalone/tests/secondary_test.cpp @@ -67,6 +67,7 @@ template static void testSecondaryBasic(void) { scudo::ScopedString Str(1024); L->getStats(&Str); Str.output(); + L->unmapTestOnly(); } struct NoCacheConfig { @@ -124,6 +125,7 @@ TEST(ScudoSecondaryTest, SecondaryCombinations) { scudo::ScopedString Str(1024); L->getStats(&Str); Str.output(); + L->unmapTestOnly(); } TEST(ScudoSecondaryTest, SecondaryIterate) { @@ -147,6 +149,7 @@ TEST(ScudoSecondaryTest, SecondaryIterate) { scudo::ScopedString Str(1024); L->getStats(&Str); Str.output(); + L->unmapTestOnly(); } TEST(ScudoSecondaryTest, SecondaryOptions) { @@ -170,6 +173,7 @@ TEST(ScudoSecondaryTest, SecondaryOptions) { EXPECT_TRUE(L->setOption(scudo::Option::MaxCacheEntrySize, 1UL << 20)); EXPECT_TRUE(L->canCache(1UL << 16)); } + L->unmapTestOnly(); } static std::mutex Mutex; @@ -216,4 +220,5 @@ TEST(ScudoSecondaryTest, SecondaryThreadsRace) { scudo::ScopedString Str(1024); L->getStats(&Str); Str.output(); + L->unmapTestOnly(); } -- cgit v1.2.3 From 865dae8703277fce73d4de475f09ee70a128b3d2 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Mon, 24 May 2021 17:13:29 -0700 Subject: [NFC][scudo] Avoid cast in test GitOrigin-RevId: f5bde3d476c2c6aee4f126d84982e8d2f0f7e408 Change-Id: I4f364bdf01bd1b38ecb80c11395ad99fc2514bd5 --- standalone/tests/common_test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/standalone/tests/common_test.cpp b/standalone/tests/common_test.cpp index f8c64b29a18..832f01da390 100644 --- a/standalone/tests/common_test.cpp +++ b/standalone/tests/common_test.cpp @@ -37,9 +37,9 @@ TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { MapPlatformData Data = {}; uptr *P = reinterpret_cast( map(nullptr, Size, "ResidentMemorySize", 0, &Data)); - const size_t N = Size / sizeof(*P); + const ptrdiff_t N = Size / sizeof(*P); ASSERT_NE(nullptr, P); - EXPECT_EQ((size_t)std::count(P, P + N, 0), N); + EXPECT_EQ(std::count(P, P + N, 0), N); EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); memset(P, 1, Size); @@ -47,7 +47,7 @@ TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { EXPECT_LT(getResidentMemorySize() - Size, Threshold); releasePagesToOS((uptr)P, 0, Size, &Data); - EXPECT_EQ((size_t)std::count(P, P + N, 0), N); + EXPECT_EQ(std::count(P, P + N, 0), N); // FIXME: does not work with QEMU-user. // EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); -- cgit v1.2.3 From 98975bb1fe065763f6b33290178e828e60b940c8 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Mon, 24 May 2021 13:12:47 -0700 Subject: [NFC][scudo] Add paramenters DCHECKs Reviewed By: hctim Differential Revision: https://reviews.llvm.org/D103042 GitOrigin-RevId: a0169b2ed198154117e82bf24ae7238454c2e9a2 Change-Id: I82ef59c82943e4adcccb97dd6779600fae2b471b --- standalone/memtag.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/standalone/memtag.h b/standalone/memtag.h index ec4b69834af..5c3ea588da3 100644 --- a/standalone/memtag.h +++ b/standalone/memtag.h @@ -146,10 +146,13 @@ inline uptr selectRandomTag(uptr Ptr, uptr ExcludeMask) { return TaggedPtr; } -inline uptr addFixedTag(uptr Ptr, uptr Tag) { return Ptr | (Tag << 56); } +inline uptr addFixedTag(uptr Ptr, uptr Tag) { + DCHECK_LT(Tag, 16); + return Ptr | (Tag << 56); +} inline uptr storeTags(uptr Begin, uptr End) { - DCHECK(Begin % 16 == 0); + DCHECK_EQ(0, Begin % 16); uptr LineSize, Next, Tmp; __asm__ __volatile__( R"( @@ -209,10 +212,12 @@ inline uptr storeTags(uptr Begin, uptr End) { [Tmp] "=&r"(Tmp) : [End] "r"(End) : "memory"); + DCHECK_EQ(0, Begin % 16); return Begin; } inline void storeTag(uptr Ptr) { + DCHECK_EQ(0, Ptr % 16); __asm__ __volatile__(R"( .arch_extension memtag stg %0, [%0] @@ -223,6 +228,7 @@ inline void storeTag(uptr Ptr) { } inline uptr loadTag(uptr Ptr) { + DCHECK_EQ(0, Ptr % 16); uptr TaggedPtr = Ptr; __asm__ __volatile__( R"( -- cgit v1.2.3 From c79ab1b2c1bbd2c950f847beac577745fb660aee Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Mon, 24 May 2021 09:26:21 -0700 Subject: [scudo] Rework dieOnMapUnmapError Said function had a few shortfalls: - didn't set an abort message on Android - was logged on several lines - didn't provide extra information like the size requested if OOM'ing This improves the function to address those points. Bug: 188581099 Differential Revision: https://reviews.llvm.org/D103034 GitOrigin-RevId: 1872283457fc1617fa750a11abdfd44e881dfcdb Change-Id: Idc91a4187ddc9ad1aef8ab85ca021a6e2cc65566 --- standalone/common.cpp | 16 +++++++++++----- standalone/common.h | 5 +++-- standalone/fuchsia.cpp | 8 ++++---- standalone/linux.cpp | 12 ++++++------ standalone/string_utils.cpp | 12 ++++++++++-- standalone/string_utils.h | 1 + 6 files changed, 35 insertions(+), 19 deletions(-) diff --git a/standalone/common.cpp b/standalone/common.cpp index d93bfc59b3c..666f95400c7 100644 --- a/standalone/common.cpp +++ b/standalone/common.cpp @@ -8,6 +8,7 @@ #include "common.h" #include "atomic_helpers.h" +#include "string_utils.h" namespace scudo { @@ -21,11 +22,16 @@ uptr getPageSizeSlow() { } // Fatal internal map() or unmap() error (potentially OOM related). -void NORETURN dieOnMapUnmapError(bool OutOfMemory) { - outputRaw("Scudo ERROR: internal map or unmap failure"); - if (OutOfMemory) - outputRaw(" (OOM)"); - outputRaw("\n"); +void NORETURN dieOnMapUnmapError(uptr SizeIfOOM) { + char Error[128] = "Scudo ERROR: internal map or unmap failure\n"; + if (SizeIfOOM) { + formatString( + Error, sizeof(Error), + "Scudo ERROR: internal map failure (NO MEMORY) requesting %zuKB\n", + SizeIfOOM >> 10); + } + outputRaw(Error); + setAbortMessage(Error); die(); } diff --git a/standalone/common.h b/standalone/common.h index d9884dfceae..3f27a3d3e1b 100644 --- a/standalone/common.h +++ b/standalone/common.h @@ -171,8 +171,9 @@ void setMemoryPermission(uptr Addr, uptr Size, uptr Flags, void releasePagesToOS(uptr BaseAddress, uptr Offset, uptr Size, MapPlatformData *Data = nullptr); -// Internal map & unmap fatal error. This must not call map(). -void NORETURN dieOnMapUnmapError(bool OutOfMemory = false); +// Internal map & unmap fatal error. This must not call map(). SizeIfOOM shall +// hold the requested size on an out-of-memory error, 0 otherwise. +void NORETURN dieOnMapUnmapError(uptr SizeIfOOM = 0); // Logging related functions. diff --git a/standalone/fuchsia.cpp b/standalone/fuchsia.cpp index 11c1de77f17..3b473bc9e22 100644 --- a/standalone/fuchsia.cpp +++ b/standalone/fuchsia.cpp @@ -41,7 +41,7 @@ static void *allocateVmar(uptr Size, MapPlatformData *Data, bool AllowNoMem) { Size, &Data->Vmar, &Data->VmarBase); if (UNLIKELY(Status != ZX_OK)) { if (Status != ZX_ERR_NO_MEMORY || !AllowNoMem) - dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY); + dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY ? Size : 0); return nullptr; } return reinterpret_cast(Data->VmarBase); @@ -71,7 +71,7 @@ void *map(void *Addr, uptr Size, const char *Name, uptr Flags, Status = _zx_vmo_set_size(Vmo, VmoSize + Size); if (Status != ZX_OK) { if (Status != ZX_ERR_NO_MEMORY || !AllowNoMem) - dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY); + dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY ? Size : 0); return nullptr; } } else { @@ -79,7 +79,7 @@ void *map(void *Addr, uptr Size, const char *Name, uptr Flags, Status = _zx_vmo_create(Size, ZX_VMO_RESIZABLE, &Vmo); if (UNLIKELY(Status != ZX_OK)) { if (Status != ZX_ERR_NO_MEMORY || !AllowNoMem) - dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY); + dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY ? Size : 0); return nullptr; } _zx_object_set_property(Vmo, ZX_PROP_NAME, Name, strlen(Name)); @@ -105,7 +105,7 @@ void *map(void *Addr, uptr Size, const char *Name, uptr Flags, } if (UNLIKELY(Status != ZX_OK)) { if (Status != ZX_ERR_NO_MEMORY || !AllowNoMem) - dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY); + dieOnMapUnmapError(Status == ZX_ERR_NO_MEMORY ? Size : 0); return nullptr; } if (Data) diff --git a/standalone/linux.cpp b/standalone/linux.cpp index 7ec69878c0e..301bdcd34da 100644 --- a/standalone/linux.cpp +++ b/standalone/linux.cpp @@ -67,7 +67,7 @@ void *map(void *Addr, uptr Size, UNUSED const char *Name, uptr Flags, void *P = mmap(Addr, Size, MmapProt, MmapFlags, -1, 0); if (P == MAP_FAILED) { if (!(Flags & MAP_ALLOWNOMEM) || errno != ENOMEM) - dieOnMapUnmapError(errno == ENOMEM); + dieOnMapUnmapError(errno == ENOMEM ? Size : 0); return nullptr; } #if SCUDO_ANDROID @@ -91,15 +91,15 @@ void setMemoryPermission(uptr Addr, uptr Size, uptr Flags, } static bool madviseNeedsMemset() { - uptr Size = getPageSizeCached(); - char *P = (char *)mmap(0, Size, PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + const uptr Size = getPageSizeCached(); + char *P = reinterpret_cast(mmap(0, Size, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)); if (!P) - dieOnMapUnmapError(errno == ENOMEM); + dieOnMapUnmapError(errno == ENOMEM ? Size : 0); *P = 1; while (madvise(P, Size, MADV_DONTNEED) == -1 && errno == EAGAIN) { } - bool R = (*P != 0); + const bool R = (*P != 0); if (munmap(P, Size) != 0) dieOnMapUnmapError(); return R; diff --git a/standalone/string_utils.cpp b/standalone/string_utils.cpp index f304491019b..25bddbce34d 100644 --- a/standalone/string_utils.cpp +++ b/standalone/string_utils.cpp @@ -115,8 +115,8 @@ static int appendPointer(char **Buffer, const char *BufferEnd, u64 ptr_value) { return Res; } -int formatString(char *Buffer, uptr BufferLength, const char *Format, - va_list Args) { +static int formatString(char *Buffer, uptr BufferLength, const char *Format, + va_list Args) { static const char *PrintfFormatsHelp = "Supported formatString formats: %([0-9]*)?(z|ll)?{d,u,x,X}; %p; " "%[-]([0-9]*)?(\\.\\*)?s; %c\n"; @@ -210,6 +210,14 @@ int formatString(char *Buffer, uptr BufferLength, const char *Format, return Res; } +int formatString(char *Buffer, uptr BufferLength, const char *Format, ...) { + va_list Args; + va_start(Args, Format); + int Res = formatString(Buffer, BufferLength, Format, Args); + va_end(Args); + return Res; +} + void ScopedString::append(const char *Format, va_list Args) { DCHECK_LT(Length, String.size()); va_list ArgsCopy; diff --git a/standalone/string_utils.h b/standalone/string_utils.h index acd60bda9d8..4880fa1e7cf 100644 --- a/standalone/string_utils.h +++ b/standalone/string_utils.h @@ -36,6 +36,7 @@ private: uptr Length; }; +int formatString(char *Buffer, uptr BufferLength, const char *Format, ...); void Printf(const char *Format, ...); } // namespace scudo -- cgit v1.2.3 From 75a7a22f45fd8a79eb3943d8d0c413e68dea6fdd Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Mon, 24 May 2021 18:12:08 -0700 Subject: [scudo] Fix CHECK implementation Cast of signed types to u64 breaks comparison. Also remove double () around operands. Reviewed By: cryptoad, hctim Differential Revision: https://reviews.llvm.org/D103060 GitOrigin-RevId: 8e30b55c82cc245f8b59ef3b29d95c9797584b63 Change-Id: I3f2db55082589ff39b56bc39c9e731a79b988c2e --- standalone/internal_defs.h | 9 +++------ standalone/report.cpp | 4 ++-- standalone/tests/report_test.cpp | 7 +++++++ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/standalone/internal_defs.h b/standalone/internal_defs.h index bbf7631be18..c9ffad136b7 100644 --- a/standalone/internal_defs.h +++ b/standalone/internal_defs.h @@ -105,14 +105,11 @@ void NORETURN die(); void NORETURN reportCheckFailed(const char *File, int Line, const char *Condition, u64 Value1, u64 Value2); - #define CHECK_IMPL(C1, Op, C2) \ do { \ - scudo::u64 V1 = (scudo::u64)(C1); \ - scudo::u64 V2 = (scudo::u64)(C2); \ - if (UNLIKELY(!(V1 Op V2))) { \ - scudo::reportCheckFailed(__FILE__, __LINE__, \ - "(" #C1 ") " #Op " (" #C2 ")", V1, V2); \ + if (UNLIKELY(!(C1 Op C2))) { \ + scudo::reportCheckFailed(__FILE__, __LINE__, #C1 " " #Op " " #C2, \ + (scudo::u64)C1, (scudo::u64)C2); \ scudo::die(); \ } \ } while (false) diff --git a/standalone/report.cpp b/standalone/report.cpp index 80cc6eda2af..292c29918d1 100644 --- a/standalone/report.cpp +++ b/standalone/report.cpp @@ -45,8 +45,8 @@ void NORETURN reportCheckFailed(const char *File, int Line, trap(); } ScopedErrorReport Report; - Report.append("CHECK failed @ %s:%d %s (%llu, %llu)\n", File, Line, Condition, - Value1, Value2); + Report.append("CHECK failed @ %s:%d %s ((u64)op1=%llu, (u64)op2=%llu)\n", + File, Line, Condition, Value1, Value2); } // Generic string fatal error message. diff --git a/standalone/tests/report_test.cpp b/standalone/tests/report_test.cpp index 09f03f1ac89..374b6b8de34 100644 --- a/standalone/tests/report_test.cpp +++ b/standalone/tests/report_test.cpp @@ -10,6 +10,13 @@ #include "report.h" +TEST(ScudoReportTest, Check) { + CHECK_LT(-1, 1); + EXPECT_DEATH(CHECK_GT(-1, 1), + "\\(-1\\) > \\(1\\) \\(\\(u64\\)op1=18446744073709551615, " + "\\(u64\\)op2=1"); +} + TEST(ScudoReportTest, Generic) { // Potentially unused if EXPECT_DEATH isn't defined. UNUSED void *P = reinterpret_cast(0x42424242U); -- cgit v1.2.3 From 64f7f019013380f69bd2de4757e26613cdea0933 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Tue, 25 May 2021 11:35:12 -0700 Subject: Revert "[NFC][scudo] Let disableMemoryTagChecksTestOnly to fail" This reverts commit 2c212db4ea42fbbc0e83647da4f62261f775388b. It's not needed. GitOrigin-RevId: d1e5f046cc9ca52301b3f5ed457272f84aa0da14 Change-Id: Iad1bdeb8756e18cb0ec86b738fe2c74deddb1a1f --- standalone/memtag.h | 5 ++--- standalone/tests/combined_test.cpp | 25 +++++++++++++------------ 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/standalone/memtag.h b/standalone/memtag.h index 5c3ea588da3..9773cb97f05 100644 --- a/standalone/memtag.h +++ b/standalone/memtag.h @@ -92,13 +92,12 @@ inline bool systemDetectsMemoryTagFaultsTestOnly() { return false; } #endif // SCUDO_LINUX -inline bool disableMemoryTagChecksTestOnly() { +inline void disableMemoryTagChecksTestOnly() { __asm__ __volatile__( R"( .arch_extension memtag msr tco, #1 )"); - return true; } inline void enableMemoryTagChecksTestOnly() { @@ -251,7 +250,7 @@ inline bool systemDetectsMemoryTagFaultsTestOnly() { UNREACHABLE("memory tagging not supported"); } -inline bool disableMemoryTagChecksTestOnly() { +inline void disableMemoryTagChecksTestOnly() { UNREACHABLE("memory tagging not supported"); } diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index 908c6354eab..a5dcaed30ab 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -399,20 +399,21 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, DisableMemoryTagging) { // Check that disabling memory tagging works correctly. void *P = Allocator->allocate(2048, Origin); EXPECT_DEATH(reinterpret_cast(P)[2048] = 0xaa, ""); - if (scudo::disableMemoryTagChecksTestOnly()) { - Allocator->disableMemoryTagging(); - reinterpret_cast(P)[2048] = 0xaa; - Allocator->deallocate(P, Origin); + scudo::disableMemoryTagChecksTestOnly(); + Allocator->disableMemoryTagging(); + reinterpret_cast(P)[2048] = 0xaa; + Allocator->deallocate(P, Origin); - P = Allocator->allocate(2048, Origin); - EXPECT_EQ(scudo::untagPointer(P), P); - reinterpret_cast(P)[2048] = 0xaa; - Allocator->deallocate(P, Origin); + P = Allocator->allocate(2048, Origin); + EXPECT_EQ(scudo::untagPointer(P), P); + reinterpret_cast(P)[2048] = 0xaa; + Allocator->deallocate(P, Origin); - // Disabling memory tag checks may interfere with subsequent tests. - // Re-enable them now. - scudo::enableMemoryTagChecksTestOnly(); - } + Allocator->releaseToOS(); + + // Disabling memory tag checks may interfere with subsequent tests. + // Re-enable them now. + scudo::enableMemoryTagChecksTestOnly(); } } -- cgit v1.2.3 From 1eba93704852c020e69ed92f44a250f405b17827 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Tue, 25 May 2021 15:25:05 -0700 Subject: [NFC][SCUDO] Fix unittest for -gtest_repeat=10 Reviewed By: cryptoad Differential Revision: https://reviews.llvm.org/D103122 GitOrigin-RevId: e14696bfd740edb3f2fa7b9f36022f36000742ec Change-Id: Ia62cb33953762a2d20754ceb86d171c49f392d4e --- standalone/tests/common_test.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/standalone/tests/common_test.cpp b/standalone/tests/common_test.cpp index 832f01da390..2c79487e77d 100644 --- a/standalone/tests/common_test.cpp +++ b/standalone/tests/common_test.cpp @@ -39,12 +39,12 @@ TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { map(nullptr, Size, "ResidentMemorySize", 0, &Data)); const ptrdiff_t N = Size / sizeof(*P); ASSERT_NE(nullptr, P); - EXPECT_EQ(std::count(P, P + N, 0), N); EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); + EXPECT_EQ(std::count(P, P + N, 0), N); memset(P, 1, Size); EXPECT_EQ(std::count(P, P + N, 0), 0); - EXPECT_LT(getResidentMemorySize() - Size, Threshold); + EXPECT_GT(getResidentMemorySize() - OnStart, Size - Threshold); releasePagesToOS((uptr)P, 0, Size, &Data); EXPECT_EQ(std::count(P, P + N, 0), N); @@ -53,7 +53,9 @@ TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { memset(P, 1, Size); EXPECT_EQ(std::count(P, P + N, 0), 0); - EXPECT_LT(getResidentMemorySize() - Size, Threshold); + EXPECT_GT(getResidentMemorySize() - OnStart, Size - Threshold); + + unmap(P, Size, 0, &Data); } } // namespace scudo -- cgit v1.2.3 From 4a435d20706eece04b759ec65de5f929f9e42c2e Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Tue, 25 May 2021 15:00:58 -0700 Subject: [scudo] Get rid of initLinkerInitialized Now that everything is forcibly linker initialized, it feels like a good time to get rid of the `init`/`initLinkerInitialized` split. This allows to get rid of various `memset` construct in `init` that gcc complains about (this fixes a Fuchsia open issue). I added various `DCHECK`s to ensure that we would get a zero-inited object when entering `init`, which required ensuring that `unmapTestOnly` leaves the object in a good state (tests are currently the only location where an allocator can be "de-initialized"). Running the tests with `--gtest_repeat=` showed no issue. Differential Revision: https://reviews.llvm.org/D103119 GitOrigin-RevId: a45877eea8c424cd91bc1f7749313c9cb3aab285 Change-Id: I6df6304120bee33b09c7bda955fc11272542bad0 --- standalone/benchmarks/malloc_benchmark.cpp | 2 -- standalone/bytemap.h | 7 +++--- standalone/combined.h | 17 ++++++--------- standalone/local_cache.h | 10 +++------ standalone/mutex.h | 1 - standalone/primary32.h | 9 +++----- standalone/primary64.h | 12 ++++++----- standalone/quarantine.h | 20 +++++------------- standalone/secondary.h | 20 ++++++------------ standalone/stats.h | 14 +++++------- standalone/tests/combined_test.cpp | 1 - standalone/tests/mutex_test.cpp | 2 -- standalone/tests/tsd_test.cpp | 15 +++++-------- standalone/tsd.h | 7 ++---- standalone/tsd_exclusive.h | 34 +++++++++++++++++------------- standalone/tsd_shared.h | 22 +++++++++++-------- 16 files changed, 78 insertions(+), 115 deletions(-) diff --git a/standalone/benchmarks/malloc_benchmark.cpp b/standalone/benchmarks/malloc_benchmark.cpp index 661fff45a8d..2adec88da3e 100644 --- a/standalone/benchmarks/malloc_benchmark.cpp +++ b/standalone/benchmarks/malloc_benchmark.cpp @@ -29,7 +29,6 @@ template static void BM_malloc_free(benchmark::State &State) { std::unique_ptr Allocator(new AllocatorT, Deleter); CurrentAllocator = Allocator.get(); - Allocator->reset(); const size_t NBytes = State.range(0); size_t PageSize = scudo::getPageSizeCached(); @@ -70,7 +69,6 @@ static void BM_malloc_free_loop(benchmark::State &State) { std::unique_ptr Allocator(new AllocatorT, Deleter); CurrentAllocator = Allocator.get(); - Allocator->reset(); const size_t NumIters = State.range(0); size_t PageSize = scudo::getPageSizeCached(); diff --git a/standalone/bytemap.h b/standalone/bytemap.h index e0d54f4e597..248e096d07b 100644 --- a/standalone/bytemap.h +++ b/standalone/bytemap.h @@ -17,10 +17,9 @@ namespace scudo { template class FlatByteMap { public: - void initLinkerInitialized() {} - void init() { memset(Map, 0, sizeof(Map)); } + void init() { DCHECK(Size == 0 || Map[0] == 0); } - void unmapTestOnly() {} + void unmapTestOnly() { memset(Map, 0, Size); } void set(uptr Index, u8 Value) { DCHECK_LT(Index, Size); @@ -36,7 +35,7 @@ public: void enable() {} private: - u8 Map[Size]; + u8 Map[Size] = {}; }; } // namespace scudo diff --git a/standalone/combined.h b/standalone/combined.h index f17c371af82..8caffcc533c 100644 --- a/standalone/combined.h +++ b/standalone/combined.h @@ -132,7 +132,7 @@ public: typedef GlobalQuarantine QuarantineT; typedef typename QuarantineT::CacheT QuarantineCacheT; - void initLinkerInitialized() { + void init() { performSanityChecks(); // Check if hardware CRC32 is supported in the binary and by the platform, @@ -166,11 +166,10 @@ public: QuarantineMaxChunkSize = static_cast(getFlags()->quarantine_max_chunk_size); - Stats.initLinkerInitialized(); + Stats.init(); const s32 ReleaseToOsIntervalMs = getFlags()->release_to_os_interval_ms; - Primary.initLinkerInitialized(ReleaseToOsIntervalMs); - Secondary.initLinkerInitialized(&Stats, ReleaseToOsIntervalMs); - + Primary.init(ReleaseToOsIntervalMs); + Secondary.init(&Stats, ReleaseToOsIntervalMs); Quarantine.init( static_cast(getFlags()->quarantine_size_kb << 10), static_cast(getFlags()->thread_local_quarantine_size_kb << 10)); @@ -210,10 +209,8 @@ public: TSDRegistry.initThreadMaybe(this, MinimalInit); } - void reset() { memset(this, 0, sizeof(*this)); } - void unmapTestOnly() { - TSDRegistry.unmapTestOnly(); + TSDRegistry.unmapTestOnly(this); Primary.unmapTestOnly(); Secondary.unmapTestOnly(); #ifdef GWP_ASAN_HOOKS @@ -226,9 +223,7 @@ public: TSDRegistryT *getTSDRegistry() { return &TSDRegistry; } // The Cache must be provided zero-initialized. - void initCache(CacheT *Cache) { - Cache->initLinkerInitialized(&Stats, &Primary); - } + void initCache(CacheT *Cache) { Cache->init(&Stats, &Primary); } // Release the resources used by a TSD, which involves: // - draining the local quarantine cache to the global quarantine; diff --git a/standalone/local_cache.h b/standalone/local_cache.h index 50039379fa0..f46645f9bad 100644 --- a/standalone/local_cache.h +++ b/standalone/local_cache.h @@ -49,18 +49,14 @@ template struct SizeClassAllocatorLocalCache { CompactPtrT Batch[MaxNumCached]; }; - void initLinkerInitialized(GlobalStats *S, SizeClassAllocator *A) { - Stats.initLinkerInitialized(); + void init(GlobalStats *S, SizeClassAllocator *A) { + DCHECK(isEmpty()); + Stats.init(); if (LIKELY(S)) S->link(&Stats); Allocator = A; } - void init(GlobalStats *S, SizeClassAllocator *A) { - memset(this, 0, sizeof(*this)); - initLinkerInitialized(S, A); - } - void destroy(GlobalStats *S) { drain(); if (LIKELY(S)) diff --git a/standalone/mutex.h b/standalone/mutex.h index a654d35c5a7..c8504c04091 100644 --- a/standalone/mutex.h +++ b/standalone/mutex.h @@ -22,7 +22,6 @@ namespace scudo { class HybridMutex { public: - void init() { M = {}; } bool tryLock(); NOINLINE void lock() { if (LIKELY(tryLock())) diff --git a/standalone/primary32.h b/standalone/primary32.h index 33d81754fb5..059680b0613 100644 --- a/standalone/primary32.h +++ b/standalone/primary32.h @@ -60,11 +60,11 @@ public: static bool canAllocate(uptr Size) { return Size <= SizeClassMap::MaxSize; } - void initLinkerInitialized(s32 ReleaseToOsInterval) { + void init(s32 ReleaseToOsInterval) { if (SCUDO_FUCHSIA) reportError("SizeClassAllocator32 is not supported on Fuchsia"); - PossibleRegions.initLinkerInitialized(); + PossibleRegions.init(); u32 Seed; const u64 Time = getMonotonicTime(); @@ -80,10 +80,6 @@ public: } setOption(Option::ReleaseInterval, static_cast(ReleaseToOsInterval)); } - void init(s32 ReleaseToOsInterval) { - memset(this, 0, sizeof(*this)); - initLinkerInitialized(ReleaseToOsInterval); - } void unmapTestOnly() { while (NumberOfStashedRegions > 0) @@ -96,6 +92,7 @@ public: MinRegionIndex = Sci->MinRegionIndex; if (Sci->MaxRegionIndex > MaxRegionIndex) MaxRegionIndex = Sci->MaxRegionIndex; + *Sci = {}; } for (uptr I = MinRegionIndex; I < MaxRegionIndex; I++) if (PossibleRegions[I]) diff --git a/standalone/primary64.h b/standalone/primary64.h index 94375fceee1..0a43f46bb2c 100644 --- a/standalone/primary64.h +++ b/standalone/primary64.h @@ -57,7 +57,8 @@ public: static bool canAllocate(uptr Size) { return Size <= SizeClassMap::MaxSize; } - void initLinkerInitialized(s32 ReleaseToOsInterval) { + void init(s32 ReleaseToOsInterval) { + DCHECK_EQ(PrimaryBase, 0U); // Reserve the space required for the Primary. PrimaryBase = reinterpret_cast( map(nullptr, PrimarySize, nullptr, MAP_NOACCESS, &Data)); @@ -77,13 +78,14 @@ public: } setOption(Option::ReleaseInterval, static_cast(ReleaseToOsInterval)); } - void init(s32 ReleaseToOsInterval) { - memset(this, 0, sizeof(*this)); - initLinkerInitialized(ReleaseToOsInterval); - } void unmapTestOnly() { + for (uptr I = 0; I < NumClasses; I++) { + RegionInfo *Region = getRegionInfo(I); + *Region = {}; + } unmap(reinterpret_cast(PrimaryBase), PrimarySize, UNMAP_ALL, &Data); + PrimaryBase = 0U; } TransferBatch *popBatch(CacheT *C, uptr ClassId) { diff --git a/standalone/quarantine.h b/standalone/quarantine.h index 8d4b38e21fc..84eb90cc0b3 100644 --- a/standalone/quarantine.h +++ b/standalone/quarantine.h @@ -64,11 +64,7 @@ static_assert(sizeof(QuarantineBatch) <= (1U << 13), ""); // 8Kb. // Per-thread cache of memory blocks. template class QuarantineCache { public: - void initLinkerInitialized() {} - void init() { - memset(this, 0, sizeof(*this)); - initLinkerInitialized(); - } + void init() { DCHECK_EQ(atomic_load_relaxed(&Size), 0U); } // Total memory used, including internal accounting. uptr getSize() const { return atomic_load_relaxed(&Size); } @@ -175,7 +171,10 @@ template class GlobalQuarantine { public: typedef QuarantineCache CacheT; - void initLinkerInitialized(uptr Size, uptr CacheSize) { + void init(uptr Size, uptr CacheSize) { + DCHECK_EQ(atomic_load_relaxed(&MaxSize), 0U); + DCHECK_EQ(atomic_load_relaxed(&MinSize), 0U); + DCHECK_EQ(atomic_load_relaxed(&MaxCacheSize), 0U); // Thread local quarantine size can be zero only when global quarantine size // is zero (it allows us to perform just one atomic read per put() call). CHECK((Size == 0 && CacheSize == 0) || CacheSize != 0); @@ -184,16 +183,7 @@ public: atomic_store_relaxed(&MinSize, Size / 10 * 9); // 90% of max size. atomic_store_relaxed(&MaxCacheSize, CacheSize); - Cache.initLinkerInitialized(); - } - void init(uptr Size, uptr CacheSize) { - CacheMutex.init(); Cache.init(); - RecycleMutex.init(); - MinSize = {}; - MaxSize = {}; - MaxCacheSize = {}; - initLinkerInitialized(Size, CacheSize); } uptr getMaxSize() const { return atomic_load_relaxed(&MaxSize); } diff --git a/standalone/secondary.h b/standalone/secondary.h index 3894b01c0a4..630e64d46ed 100644 --- a/standalone/secondary.h +++ b/standalone/secondary.h @@ -71,7 +71,6 @@ static void unmap(LargeBlock::Header *H) { class MapAllocatorNoCache { public: - void initLinkerInitialized(UNUSED s32 ReleaseToOsInterval) {} void init(UNUSED s32 ReleaseToOsInterval) {} bool retrieve(UNUSED Options Options, UNUSED uptr Size, UNUSED uptr Alignment, UNUSED LargeBlock::Header **H, UNUSED bool *Zeroed) { @@ -121,17 +120,14 @@ public: Config::SecondaryCacheEntriesArraySize, ""); - void initLinkerInitialized(s32 ReleaseToOsInterval) { + void init(s32 ReleaseToOsInterval) { + DCHECK_EQ(EntriesCount, 0U); setOption(Option::MaxCacheEntriesCount, static_cast(Config::SecondaryCacheDefaultMaxEntriesCount)); setOption(Option::MaxCacheEntrySize, static_cast(Config::SecondaryCacheDefaultMaxEntrySize)); setOption(Option::ReleaseInterval, static_cast(ReleaseToOsInterval)); } - void init(s32 ReleaseToOsInterval) { - memset(this, 0, sizeof(*this)); - initLinkerInitialized(ReleaseToOsInterval); - } void store(Options Options, LargeBlock::Header *H) { if (!canCache(H->CommitSize)) @@ -404,16 +400,14 @@ private: template class MapAllocator { public: - void initLinkerInitialized(GlobalStats *S, s32 ReleaseToOsInterval = -1) { - Cache.initLinkerInitialized(ReleaseToOsInterval); - Stats.initLinkerInitialized(); + void init(GlobalStats *S, s32 ReleaseToOsInterval = -1) { + DCHECK_EQ(AllocatedBytes, 0U); + DCHECK_EQ(FreedBytes, 0U); + Cache.init(ReleaseToOsInterval); + Stats.init(); if (LIKELY(S)) S->link(&Stats); } - void init(GlobalStats *S, s32 ReleaseToOsInterval = -1) { - memset(this, 0, sizeof(*this)); - initLinkerInitialized(S, ReleaseToOsInterval); - } void *allocate(Options Options, uptr Size, uptr AlignmentHint = 0, uptr *BlockEnd = nullptr, diff --git a/standalone/stats.h b/standalone/stats.h index e15c0569497..be5bf2d3720 100644 --- a/standalone/stats.h +++ b/standalone/stats.h @@ -29,8 +29,10 @@ typedef uptr StatCounters[StatCount]; // LocalStats::add'ing, this is OK, we will still get a meaningful number. class LocalStats { public: - void initLinkerInitialized() {} - void init() { memset(this, 0, sizeof(*this)); } + void init() { + for (uptr I = 0; I < StatCount; I++) + DCHECK_EQ(get(static_cast(I)), 0U); + } void add(StatType I, uptr V) { V += atomic_load_relaxed(&StatsArray[I]); @@ -56,13 +58,7 @@ private: // Global stats, used for aggregation and querying. class GlobalStats : public LocalStats { public: - void initLinkerInitialized() {} - void init() { - LocalStats::init(); - Mutex.init(); - StatsList = {}; - initLinkerInitialized(); - } + void init() { LocalStats::init(); } void link(LocalStats *S) { ScopedLock L(Mutex); diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index a5dcaed30ab..983e9b48ad2 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -68,7 +68,6 @@ void checkMemoryTaggingMaybe(AllocatorT *Allocator, void *P, scudo::uptr Size, template struct TestAllocator : scudo::Allocator { TestAllocator() { - this->reset(); this->initThreadMaybe(); if (scudo::archSupportsMemoryTagging() && !scudo::systemDetectsMemoryTagFaultsTestOnly()) diff --git a/standalone/tests/mutex_test.cpp b/standalone/tests/mutex_test.cpp index ed56cb5219e..efee6fea226 100644 --- a/standalone/tests/mutex_test.cpp +++ b/standalone/tests/mutex_test.cpp @@ -82,7 +82,6 @@ static void *tryThread(void *Param) { TEST(ScudoMutexTest, Mutex) { scudo::HybridMutex M; - M.init(); TestData Data(M); pthread_t Threads[NumberOfThreads]; for (scudo::u32 I = 0; I < NumberOfThreads; I++) @@ -93,7 +92,6 @@ TEST(ScudoMutexTest, Mutex) { TEST(ScudoMutexTest, MutexTry) { scudo::HybridMutex M; - M.init(); TestData Data(M); pthread_t Threads[NumberOfThreads]; for (scudo::u32 I = 0; I < NumberOfThreads; I++) diff --git a/standalone/tests/tsd_test.cpp b/standalone/tests/tsd_test.cpp index 58ac9e74b98..35e579d5dc5 100644 --- a/standalone/tests/tsd_test.cpp +++ b/standalone/tests/tsd_test.cpp @@ -26,15 +26,14 @@ public: using CacheT = struct MockCache { volatile scudo::uptr Canary; }; using QuarantineCacheT = struct MockQuarantine {}; - void initLinkerInitialized() { + void init() { // This should only be called once by the registry. EXPECT_FALSE(Initialized); Initialized = true; } - void reset() { memset(this, 0, sizeof(*this)); } - void unmapTestOnly() { TSDRegistry.unmapTestOnly(); } - void initCache(CacheT *Cache) { memset(Cache, 0, sizeof(*Cache)); } + void unmapTestOnly() { TSDRegistry.unmapTestOnly(this); } + void initCache(CacheT *Cache) { *Cache = {}; } void commitBack(scudo::TSD *TSD) {} TSDRegistryT *getTSDRegistry() { return &TSDRegistry; } void callPostInitCallback() {} @@ -42,7 +41,7 @@ public: bool isInitialized() { return Initialized; } private: - bool Initialized; + bool Initialized = false; TSDRegistryT TSDRegistry; }; @@ -69,11 +68,10 @@ TEST(ScudoTSDTest, TSDRegistryInit) { }; std::unique_ptr Allocator(new AllocatorT, Deleter); - Allocator->reset(); EXPECT_FALSE(Allocator->isInitialized()); auto Registry = Allocator->getTSDRegistry(); - Registry->initLinkerInitialized(Allocator.get()); + Registry->init(Allocator.get()); EXPECT_TRUE(Allocator->isInitialized()); } @@ -84,7 +82,6 @@ template static void testRegistry() { }; std::unique_ptr Allocator(new AllocatorT, Deleter); - Allocator->reset(); EXPECT_FALSE(Allocator->isInitialized()); auto Registry = Allocator->getTSDRegistry(); @@ -153,7 +150,6 @@ template static void testRegistryThreaded() { }; std::unique_ptr Allocator(new AllocatorT, Deleter); - Allocator->reset(); std::thread Threads[32]; for (scudo::uptr I = 0; I < ARRAY_SIZE(Threads); I++) Threads[I] = std::thread(stressCache, Allocator.get()); @@ -209,7 +205,6 @@ TEST(ScudoTSDTest, TSDRegistryTSDsCount) { }; std::unique_ptr Allocator(new AllocatorT, Deleter); - Allocator->reset(); // We attempt to use as many TSDs as the shared cache offers by creating a // decent amount of threads that will be run concurrently and attempt to get // and lock TSDs. We put them all in a set and count the number of entries diff --git a/standalone/tsd.h b/standalone/tsd.h index a6e669b66e6..e376df05af5 100644 --- a/standalone/tsd.h +++ b/standalone/tsd.h @@ -28,14 +28,11 @@ template struct alignas(SCUDO_CACHE_LINE_SIZE) TSD { typename Allocator::QuarantineCacheT QuarantineCache; u8 DestructorIterations = 0; - void initLinkerInitialized(Allocator *Instance) { + void init(Allocator *Instance) { + DCHECK_EQ(DestructorIterations, 0U); Instance->initCache(&Cache); DestructorIterations = PTHREAD_DESTRUCTOR_ITERATIONS; } - void init(Allocator *Instance) { - memset(this, 0, sizeof(*this)); - initLinkerInitialized(Instance); - } void commitBack(Allocator *Instance) { Instance->commitBack(this); } diff --git a/standalone/tsd_exclusive.h b/standalone/tsd_exclusive.h index a907ed4684a..bba0c277c6a 100644 --- a/standalone/tsd_exclusive.h +++ b/standalone/tsd_exclusive.h @@ -25,31 +25,35 @@ struct ThreadState { template void teardownThread(void *Ptr); template struct TSDRegistryExT { - void initLinkerInitialized(Allocator *Instance) { - Instance->initLinkerInitialized(); + void init(Allocator *Instance) { + DCHECK(!Initialized); + Instance->init(); CHECK_EQ(pthread_key_create(&PThreadKey, teardownThread), 0); - FallbackTSD.initLinkerInitialized(Instance); + FallbackTSD.init(Instance); Initialized = true; } - void init(Allocator *Instance) { - memset(this, 0, sizeof(*this)); - initLinkerInitialized(Instance); - } void initOnceMaybe(Allocator *Instance) { ScopedLock L(Mutex); if (LIKELY(Initialized)) return; - initLinkerInitialized(Instance); // Sets Initialized. + init(Instance); // Sets Initialized. } - void unmapTestOnly() { - Allocator *Instance = - reinterpret_cast(pthread_getspecific(PThreadKey)); - if (!Instance) - return; - ThreadTSD.commitBack(Instance); + void unmapTestOnly(Allocator *Instance) { + DCHECK(Instance); + if (reinterpret_cast(pthread_getspecific(PThreadKey))) { + DCHECK_EQ(reinterpret_cast(pthread_getspecific(PThreadKey)), + Instance); + ThreadTSD.commitBack(Instance); + ThreadTSD = {}; + } + CHECK_EQ(pthread_key_delete(PThreadKey), 0); + PThreadKey = {}; + FallbackTSD.commitBack(Instance); + FallbackTSD = {}; State = {}; + Initialized = false; } ALWAYS_INLINE void initThreadMaybe(Allocator *Instance, bool MinimalInit) { @@ -103,7 +107,7 @@ private: return; CHECK_EQ( pthread_setspecific(PThreadKey, reinterpret_cast(Instance)), 0); - ThreadTSD.initLinkerInitialized(Instance); + ThreadTSD.init(Instance); State.InitState = ThreadState::Initialized; Instance->callPostInitCallback(); } diff --git a/standalone/tsd_shared.h b/standalone/tsd_shared.h index afe3623ce40..1c2a880416b 100644 --- a/standalone/tsd_shared.h +++ b/standalone/tsd_shared.h @@ -24,28 +24,32 @@ namespace scudo { template struct TSDRegistrySharedT { - void initLinkerInitialized(Allocator *Instance) { - Instance->initLinkerInitialized(); + void init(Allocator *Instance) { + DCHECK(!Initialized); + Instance->init(); for (u32 I = 0; I < TSDsArraySize; I++) - TSDs[I].initLinkerInitialized(Instance); + TSDs[I].init(Instance); const u32 NumberOfCPUs = getNumberOfCPUs(); setNumberOfTSDs((NumberOfCPUs == 0) ? DefaultTSDCount : Min(NumberOfCPUs, DefaultTSDCount)); Initialized = true; } - void init(Allocator *Instance) { - memset(this, 0, sizeof(*this)); - initLinkerInitialized(Instance); - } void initOnceMaybe(Allocator *Instance) { ScopedLock L(Mutex); if (LIKELY(Initialized)) return; - initLinkerInitialized(Instance); // Sets Initialized. + init(Instance); // Sets Initialized. } - void unmapTestOnly() { setCurrentTSD(nullptr); } + void unmapTestOnly(Allocator *Instance) { + for (u32 I = 0; I < TSDsArraySize; I++) { + TSDs[I].commitBack(Instance); + TSDs[I] = {}; + } + setCurrentTSD(nullptr); + Initialized = false; + } ALWAYS_INLINE void initThreadMaybe(Allocator *Instance, UNUSED bool MinimalInit) { -- cgit v1.2.3 From d78d2dbfe73bb231ce210b10a192b9f476fe84bd Mon Sep 17 00:00:00 2001 From: Mitch Phillips <31459023+hctim@users.noreply.github.com> Date: Wed, 26 May 2021 10:03:10 -0700 Subject: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests. This patch moves -fsanitize=scudo to link the standalone scudo library, rather than the original compiler-rt based library. This is one of the major remaining roadblocks to deleting the compiler-rt based scudo, which should not be used any more. The standalone Scudo is better in pretty much every way and is much more suitable for production usage. As well as patching the litmus tests for checking that the scudo_standalone lib is linked instead of the scudo lib, this patch also ports all the scudo lit tests to run under scudo standalone. This patch also adds a feature to scudo standalone that was under test in the original scudo - that arguments passed to an aligned operator new were checked that the alignment was a power of two. Some lit tests could not be migrated, due to the following issues: 1. Features that aren't supported in scudo standalone, like the rss limit. 2. Different quarantine implementation where the test needs some more thought. 3. Small bugs in scudo standalone that should probably be fixed, like the Secondary allocator having a full page on the LHS of an allocation that only contains the chunk header, so underflows by <= a page aren't caught. 4. Slight differences in behaviour that's technically correct, like 'realloc(malloc(1), 0)' returns nullptr in standalone, but a real pointer in old scudo. 5. Some tests that might be migratable, but not easily. Tests that are obviously not applicable to scudo standalone (like testing that no sanitizer symbols made it into the DSO) have been deleted. After this patch, the remaining work is: 1. Update the Scudo documentation. The flags have changed, etc. 2. Delete the old version of scudo. 3. Patch up the tests in lit-unmigrated, or fix Scudo standalone. Reviewed By: cryptoad, vitalybuka Differential Revision: https://reviews.llvm.org/D102543 GitOrigin-RevId: 6911114d8cbed06a8a809c34ae07f4e3e89ab252 Change-Id: I2918c9683874614bfb7603c45d29452b8c32d185 --- standalone/wrappers_cpp.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/standalone/wrappers_cpp.cpp b/standalone/wrappers_cpp.cpp index adb10411812..060117981e6 100644 --- a/standalone/wrappers_cpp.cpp +++ b/standalone/wrappers_cpp.cpp @@ -23,6 +23,27 @@ struct nothrow_t {}; enum class align_val_t : size_t {}; } // namespace std +// It's not valid for a non-throwing non-noreturn operator new to return +// nullptr. In these cases, just terminate. +#define CHECK_ALIGN_NORETURN(align) \ + do { \ + scudo::uptr alignment = static_cast(align); \ + if (UNLIKELY(!scudo::isPowerOfTwo(alignment))) { \ + scudo::reportAlignmentNotPowerOfTwo(alignment); \ + } \ + } while (0) + +#define CHECK_ALIGN(align) \ + do { \ + scudo::uptr alignment = static_cast(align); \ + if (UNLIKELY(!scudo::isPowerOfTwo(alignment))) { \ + if (Allocator.canReturnNull()) { \ + return nullptr; \ + } \ + scudo::reportAlignmentNotPowerOfTwo(alignment); \ + } \ + } while (0) + INTERFACE WEAK void *operator new(size_t size) { return Allocator.allocate(size, scudo::Chunk::Origin::New); } @@ -38,20 +59,24 @@ INTERFACE WEAK void *operator new[](size_t size, return Allocator.allocate(size, scudo::Chunk::Origin::NewArray); } INTERFACE WEAK void *operator new(size_t size, std::align_val_t align) { + CHECK_ALIGN_NORETURN(align); return Allocator.allocate(size, scudo::Chunk::Origin::New, static_cast(align)); } INTERFACE WEAK void *operator new[](size_t size, std::align_val_t align) { + CHECK_ALIGN_NORETURN(align); return Allocator.allocate(size, scudo::Chunk::Origin::NewArray, static_cast(align)); } INTERFACE WEAK void *operator new(size_t size, std::align_val_t align, std::nothrow_t const &) NOEXCEPT { + CHECK_ALIGN(align); return Allocator.allocate(size, scudo::Chunk::Origin::New, static_cast(align)); } INTERFACE WEAK void *operator new[](size_t size, std::align_val_t align, std::nothrow_t const &) NOEXCEPT { + CHECK_ALIGN(align); return Allocator.allocate(size, scudo::Chunk::Origin::NewArray, static_cast(align)); } -- cgit v1.2.3 From 96096b514f84fa7328e9937de1c8edc9fc6f7680 Mon Sep 17 00:00:00 2001 From: Mitch Phillips <31459023+hctim@users.noreply.github.com> Date: Wed, 26 May 2021 10:50:26 -0700 Subject: Revert "[Scudo] Make -fsanitize=scudo use standalone. Migrate tests." This reverts commit 6911114d8cbed06a8a809c34ae07f4e3e89ab252. Broke the QEMU sanitizer bots due to a missing header dependency. This actually needs to be fixed on the bot-side, but for now reverting this patch until I can fix up the bot. GitOrigin-RevId: f7c5c0d87b8ae5e55006fd3a31994cd68d64f102 Change-Id: Iddb9bbf9c161b3c662fd3e17ea65dc5734f51a11 --- standalone/wrappers_cpp.cpp | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/standalone/wrappers_cpp.cpp b/standalone/wrappers_cpp.cpp index 060117981e6..adb10411812 100644 --- a/standalone/wrappers_cpp.cpp +++ b/standalone/wrappers_cpp.cpp @@ -23,27 +23,6 @@ struct nothrow_t {}; enum class align_val_t : size_t {}; } // namespace std -// It's not valid for a non-throwing non-noreturn operator new to return -// nullptr. In these cases, just terminate. -#define CHECK_ALIGN_NORETURN(align) \ - do { \ - scudo::uptr alignment = static_cast(align); \ - if (UNLIKELY(!scudo::isPowerOfTwo(alignment))) { \ - scudo::reportAlignmentNotPowerOfTwo(alignment); \ - } \ - } while (0) - -#define CHECK_ALIGN(align) \ - do { \ - scudo::uptr alignment = static_cast(align); \ - if (UNLIKELY(!scudo::isPowerOfTwo(alignment))) { \ - if (Allocator.canReturnNull()) { \ - return nullptr; \ - } \ - scudo::reportAlignmentNotPowerOfTwo(alignment); \ - } \ - } while (0) - INTERFACE WEAK void *operator new(size_t size) { return Allocator.allocate(size, scudo::Chunk::Origin::New); } @@ -59,24 +38,20 @@ INTERFACE WEAK void *operator new[](size_t size, return Allocator.allocate(size, scudo::Chunk::Origin::NewArray); } INTERFACE WEAK void *operator new(size_t size, std::align_val_t align) { - CHECK_ALIGN_NORETURN(align); return Allocator.allocate(size, scudo::Chunk::Origin::New, static_cast(align)); } INTERFACE WEAK void *operator new[](size_t size, std::align_val_t align) { - CHECK_ALIGN_NORETURN(align); return Allocator.allocate(size, scudo::Chunk::Origin::NewArray, static_cast(align)); } INTERFACE WEAK void *operator new(size_t size, std::align_val_t align, std::nothrow_t const &) NOEXCEPT { - CHECK_ALIGN(align); return Allocator.allocate(size, scudo::Chunk::Origin::New, static_cast(align)); } INTERFACE WEAK void *operator new[](size_t size, std::align_val_t align, std::nothrow_t const &) NOEXCEPT { - CHECK_ALIGN(align); return Allocator.allocate(size, scudo::Chunk::Origin::NewArray, static_cast(align)); } -- cgit v1.2.3 From 63aa02c3f28b27d501d570aedf0227378c4a35cd Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 27 May 2021 09:49:34 -0700 Subject: [NFC][scudo] Rename internal function GitOrigin-RevId: eb69763ad8ea18ef1b0d739847da0be4ab099d51 Change-Id: I5a9abc10f97a06cab2801558a4727ab00625bef9 --- standalone/linux.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/standalone/linux.cpp b/standalone/linux.cpp index 301bdcd34da..75757cae267 100644 --- a/standalone/linux.cpp +++ b/standalone/linux.cpp @@ -90,7 +90,7 @@ void setMemoryPermission(uptr Addr, uptr Size, uptr Flags, dieOnMapUnmapError(); } -static bool madviseNeedsMemset() { +static bool madviseNotNeedFails() { const uptr Size = getPageSizeCached(); char *P = reinterpret_cast(mmap(0, Size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)); @@ -105,12 +105,12 @@ static bool madviseNeedsMemset() { return R; } -static bool madviseNeedsMemsetCached() { +static bool madviseNotNeedFailsCached() { static atomic_u8 Cache; enum State : u8 { Unknown = 0, Yes = 1, No = 2 }; State NeedsMemset = static_cast(atomic_load_relaxed(&Cache)); if (NeedsMemset == Unknown) { - NeedsMemset = madviseNeedsMemset() ? Yes : No; + NeedsMemset = madviseNotNeedFails() ? Yes : No; atomic_store_relaxed(&Cache, NeedsMemset); } return NeedsMemset == Yes; @@ -119,7 +119,7 @@ static bool madviseNeedsMemsetCached() { void releasePagesToOS(uptr BaseAddress, uptr Offset, uptr Size, UNUSED MapPlatformData *Data) { void *Addr = reinterpret_cast(BaseAddress + Offset); - if (madviseNeedsMemsetCached()) { + if (madviseNotNeedFailsCached()) { // Workaround for QEMU-user ignoring MADV_DONTNEED. // https://github.com/qemu/qemu/blob/b1cffefa1b163bce9aebc3416f562c1d3886eeaa/linux-user/syscall.c#L11941 // https://bugs.launchpad.net/qemu/+bug/1926521 -- cgit v1.2.3 From a05cb57f8a189b0e416db58ee9bc023c9d0d3d59 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 27 May 2021 10:02:47 -0700 Subject: [NFC][scudo] Check zeros on smaller allocations 1Tb counting was the slowest test under the QEMU with MTE. GitOrigin-RevId: c261edb277020471b7670a8b2f826efc73c5d941 Change-Id: I0d0d2e0e76e6885e57805f5b4cf83f2f57536a4a --- standalone/tests/common_test.cpp | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/standalone/tests/common_test.cpp b/standalone/tests/common_test.cpp index 2c79487e77d..eb3a5bceca9 100644 --- a/standalone/tests/common_test.cpp +++ b/standalone/tests/common_test.cpp @@ -35,27 +35,39 @@ TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { const uptr Threshold = Size >> 3; MapPlatformData Data = {}; - uptr *P = reinterpret_cast( - map(nullptr, Size, "ResidentMemorySize", 0, &Data)); - const ptrdiff_t N = Size / sizeof(*P); + void *P = map(nullptr, Size, "ResidentMemorySize", 0, &Data); ASSERT_NE(nullptr, P); EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); - EXPECT_EQ(std::count(P, P + N, 0), N); memset(P, 1, Size); - EXPECT_EQ(std::count(P, P + N, 0), 0); EXPECT_GT(getResidentMemorySize() - OnStart, Size - Threshold); releasePagesToOS((uptr)P, 0, Size, &Data); - EXPECT_EQ(std::count(P, P + N, 0), N); // FIXME: does not work with QEMU-user. // EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); memset(P, 1, Size); - EXPECT_EQ(std::count(P, P + N, 0), 0); EXPECT_GT(getResidentMemorySize() - OnStart, Size - Threshold); unmap(P, Size, 0, &Data); } +TEST(ScudoCommonTest, Zeros) { + const uptr Size = 1ull << 20; + + MapPlatformData Data = {}; + uptr *P = reinterpret_cast(map(nullptr, Size, "Zeros", 0, &Data)); + const ptrdiff_t N = Size / sizeof(*P); + ASSERT_NE(nullptr, P); + EXPECT_EQ(std::count(P, P + N, 0), N); + + memset(P, 1, Size); + EXPECT_EQ(std::count(P, P + N, 0), 0); + + releasePagesToOS((uptr)P, 0, Size, &Data); + EXPECT_EQ(std::count(P, P + N, 0), N); + + unmap(P, Size, 0, &Data); +} + } // namespace scudo -- cgit v1.2.3 From 6c5c1bbfc92ff0d5103fbe99c25f66e36719c735 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 28 May 2021 01:53:42 -0700 Subject: Revert "[scudo] Check if MADV_DONTNEED zeroes memory" This reverts commit d423509b8036c29bbf94dab192d12097555ce0f8. We are going to use patched QEMU. GitOrigin-RevId: 4458e8c4b42f3ebd44f909ffc0836224b7141d2e Change-Id: I3a2629abaddeaca6e6290223a51ddcac41d799de --- standalone/linux.cpp | 34 +--------------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/standalone/linux.cpp b/standalone/linux.cpp index 75757cae267..dedab61631b 100644 --- a/standalone/linux.cpp +++ b/standalone/linux.cpp @@ -10,7 +10,6 @@ #if SCUDO_LINUX -#include "atomic_helpers.h" #include "common.h" #include "linux.h" #include "mutex.h" @@ -90,41 +89,10 @@ void setMemoryPermission(uptr Addr, uptr Size, uptr Flags, dieOnMapUnmapError(); } -static bool madviseNotNeedFails() { - const uptr Size = getPageSizeCached(); - char *P = reinterpret_cast(mmap(0, Size, PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)); - if (!P) - dieOnMapUnmapError(errno == ENOMEM ? Size : 0); - *P = 1; - while (madvise(P, Size, MADV_DONTNEED) == -1 && errno == EAGAIN) { - } - const bool R = (*P != 0); - if (munmap(P, Size) != 0) - dieOnMapUnmapError(); - return R; -} - -static bool madviseNotNeedFailsCached() { - static atomic_u8 Cache; - enum State : u8 { Unknown = 0, Yes = 1, No = 2 }; - State NeedsMemset = static_cast(atomic_load_relaxed(&Cache)); - if (NeedsMemset == Unknown) { - NeedsMemset = madviseNotNeedFails() ? Yes : No; - atomic_store_relaxed(&Cache, NeedsMemset); - } - return NeedsMemset == Yes; -} - void releasePagesToOS(uptr BaseAddress, uptr Offset, uptr Size, UNUSED MapPlatformData *Data) { void *Addr = reinterpret_cast(BaseAddress + Offset); - if (madviseNotNeedFailsCached()) { - // Workaround for QEMU-user ignoring MADV_DONTNEED. - // https://github.com/qemu/qemu/blob/b1cffefa1b163bce9aebc3416f562c1d3886eeaa/linux-user/syscall.c#L11941 - // https://bugs.launchpad.net/qemu/+bug/1926521 - memset(Addr, 0, Size); - } + while (madvise(Addr, Size, MADV_DONTNEED) == -1 && errno == EAGAIN) { } } -- cgit v1.2.3 From 611951b6fc15cecbf511690385ad539f98307c2f Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 28 May 2021 01:57:55 -0700 Subject: [NFC][scudo] Re-enable check in the test It should pass with patched QEMU. GitOrigin-RevId: a6e5a4b464bebb60f4fdfc27700aa37bbfa3fc54 Change-Id: I707f60778ba3ffd4c63b95701af67887f4b58f0f --- standalone/tests/common_test.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/standalone/tests/common_test.cpp b/standalone/tests/common_test.cpp index eb3a5bceca9..d8184a72361 100644 --- a/standalone/tests/common_test.cpp +++ b/standalone/tests/common_test.cpp @@ -43,8 +43,7 @@ TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { EXPECT_GT(getResidentMemorySize() - OnStart, Size - Threshold); releasePagesToOS((uptr)P, 0, Size, &Data); - // FIXME: does not work with QEMU-user. - // EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); + EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); memset(P, 1, Size); EXPECT_GT(getResidentMemorySize() - OnStart, Size - Threshold); -- cgit v1.2.3 From 58be7da29f92dcb52f5a80da08147418ffe82823 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 27 May 2021 10:10:50 -0700 Subject: [scudo] Enabled MTE in tests Reviewed By: pcc, hctim Differential Revision: https://reviews.llvm.org/D103305 GitOrigin-RevId: 4124bca309586234748c9c99600ec2f3b2f6d775 Change-Id: Ief6930231abbf7e15383401fc88e7cbcd8367783 --- standalone/memtag.h | 30 +++++++++++++++++++++++++++++- standalone/tests/scudo_unit_test_main.cpp | 3 +++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/standalone/memtag.h b/standalone/memtag.h index 9773cb97f05..fec258b11a4 100644 --- a/standalone/memtag.h +++ b/standalone/memtag.h @@ -67,15 +67,27 @@ inline bool systemSupportsMemoryTagging() { } inline bool systemDetectsMemoryTagFaultsTestOnly() { +#ifndef PR_SET_TAGGED_ADDR_CTRL +#define PR_SET_TAGGED_ADDR_CTRL 54 +#endif #ifndef PR_GET_TAGGED_ADDR_CTRL #define PR_GET_TAGGED_ADDR_CTRL 56 #endif +#ifndef PR_TAGGED_ADDR_ENABLE +#define PR_TAGGED_ADDR_ENABLE (1UL << 0) +#endif #ifndef PR_MTE_TCF_SHIFT #define PR_MTE_TCF_SHIFT 1 #endif +#ifndef PR_MTE_TAG_SHIFT +#define PR_MTE_TAG_SHIFT 3 +#endif #ifndef PR_MTE_TCF_NONE #define PR_MTE_TCF_NONE (0UL << PR_MTE_TCF_SHIFT) #endif +#ifndef PR_MTE_TCF_SYNC +#define PR_MTE_TCF_SYNC (1UL << PR_MTE_TCF_SHIFT) +#endif #ifndef PR_MTE_TCF_MASK #define PR_MTE_TCF_MASK (3UL << PR_MTE_TCF_SHIFT) #endif @@ -84,11 +96,23 @@ inline bool systemDetectsMemoryTagFaultsTestOnly() { PR_MTE_TCF_MASK) != PR_MTE_TCF_NONE; } +inline void enableSystemMemoryTaggingTestOnly() { + prctl(PR_SET_TAGGED_ADDR_CTRL, + PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC | (0xfffe << PR_MTE_TAG_SHIFT), + 0, 0, 0); +} + #else // !SCUDO_LINUX inline bool systemSupportsMemoryTagging() { return false; } -inline bool systemDetectsMemoryTagFaultsTestOnly() { return false; } +inline bool systemDetectsMemoryTagFaultsTestOnly() { + UNREACHABLE("memory tagging not supported"); +} + +inline void enableSystemMemoryTaggingTestOnly() { + UNREACHABLE("memory tagging not supported"); +} #endif // SCUDO_LINUX @@ -250,6 +274,10 @@ inline bool systemDetectsMemoryTagFaultsTestOnly() { UNREACHABLE("memory tagging not supported"); } +inline void enableSystemMemoryTaggingTestOnly() { + UNREACHABLE("memory tagging not supported"); +} + inline void disableMemoryTagChecksTestOnly() { UNREACHABLE("memory tagging not supported"); } diff --git a/standalone/tests/scudo_unit_test_main.cpp b/standalone/tests/scudo_unit_test_main.cpp index 9bbf6e75a5c..1983b80ac0c 100644 --- a/standalone/tests/scudo_unit_test_main.cpp +++ b/standalone/tests/scudo_unit_test_main.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "memtag.h" #include "tests/scudo_unit_test.h" // Match Android's default configuration, which disables Scudo's mismatch @@ -33,6 +34,8 @@ __scudo_default_options() { // for Fuchsia builds. #if !SCUDO_FUCHSIA int main(int argc, char **argv) { + if (scudo::archSupportsMemoryTagging()) + scudo::enableSystemMemoryTaggingTestOnly(); testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } -- cgit v1.2.3 From 24f3bdc31da8557a618566bfa0d176c4234bb228 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 3 Jun 2021 00:06:45 -0700 Subject: [NFC][scudo] Avoid integer overflow in test releasePagesToOS may shrink RSS below the value stored on start. GitOrigin-RevId: b40908e639b6075c77d7b2a990a2a273a39f4102 Change-Id: Ifec8ff4525c8e715f0f0d998d9276cfa881b7133 --- standalone/tests/common_test.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/standalone/tests/common_test.cpp b/standalone/tests/common_test.cpp index d8184a72361..711e3b28e31 100644 --- a/standalone/tests/common_test.cpp +++ b/standalone/tests/common_test.cpp @@ -37,16 +37,16 @@ TEST(ScudoCommonTest, SKIP_ON_FUCHSIA(ResidentMemorySize)) { MapPlatformData Data = {}; void *P = map(nullptr, Size, "ResidentMemorySize", 0, &Data); ASSERT_NE(nullptr, P); - EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); + EXPECT_LT(getResidentMemorySize(), OnStart + Threshold); memset(P, 1, Size); - EXPECT_GT(getResidentMemorySize() - OnStart, Size - Threshold); + EXPECT_GT(getResidentMemorySize(), OnStart + Size - Threshold); releasePagesToOS((uptr)P, 0, Size, &Data); - EXPECT_LT(getResidentMemorySize() - OnStart, Threshold); + EXPECT_LT(getResidentMemorySize(), OnStart + Threshold); memset(P, 1, Size); - EXPECT_GT(getResidentMemorySize() - OnStart, Size - Threshold); + EXPECT_GT(getResidentMemorySize(), OnStart + Size - Threshold); unmap(P, Size, 0, &Data); } -- cgit v1.2.3 From 53aea5c442dd86951eef46a0e0f0cccd7c1dc39f Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Thu, 3 Jun 2021 12:11:05 -0700 Subject: [scudo] Rework Vector/String Some platforms (eg: Trusty) are extremelly memory constrained, which doesn't necessarily work well with some of Scudo's current assumptions. `Vector` by default (and as such `String` and `ScopedString`) maps a page, which is a bit of a waste. This CL changes `Vector` to use a buffer local to the class first, then potentially map more memory if needed (`ScopedString` currently are all stack based so it would be stack data). We also want to allow a platform to prevent any dynamic resizing, so I added a `CanGrow` templated parameter that for now is always `true` but would be set to `false` on Trusty. Differential Revision: https://reviews.llvm.org/D103641 GitOrigin-RevId: 868317b3fd765830c07ecf16cbfcf6c7708adc3c Change-Id: Ia16abe9b3f18c6a719f1c202a943b1381ea5ecbb --- standalone/combined.h | 4 ++-- standalone/primary64.h | 2 +- standalone/report.cpp | 2 +- standalone/size_class_map.h | 2 +- standalone/string_utils.cpp | 6 +++--- standalone/string_utils.h | 7 ++++--- standalone/tests/primary_test.cpp | 8 ++++---- standalone/tests/quarantine_test.cpp | 4 ++-- standalone/tests/secondary_test.cpp | 8 ++++---- standalone/tests/strings_test.cpp | 8 ++++---- standalone/tests/vector_test.cpp | 14 +++++++------- standalone/vector.h | 31 ++++++++++++++----------------- 12 files changed, 47 insertions(+), 49 deletions(-) diff --git a/standalone/combined.h b/standalone/combined.h index 8caffcc533c..f58eaa945af 100644 --- a/standalone/combined.h +++ b/standalone/combined.h @@ -694,7 +694,7 @@ public: // function. This can be called with a null buffer or zero size for buffer // sizing purposes. uptr getStats(char *Buffer, uptr Size) { - ScopedString Str(1024); + ScopedString Str; disable(); const uptr Length = getStats(&Str) + 1; enable(); @@ -708,7 +708,7 @@ public: } void printStats() { - ScopedString Str(1024); + ScopedString Str; disable(); getStats(&Str); enable(); diff --git a/standalone/primary64.h b/standalone/primary64.h index 0a43f46bb2c..a3456b6b99c 100644 --- a/standalone/primary64.h +++ b/standalone/primary64.h @@ -341,7 +341,7 @@ private: if (UNLIKELY(RegionBase + MappedUser + MapSize > RegionSize)) { if (!Region->Exhausted) { Region->Exhausted = true; - ScopedString Str(1024); + ScopedString Str; getStats(&Str); Str.append( "Scudo OOM: The process has exhausted %zuM for size class %zu.\n", diff --git a/standalone/report.cpp b/standalone/report.cpp index 292c29918d1..561c7c51f4e 100644 --- a/standalone/report.cpp +++ b/standalone/report.cpp @@ -17,7 +17,7 @@ namespace scudo { class ScopedErrorReport { public: - ScopedErrorReport() : Message(512) { Message.append("Scudo ERROR: "); } + ScopedErrorReport() : Message() { Message.append("Scudo ERROR: "); } void append(const char *Format, ...) { va_list Args; va_start(Args, Format); diff --git a/standalone/size_class_map.h b/standalone/size_class_map.h index 2736e995dba..e1c2edac476 100644 --- a/standalone/size_class_map.h +++ b/standalone/size_class_map.h @@ -309,7 +309,7 @@ struct SvelteSizeClassConfig { typedef FixedSizeClassMap SvelteSizeClassMap; template inline void printMap() { - ScopedString Buffer(1024); + ScopedString Buffer; uptr PrevS = 0; uptr TotalCached = 0; for (uptr I = 0; I < SCMap::NumClasses; I++) { diff --git a/standalone/string_utils.cpp b/standalone/string_utils.cpp index 25bddbce34d..ca80834cca8 100644 --- a/standalone/string_utils.cpp +++ b/standalone/string_utils.cpp @@ -219,7 +219,7 @@ int formatString(char *Buffer, uptr BufferLength, const char *Format, ...) { } void ScopedString::append(const char *Format, va_list Args) { - DCHECK_LT(Length, String.size()); + RAW_CHECK(Length <= String.size()); va_list ArgsCopy; va_copy(ArgsCopy, Args); // formatString doesn't currently support a null buffer or zero buffer length, @@ -232,7 +232,7 @@ void ScopedString::append(const char *Format, va_list Args) { formatString(String.data() + Length, AdditionalLength, Format, ArgsCopy); va_end(ArgsCopy); Length = strlen(String.data()); - CHECK_LT(Length, String.size()); + RAW_CHECK(Length < String.size()); } FORMAT(2, 3) @@ -247,7 +247,7 @@ FORMAT(1, 2) void Printf(const char *Format, ...) { va_list Args; va_start(Args, Format); - ScopedString Msg(1024); + ScopedString Msg; Msg.append(Format, Args); outputRaw(Msg.data()); va_end(Args); diff --git a/standalone/string_utils.h b/standalone/string_utils.h index 4880fa1e7cf..7a57da3a53f 100644 --- a/standalone/string_utils.h +++ b/standalone/string_utils.h @@ -18,8 +18,9 @@ namespace scudo { class ScopedString { public: - explicit ScopedString(uptr MaxLength) : String(MaxLength), Length(0) { - String[0] = '\0'; + explicit ScopedString() : String() { + if (String.capacity() > 0) + String[0] = '\0'; } uptr length() { return Length; } const char *data() { return String.data(); } @@ -33,7 +34,7 @@ public: private: Vector String; - uptr Length; + uptr Length = 0; }; int formatString(char *Buffer, uptr BufferLength, const char *Format, ...); diff --git a/standalone/tests/primary_test.cpp b/standalone/tests/primary_test.cpp index 3574c28c6c0..93157f942c5 100644 --- a/standalone/tests/primary_test.cpp +++ b/standalone/tests/primary_test.cpp @@ -132,7 +132,7 @@ SCUDO_TYPED_TEST(ScudoPrimaryTest, BasicPrimary) { } Cache.destroy(nullptr); Allocator->releaseToOS(); - scudo::ScopedString Str(1024); + scudo::ScopedString Str; Allocator->getStats(&Str); Str.output(); } @@ -178,7 +178,7 @@ TEST(ScudoPrimaryTest, Primary64OOM) { } Cache.destroy(nullptr); Allocator.releaseToOS(); - scudo::ScopedString Str(1024); + scudo::ScopedString Str; Allocator.getStats(&Str); Str.output(); EXPECT_EQ(AllocationFailed, true); @@ -216,7 +216,7 @@ SCUDO_TYPED_TEST(ScudoPrimaryTest, PrimaryIterate) { } Cache.destroy(nullptr); Allocator->releaseToOS(); - scudo::ScopedString Str(1024); + scudo::ScopedString Str; Allocator->getStats(&Str); Str.output(); } @@ -263,7 +263,7 @@ SCUDO_TYPED_TEST(ScudoPrimaryTest, PrimaryThreaded) { for (auto &T : Threads) T.join(); Allocator->releaseToOS(); - scudo::ScopedString Str(1024); + scudo::ScopedString Str; Allocator->getStats(&Str); Str.output(); } diff --git a/standalone/tests/quarantine_test.cpp b/standalone/tests/quarantine_test.cpp index 91de56a78c9..972c98d510a 100644 --- a/standalone/tests/quarantine_test.cpp +++ b/standalone/tests/quarantine_test.cpp @@ -214,7 +214,7 @@ TEST(ScudoQuarantineTest, GlobalQuarantine) { Quarantine.drainAndRecycle(&Cache, Cb); EXPECT_EQ(Cache.getSize(), 0UL); - scudo::ScopedString Str(1024); + scudo::ScopedString Str; Quarantine.getStats(&Str); Str.output(); } @@ -246,7 +246,7 @@ TEST(ScudoQuarantineTest, ThreadedGlobalQuarantine) { for (scudo::uptr I = 0; I < NumberOfThreads; I++) pthread_join(T[I].Thread, 0); - scudo::ScopedString Str(1024); + scudo::ScopedString Str; Quarantine.getStats(&Str); Str.output(); diff --git a/standalone/tests/secondary_test.cpp b/standalone/tests/secondary_test.cpp index 2320a4d546a..2dc041b94a8 100644 --- a/standalone/tests/secondary_test.cpp +++ b/standalone/tests/secondary_test.cpp @@ -64,7 +64,7 @@ template static void testSecondaryBasic(void) { L->deallocate(scudo::Options{}, V.back()); V.pop_back(); } - scudo::ScopedString Str(1024); + scudo::ScopedString Str; L->getStats(&Str); Str.output(); L->unmapTestOnly(); @@ -122,7 +122,7 @@ TEST(ScudoSecondaryTest, SecondaryCombinations) { } } } - scudo::ScopedString Str(1024); + scudo::ScopedString Str; L->getStats(&Str); Str.output(); L->unmapTestOnly(); @@ -146,7 +146,7 @@ TEST(ScudoSecondaryTest, SecondaryIterate) { L->deallocate(scudo::Options{}, V.back()); V.pop_back(); } - scudo::ScopedString Str(1024); + scudo::ScopedString Str; L->getStats(&Str); Str.output(); L->unmapTestOnly(); @@ -217,7 +217,7 @@ TEST(ScudoSecondaryTest, SecondaryThreadsRace) { } for (auto &T : Threads) T.join(); - scudo::ScopedString Str(1024); + scudo::ScopedString Str; L->getStats(&Str); Str.output(); L->unmapTestOnly(); diff --git a/standalone/tests/strings_test.cpp b/standalone/tests/strings_test.cpp index eed174dc586..332bac2f796 100644 --- a/standalone/tests/strings_test.cpp +++ b/standalone/tests/strings_test.cpp @@ -13,7 +13,7 @@ #include TEST(ScudoStringsTest, Basic) { - scudo::ScopedString Str(128); + scudo::ScopedString Str; Str.append("a%db%zdc%ue%zuf%xh%zxq%pe%sr", static_cast(-1), static_cast(-2), static_cast(-4), static_cast(5), static_cast(10), @@ -29,7 +29,7 @@ TEST(ScudoStringsTest, Basic) { } TEST(ScudoStringsTest, Precision) { - scudo::ScopedString Str(128); + scudo::ScopedString Str; Str.append("%.*s", 3, "12345"); EXPECT_EQ(Str.length(), strlen(Str.data())); EXPECT_STREQ("123", Str.data()); @@ -52,7 +52,7 @@ TEST(ScudoStringTest, PotentialOverflows) { // Use a ScopedString that spans a page, and attempt to write past the end // of it with variations of append. The expectation is for nothing to crash. const scudo::uptr PageSize = scudo::getPageSizeCached(); - scudo::ScopedString Str(PageSize); + scudo::ScopedString Str; Str.clear(); fillString(Str, 2 * PageSize); Str.clear(); @@ -68,7 +68,7 @@ TEST(ScudoStringTest, PotentialOverflows) { template static void testAgainstLibc(const char *Format, T Arg1, T Arg2) { - scudo::ScopedString Str(128); + scudo::ScopedString Str; Str.append(Format, Arg1, Arg2); char Buffer[128]; snprintf(Buffer, sizeof(Buffer), Format, Arg1, Arg2); diff --git a/standalone/tests/vector_test.cpp b/standalone/tests/vector_test.cpp index d2c6a9b6bb3..dc23c2a3471 100644 --- a/standalone/tests/vector_test.cpp +++ b/standalone/tests/vector_test.cpp @@ -23,14 +23,14 @@ TEST(ScudoVectorTest, Basic) { } TEST(ScudoVectorTest, Stride) { - scudo::Vector V; - for (int i = 0; i < 1000; i++) { - V.push_back(i); - EXPECT_EQ(V.size(), i + 1U); - EXPECT_EQ(V[i], i); + scudo::Vector V; + for (scudo::uptr I = 0; I < 1000; I++) { + V.push_back(I); + EXPECT_EQ(V.size(), I + 1U); + EXPECT_EQ(V[I], I); } - for (int i = 0; i < 1000; i++) - EXPECT_EQ(V[i], i); + for (scudo::uptr I = 0; I < 1000; I++) + EXPECT_EQ(V[I], I); } TEST(ScudoVectorTest, ResizeReduction) { diff --git a/standalone/vector.h b/standalone/vector.h index 6ca350a2577..2c9a6e2aa65 100644 --- a/standalone/vector.h +++ b/standalone/vector.h @@ -19,14 +19,13 @@ namespace scudo { // small vectors. The current implementation supports only POD types. template class VectorNoCtor { public: - void init(uptr InitialCapacity) { - CapacityBytes = 0; - Size = 0; - Data = nullptr; + void init(uptr InitialCapacity = 0) { + Data = reinterpret_cast(&LocalData[0]); + CapacityBytes = sizeof(LocalData); reserve(InitialCapacity); } void destroy() { - if (Data) + if (Data != reinterpret_cast(&LocalData[0])) unmap(Data, CapacityBytes); } T &operator[](uptr I) { @@ -82,26 +81,24 @@ private: void reallocate(uptr NewCapacity) { DCHECK_GT(NewCapacity, 0); DCHECK_LE(Size, NewCapacity); - const uptr NewCapacityBytes = - roundUpTo(NewCapacity * sizeof(T), getPageSizeCached()); + NewCapacity = roundUpTo(NewCapacity * sizeof(T), getPageSizeCached()); T *NewData = - reinterpret_cast(map(nullptr, NewCapacityBytes, "scudo:vector")); - if (Data) { - memcpy(NewData, Data, Size * sizeof(T)); - unmap(Data, CapacityBytes); - } + reinterpret_cast(map(nullptr, NewCapacity, "scudo:vector")); + memcpy(NewData, Data, Size * sizeof(T)); + destroy(); Data = NewData; - CapacityBytes = NewCapacityBytes; + CapacityBytes = NewCapacity; } - T *Data; - uptr CapacityBytes; - uptr Size; + T *Data = nullptr; + u8 LocalData[256] = {}; + uptr CapacityBytes = 0; + uptr Size = 0; }; template class Vector : public VectorNoCtor { public: - Vector() { VectorNoCtor::init(1); } + Vector() { VectorNoCtor::init(); } explicit Vector(uptr Count) { VectorNoCtor::init(Count); this->resize(Count); -- cgit v1.2.3 From ef0e3548ab99afca9c662f35a0f1e6c676f9c052 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 28 May 2021 02:08:44 -0700 Subject: [scudo] Always exclude Tag 0 prepareTaggedChunk uses Tag 0 for header. Android already PR_MTE_TAG_MASK to 0xfffe, but with the patch we will not need to deppend on the system configuration. Reviewed By: pcc Differential Revision: https://reviews.llvm.org/D103134 GitOrigin-RevId: ba04c7c128b1214edf8888b129f9e841852a629b Change-Id: I4bb78ac7f872ed8bd275ab06d507aa04825ecefb --- standalone/memtag.h | 1 + 1 file changed, 1 insertion(+) diff --git a/standalone/memtag.h b/standalone/memtag.h index fec258b11a4..26502e2b1ba 100644 --- a/standalone/memtag.h +++ b/standalone/memtag.h @@ -158,6 +158,7 @@ public: }; inline uptr selectRandomTag(uptr Ptr, uptr ExcludeMask) { + ExcludeMask |= 1; // Always exclude Tag 0. uptr TaggedPtr; __asm__ __volatile__( R"( -- cgit v1.2.3 From 70168a659f7feff1dbf0d623cbe4d80759871f53 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 4 Jun 2021 11:25:47 -0700 Subject: [scudo] Remove disableMemoryTagChecksTestOnly And replace with ScopedDisableMemoryTagChecks. Differential Revision: https://reviews.llvm.org/D103708 GitOrigin-RevId: 39f928ed01fde8604515421c5438c3ecb537fbf2 Change-Id: Ibe342f6dae5c24a990de0cb5c331bfefadcd67c6 --- standalone/memtag.h | 24 ------------------------ standalone/tests/combined_test.cpp | 7 ++----- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/standalone/memtag.h b/standalone/memtag.h index 26502e2b1ba..13e9939cae7 100644 --- a/standalone/memtag.h +++ b/standalone/memtag.h @@ -116,22 +116,6 @@ inline void enableSystemMemoryTaggingTestOnly() { #endif // SCUDO_LINUX -inline void disableMemoryTagChecksTestOnly() { - __asm__ __volatile__( - R"( - .arch_extension memtag - msr tco, #1 - )"); -} - -inline void enableMemoryTagChecksTestOnly() { - __asm__ __volatile__( - R"( - .arch_extension memtag - msr tco, #0 - )"); -} - class ScopedDisableMemoryTagChecks { uptr PrevTCO; @@ -279,14 +263,6 @@ inline void enableSystemMemoryTaggingTestOnly() { UNREACHABLE("memory tagging not supported"); } -inline void disableMemoryTagChecksTestOnly() { - UNREACHABLE("memory tagging not supported"); -} - -inline void enableMemoryTagChecksTestOnly() { - UNREACHABLE("memory tagging not supported"); -} - struct ScopedDisableMemoryTagChecks { ScopedDisableMemoryTagChecks() {} }; diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index 983e9b48ad2..50b55bb089f 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "memtag.h" #include "tests/scudo_unit_test.h" #include "allocator_config.h" @@ -398,7 +399,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, DisableMemoryTagging) { // Check that disabling memory tagging works correctly. void *P = Allocator->allocate(2048, Origin); EXPECT_DEATH(reinterpret_cast(P)[2048] = 0xaa, ""); - scudo::disableMemoryTagChecksTestOnly(); + scudo::ScopedDisableMemoryTagChecks NoTagChecks; Allocator->disableMemoryTagging(); reinterpret_cast(P)[2048] = 0xaa; Allocator->deallocate(P, Origin); @@ -409,10 +410,6 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, DisableMemoryTagging) { Allocator->deallocate(P, Origin); Allocator->releaseToOS(); - - // Disabling memory tag checks may interfere with subsequent tests. - // Re-enable them now. - scudo::enableMemoryTagChecksTestOnly(); } } -- cgit v1.2.3 From ecf52a746a9f2f877c7615bfacbf7c1df44e9310 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Mon, 24 May 2021 21:40:44 -0700 Subject: [scudo] Add memtag_test Differential Revision: https://reviews.llvm.org/D103074 GitOrigin-RevId: 07c92b2e958191a43464a5ca08933be56d72f2df Change-Id: Ic9231a5265f038c09571c91d023b8b8a797b1c04 --- standalone/memtag.h | 1 + standalone/tests/memtag_test.cpp | 186 +++++++++++++++++++++++++++++++++++++ standalone/tests/scudo_unit_test.h | 6 ++ 3 files changed, 193 insertions(+) create mode 100644 standalone/tests/memtag_test.cpp diff --git a/standalone/memtag.h b/standalone/memtag.h index 13e9939cae7..0c47f67a574 100644 --- a/standalone/memtag.h +++ b/standalone/memtag.h @@ -156,6 +156,7 @@ inline uptr selectRandomTag(uptr Ptr, uptr ExcludeMask) { inline uptr addFixedTag(uptr Ptr, uptr Tag) { DCHECK_LT(Tag, 16); + DCHECK_EQ(untagPointer(Ptr), Ptr); return Ptr | (Tag << 56); } diff --git a/standalone/tests/memtag_test.cpp b/standalone/tests/memtag_test.cpp new file mode 100644 index 00000000000..13c853f40e0 --- /dev/null +++ b/standalone/tests/memtag_test.cpp @@ -0,0 +1,186 @@ +//===-- memtag_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 "common.h" +#include "memtag.h" +#include "platform.h" +#include "tests/scudo_unit_test.h" + +#if SCUDO_LINUX +namespace scudo { + +TEST(MemtagBasicTest, Unsupported) { + if (archSupportsMemoryTagging()) + GTEST_SKIP(); + + EXPECT_DEATH(archMemoryTagGranuleSize(), "not supported"); + EXPECT_DEATH(untagPointer((uptr)0), "not supported"); + EXPECT_DEATH(extractTag((uptr)0), "not supported"); + + EXPECT_DEATH(systemSupportsMemoryTagging(), "not supported"); + EXPECT_DEATH(systemDetectsMemoryTagFaultsTestOnly(), "not supported"); + EXPECT_DEATH(enableSystemMemoryTaggingTestOnly(), "not supported"); + + EXPECT_DEATH(selectRandomTag((uptr)0, 0), "not supported"); + EXPECT_DEATH(addFixedTag((uptr)0, 1), "not supported"); + EXPECT_DEATH(storeTags((uptr)0, (uptr)0 + sizeof(0)), "not supported"); + EXPECT_DEATH(storeTag((uptr)0), "not supported"); + EXPECT_DEATH(loadTag((uptr)0), "not supported"); + + EXPECT_DEATH(setRandomTag(nullptr, 64, 0, nullptr, nullptr), "not supported"); + EXPECT_DEATH(untagPointer(nullptr), "not supported"); + EXPECT_DEATH(loadTag(nullptr), "not supported"); + EXPECT_DEATH(addFixedTag(nullptr, 0), "not supported"); +} + +class MemtagTest : public ::testing::Test { +protected: + void SetUp() override { + if (!archSupportsMemoryTagging() || !systemDetectsMemoryTagFaultsTestOnly()) + GTEST_SKIP() << "Memory tagging is not supported"; + + BufferSize = getPageSizeCached(); + Buffer = reinterpret_cast( + map(nullptr, BufferSize, "MemtagTest", MAP_MEMTAG, &Data)); + Addr = reinterpret_cast(Buffer); + EXPECT_TRUE(isAligned(Addr, archMemoryTagGranuleSize())); + EXPECT_EQ(Addr, untagPointer(Addr)); + } + + void TearDown() override { + if (Buffer) + unmap(Buffer, BufferSize, 0, &Data); + } + + uptr BufferSize = 0; + MapPlatformData Data = {}; + u8 *Buffer = nullptr; + uptr Addr = 0; +}; + +TEST_F(MemtagTest, ArchMemoryTagGranuleSize) { + EXPECT_GT(archMemoryTagGranuleSize(), 1u); + EXPECT_TRUE(isPowerOfTwo(archMemoryTagGranuleSize())); +} + +TEST_F(MemtagTest, ExtractTag) { + uptr Tags = 0; + // Try all value for the top byte and check the tags values are in the + // expected range. + for (u64 Top = 0; Top < 0x100; ++Top) + Tags = Tags | (1u << extractTag(Addr | (Top << 56))); + EXPECT_EQ(0xffff, Tags); +} + +TEST_F(MemtagTest, AddFixedTag) { + for (uptr Tag = 0; Tag < 0x10; ++Tag) + EXPECT_EQ(Tag, extractTag(addFixedTag(Addr, Tag))); + if (SCUDO_DEBUG) { + EXPECT_DEBUG_DEATH(addFixedTag(Addr, 16), ""); + EXPECT_DEBUG_DEATH(addFixedTag(~Addr, 0), ""); + } +} + +TEST_F(MemtagTest, UntagPointer) { + uptr UnTagMask = untagPointer(~uptr(0)); + for (u64 Top = 0; Top < 0x100; ++Top) { + uptr Ptr = (Addr | (Top << 56)) & UnTagMask; + EXPECT_EQ(addFixedTag(Ptr, 0), untagPointer(Ptr)); + } +} + +TEST_F(MemtagTest, ScopedDisableMemoryTagChecks) { + u8 *P = reinterpret_cast(addFixedTag(Addr, 1)); + EXPECT_NE(P, Buffer); + + EXPECT_DEATH(*P = 20, ""); + ScopedDisableMemoryTagChecks Disable; + *P = 10; +} + +TEST_F(MemtagTest, SelectRandomTag) { + for (uptr SrcTag = 0; SrcTag < 0x10; ++SrcTag) { + uptr Ptr = addFixedTag(Addr, SrcTag); + uptr Tags = 0; + for (uptr I = 0; I < 100000; ++I) + Tags = Tags | (1u << extractTag(selectRandomTag(Ptr, 0))); + EXPECT_EQ(0xfffe, Tags); + } +} + +TEST_F(MemtagTest, SelectRandomTagWithMask) { + for (uptr j = 0; j < 32; ++j) { + for (uptr i = 0; i < 1000; ++i) + EXPECT_NE(j, extractTag(selectRandomTag(Addr, 1ull << j))); + } +} + +TEST_F(MemtagTest, SKIP_NO_DEBUG(LoadStoreTagUnaligned)) { + for (uptr P = Addr; P < Addr + 4 * archMemoryTagGranuleSize(); ++P) { + if (P % archMemoryTagGranuleSize() == 0) + continue; + EXPECT_DEBUG_DEATH(loadTag(P), ""); + EXPECT_DEBUG_DEATH(storeTag(P), ""); + } +} + +TEST_F(MemtagTest, LoadStoreTag) { + uptr Base = Addr + 0x100; + uptr Tagged = addFixedTag(Base, 7); + storeTag(Tagged); + + EXPECT_EQ(Base - archMemoryTagGranuleSize(), + loadTag(Base - archMemoryTagGranuleSize())); + EXPECT_EQ(Tagged, loadTag(Base)); + EXPECT_EQ(Base + archMemoryTagGranuleSize(), + loadTag(Base + archMemoryTagGranuleSize())); +} + +TEST_F(MemtagTest, SKIP_NO_DEBUG(StoreTagsUnaligned)) { + for (uptr P = Addr; P < Addr + 4 * archMemoryTagGranuleSize(); ++P) { + uptr Tagged = addFixedTag(P, 5); + if (Tagged % archMemoryTagGranuleSize() == 0) + continue; + EXPECT_DEBUG_DEATH(storeTags(Tagged, Tagged), ""); + } +} + +TEST_F(MemtagTest, StoreTags) { + const uptr MaxTaggedSize = 4 * archMemoryTagGranuleSize(); + for (uptr Size = 0; Size <= MaxTaggedSize; ++Size) { + uptr NoTagBegin = Addr + archMemoryTagGranuleSize(); + uptr NoTagEnd = NoTagBegin + Size; + + u8 Tag = 5; + + uptr TaggedBegin = addFixedTag(NoTagBegin, Tag); + uptr TaggedEnd = addFixedTag(NoTagEnd, Tag); + + EXPECT_EQ(roundUpTo(TaggedEnd, archMemoryTagGranuleSize()), + storeTags(TaggedBegin, TaggedEnd)); + + uptr LoadPtr = Addr; + // Untagged left granule. + EXPECT_EQ(LoadPtr, loadTag(LoadPtr)); + + for (LoadPtr += archMemoryTagGranuleSize(); LoadPtr < NoTagEnd; + LoadPtr += archMemoryTagGranuleSize()) { + EXPECT_EQ(addFixedTag(LoadPtr, 5), loadTag(LoadPtr)); + } + + // Untagged right granule. + EXPECT_EQ(LoadPtr, loadTag(LoadPtr)); + + // Reset tags without using StoreTags. + releasePagesToOS(Addr, 0, BufferSize, &Data); + } +} + +} // namespace scudo + +#endif diff --git a/standalone/tests/scudo_unit_test.h b/standalone/tests/scudo_unit_test.h index 7b8218a1b28..1665fa87e5f 100644 --- a/standalone/tests/scudo_unit_test.h +++ b/standalone/tests/scudo_unit_test.h @@ -39,4 +39,10 @@ using Test = ::testing::Test; #define SKIP_ON_FUCHSIA(T) T #endif +#if SCUDO_DEBUG +#define SKIP_NO_DEBUG(T) T +#else +#define SKIP_NO_DEBUG(T) DISABLED_##T +#endif + extern bool UseQuarantine; -- cgit v1.2.3 From a78a4a7f70cc7cad99e2a77f20c51e31517f64bc Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Sat, 29 May 2021 17:18:17 -0700 Subject: [scudo] Untag pointer in iterateOverChunks Pointer comparison in Lambda will not work on tagged pointers. Reviewed By: pcc Differential Revision: https://reviews.llvm.org/D103496 GitOrigin-RevId: 1e6d135325357d8c32fd0b0d7f668cad91d478bc Change-Id: Icc46fb4d685cf727ea4007becfc40a8ea61c6fe3 --- standalone/combined.h | 2 ++ standalone/tests/wrappers_c_test.cpp | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/standalone/combined.h b/standalone/combined.h index f58eaa945af..079edab1875 100644 --- a/standalone/combined.h +++ b/standalone/combined.h @@ -727,6 +727,8 @@ public: void iterateOverChunks(uptr Base, uptr Size, iterate_callback Callback, void *Arg) { initThreadMaybe(); + if (archSupportsMemoryTagging()) + Base = untagPointer(Base); const uptr From = Base; const uptr To = Base + Size; bool MayHaveTaggedPrimary = allocatorSupportsMemoryTagging() && diff --git a/standalone/tests/wrappers_c_test.cpp b/standalone/tests/wrappers_c_test.cpp index eed8f031933..62410fc393a 100644 --- a/standalone/tests/wrappers_c_test.cpp +++ b/standalone/tests/wrappers_c_test.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "memtag.h" #include "scudo/interface.h" #include "tests/scudo_unit_test.h" @@ -277,6 +278,10 @@ static uintptr_t BoundaryP; static size_t Count; static void callback(uintptr_t Base, size_t Size, void *Arg) { + if (scudo::archSupportsMemoryTagging()) { + Base = scudo::untagPointer(Base); + BoundaryP = scudo::untagPointer(BoundaryP); + } if (Base == BoundaryP) Count++; } -- cgit v1.2.3 From 02efe40f2645ae747da76bde96e9aaa0fe115b8e Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Fri, 4 Jun 2021 13:38:26 -0700 Subject: [scudo] Fix String DCHECK This resolves an issue tripping a `DCHECK`, as I was checking for the capacity and not the size. We don't need to 0-init the Vector as it's done already, and make sure we only 0-out the string on clear if it's not empty. Differential Revision: https://reviews.llvm.org/D103716 GitOrigin-RevId: 5019b0a56588fc13e6a37c49e22a812afe6c4416 Change-Id: I0aacc20416a642198d0e49d1559c259621f625db --- standalone/string_utils.h | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/standalone/string_utils.h b/standalone/string_utils.h index 7a57da3a53f..7c1cd575306 100644 --- a/standalone/string_utils.h +++ b/standalone/string_utils.h @@ -18,14 +18,12 @@ namespace scudo { class ScopedString { public: - explicit ScopedString() : String() { - if (String.capacity() > 0) - String[0] = '\0'; - } + explicit ScopedString() : String() {} uptr length() { return Length; } const char *data() { return String.data(); } void clear() { - String[0] = '\0'; + if (!String.empty()) + String[0] = '\0'; Length = 0; } void append(const char *Format, va_list Args); -- cgit v1.2.3 From 5a56cb468245eea50ef466dc4864feea37e25cb7 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 4 Jun 2021 18:22:32 -0700 Subject: [NFC][scudo] Fix sign-compare warning in test GitOrigin-RevId: b850798f11d3428d6edc22ad63b9cabe2e6baec1 Change-Id: I7d8995c9661a0e736d310bc37c4ad3c1e2be7ed2 --- standalone/tests/memtag_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/standalone/tests/memtag_test.cpp b/standalone/tests/memtag_test.cpp index 13c853f40e0..8726fc2e171 100644 --- a/standalone/tests/memtag_test.cpp +++ b/standalone/tests/memtag_test.cpp @@ -74,7 +74,7 @@ TEST_F(MemtagTest, ExtractTag) { // expected range. for (u64 Top = 0; Top < 0x100; ++Top) Tags = Tags | (1u << extractTag(Addr | (Top << 56))); - EXPECT_EQ(0xffff, Tags); + EXPECT_EQ(0xffffull, Tags); } TEST_F(MemtagTest, AddFixedTag) { -- cgit v1.2.3 From 04f3ad960c5d8b97c2587fbea2a8cb9cf268d86b Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 4 Jun 2021 13:48:54 -0700 Subject: [Scudo] Improve ScopedString constructor Avoid referencing elements beyond internal vector size. Reviewed By: cryptoad Differential Revision: https://reviews.llvm.org/D103718 GitOrigin-RevId: df87aeb8268240a1f9042aff1450d84900c52ae5 Change-Id: Ib3e665a97b9bd45098f907bcb4e79cfb529fd562 --- standalone/string_utils.h | 6 +++--- standalone/tests/strings_test.cpp | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/standalone/string_utils.h b/standalone/string_utils.h index 7c1cd575306..7d9c539c271 100644 --- a/standalone/string_utils.h +++ b/standalone/string_utils.h @@ -18,12 +18,12 @@ namespace scudo { class ScopedString { public: - explicit ScopedString() : String() {} + explicit ScopedString() { String.push_back('\0'); } uptr length() { return Length; } const char *data() { return String.data(); } void clear() { - if (!String.empty()) - String[0] = '\0'; + String.clear(); + String.push_back('\0'); Length = 0; } void append(const char *Format, va_list Args); diff --git a/standalone/tests/strings_test.cpp b/standalone/tests/strings_test.cpp index 332bac2f796..298977a14c8 100644 --- a/standalone/tests/strings_test.cpp +++ b/standalone/tests/strings_test.cpp @@ -12,6 +12,12 @@ #include +TEST(ScudoStringsTest, Constructor) { + scudo::ScopedString Str; + EXPECT_EQ(0, Str.length()); + EXPECT_EQ('\0', *Str.data()); +} + TEST(ScudoStringsTest, Basic) { scudo::ScopedString Str; Str.append("a%db%zdc%ue%zuf%xh%zxq%pe%sr", static_cast(-1), @@ -28,6 +34,23 @@ TEST(ScudoStringsTest, Basic) { EXPECT_STREQ(expectedString.c_str(), Str.data()); } +TEST(ScudoStringsTest, Clear) { + scudo::ScopedString Str; + Str.append("123"); + Str.clear(); + EXPECT_EQ(0, Str.length()); + EXPECT_EQ('\0', *Str.data()); +} + +TEST(ScudoStringsTest, ClearLarge) { + scudo::ScopedString Str; + for (int i = 0; i < 10000; ++i) + Str.append("123"); + Str.clear(); + EXPECT_EQ(0, Str.length()); + EXPECT_EQ('\0', *Str.data()); +} + TEST(ScudoStringsTest, Precision) { scudo::ScopedString Str; Str.append("%.*s", 3, "12345"); -- cgit v1.2.3 From 6eb890afb629b97bf40e52105146685983f6e978 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 4 Jun 2021 15:56:13 -0700 Subject: [scudo] Remove ScopedString::Length Differential Revision: https://reviews.llvm.org/D103725 GitOrigin-RevId: 70b29213eb3446ffb85678da3cc447f478e27e15 Change-Id: I604f0d564ad9612bd146e59c578624066f32a63c --- standalone/string_utils.cpp | 9 +++++---- standalone/string_utils.h | 4 +--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/standalone/string_utils.cpp b/standalone/string_utils.cpp index ca80834cca8..acf85889fcf 100644 --- a/standalone/string_utils.cpp +++ b/standalone/string_utils.cpp @@ -219,7 +219,6 @@ int formatString(char *Buffer, uptr BufferLength, const char *Format, ...) { } void ScopedString::append(const char *Format, va_list Args) { - RAW_CHECK(Length <= String.size()); va_list ArgsCopy; va_copy(ArgsCopy, Args); // formatString doesn't currently support a null buffer or zero buffer length, @@ -228,11 +227,13 @@ void ScopedString::append(const char *Format, va_list Args) { char C[1]; const uptr AdditionalLength = static_cast(formatString(C, sizeof(C), Format, Args)) + 1; + const uptr Length = length(); String.resize(Length + AdditionalLength); - formatString(String.data() + Length, AdditionalLength, Format, ArgsCopy); + const uptr FormattedLength = static_cast(formatString( + String.data() + Length, String.size() - Length, Format, ArgsCopy)); + RAW_CHECK(data()[length()] == '\0'); + RAW_CHECK(FormattedLength + 1 == AdditionalLength); va_end(ArgsCopy); - Length = strlen(String.data()); - RAW_CHECK(Length < String.size()); } FORMAT(2, 3) diff --git a/standalone/string_utils.h b/standalone/string_utils.h index 7d9c539c271..06d23d42246 100644 --- a/standalone/string_utils.h +++ b/standalone/string_utils.h @@ -19,12 +19,11 @@ namespace scudo { class ScopedString { public: explicit ScopedString() { String.push_back('\0'); } - uptr length() { return Length; } + uptr length() { return String.size() - 1; } const char *data() { return String.data(); } void clear() { String.clear(); String.push_back('\0'); - Length = 0; } void append(const char *Format, va_list Args); void append(const char *Format, ...); @@ -32,7 +31,6 @@ public: private: Vector String; - uptr Length = 0; }; int formatString(char *Buffer, uptr BufferLength, const char *Format, ...); -- cgit v1.2.3 From 1a0edff9a72bc8b76bbec8ca683430f2a7595ac9 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 4 Jun 2021 19:26:03 -0700 Subject: [NFC][scudo] Convert to TYPED more ScudoCombinedTest GitOrigin-RevId: 57ba226296411f0a3918573479ca750e375c13c0 Change-Id: Ifc49271cc9e8095453b3d24aa6e49251dd6d725a --- standalone/tests/combined_test.cpp | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index 50b55bb089f..afa064e60bc 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -559,15 +559,6 @@ TEST(ScudoCombinedTest, DeathCombined) { EXPECT_DEATH(Allocator->getUsableSize(P), ""); } -// Ensure that releaseToOS can be called prior to any other allocator -// operation without issue. -TEST(ScudoCombinedTest, ReleaseToOS) { - using AllocatorT = TestAllocator; - auto Allocator = std::unique_ptr(new AllocatorT()); - - Allocator->releaseToOS(); -} - // Verify that when a region gets full, the allocator will still manage to // fulfill the allocation through a larger size class. TEST(ScudoCombinedTest, FullRegion) { @@ -600,10 +591,15 @@ TEST(ScudoCombinedTest, FullRegion) { EXPECT_EQ(FailedAllocationsCount, 0U); } -TEST(ScudoCombinedTest, OddEven) { - using AllocatorT = TestAllocator; - using SizeClassMap = AllocatorT::PrimaryT::SizeClassMap; - auto Allocator = std::unique_ptr(new AllocatorT()); +// Ensure that releaseToOS can be called prior to any other allocator +// operation without issue. +SCUDO_TYPED_TEST(ScudoCombinedTest, ReleaseToOS) { + auto *Allocator = this->Allocator.get(); + Allocator->releaseToOS(); +} + +SCUDO_TYPED_TEST(ScudoCombinedTest, OddEven) { + auto *Allocator = this->Allocator.get(); if (!Allocator->useMemoryTaggingTestOnly()) return; @@ -614,6 +610,7 @@ TEST(ScudoCombinedTest, OddEven) { EXPECT_NE(Tag1 % 2, Tag2 % 2); }; + using SizeClassMap = typename TypeParam::Primary::SizeClassMap; for (scudo::uptr ClassId = 1U; ClassId <= SizeClassMap::LargestClassId; ClassId++) { const scudo::uptr Size = SizeClassMap::getSizeByClassId(ClassId); @@ -639,10 +636,8 @@ TEST(ScudoCombinedTest, OddEven) { } } -TEST(ScudoCombinedTest, DisableMemInit) { - using AllocatorT = TestAllocator; - using SizeClassMap = AllocatorT::PrimaryT::SizeClassMap; - auto Allocator = std::unique_ptr(new AllocatorT()); +SCUDO_TYPED_TEST(ScudoCombinedTest, DisableMemInit) { + auto *Allocator = this->Allocator.get(); std::vector Ptrs(65536, nullptr); @@ -654,6 +649,7 @@ TEST(ScudoCombinedTest, DisableMemInit) { // expected. This is tricky to ensure when MTE is enabled, so this test tries // to exercise the relevant code on our MTE path. for (scudo::uptr ClassId = 1U; ClassId <= 8; ClassId++) { + using SizeClassMap = typename TypeParam::Primary::SizeClassMap; const scudo::uptr Size = SizeClassMap::getSizeByClassId(ClassId) - scudo::Chunk::getHeaderSize(); if (Size < 8) -- cgit v1.2.3 From c1d01dffbd44aeb3840b50d8a7d95a2660ada8e4 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 4 Jun 2021 19:32:24 -0700 Subject: [NFC][scudo] Fix sign-compare warning in test GitOrigin-RevId: 9ff982dbbdd4d0f851e0b32440e13ac92635ac6f Change-Id: I230ec7abd3c334b4a104fdc97321bfbb3fdfc4de --- standalone/tests/memtag_test.cpp | 2 +- standalone/tests/strings_test.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/standalone/tests/memtag_test.cpp b/standalone/tests/memtag_test.cpp index 8726fc2e171..50ba0fc82ce 100644 --- a/standalone/tests/memtag_test.cpp +++ b/standalone/tests/memtag_test.cpp @@ -109,7 +109,7 @@ TEST_F(MemtagTest, SelectRandomTag) { uptr Tags = 0; for (uptr I = 0; I < 100000; ++I) Tags = Tags | (1u << extractTag(selectRandomTag(Ptr, 0))); - EXPECT_EQ(0xfffe, Tags); + EXPECT_EQ(0xfffeull, Tags); } } diff --git a/standalone/tests/strings_test.cpp b/standalone/tests/strings_test.cpp index 298977a14c8..6d7e78a816a 100644 --- a/standalone/tests/strings_test.cpp +++ b/standalone/tests/strings_test.cpp @@ -14,7 +14,7 @@ TEST(ScudoStringsTest, Constructor) { scudo::ScopedString Str; - EXPECT_EQ(0, Str.length()); + EXPECT_EQ(0ul, Str.length()); EXPECT_EQ('\0', *Str.data()); } @@ -38,7 +38,7 @@ TEST(ScudoStringsTest, Clear) { scudo::ScopedString Str; Str.append("123"); Str.clear(); - EXPECT_EQ(0, Str.length()); + EXPECT_EQ(0ul, Str.length()); EXPECT_EQ('\0', *Str.data()); } @@ -47,7 +47,7 @@ TEST(ScudoStringsTest, ClearLarge) { for (int i = 0; i < 10000; ++i) Str.append("123"); Str.clear(); - EXPECT_EQ(0, Str.length()); + EXPECT_EQ(0ul, Str.length()); EXPECT_EQ('\0', *Str.data()); } -- cgit v1.2.3 From cfd920459073cedbf63e2a75697e920718405582 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Mon, 7 Jun 2021 18:54:14 -0700 Subject: [NFC][scudo] Print errno of fork failure This fork fails sometime on sanitizer-x86_64-linux-qemu bot. GitOrigin-RevId: b41b76b303cdfde389f70e0a311698d7391423f1 Change-Id: I9a1fcb460f3b75142753c5c927f50511e99f195a --- standalone/tests/wrappers_c_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/standalone/tests/wrappers_c_test.cpp b/standalone/tests/wrappers_c_test.cpp index 62410fc393a..b82b5fb4b51 100644 --- a/standalone/tests/wrappers_c_test.cpp +++ b/standalone/tests/wrappers_c_test.cpp @@ -371,7 +371,7 @@ TEST(ScudoWrappersCTest, MallocInfo) { TEST(ScudoWrappersCTest, Fork) { void *P; pid_t Pid = fork(); - EXPECT_GE(Pid, 0); + EXPECT_GE(Pid, 0) << strerror(errno); if (Pid == 0) { P = malloc(Size); EXPECT_NE(P, nullptr); -- cgit v1.2.3 From 63933f12612917323ce76fd5a14c7f1de8c5fd91 Mon Sep 17 00:00:00 2001 From: Daniel Michael Date: Mon, 7 Jun 2021 09:10:46 -0700 Subject: [scudo] Add Scudo support for Trusty OS trusty.cpp and trusty.h define Trusty implementations of map and other platform-specific functions. In addition to adding Trusty configurations in allocator_config.h and size_class_map.h, MapSizeIncrement and PrimaryEnableRandomOffset are added as configurable options in allocator_config.h. Background on Trusty: https://source.android.com/security/trusty Differential Revision: https://reviews.llvm.org/D103578 GitOrigin-RevId: 2551053e8d8df464d5b60e7c9b0add8f85cc1e10 Change-Id: I790ba0e1932e7d2174b5cf8bb2c1393aec9cf6f6 --- standalone/allocator_config.h | 36 +++++++++++++ standalone/common.h | 1 + standalone/platform.h | 8 ++- standalone/primary32.h | 3 ++ standalone/primary64.h | 17 ++++--- standalone/size_class_map.h | 14 ++++++ standalone/tests/combined_test.cpp | 2 + standalone/tests/primary_test.cpp | 8 +++ standalone/trusty.cpp | 100 +++++++++++++++++++++++++++++++++++++ standalone/trusty.h | 24 +++++++++ 10 files changed, 205 insertions(+), 8 deletions(-) create mode 100644 standalone/trusty.cpp create mode 100644 standalone/trusty.h diff --git a/standalone/allocator_config.h b/standalone/allocator_config.h index 36c160637b5..7f556360c62 100644 --- a/standalone/allocator_config.h +++ b/standalone/allocator_config.h @@ -40,6 +40,12 @@ namespace scudo { // // eg: Ptr = Base + (CompactPtr << Scale). // typedef u32 PrimaryCompactPtrT; // static const uptr PrimaryCompactPtrScale = SCUDO_MIN_ALIGNMENT_LOG; +// // Indicates support for offsetting the start of a region by +// // a random number of pages. Only used with primary64. +// static const bool PrimaryEnableRandomOffset = true; +// // Call map for user memory with at least this size. Only used with +// // primary64. +// static const uptr PrimaryMapSizeIncrement = 1UL << 18; // // Defines the minimal & maximal release interval that can be set. // static const s32 PrimaryMinReleaseToOsIntervalMs = INT32_MIN; // static const s32 PrimaryMaxReleaseToOsIntervalMs = INT32_MAX; @@ -61,6 +67,8 @@ struct DefaultConfig { static const uptr PrimaryRegionSizeLog = 32U; typedef uptr PrimaryCompactPtrT; static const uptr PrimaryCompactPtrScale = 0; + static const bool PrimaryEnableRandomOffset = true; + static const uptr PrimaryMapSizeIncrement = 1UL << 18; #else typedef SizeClassAllocator32 Primary; static const uptr PrimaryRegionSizeLog = 19U; @@ -89,6 +97,8 @@ struct AndroidConfig { static const uptr PrimaryRegionSizeLog = 28U; typedef u32 PrimaryCompactPtrT; static const uptr PrimaryCompactPtrScale = SCUDO_MIN_ALIGNMENT_LOG; + static const bool PrimaryEnableRandomOffset = true; + static const uptr PrimaryMapSizeIncrement = 1UL << 18; #else typedef SizeClassAllocator32 Primary; static const uptr PrimaryRegionSizeLog = 18U; @@ -118,6 +128,8 @@ struct AndroidSvelteConfig { static const uptr PrimaryRegionSizeLog = 27U; typedef u32 PrimaryCompactPtrT; static const uptr PrimaryCompactPtrScale = SCUDO_MIN_ALIGNMENT_LOG; + static const bool PrimaryEnableRandomOffset = true; + static const uptr PrimaryMapSizeIncrement = 1UL << 18; #else typedef SizeClassAllocator32 Primary; static const uptr PrimaryRegionSizeLog = 16U; @@ -146,6 +158,8 @@ struct FuchsiaConfig { typedef SizeClassAllocator64 Primary; static const uptr PrimaryRegionSizeLog = 30U; typedef u32 PrimaryCompactPtrT; + static const bool PrimaryEnableRandomOffset = true; + static const uptr PrimaryMapSizeIncrement = 1UL << 18; static const uptr PrimaryCompactPtrScale = SCUDO_MIN_ALIGNMENT_LOG; static const s32 PrimaryMinReleaseToOsIntervalMs = INT32_MIN; static const s32 PrimaryMaxReleaseToOsIntervalMs = INT32_MAX; @@ -154,12 +168,34 @@ struct FuchsiaConfig { template using TSDRegistryT = TSDRegistrySharedT; // Shared, max 8 TSDs. }; + +struct TrustyConfig { + using SizeClassMap = TrustySizeClassMap; + static const bool MaySupportMemoryTagging = false; + + typedef SizeClassAllocator64 Primary; + // Some apps have 1 page of heap total so small regions are necessary. + static const uptr PrimaryRegionSizeLog = 10U; + typedef u32 PrimaryCompactPtrT; + static const bool PrimaryEnableRandomOffset = false; + // Trusty is extremely memory-constrained so minimally round up map calls. + static const uptr PrimaryMapSizeIncrement = 1UL << 4; + static const uptr PrimaryCompactPtrScale = SCUDO_MIN_ALIGNMENT_LOG; + static const s32 PrimaryMinReleaseToOsIntervalMs = INT32_MIN; + static const s32 PrimaryMaxReleaseToOsIntervalMs = INT32_MAX; + + typedef MapAllocatorNoCache SecondaryCache; + template + using TSDRegistryT = TSDRegistrySharedT; // Shared, max 1 TSD. +}; #endif #if SCUDO_ANDROID typedef AndroidConfig Config; #elif SCUDO_FUCHSIA typedef FuchsiaConfig Config; +#elif SCUDO_TRUSTY +typedef TrustyConfig Config; #else typedef DefaultConfig Config; #endif diff --git a/standalone/common.h b/standalone/common.h index 3f27a3d3e1b..bc3dfec6dbb 100644 --- a/standalone/common.h +++ b/standalone/common.h @@ -13,6 +13,7 @@ #include "fuchsia.h" #include "linux.h" +#include "trusty.h" #include #include diff --git a/standalone/platform.h b/standalone/platform.h index a4c2a0b2660..36378d14d84 100644 --- a/standalone/platform.h +++ b/standalone/platform.h @@ -12,7 +12,7 @@ // Transitive includes of stdint.h specify some of the defines checked below. #include -#if defined(__linux__) +#if defined(__linux__) && !defined(__TRUSTY__) #define SCUDO_LINUX 1 #else #define SCUDO_LINUX 0 @@ -31,6 +31,12 @@ #define SCUDO_FUCHSIA 0 #endif +#if defined(__TRUSTY__) +#define SCUDO_TRUSTY 1 +#else +#define SCUDO_TRUSTY 0 +#endif + #if __LP64__ #define SCUDO_WORDSIZE 64U #else diff --git a/standalone/primary32.h b/standalone/primary32.h index 059680b0613..36ae083cfc7 100644 --- a/standalone/primary32.h +++ b/standalone/primary32.h @@ -64,6 +64,9 @@ public: if (SCUDO_FUCHSIA) reportError("SizeClassAllocator32 is not supported on Fuchsia"); + if (SCUDO_TRUSTY) + reportError("SizeClassAllocator32 is not supported on Trusty"); + PossibleRegions.init(); u32 Seed; diff --git a/standalone/primary64.h b/standalone/primary64.h index a3456b6b99c..27634c96c71 100644 --- a/standalone/primary64.h +++ b/standalone/primary64.h @@ -25,8 +25,9 @@ namespace scudo { // // It starts by reserving NumClasses * 2^RegionSizeLog bytes, equally divided in // Regions, specific to each size class. Note that the base of that mapping is -// random (based to the platform specific map() capabilities), and that each -// Region actually starts at a random offset from its base. +// random (based to the platform specific map() capabilities). If +// PrimaryEnableRandomOffset is set, each Region actually starts at a random +// offset from its base. // // Regions are mapped incrementally on demand to fulfill allocation requests, // those mappings being split into equally sized Blocks based on the size class @@ -70,9 +71,12 @@ public: const uptr PageSize = getPageSizeCached(); for (uptr I = 0; I < NumClasses; I++) { RegionInfo *Region = getRegionInfo(I); - // The actual start of a region is offseted by a random number of pages. - Region->RegionBeg = - getRegionBaseByClassId(I) + (getRandomModN(&Seed, 16) + 1) * PageSize; + // The actual start of a region is offset by a random number of pages + // when PrimaryEnableRandomOffset is set. + Region->RegionBeg = getRegionBaseByClassId(I) + + (Config::PrimaryEnableRandomOffset + ? ((getRandomModN(&Seed, 16) + 1) * PageSize) + : 0); Region->RandState = getRandomU32(&Seed); Region->ReleaseInfo.LastReleaseAtNs = Time; } @@ -267,8 +271,7 @@ private: static const uptr NumClasses = SizeClassMap::NumClasses; static const uptr PrimarySize = RegionSize * NumClasses; - // Call map for user memory with at least this size. - static const uptr MapSizeIncrement = 1UL << 18; + static const uptr MapSizeIncrement = Config::PrimaryMapSizeIncrement; // Fill at most this number of batches from the newly map'd memory. static const u32 MaxNumBatches = SCUDO_ANDROID ? 4U : 8U; diff --git a/standalone/size_class_map.h b/standalone/size_class_map.h index e1c2edac476..ba0f78453bc 100644 --- a/standalone/size_class_map.h +++ b/standalone/size_class_map.h @@ -308,6 +308,20 @@ struct SvelteSizeClassConfig { typedef FixedSizeClassMap SvelteSizeClassMap; +// Trusty is configured to only have one region containing blocks of size +// 2^7 bytes. +struct TrustySizeClassConfig { + static const uptr NumBits = 1; + static const uptr MinSizeLog = 7; + static const uptr MidSizeLog = 7; + static const uptr MaxSizeLog = 7; + static const u32 MaxNumCachedHint = 8; + static const uptr MaxBytesCachedLog = 10; + static const uptr SizeDelta = 0; +}; + +typedef FixedSizeClassMap TrustySizeClassMap; + template inline void printMap() { ScopedString Buffer; uptr PrevS = 0; diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index afa064e60bc..6716d5df1e0 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -521,6 +521,8 @@ struct DeathConfig { static const scudo::s32 PrimaryMaxReleaseToOsIntervalMs = INT32_MAX; typedef scudo::uptr PrimaryCompactPtrT; static const scudo::uptr PrimaryCompactPtrScale = 0; + static const bool PrimaryEnableRandomOffset = true; + static const scudo::uptr PrimaryMapSizeIncrement = 1UL << 18; typedef scudo::MapAllocatorNoCache SecondaryCache; template using TSDRegistryT = scudo::TSDRegistrySharedT; diff --git a/standalone/tests/primary_test.cpp b/standalone/tests/primary_test.cpp index 93157f942c5..5ec43619a2a 100644 --- a/standalone/tests/primary_test.cpp +++ b/standalone/tests/primary_test.cpp @@ -29,6 +29,8 @@ struct TestConfig1 { static const bool MaySupportMemoryTagging = false; typedef scudo::uptr PrimaryCompactPtrT; static const scudo::uptr PrimaryCompactPtrScale = 0; + static const bool PrimaryEnableRandomOffset = true; + static const scudo::uptr PrimaryMapSizeIncrement = 1UL << 18; }; struct TestConfig2 { @@ -43,6 +45,8 @@ struct TestConfig2 { static const bool MaySupportMemoryTagging = false; typedef scudo::uptr PrimaryCompactPtrT; static const scudo::uptr PrimaryCompactPtrScale = 0; + static const bool PrimaryEnableRandomOffset = true; + static const scudo::uptr PrimaryMapSizeIncrement = 1UL << 18; }; struct TestConfig3 { @@ -57,6 +61,8 @@ struct TestConfig3 { static const bool MaySupportMemoryTagging = true; typedef scudo::uptr PrimaryCompactPtrT; static const scudo::uptr PrimaryCompactPtrScale = 0; + static const bool PrimaryEnableRandomOffset = true; + static const scudo::uptr PrimaryMapSizeIncrement = 1UL << 18; }; template @@ -145,6 +151,8 @@ struct SmallRegionsConfig { static const bool MaySupportMemoryTagging = false; typedef scudo::uptr PrimaryCompactPtrT; static const scudo::uptr PrimaryCompactPtrScale = 0; + static const bool PrimaryEnableRandomOffset = true; + static const scudo::uptr PrimaryMapSizeIncrement = 1UL << 18; }; // The 64-bit SizeClassAllocator can be easily OOM'd with small region sizes. diff --git a/standalone/trusty.cpp b/standalone/trusty.cpp new file mode 100644 index 00000000000..81d6bc585f0 --- /dev/null +++ b/standalone/trusty.cpp @@ -0,0 +1,100 @@ +//===-- trusty.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 "platform.h" + +#if SCUDO_TRUSTY + +#include "common.h" +#include "mutex.h" +#include "string_utils.h" +#include "trusty.h" + +#include // for errno +#include // for printf() +#include // for getenv() +#include // for getauxval() +#include // for clock_gettime() +#include // for _trusty_brk() + +#define SBRK_ALIGN 32 + +namespace scudo { + +uptr getPageSize() { return getauxval(AT_PAGESZ); } + +void NORETURN die() { abort(); } + +void *map(UNUSED void *Addr, uptr Size, UNUSED const char *Name, uptr Flags, + UNUSED MapPlatformData *Data) { + // Calling _trusty_brk(0) returns the current program break. + uptr ProgramBreak = reinterpret_cast(_trusty_brk(0)); + uptr Start; + uptr End; + + Start = roundUpTo(ProgramBreak, SBRK_ALIGN); + // Don't actually extend the heap if MAP_NOACCESS flag is set since this is + // the case where Scudo tries to reserve a memory region without mapping + // physical pages. + if (Flags & MAP_NOACCESS) + return reinterpret_cast(Start); + + // Attempt to extend the heap by Size bytes using _trusty_brk. + End = roundUpTo(Start + Size, SBRK_ALIGN); + ProgramBreak = + reinterpret_cast(_trusty_brk(reinterpret_cast(End))); + if (ProgramBreak < End) { + errno = ENOMEM; + dieOnMapUnmapError(Size); + return nullptr; + } + return reinterpret_cast(Start); // Base of new reserved region. +} + +// Unmap is a no-op since Trusty uses sbrk instead of memory mapping. +void unmap(UNUSED void *Addr, UNUSED uptr Size, UNUSED uptr Flags, + UNUSED MapPlatformData *Data) {} + +void setMemoryPermission(UNUSED uptr Addr, UNUSED uptr Size, UNUSED uptr Flags, + UNUSED MapPlatformData *Data) {} + +void releasePagesToOS(UNUSED uptr BaseAddress, UNUSED uptr Offset, + UNUSED uptr Size, UNUSED MapPlatformData *Data) {} + +const char *getEnv(const char *Name) { return getenv(Name); } + +// All mutex operations are a no-op since Trusty doesn't currently support +// threads. +bool HybridMutex::tryLock() { return true; } + +void HybridMutex::lockSlow() {} + +void HybridMutex::unlock() {} + +u64 getMonotonicTime() { + timespec TS; + clock_gettime(CLOCK_MONOTONIC, &TS); + return static_cast(TS.tv_sec) * (1000ULL * 1000 * 1000) + + static_cast(TS.tv_nsec); +} + +u32 getNumberOfCPUs() { return 0; } + +u32 getThreadID() { return 0; } + +bool getRandom(UNUSED void *Buffer, UNUSED uptr Length, UNUSED bool Blocking) { + return false; +} + +void outputRaw(const char *Buffer) { printf("%s", Buffer); } + +void setAbortMessage(UNUSED const char *Message) {} + +} // namespace scudo + +#endif // SCUDO_TRUSTY diff --git a/standalone/trusty.h b/standalone/trusty.h new file mode 100644 index 00000000000..50edd1c6fe6 --- /dev/null +++ b/standalone/trusty.h @@ -0,0 +1,24 @@ +//===-- trusty.h -----------------------------------------------*- 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 +// +//===----------------------------------------------------------------------===// + +#ifndef SCUDO_TRUSTY_H_ +#define SCUDO_TRUSTY_H_ + +#include "platform.h" + +#if SCUDO_TRUSTY + +namespace scudo { +// MapPlatformData is unused on Trusty, define it as a minimially sized +// structure. +struct MapPlatformData {}; +} // namespace scudo + +#endif // SCUDO_TRUSTY + +#endif // SCUDO_TRUSTY_H_ -- cgit v1.2.3 From 43839dc4211b18caeb87c3889d6ac09eaf388f77 Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Wed, 16 Jun 2021 10:51:51 -0700 Subject: [scudo] Ensure proper allocator alignment in TSD test The `MockAllocator` used in `ScudoTSDTest` wasn't allocated properly aligned, which resulted in the `TSDs` of the shared registry not being aligned either. This lead to some failures like: https://reviews.llvm.org/D103119#2822008 This changes how the `MockAllocator` is allocated, same as Vitaly did in the combined tests, properly aligning it, which results in the `TSDs` being aligned as well. Add a `DCHECK` in the shared registry to check that it is. Differential Revision: https://reviews.llvm.org/D104402 GitOrigin-RevId: 8b062b61606270950645d73b68036c9ab2c7c4bc Change-Id: I738df5f9848d58d32c11f5be676c44833d873bc2 --- standalone/primary32.h | 2 +- standalone/primary64.h | 1 + standalone/quarantine.h | 2 ++ standalone/tests/tsd_test.cpp | 9 +++++++++ standalone/tsd.h | 2 ++ 5 files changed, 15 insertions(+), 1 deletion(-) diff --git a/standalone/primary32.h b/standalone/primary32.h index 36ae083cfc7..326c10a32a8 100644 --- a/standalone/primary32.h +++ b/standalone/primary32.h @@ -67,8 +67,8 @@ public: if (SCUDO_TRUSTY) reportError("SizeClassAllocator32 is not supported on Trusty"); + DCHECK(isAligned(reinterpret_cast(this), alignof(ThisT))); PossibleRegions.init(); - u32 Seed; const u64 Time = getMonotonicTime(); if (!getRandom(reinterpret_cast(&Seed), sizeof(Seed))) diff --git a/standalone/primary64.h b/standalone/primary64.h index 27634c96c71..13420bf3d22 100644 --- a/standalone/primary64.h +++ b/standalone/primary64.h @@ -59,6 +59,7 @@ public: static bool canAllocate(uptr Size) { return Size <= SizeClassMap::MaxSize; } void init(s32 ReleaseToOsInterval) { + DCHECK(isAligned(reinterpret_cast(this), alignof(ThisT))); DCHECK_EQ(PrimaryBase, 0U); // Reserve the space required for the Primary. PrimaryBase = reinterpret_cast( diff --git a/standalone/quarantine.h b/standalone/quarantine.h index 84eb90cc0b3..2d231c3a28d 100644 --- a/standalone/quarantine.h +++ b/standalone/quarantine.h @@ -170,8 +170,10 @@ private: template class GlobalQuarantine { public: typedef QuarantineCache CacheT; + using ThisT = GlobalQuarantine; void init(uptr Size, uptr CacheSize) { + DCHECK(isAligned(reinterpret_cast(this), alignof(ThisT))); DCHECK_EQ(atomic_load_relaxed(&MaxSize), 0U); DCHECK_EQ(atomic_load_relaxed(&MinSize), 0U); DCHECK_EQ(atomic_load_relaxed(&MaxCacheSize), 0U); diff --git a/standalone/tests/tsd_test.cpp b/standalone/tests/tsd_test.cpp index 35e579d5dc5..17387ee7c57 100644 --- a/standalone/tests/tsd_test.cpp +++ b/standalone/tests/tsd_test.cpp @@ -11,6 +11,8 @@ #include "tsd_exclusive.h" #include "tsd_shared.h" +#include + #include #include #include @@ -40,6 +42,13 @@ public: bool isInitialized() { return Initialized; } + void *operator new(size_t Size) { + void *P = nullptr; + EXPECT_EQ(0, posix_memalign(&P, alignof(ThisT), Size)); + return P; + } + void operator delete(void *P) { free(P); } + private: bool Initialized = false; TSDRegistryT TSDRegistry; diff --git a/standalone/tsd.h b/standalone/tsd.h index e376df05af5..b400a3b56da 100644 --- a/standalone/tsd.h +++ b/standalone/tsd.h @@ -26,10 +26,12 @@ namespace scudo { template struct alignas(SCUDO_CACHE_LINE_SIZE) TSD { typename Allocator::CacheT Cache; typename Allocator::QuarantineCacheT QuarantineCache; + using ThisT = TSD; u8 DestructorIterations = 0; void init(Allocator *Instance) { DCHECK_EQ(DestructorIterations, 0U); + DCHECK(isAligned(reinterpret_cast(this), alignof(ThisT))); Instance->initCache(&Cache); DestructorIterations = PTHREAD_DESTRUCTOR_ITERATIONS; } -- cgit v1.2.3 From fdad5742fc12edb9dea42d3912ff210e83ee0ea1 Mon Sep 17 00:00:00 2001 From: Evgenii Stepanov Date: Tue, 22 Jun 2021 16:40:17 -0700 Subject: [scudo] Handle predefined M_MEMTAG_TUNING_* constants (NFC). Bionic may provide the definitions of M_MEMTAG_TUNING_* constants. Do not redefine them in that case. Differential Revision: https://reviews.llvm.org/D104758 GitOrigin-RevId: d693957e58ff498dba7ef1d05c08849693abff35 Change-Id: Iab42b389fe7c52716db3bd498f815ac8948f38b8 --- standalone/include/scudo/interface.h | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/standalone/include/scudo/interface.h b/standalone/include/scudo/interface.h index 0e6cf3d4e25..9b9a84623c5 100644 --- a/standalone/include/scudo/interface.h +++ b/standalone/include/scudo/interface.h @@ -120,7 +120,7 @@ size_t __scudo_get_ring_buffer_size(); // Tune the allocator's choice of memory tags to make it more likely that // a certain class of memory errors will be detected. The value argument should -// be one of the enumerators of the scudo_memtag_tuning enum below. +// be one of the M_MEMTAG_TUNING_* constants below. #ifndef M_MEMTAG_TUNING #define M_MEMTAG_TUNING -102 #endif @@ -145,13 +145,15 @@ size_t __scudo_get_ring_buffer_size(); #define M_TSDS_COUNT_MAX -202 #endif -enum scudo_memtag_tuning { - // Tune for buffer overflows. - M_MEMTAG_TUNING_BUFFER_OVERFLOW, +// Tune for buffer overflows. +#ifndef M_MEMTAG_TUNING_BUFFER_OVERFLOW +#define M_MEMTAG_TUNING_BUFFER_OVERFLOW 0 +#endif - // Tune for use-after-free. - M_MEMTAG_TUNING_UAF, -}; +// Tune for use-after-free. +#ifndef M_MEMTAG_TUNING_UAF +#define M_MEMTAG_TUNING_UAF 1 +#endif } // extern "C" -- cgit v1.2.3 From b5ac5933e2d255a73862c83f1a55bd88b7f25c26 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Sat, 29 May 2021 17:11:36 -0700 Subject: [scudo] Enabled MTE before the first allocator Reviewed By: pcc Differential Revision: https://reviews.llvm.org/D103726 GitOrigin-RevId: 533abb7ecf1c0e80429ad7cd850e9720d2b2ae1c Change-Id: I91f90b67807b16362f985a2215c3646044f882a3 --- standalone/allocator_config.h | 3 +-- standalone/tests/scudo_unit_test_main.cpp | 18 ++++++++++++++++-- standalone/tests/wrappers_cpp_test.cpp | 6 ++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/standalone/allocator_config.h b/standalone/allocator_config.h index 7f556360c62..e6f46b511db 100644 --- a/standalone/allocator_config.h +++ b/standalone/allocator_config.h @@ -60,7 +60,7 @@ namespace scudo { struct DefaultConfig { using SizeClassMap = DefaultSizeClassMap; - static const bool MaySupportMemoryTagging = false; + static const bool MaySupportMemoryTagging = true; #if SCUDO_CAN_USE_PRIMARY64 typedef SizeClassAllocator64 Primary; @@ -87,7 +87,6 @@ struct DefaultConfig { template using TSDRegistryT = TSDRegistryExT; // Exclusive }; - struct AndroidConfig { using SizeClassMap = AndroidSizeClassMap; static const bool MaySupportMemoryTagging = true; diff --git a/standalone/tests/scudo_unit_test_main.cpp b/standalone/tests/scudo_unit_test_main.cpp index 1983b80ac0c..fbfefa5c93d 100644 --- a/standalone/tests/scudo_unit_test_main.cpp +++ b/standalone/tests/scudo_unit_test_main.cpp @@ -17,12 +17,27 @@ #define DEALLOC_TYPE_MISMATCH "true" #endif +static void EnableMemoryTaggingIfSupported() { + if (!scudo::archSupportsMemoryTagging()) + return; + static bool Done = []() { + if (!scudo::systemDetectsMemoryTagFaultsTestOnly()) + scudo::enableSystemMemoryTaggingTestOnly(); + return true; + }(); + (void)Done; +} + // This allows us to turn on/off a Quarantine for specific tests. The Quarantine // parameters are on the low end, to avoid having to loop excessively in some // tests. bool UseQuarantine = true; extern "C" __attribute__((visibility("default"))) const char * __scudo_default_options() { + // The wrapper tests initialize the global allocator early, before main(). We + // need to have Memory Tagging enabled before that happens or the allocator + // will disable the feature entirely. + EnableMemoryTaggingIfSupported(); if (!UseQuarantine) return "dealloc_type_mismatch=" DEALLOC_TYPE_MISMATCH; return "quarantine_size_kb=256:thread_local_quarantine_size_kb=128:" @@ -34,8 +49,7 @@ __scudo_default_options() { // for Fuchsia builds. #if !SCUDO_FUCHSIA int main(int argc, char **argv) { - if (scudo::archSupportsMemoryTagging()) - scudo::enableSystemMemoryTaggingTestOnly(); + EnableMemoryTaggingIfSupported(); testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } diff --git a/standalone/tests/wrappers_cpp_test.cpp b/standalone/tests/wrappers_cpp_test.cpp index 9df06dcdf14..64a0917d477 100644 --- a/standalone/tests/wrappers_cpp_test.cpp +++ b/standalone/tests/wrappers_cpp_test.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "memtag.h" #include "tests/scudo_unit_test.h" #include @@ -107,6 +108,11 @@ static void stressNew() { } TEST(ScudoWrappersCppTest, ThreadedNew) { +#if !SCUDO_ANDROID + // TODO: Investigate why libc sometimes crashes with tag missmatch in + // __pthread_clockjoin_ex. + scudo::ScopedDisableMemoryTagChecks NoTags; +#endif Ready = false; std::thread Threads[32]; for (size_t I = 0U; I < sizeof(Threads) / sizeof(Threads[0]); I++) -- cgit v1.2.3 From c4311c7b2291758c71d51b16874c9932eb2f08f3 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Wed, 23 Jun 2021 23:52:47 -0700 Subject: [scudo] Fix use of ScopedDisableMemoryTagChecks in test GitOrigin-RevId: 99ec78c0feded25a2b01dd5ae822b37a70b63c17 Change-Id: Ifd631b490dd4cd8672b1c35a9a135e211c1d8391 --- standalone/tests/wrappers_cpp_test.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/standalone/tests/wrappers_cpp_test.cpp b/standalone/tests/wrappers_cpp_test.cpp index 64a0917d477..24fbf277e03 100644 --- a/standalone/tests/wrappers_cpp_test.cpp +++ b/standalone/tests/wrappers_cpp_test.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -111,7 +112,9 @@ TEST(ScudoWrappersCppTest, ThreadedNew) { #if !SCUDO_ANDROID // TODO: Investigate why libc sometimes crashes with tag missmatch in // __pthread_clockjoin_ex. - scudo::ScopedDisableMemoryTagChecks NoTags; + std::unique_ptr NoTags; + if (scudo::systemSupportsMemoryTagging()) + NoTags = std::make_unique(); #endif Ready = false; std::thread Threads[32]; -- cgit v1.2.3 From c971fbfce26017bb75a4f5ccbb56445f55c9f077 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Wed, 23 Jun 2021 23:58:09 -0700 Subject: [scudo] Avoid ifdef in test GitOrigin-RevId: 6fd963ab64e7f0771bd59056b71ce915bc03d951 Change-Id: Id60867692a1a96d81ac7a3eac6eb34040e65639a --- standalone/tests/wrappers_cpp_test.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/standalone/tests/wrappers_cpp_test.cpp b/standalone/tests/wrappers_cpp_test.cpp index 24fbf277e03..461c8649eb5 100644 --- a/standalone/tests/wrappers_cpp_test.cpp +++ b/standalone/tests/wrappers_cpp_test.cpp @@ -109,13 +109,12 @@ static void stressNew() { } TEST(ScudoWrappersCppTest, ThreadedNew) { -#if !SCUDO_ANDROID // TODO: Investigate why libc sometimes crashes with tag missmatch in // __pthread_clockjoin_ex. std::unique_ptr NoTags; - if (scudo::systemSupportsMemoryTagging()) + if (scudo::systemSupportsMemoryTagging() && !SCUDO_ANDROID) NoTags = std::make_unique(); -#endif + Ready = false; std::thread Threads[32]; for (size_t I = 0U; I < sizeof(Threads) / sizeof(Threads[0]); I++) -- cgit v1.2.3 From b581108c5d63aeadd8ead6012342be2b07e890fd Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 24 Jun 2021 00:07:24 -0700 Subject: [scudo] Fix test on arch without MTE GitOrigin-RevId: 35e1dbd18920a0f1863cd06651d217693a94ad60 Change-Id: I70f15b69c1db3768f3abd6f48676e1bd98b5a99c --- standalone/tests/wrappers_cpp_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/standalone/tests/wrappers_cpp_test.cpp b/standalone/tests/wrappers_cpp_test.cpp index 461c8649eb5..747638b2486 100644 --- a/standalone/tests/wrappers_cpp_test.cpp +++ b/standalone/tests/wrappers_cpp_test.cpp @@ -112,7 +112,7 @@ TEST(ScudoWrappersCppTest, ThreadedNew) { // TODO: Investigate why libc sometimes crashes with tag missmatch in // __pthread_clockjoin_ex. std::unique_ptr NoTags; - if (scudo::systemSupportsMemoryTagging() && !SCUDO_ANDROID) + if (!SCUDO_ANDROID && scudo::archSupportsMemoryTagging() && scudo::systemSupportsMemoryTagging()) NoTags = std::make_unique(); Ready = false; -- cgit v1.2.3 From cba248f18d05f028eb529ede39b9a69661f3acb8 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 24 Jun 2021 17:28:01 -0700 Subject: [scudo] Clang-format tests GitOrigin-RevId: a4357411107e48e2580b06f4b4f518b2213951b8 Change-Id: I928a4d7d1dfa5b86968187365c95be7bf69840da --- standalone/tests/wrappers_cpp_test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/standalone/tests/wrappers_cpp_test.cpp b/standalone/tests/wrappers_cpp_test.cpp index 747638b2486..eb098c7f4d2 100644 --- a/standalone/tests/wrappers_cpp_test.cpp +++ b/standalone/tests/wrappers_cpp_test.cpp @@ -112,7 +112,8 @@ TEST(ScudoWrappersCppTest, ThreadedNew) { // TODO: Investigate why libc sometimes crashes with tag missmatch in // __pthread_clockjoin_ex. std::unique_ptr NoTags; - if (!SCUDO_ANDROID && scudo::archSupportsMemoryTagging() && scudo::systemSupportsMemoryTagging()) + if (!SCUDO_ANDROID && scudo::archSupportsMemoryTagging() && + scudo::systemSupportsMemoryTagging()) NoTags = std::make_unique(); Ready = false; -- cgit v1.2.3 From 137ec5adf540e5f91cdf8ac55aef4482503f82c3 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Wed, 30 Jun 2021 14:42:41 -0700 Subject: [scudo] GWP_ASAN runs on untagged pointers It's already covered by multiple tests, but to trigger this path we need MTE+GWP which disabled. Reviewed By: hctim, pcc Differential Revision: https://reviews.llvm.org/D105232 GitOrigin-RevId: b1fd009aab4bfe4f16bd78b7ac779c3f665ae060 Change-Id: I8681f8ba41b847ee77bf7d4c97ee96e49efcba78 --- standalone/combined.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/standalone/combined.h b/standalone/combined.h index 079edab1875..e8bb8bf207b 100644 --- a/standalone/combined.h +++ b/standalone/combined.h @@ -569,9 +569,6 @@ public: reportAllocationSizeTooBig(NewSize, 0, MaxAllowedMallocSize); } - void *OldTaggedPtr = OldPtr; - OldPtr = getHeaderTaggedPointer(OldPtr); - // The following cases are handled by the C wrappers. DCHECK_NE(OldPtr, nullptr); DCHECK_NE(NewSize, 0); @@ -591,6 +588,9 @@ public: } #endif // GWP_ASAN_HOOKS + void *OldTaggedPtr = OldPtr; + OldPtr = getHeaderTaggedPointer(OldPtr); + if (UNLIKELY(!isAligned(reinterpret_cast(OldPtr), MinAlignment))) reportMisalignedPointer(AllocatorAction::Reallocating, OldPtr); -- cgit v1.2.3 From d3199321d76813124a7f25d4a6efa09e386ec69e Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 1 Jul 2021 10:22:35 -0700 Subject: [NFC][scudo] Extract MapAllocatorTest for TEST_F GitOrigin-RevId: 39a15b5ae00df9a5e35f67dbffaed082b7e54d50 Change-Id: Iccee37cb8bb001dab5f5caacf08b3b3e808ed43e --- standalone/tests/secondary_test.cpp | 142 +++++++++++++++++++----------------- 1 file changed, 74 insertions(+), 68 deletions(-) diff --git a/standalone/tests/secondary_test.cpp b/standalone/tests/secondary_test.cpp index 2dc041b94a8..bbaf79261ba 100644 --- a/standalone/tests/secondary_test.cpp +++ b/standalone/tests/secondary_test.cpp @@ -11,11 +11,11 @@ #include "allocator_config.h" #include "secondary.h" -#include - #include +#include #include #include +#include #include #include @@ -94,14 +94,22 @@ TEST(ScudoSecondaryTest, SecondaryBasic) { using LargeAllocator = scudo::MapAllocator; +struct MapAllocatorTest : public Test { + void SetUp() override { Allocator->init(nullptr); } + + void TearDown() override { Allocator->unmapTestOnly(); } + + std::unique_ptr Allocator = + std::make_unique(); + scudo::Options Options = {}; +}; + // This exercises a variety of combinations of size and alignment for the // MapAllocator. The size computation done here mimic the ones done by the // combined allocator. -TEST(ScudoSecondaryTest, SecondaryCombinations) { +TEST_F(MapAllocatorTest, SecondaryCombinations) { constexpr scudo::uptr MinAlign = FIRST_32_SECOND_64(8, 16); constexpr scudo::uptr HeaderSize = scudo::roundUpTo(8, MinAlign); - std::unique_ptr L(new LargeAllocator); - L->init(nullptr); for (scudo::uptr SizeLog = 0; SizeLog <= 20; SizeLog++) { for (scudo::uptr AlignLog = FIRST_32_SECOND_64(3, 4); AlignLog <= 16; AlignLog++) { @@ -113,103 +121,102 @@ TEST(ScudoSecondaryTest, SecondaryCombinations) { scudo::roundUpTo((1U << SizeLog) + Delta, MinAlign); const scudo::uptr Size = HeaderSize + UserSize + (Align > MinAlign ? Align - HeaderSize : 0); - void *P = L->allocate(scudo::Options{}, Size, Align); + void *P = Allocator->allocate(Options, Size, Align); EXPECT_NE(P, nullptr); void *AlignedP = reinterpret_cast( scudo::roundUpTo(reinterpret_cast(P), Align)); memset(AlignedP, 0xff, UserSize); - L->deallocate(scudo::Options{}, P); + Allocator->deallocate(Options, P); } } } scudo::ScopedString Str; - L->getStats(&Str); + Allocator->getStats(&Str); Str.output(); - L->unmapTestOnly(); } -TEST(ScudoSecondaryTest, SecondaryIterate) { - std::unique_ptr L(new LargeAllocator); - L->init(nullptr); +TEST_F(MapAllocatorTest, SecondaryIterate) { std::vector V; const scudo::uptr PageSize = scudo::getPageSizeCached(); for (scudo::uptr I = 0; I < 32U; I++) - V.push_back(L->allocate(scudo::Options{}, (std::rand() % 16) * PageSize)); + V.push_back(Allocator->allocate(Options, (std::rand() % 16) * PageSize)); auto Lambda = [V](scudo::uptr Block) { EXPECT_NE(std::find(V.begin(), V.end(), reinterpret_cast(Block)), V.end()); }; - L->disable(); - L->iterateOverBlocks(Lambda); - L->enable(); + Allocator->disable(); + Allocator->iterateOverBlocks(Lambda); + Allocator->enable(); while (!V.empty()) { - L->deallocate(scudo::Options{}, V.back()); + Allocator->deallocate(Options, V.back()); V.pop_back(); } scudo::ScopedString Str; - L->getStats(&Str); + Allocator->getStats(&Str); Str.output(); - L->unmapTestOnly(); } -TEST(ScudoSecondaryTest, SecondaryOptions) { - std::unique_ptr L(new LargeAllocator); - L->init(nullptr); +TEST_F(MapAllocatorTest, SecondaryOptions) { // Attempt to set a maximum number of entries higher than the array size. - EXPECT_FALSE(L->setOption(scudo::Option::MaxCacheEntriesCount, 4096U)); + EXPECT_FALSE( + Allocator->setOption(scudo::Option::MaxCacheEntriesCount, 4096U)); // A negative number will be cast to a scudo::u32, and fail. - EXPECT_FALSE(L->setOption(scudo::Option::MaxCacheEntriesCount, -1)); - if (L->canCache(0U)) { + EXPECT_FALSE(Allocator->setOption(scudo::Option::MaxCacheEntriesCount, -1)); + if (Allocator->canCache(0U)) { // Various valid combinations. - EXPECT_TRUE(L->setOption(scudo::Option::MaxCacheEntriesCount, 4U)); - EXPECT_TRUE(L->setOption(scudo::Option::MaxCacheEntrySize, 1UL << 20)); - EXPECT_TRUE(L->canCache(1UL << 18)); - EXPECT_TRUE(L->setOption(scudo::Option::MaxCacheEntrySize, 1UL << 17)); - EXPECT_FALSE(L->canCache(1UL << 18)); - EXPECT_TRUE(L->canCache(1UL << 16)); - EXPECT_TRUE(L->setOption(scudo::Option::MaxCacheEntriesCount, 0U)); - EXPECT_FALSE(L->canCache(1UL << 16)); - EXPECT_TRUE(L->setOption(scudo::Option::MaxCacheEntriesCount, 4U)); - EXPECT_TRUE(L->setOption(scudo::Option::MaxCacheEntrySize, 1UL << 20)); - EXPECT_TRUE(L->canCache(1UL << 16)); + EXPECT_TRUE(Allocator->setOption(scudo::Option::MaxCacheEntriesCount, 4U)); + EXPECT_TRUE( + Allocator->setOption(scudo::Option::MaxCacheEntrySize, 1UL << 20)); + EXPECT_TRUE(Allocator->canCache(1UL << 18)); + EXPECT_TRUE( + Allocator->setOption(scudo::Option::MaxCacheEntrySize, 1UL << 17)); + EXPECT_FALSE(Allocator->canCache(1UL << 18)); + EXPECT_TRUE(Allocator->canCache(1UL << 16)); + EXPECT_TRUE(Allocator->setOption(scudo::Option::MaxCacheEntriesCount, 0U)); + EXPECT_FALSE(Allocator->canCache(1UL << 16)); + EXPECT_TRUE(Allocator->setOption(scudo::Option::MaxCacheEntriesCount, 4U)); + EXPECT_TRUE( + Allocator->setOption(scudo::Option::MaxCacheEntrySize, 1UL << 20)); + EXPECT_TRUE(Allocator->canCache(1UL << 16)); } - L->unmapTestOnly(); } -static std::mutex Mutex; -static std::condition_variable Cv; -static bool Ready; +struct MapAllocatorWithReleaseTest : public MapAllocatorTest { + void SetUp() override { Allocator->init(nullptr, /*ReleaseToOsInterval=*/0); } -static void performAllocations(LargeAllocator *L) { - std::vector V; - const scudo::uptr PageSize = scudo::getPageSizeCached(); - { - std::unique_lock Lock(Mutex); - while (!Ready) - Cv.wait(Lock); - } - for (scudo::uptr I = 0; I < 128U; I++) { - // Deallocate 75% of the blocks. - const bool Deallocate = (rand() & 3) != 0; - void *P = L->allocate(scudo::Options{}, (std::rand() % 16) * PageSize); - if (Deallocate) - L->deallocate(scudo::Options{}, P); - else - V.push_back(P); - } - while (!V.empty()) { - L->deallocate(scudo::Options{}, V.back()); - V.pop_back(); + void performAllocations() { + std::vector V; + const scudo::uptr PageSize = scudo::getPageSizeCached(); + { + std::unique_lock Lock(Mutex); + while (!Ready) + Cv.wait(Lock); + } + for (scudo::uptr I = 0; I < 128U; I++) { + // Deallocate 75% of the blocks. + const bool Deallocate = (rand() & 3) != 0; + void *P = Allocator->allocate(Options, (std::rand() % 16) * PageSize); + if (Deallocate) + Allocator->deallocate(Options, P); + else + V.push_back(P); + } + while (!V.empty()) { + Allocator->deallocate(Options, V.back()); + V.pop_back(); + } } -} -TEST(ScudoSecondaryTest, SecondaryThreadsRace) { - Ready = false; - std::unique_ptr L(new LargeAllocator); - L->init(nullptr, /*ReleaseToOsInterval=*/0); + std::mutex Mutex; + std::condition_variable Cv; + bool Ready = false; +}; + +TEST_F(MapAllocatorWithReleaseTest, SecondaryThreadsRace) { std::thread Threads[16]; for (scudo::uptr I = 0; I < ARRAY_SIZE(Threads); I++) - Threads[I] = std::thread(performAllocations, L.get()); + Threads[I] = + std::thread(&MapAllocatorWithReleaseTest::performAllocations, this); { std::unique_lock Lock(Mutex); Ready = true; @@ -218,7 +225,6 @@ TEST(ScudoSecondaryTest, SecondaryThreadsRace) { for (auto &T : Threads) T.join(); scudo::ScopedString Str; - L->getStats(&Str); + Allocator->getStats(&Str); Str.output(); - L->unmapTestOnly(); } -- cgit v1.2.3 From 97742dd0506e4f512ca5511d58210ebd9961b24f Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 1 Jul 2021 11:56:11 -0700 Subject: [NFC][scudo] Exctract getOptionsForConfig in test GitOrigin-RevId: 33b579c8a5efa476b8a1bd528fe5e47429249847 Change-Id: I51d16e08902a460271f976f5316fe1c618a7f4fb --- standalone/tests/secondary_test.cpp | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/standalone/tests/secondary_test.cpp b/standalone/tests/secondary_test.cpp index bbaf79261ba..6b8a60d386b 100644 --- a/standalone/tests/secondary_test.cpp +++ b/standalone/tests/secondary_test.cpp @@ -19,19 +19,24 @@ #include #include +template static scudo::Options getOptionsForConfig() { + return {}; +} + template static void testSecondaryBasic(void) { using SecondaryT = scudo::MapAllocator; + scudo::Options Options = getOptionsForConfig(); scudo::GlobalStats S; S.init(); std::unique_ptr L(new SecondaryT); L->init(&S); const scudo::uptr Size = 1U << 16; - void *P = L->allocate(scudo::Options{}, Size); + void *P = L->allocate(Options, Size); EXPECT_NE(P, nullptr); memset(P, 'A', Size); EXPECT_GE(SecondaryT::getBlockSize(P), Size); - L->deallocate(scudo::Options{}, P); + L->deallocate(Options, P); // If the Secondary can't cache that pointer, it will be unmapped. if (!L->canCache(Size)) { @@ -40,8 +45,8 @@ template static void testSecondaryBasic(void) { // Repeat few time to avoid missing crash if it's mmaped by unrelated // code. for (int i = 0; i < 10; ++i) { - P = L->allocate(scudo::Options{}, Size); - L->deallocate(scudo::Options{}, P); + P = L->allocate(Options, Size); + L->deallocate(Options, P); memset(P, 'A', Size); } }, @@ -49,19 +54,19 @@ template static void testSecondaryBasic(void) { } const scudo::uptr Align = 1U << 16; - P = L->allocate(scudo::Options{}, Size + Align, Align); + P = L->allocate(Options, Size + Align, Align); EXPECT_NE(P, nullptr); void *AlignedP = reinterpret_cast( scudo::roundUpTo(reinterpret_cast(P), Align)); memset(AlignedP, 'A', Size); - L->deallocate(scudo::Options{}, P); + L->deallocate(Options, P); std::vector V; for (scudo::uptr I = 0; I < 32U; I++) - V.push_back(L->allocate(scudo::Options{}, Size)); + V.push_back(L->allocate(Options, Size)); std::shuffle(V.begin(), V.end(), std::mt19937(std::random_device()())); while (!V.empty()) { - L->deallocate(scudo::Options{}, V.back()); + L->deallocate(Options, V.back()); V.pop_back(); } scudo::ScopedString Str; @@ -92,16 +97,17 @@ TEST(ScudoSecondaryTest, SecondaryBasic) { testSecondaryBasic(); } -using LargeAllocator = scudo::MapAllocator; - struct MapAllocatorTest : public Test { + using Config = scudo::DefaultConfig; + using LargeAllocator = scudo::MapAllocator; + void SetUp() override { Allocator->init(nullptr); } void TearDown() override { Allocator->unmapTestOnly(); } std::unique_ptr Allocator = std::make_unique(); - scudo::Options Options = {}; + scudo::Options Options = getOptionsForConfig(); }; // This exercises a variety of combinations of size and alignment for the -- cgit v1.2.3 From ace22c7c3a8bbafc6bd1df6394a68c489eabf6e1 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Wed, 30 Jun 2021 23:39:12 -0700 Subject: [scudo] Remove false DCHECK MTE Cache.store passes MAP_NOACCESS here. Reviewed By: pcc, cryptoad Differential Revision: https://reviews.llvm.org/D105266 GitOrigin-RevId: 78e70cee0d46bb14dcbedec993fbf855a4d13266 Change-Id: Id3440ab4f8244a7515ef15d0cf761efa3899f23c --- standalone/linux.cpp | 5 +---- standalone/tests/secondary_test.cpp | 7 ++++++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/standalone/linux.cpp b/standalone/linux.cpp index dedab61631b..c77c1bb600d 100644 --- a/standalone/linux.cpp +++ b/standalone/linux.cpp @@ -58,11 +58,8 @@ void *map(void *Addr, uptr Size, UNUSED const char *Name, uptr Flags, if (Flags & MAP_MEMTAG) MmapProt |= PROT_MTE; #endif - if (Addr) { - // Currently no scenario for a noaccess mapping with a fixed address. - DCHECK_EQ(Flags & MAP_NOACCESS, 0); + if (Addr) MmapFlags |= MAP_FIXED; - } void *P = mmap(Addr, Size, MmapProt, MmapFlags, -1, 0); if (P == MAP_FAILED) { if (!(Flags & MAP_ALLOWNOMEM) || errno != ENOMEM) diff --git a/standalone/tests/secondary_test.cpp b/standalone/tests/secondary_test.cpp index 6b8a60d386b..d3b7c486f7c 100644 --- a/standalone/tests/secondary_test.cpp +++ b/standalone/tests/secondary_test.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include "memtag.h" #include "tests/scudo_unit_test.h" #include "allocator_config.h" @@ -20,7 +21,11 @@ #include template static scudo::Options getOptionsForConfig() { - return {}; + if (!Config::MaySupportMemoryTagging || !scudo::archSupportsMemoryTagging()) + return {}; + scudo::AtomicOptions AO; + AO.set(scudo::OptionBit::UseMemoryTagging); + return AO.load(); } template static void testSecondaryBasic(void) { -- cgit v1.2.3 From 5df4673efd351b357998c1007b5a2360ab3b67e0 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Wed, 30 Jun 2021 20:22:41 -0700 Subject: [scudo] Untag BlockEnd in reallocate If we get here from reallocate, BlockEnd is tagged. Then we will storeTag(UntaggedEnd) into the header of the next chunk. Luckily header tag is 0 so unpatched code still works. Reviewed By: pcc Differential Revision: https://reviews.llvm.org/D105261 GitOrigin-RevId: fe30963600ea579d4046c9a92c6e38cc2be0e9a2 Change-Id: Idfbf127080f09a2a111741f60efa5422414c2009 --- standalone/combined.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/standalone/combined.h b/standalone/combined.h index e8bb8bf207b..fd5360ce0f5 100644 --- a/standalone/combined.h +++ b/standalone/combined.h @@ -639,7 +639,7 @@ public: if (ClassId) { resizeTaggedChunk(reinterpret_cast(OldTaggedPtr) + OldSize, reinterpret_cast(OldTaggedPtr) + NewSize, - NewSize, BlockEnd); + NewSize, untagPointer(BlockEnd)); storePrimaryAllocationStackMaybe(Options, OldPtr); } else { storeSecondaryAllocationStackMaybe(Options, OldPtr, NewSize); @@ -1154,6 +1154,7 @@ private: // address tags against chunks. To allow matching in this case we store the // address tag in the first byte of the chunk. void storeEndMarker(uptr End, uptr Size, uptr BlockEnd) { + DCHECK_EQ(BlockEnd, untagPointer(BlockEnd)); uptr UntaggedEnd = untagPointer(End); if (UntaggedEnd != BlockEnd) { storeTag(UntaggedEnd); -- cgit v1.2.3 From 7878d87edc0864ff54c2bae1dd8c2d3bb873db26 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Thu, 1 Jul 2021 21:40:04 -0700 Subject: [scudo] Fix test on aarch64 without MTE GitOrigin-RevId: 07a1f3513e2e3802671a0a4ca1edf2fe577fad03 Change-Id: Id5f6fb88ab0426907455119102eedf2775fc6c2a --- standalone/tests/secondary_test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/standalone/tests/secondary_test.cpp b/standalone/tests/secondary_test.cpp index d3b7c486f7c..723679228cb 100644 --- a/standalone/tests/secondary_test.cpp +++ b/standalone/tests/secondary_test.cpp @@ -21,7 +21,8 @@ #include template static scudo::Options getOptionsForConfig() { - if (!Config::MaySupportMemoryTagging || !scudo::archSupportsMemoryTagging()) + if (!Config::MaySupportMemoryTagging || !scudo::archSupportsMemoryTagging() || + !scudo::systemSupportsMemoryTagging()) return {}; scudo::AtomicOptions AO; AO.set(scudo::OptionBit::UseMemoryTagging); -- cgit v1.2.3 From 3a4cbda66ad22efb587737e8f24dd46bab390f9a Mon Sep 17 00:00:00 2001 From: Mitch Phillips <31459023+hctim@users.noreply.github.com> Date: Fri, 2 Jul 2021 13:50:52 -0700 Subject: Name all DEATH tests using 'DeathTest' suffix. gtest highly recommends this prefix, and runs death tests first (https://github.com/google/googletest/blob/master/docs/advanced.md#death-test-naming). This may help with some spurious bot failures like https://lab.llvm.org/buildbot/#/builders/169/builds/1290/steps/25/logs/stdio. Reviewed By: cryptoad, vitalybuka Differential Revision: https://reviews.llvm.org/D105371 GitOrigin-RevId: dd1c4bd09dd7a48c744f58847862f2e2bd633477 Change-Id: I038fa1cd26714fb8dd2c415ee9ec85ba26772476 --- standalone/tests/chunk_test.cpp | 4 ++-- standalone/tests/combined_test.cpp | 14 ++++++++------ standalone/tests/map_test.cpp | 6 +++--- standalone/tests/memtag_test.cpp | 12 +++++++----- standalone/tests/report_test.cpp | 6 +++--- standalone/tests/wrappers_c_test.cpp | 8 ++++---- standalone/tests/wrappers_cpp_test.cpp | 2 +- 7 files changed, 28 insertions(+), 24 deletions(-) diff --git a/standalone/tests/chunk_test.cpp b/standalone/tests/chunk_test.cpp index 6458e23e142..7a29f3c11b7 100644 --- a/standalone/tests/chunk_test.cpp +++ b/standalone/tests/chunk_test.cpp @@ -21,7 +21,7 @@ static void initChecksum(void) { scudo::HashAlgorithm = scudo::Checksum::HardwareCRC32; } -TEST(ScudoChunkTest, ChunkBasic) { +TEST(ScudoChunkDeathTest, ChunkBasic) { initChecksum(); const scudo::uptr Size = 0x100U; scudo::Chunk::UnpackedHeader Header = {}; @@ -60,7 +60,7 @@ TEST(ScudoChunkTest, ChunkCmpXchg) { free(Block); } -TEST(ScudoChunkTest, CorruptHeader) { +TEST(ScudoChunkDeathTest, CorruptHeader) { initChecksum(); const scudo::uptr Size = 0x100U; scudo::Chunk::UnpackedHeader Header = {}; diff --git a/standalone/tests/combined_test.cpp b/standalone/tests/combined_test.cpp index 6716d5df1e0..a2461c53cd9 100644 --- a/standalone/tests/combined_test.cpp +++ b/standalone/tests/combined_test.cpp @@ -103,6 +103,8 @@ template struct ScudoCombinedTest : public Test { std::unique_ptr Allocator; }; +template using ScudoCombinedDeathTest = ScudoCombinedTest; + #if SCUDO_FUCHSIA #define SCUDO_TYPED_TEST_ALL_TYPES(FIXTURE, NAME) \ SCUDO_TYPED_TEST_TYPE(FIXTURE, NAME, AndroidSvelteConfig) \ @@ -166,7 +168,7 @@ void ScudoCombinedTest::BasicTest(scudo::uptr SizeLog) { } #define SCUDO_MAKE_BASIC_TEST(SizeLog) \ - SCUDO_TYPED_TEST(ScudoCombinedTest, BasicCombined##SizeLog) { \ + SCUDO_TYPED_TEST(ScudoCombinedDeathTest, BasicCombined##SizeLog) { \ this->BasicTest(SizeLog); \ } @@ -314,7 +316,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, ReallocateLargeDecreasing) { Allocator->deallocate(P, Origin); } -SCUDO_TYPED_TEST(ScudoCombinedTest, ReallocateSame) { +SCUDO_TYPED_TEST(ScudoCombinedDeathTest, ReallocateSame) { auto *Allocator = this->Allocator.get(); // Check that reallocating a chunk to a slightly smaller or larger size @@ -365,7 +367,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, IterateOverChunks) { } } -SCUDO_TYPED_TEST(ScudoCombinedTest, UseAfterFree) { +SCUDO_TYPED_TEST(ScudoCombinedDeathTest, UseAfterFree) { auto *Allocator = this->Allocator.get(); // Check that use-after-free is detected. @@ -392,7 +394,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, UseAfterFree) { } } -SCUDO_TYPED_TEST(ScudoCombinedTest, DisableMemoryTagging) { +SCUDO_TYPED_TEST(ScudoCombinedDeathTest, DisableMemoryTagging) { auto *Allocator = this->Allocator.get(); if (Allocator->useMemoryTaggingTestOnly()) { @@ -490,7 +492,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, ThreadedCombined) { // Test that multiple instantiations of the allocator have not messed up the // process's signal handlers (GWP-ASan used to do this). -TEST(ScudoCombinedTest, SKIP_ON_FUCHSIA(testSEGV)) { +TEST(ScudoCombinedDeathTest, SKIP_ON_FUCHSIA(testSEGV)) { const scudo::uptr Size = 4 * scudo::getPageSizeCached(); scudo::MapPlatformData Data = {}; void *P = scudo::map(nullptr, Size, "testSEGV", MAP_NOACCESS, &Data); @@ -528,7 +530,7 @@ struct DeathConfig { template using TSDRegistryT = scudo::TSDRegistrySharedT; }; -TEST(ScudoCombinedTest, DeathCombined) { +TEST(ScudoCombinedDeathTest, DeathCombined) { using AllocatorT = TestAllocator; auto Allocator = std::unique_ptr(new AllocatorT()); diff --git a/standalone/tests/map_test.cpp b/standalone/tests/map_test.cpp index 149a704e4dd..095e1b6a5d2 100644 --- a/standalone/tests/map_test.cpp +++ b/standalone/tests/map_test.cpp @@ -20,7 +20,7 @@ TEST(ScudoMapTest, PageSize) { static_cast(getpagesize())); } -TEST(ScudoMapTest, MapNoAccessUnmap) { +TEST(ScudoMapDeathTest, MapNoAccessUnmap) { const scudo::uptr Size = 4 * scudo::getPageSizeCached(); scudo::MapPlatformData Data = {}; void *P = scudo::map(nullptr, Size, MappingName, MAP_NOACCESS, &Data); @@ -29,7 +29,7 @@ TEST(ScudoMapTest, MapNoAccessUnmap) { scudo::unmap(P, Size, UNMAP_ALL, &Data); } -TEST(ScudoMapTest, MapUnmap) { +TEST(ScudoMapDeathTest, MapUnmap) { const scudo::uptr Size = 4 * scudo::getPageSizeCached(); EXPECT_DEATH( { @@ -46,7 +46,7 @@ TEST(ScudoMapTest, MapUnmap) { ""); } -TEST(ScudoMapTest, MapWithGuardUnmap) { +TEST(ScudoMapDeathTest, MapWithGuardUnmap) { const scudo::uptr PageSize = scudo::getPageSizeCached(); const scudo::uptr Size = 4 * PageSize; scudo::MapPlatformData Data = {}; diff --git a/standalone/tests/memtag_test.cpp b/standalone/tests/memtag_test.cpp index 50ba0fc82ce..72c9de36b8b 100644 --- a/standalone/tests/memtag_test.cpp +++ b/standalone/tests/memtag_test.cpp @@ -14,7 +14,7 @@ #if SCUDO_LINUX namespace scudo { -TEST(MemtagBasicTest, Unsupported) { +TEST(MemtagBasicDeathTest, Unsupported) { if (archSupportsMemoryTagging()) GTEST_SKIP(); @@ -63,6 +63,8 @@ protected: uptr Addr = 0; }; +using MemtagDeathTest = MemtagTest; + TEST_F(MemtagTest, ArchMemoryTagGranuleSize) { EXPECT_GT(archMemoryTagGranuleSize(), 1u); EXPECT_TRUE(isPowerOfTwo(archMemoryTagGranuleSize())); @@ -77,7 +79,7 @@ TEST_F(MemtagTest, ExtractTag) { EXPECT_EQ(0xffffull, Tags); } -TEST_F(MemtagTest, AddFixedTag) { +TEST_F(MemtagDeathTest, AddFixedTag) { for (uptr Tag = 0; Tag < 0x10; ++Tag) EXPECT_EQ(Tag, extractTag(addFixedTag(Addr, Tag))); if (SCUDO_DEBUG) { @@ -94,7 +96,7 @@ TEST_F(MemtagTest, UntagPointer) { } } -TEST_F(MemtagTest, ScopedDisableMemoryTagChecks) { +TEST_F(MemtagDeathTest, ScopedDisableMemoryTagChecks) { u8 *P = reinterpret_cast(addFixedTag(Addr, 1)); EXPECT_NE(P, Buffer); @@ -120,7 +122,7 @@ TEST_F(MemtagTest, SelectRandomTagWithMask) { } } -TEST_F(MemtagTest, SKIP_NO_DEBUG(LoadStoreTagUnaligned)) { +TEST_F(MemtagDeathTest, SKIP_NO_DEBUG(LoadStoreTagUnaligned)) { for (uptr P = Addr; P < Addr + 4 * archMemoryTagGranuleSize(); ++P) { if (P % archMemoryTagGranuleSize() == 0) continue; @@ -141,7 +143,7 @@ TEST_F(MemtagTest, LoadStoreTag) { loadTag(Base + archMemoryTagGranuleSize())); } -TEST_F(MemtagTest, SKIP_NO_DEBUG(StoreTagsUnaligned)) { +TEST_F(MemtagDeathTest, SKIP_NO_DEBUG(StoreTagsUnaligned)) { for (uptr P = Addr; P < Addr + 4 * archMemoryTagGranuleSize(); ++P) { uptr Tagged = addFixedTag(P, 5); if (Tagged % archMemoryTagGranuleSize() == 0) diff --git a/standalone/tests/report_test.cpp b/standalone/tests/report_test.cpp index 374b6b8de34..81587bae6b5 100644 --- a/standalone/tests/report_test.cpp +++ b/standalone/tests/report_test.cpp @@ -10,14 +10,14 @@ #include "report.h" -TEST(ScudoReportTest, Check) { +TEST(ScudoReportDeathTest, Check) { CHECK_LT(-1, 1); EXPECT_DEATH(CHECK_GT(-1, 1), "\\(-1\\) > \\(1\\) \\(\\(u64\\)op1=18446744073709551615, " "\\(u64\\)op2=1"); } -TEST(ScudoReportTest, Generic) { +TEST(ScudoReportDeathTest, Generic) { // Potentially unused if EXPECT_DEATH isn't defined. UNUSED void *P = reinterpret_cast(0x42424242U); EXPECT_DEATH(scudo::reportError("TEST123"), "Scudo ERROR.*TEST123"); @@ -45,7 +45,7 @@ TEST(ScudoReportTest, Generic) { "Scudo ERROR.*42424242.*123.*456"); } -TEST(ScudoReportTest, CSpecific) { +TEST(ScudoReportDeathTest, CSpecific) { EXPECT_DEATH(scudo::reportAlignmentNotPowerOfTwo(123), "Scudo ERROR.*123"); EXPECT_DEATH(scudo::reportCallocOverflow(123, 456), "Scudo ERROR.*123.*456"); EXPECT_DEATH(scudo::reportInvalidPosixMemalignAlignment(789), diff --git a/standalone/tests/wrappers_c_test.cpp b/standalone/tests/wrappers_c_test.cpp index b82b5fb4b51..f607ba70ab4 100644 --- a/standalone/tests/wrappers_c_test.cpp +++ b/standalone/tests/wrappers_c_test.cpp @@ -38,7 +38,7 @@ void *pvalloc(size_t size); static const size_t Size = 100U; -TEST(ScudoWrappersCTest, Malloc) { +TEST(ScudoWrappersCDeathTest, Malloc) { void *P = malloc(Size); EXPECT_NE(P, nullptr); EXPECT_LE(Size, malloc_usable_size(P)); @@ -154,7 +154,7 @@ TEST(ScudoWrappersCTest, AlignedAlloc) { EXPECT_EQ(errno, EINVAL); } -TEST(ScudoWrappersCTest, Realloc) { +TEST(ScudoWrappersCDeathTest, Realloc) { // realloc(nullptr, N) is malloc(N) void *P = realloc(nullptr, 0U); EXPECT_NE(P, nullptr); @@ -333,7 +333,7 @@ TEST(ScudoWrappersCTest, MallocIterateBoundary) { // Fuchsia doesn't have alarm, fork or malloc_info. #if !SCUDO_FUCHSIA -TEST(ScudoWrappersCTest, MallocDisableDeadlock) { +TEST(ScudoWrappersCDeathTest, MallocDisableDeadlock) { // We expect heap operations within a disable/enable scope to deadlock. EXPECT_DEATH( { @@ -368,7 +368,7 @@ TEST(ScudoWrappersCTest, MallocInfo) { free(P2); } -TEST(ScudoWrappersCTest, Fork) { +TEST(ScudoWrappersCDeathTest, Fork) { void *P; pid_t Pid = fork(); EXPECT_GE(Pid, 0) << strerror(errno); diff --git a/standalone/tests/wrappers_cpp_test.cpp b/standalone/tests/wrappers_cpp_test.cpp index eb098c7f4d2..114196778a9 100644 --- a/standalone/tests/wrappers_cpp_test.cpp +++ b/standalone/tests/wrappers_cpp_test.cpp @@ -67,7 +67,7 @@ public: Color C = Color::Red; }; -TEST(ScudoWrappersCppTest, New) { +TEST(ScudoWrappersCppDeathTest, New) { if (getenv("SKIP_TYPE_MISMATCH")) { printf("Skipped type mismatch tests.\n"); return; -- cgit v1.2.3 From 068afb487233cecf9ab93b5f1996233b7a894709 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 9 Jul 2021 12:24:50 -0700 Subject: [scudo] Check if we use __clang_major__ >= 12 This makes sure we have support for MTE instructions. Later the check can be extended to support MTE on other compilers. Reviewed By: pcc Differential Revision: https://reviews.llvm.org/D105722 GitOrigin-RevId: db4c25822a1d9458f961eeb6cecf8a7bbce05b13 Change-Id: Ie9367cf1743b78ae06be951ba007107a43604604 --- standalone/memtag.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/standalone/memtag.h b/standalone/memtag.h index 0c47f67a574..cd8d95a3f75 100644 --- a/standalone/memtag.h +++ b/standalone/memtag.h @@ -18,7 +18,7 @@ namespace scudo { -#if defined(__aarch64__) || defined(SCUDO_FUZZ) +#if (__clang_major__ >= 12 && defined(__aarch64__)) || defined(SCUDO_FUZZ) // We assume that Top-Byte Ignore is enabled if the architecture supports memory // tagging. Not all operating systems enable TBI, so we only claim architectural @@ -55,7 +55,7 @@ inline uint8_t extractTag(uptr Ptr) { #endif -#if defined(__aarch64__) +#if __clang_major__ >= 12 && defined(__aarch64__) #if SCUDO_LINUX -- cgit v1.2.3 From aec1844db745dd0efb7ffc6ccc4c8d03055c6692 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Tue, 13 Jul 2021 18:11:57 -0700 Subject: [scudo] Don't enabled MTE for small alignment Differential Revision: https://reviews.llvm.org/D105954 GitOrigin-RevId: 14362bf1b2589f8a769709be599f24ab7eee6d6e Change-Id: I907f146ea2195699a8d27b7f6f4f33f897b16b37 --- standalone/memtag.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/standalone/memtag.h b/standalone/memtag.h index cd8d95a3f75..c48e228fbe4 100644 --- a/standalone/memtag.h +++ b/standalone/memtag.h @@ -319,7 +319,8 @@ inline void *addFixedTag(void *Ptr, uptr Tag) { template inline constexpr bool allocatorSupportsMemoryTagging() { - return archSupportsMemoryTagging() && Config::MaySupportMemoryTagging; + return archSupportsMemoryTagging() && Config::MaySupportMemoryTagging && + (1 << SCUDO_MIN_ALIGNMENT_LOG) >= archMemoryTagGranuleSize(); } } // namespace scudo -- cgit v1.2.3 From 543c7adb459ceada6859cb0daa2b558986409203 Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Mon, 2 Aug 2021 13:38:14 -0700 Subject: [scudo] Make Vector() constexpr A `Vector` that doesn't require an initial `reserve()` (eg: with a default, or small enough capacity) can have a constant initializer. This changes the code in a few places to make that possible: - mark a few other functions as `constexpr` - do without any `reinterpret_cast` - allow to skip `reserve` from `init` Differential Revision: https://reviews.llvm.org/D107308 GitOrigin-RevId: 23a94af44939b094f9ba2d6bb969f5a48b78fa8c Change-Id: I9122b8511df7bb97b59b89c529e5d2e0aaaf6455 --- standalone/vector.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/standalone/vector.h b/standalone/vector.h index 2c9a6e2aa65..eae774b56e2 100644 --- a/standalone/vector.h +++ b/standalone/vector.h @@ -19,13 +19,14 @@ namespace scudo { // small vectors. The current implementation supports only POD types. template class VectorNoCtor { public: - void init(uptr InitialCapacity = 0) { - Data = reinterpret_cast(&LocalData[0]); + constexpr void init(uptr InitialCapacity = 0) { + Data = &LocalData[0]; CapacityBytes = sizeof(LocalData); - reserve(InitialCapacity); + if (InitialCapacity > capacity()) + reserve(InitialCapacity); } void destroy() { - if (Data != reinterpret_cast(&LocalData[0])) + if (Data != &LocalData[0]) unmap(Data, CapacityBytes); } T &operator[](uptr I) { @@ -55,7 +56,7 @@ public: uptr size() const { return Size; } const T *data() const { return Data; } T *data() { return Data; } - uptr capacity() const { return CapacityBytes / sizeof(T); } + constexpr uptr capacity() const { return CapacityBytes / sizeof(T); } void reserve(uptr NewSize) { // Never downsize internal buffer. if (NewSize > capacity()) @@ -91,14 +92,14 @@ private: } T *Data = nullptr; - u8 LocalData[256] = {}; + T LocalData[256 / sizeof(T)] = {}; uptr CapacityBytes = 0; uptr Size = 0; }; template class Vector : public VectorNoCtor { public: - Vector() { VectorNoCtor::init(); } + constexpr Vector() { VectorNoCtor::init(); } explicit Vector(uptr Count) { VectorNoCtor::init(Count); this->resize(Count); -- cgit v1.2.3 From 22935fdc3f71a62adb7928f88d2b88d062b8d60f Mon Sep 17 00:00:00 2001 From: Mitch Phillips <31459023+hctim@users.noreply.github.com> Date: Wed, 11 Aug 2021 09:21:28 -0700 Subject: [scudo] Add GWP-ASan state/metadata pointer proxies. Provide accessor proxies for the gwp-asan regions that are useful in symbolizing dumps offline. Should be useful for Fuchsia to be able to locate these internal pointers to stash the data in a minidump. Reviewed By: cryptoad Differential Revision: https://reviews.llvm.org/D107909 GitOrigin-RevId: 32adf108c8edd989caf9392131a5ebaf9de762f2 Change-Id: I4da14c8930a7b825cabf41da2fa221fa9fc3489d --- standalone/combined.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/standalone/combined.h b/standalone/combined.h index fd5360ce0f5..922c2e50bb0 100644 --- a/standalone/combined.h +++ b/standalone/combined.h @@ -205,6 +205,16 @@ public: #endif // GWP_ASAN_HOOKS } +#ifdef GWP_ASAN_HOOKS + const gwp_asan::AllocationMetadata *getGwpAsanAllocationMetadata() { + return GuardedAlloc.getMetadataRegion(); + } + + const gwp_asan::AllocatorState *getGwpAsanAllocatorState() { + return GuardedAlloc.getAllocatorState(); + } +#endif // GWP_ASAN_HOOKS + ALWAYS_INLINE void initThreadMaybe(bool MinimalInit = false) { TSDRegistry.initThreadMaybe(this, MinimalInit); } -- cgit v1.2.3 From b0e1b4d2dc6138172b322c85f63583978a5637d7 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Fri, 13 Aug 2021 07:29:41 +0200 Subject: scudo/standalone: prepare for enabling format string checking Move __attribute__((format)) to the function declarations in the header file. It's almost pointless in the source file. But disable the warning with -Wno-format for now since there is a number of existing warnings. Depends on D107984. Reviewed By: vitalybuka Differential Revision: https://reviews.llvm.org/D108014 GitOrigin-RevId: 1fbe5fb81c80ebd481430cbe0b457d3726329b29 Change-Id: Ifa183cec5d09902ae646b904ddb9c410c2ee0ce2 --- standalone/string_utils.cpp | 2 -- standalone/string_utils.h | 7 ++++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/standalone/string_utils.cpp b/standalone/string_utils.cpp index acf85889fcf..13fdb9c6ca6 100644 --- a/standalone/string_utils.cpp +++ b/standalone/string_utils.cpp @@ -236,7 +236,6 @@ void ScopedString::append(const char *Format, va_list Args) { va_end(ArgsCopy); } -FORMAT(2, 3) void ScopedString::append(const char *Format, ...) { va_list Args; va_start(Args, Format); @@ -244,7 +243,6 @@ void ScopedString::append(const char *Format, ...) { va_end(Args); } -FORMAT(1, 2) void Printf(const char *Format, ...) { va_list Args; va_start(Args, Format); diff --git a/standalone/string_utils.h b/standalone/string_utils.h index 06d23d42246..dd6ff7893b8 100644 --- a/standalone/string_utils.h +++ b/standalone/string_utils.h @@ -26,15 +26,16 @@ public: String.push_back('\0'); } void append(const char *Format, va_list Args); - void append(const char *Format, ...); + void append(const char *Format, ...) FORMAT(2, 3); void output() const { outputRaw(String.data()); } private: Vector String; }; -int formatString(char *Buffer, uptr BufferLength, const char *Format, ...); -void Printf(const char *Format, ...); +int formatString(char *Buffer, uptr BufferLength, const char *Format, ...) + FORMAT(3, 4); +void Printf(const char *Format, ...) FORMAT(1, 2); } // namespace scudo -- cgit v1.2.3 From d5ce6136c40ee6221ef22ae2bb840cc790915c79 Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Sun, 15 Aug 2021 09:09:46 -0700 Subject: [scudo] Use stdint types for internal scudo types `scudo::uptr` was defined as an `unsigned long` on 32-b platform, while a `uintptr_t` is usually defined as an `unsigned int`. This worked, this was not consistent, particularly with regard to format string specifiers. As suggested by Vitaly, since we are including `stdint.h`, define the internal `scudo` integer types to those. Differential Revision: https://reviews.llvm.org/D108089 GitOrigin-RevId: 5fc841d8a278ea16bae457deba35d0db6b716dd6 Change-Id: I449369fdb0a5f04c717dad75b45a4e9996f4d5ec --- standalone/internal_defs.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/standalone/internal_defs.h b/standalone/internal_defs.h index c9ffad136b7..621fc9c45e9 100644 --- a/standalone/internal_defs.h +++ b/standalone/internal_defs.h @@ -78,16 +78,16 @@ namespace scudo { -typedef unsigned long uptr; -typedef unsigned char u8; -typedef unsigned short u16; -typedef unsigned int u32; -typedef unsigned long long u64; -typedef signed long sptr; -typedef signed char s8; -typedef signed short s16; -typedef signed int s32; -typedef signed long long s64; +typedef uintptr_t uptr; +typedef uint8_t u8; +typedef uint16_t u16; +typedef uint32_t u32; +typedef uint64_t u64; +typedef intptr_t sptr; +typedef int8_t s8; +typedef int16_t s16; +typedef int32_t s32; +typedef int64_t s64; // The following two functions have platform specific implementations. void outputRaw(const char *Buffer); -- cgit v1.2.3 From f67abf9ac7b22352644757e668dd4c7edc3c2b52 Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Mon, 16 Aug 2021 11:13:48 -0700 Subject: Revert "[scudo] Use stdint types for internal scudo types" This reverts commit 5fc841d8a278ea16bae457deba35d0db6b716dd6. GitOrigin-RevId: 427c9aa7c440aa9003e322a3107f8b222fa17ef4 Change-Id: I0facd472c773b29d62ba1ef6ed931e628dcc3e69 --- standalone/internal_defs.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/standalone/internal_defs.h b/standalone/internal_defs.h index 621fc9c45e9..c9ffad136b7 100644 --- a/standalone/internal_defs.h +++ b/standalone/internal_defs.h @@ -78,16 +78,16 @@ namespace scudo { -typedef uintptr_t uptr; -typedef uint8_t u8; -typedef uint16_t u16; -typedef uint32_t u32; -typedef uint64_t u64; -typedef intptr_t sptr; -typedef int8_t s8; -typedef int16_t s16; -typedef int32_t s32; -typedef int64_t s64; +typedef unsigned long uptr; +typedef unsigned char u8; +typedef unsigned short u16; +typedef unsigned int u32; +typedef unsigned long long u64; +typedef signed long sptr; +typedef signed char s8; +typedef signed short s16; +typedef signed int s32; +typedef signed long long s64; // The following two functions have platform specific implementations. void outputRaw(const char *Buffer); -- cgit v1.2.3 From 5b9868d9e6830e9205ab69e5bbcac6a6038b2b79 Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Mon, 16 Aug 2021 11:59:15 -0700 Subject: [scudo] Use stdint types for internal types (redo) This is a redo of D108089 that broke some 32-bit builds. `scudo::uptr` was defined as an `unsigned long` on 32-b platform, while a `uintptr_t` is usually defined as an `unsigned int`. This worked, this was not consistent, particularly with regard to format string specifiers. As suggested by Vitaly, since we are including `stdint.h`, define the internal scudo integer types to those. Differential Revision: https://reviews.llvm.org/D108152 GitOrigin-RevId: b2aaafb8377ac1ab081e7c0d3ba92ee5eb4de07c Change-Id: Id16c7b4652b873a759ac2f1523e59d3ab157a298 --- standalone/combined.h | 2 +- standalone/internal_defs.h | 20 ++++++++++---------- standalone/secondary.h | 2 +- standalone/wrappers_c_checks.h | 4 +++- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/standalone/combined.h b/standalone/combined.h index 922c2e50bb0..371fb783a06 100644 --- a/standalone/combined.h +++ b/standalone/combined.h @@ -920,7 +920,7 @@ public: if (!Depot->find(Hash, &RingPos, &Size)) return; for (unsigned I = 0; I != Size && I != MaxTraceSize; ++I) - Trace[I] = (*Depot)[RingPos + I]; + Trace[I] = static_cast((*Depot)[RingPos + I]); } static void getErrorInfo(struct scudo_error_info *ErrorInfo, diff --git a/standalone/internal_defs.h b/standalone/internal_defs.h index c9ffad136b7..621fc9c45e9 100644 --- a/standalone/internal_defs.h +++ b/standalone/internal_defs.h @@ -78,16 +78,16 @@ namespace scudo { -typedef unsigned long uptr; -typedef unsigned char u8; -typedef unsigned short u16; -typedef unsigned int u32; -typedef unsigned long long u64; -typedef signed long sptr; -typedef signed char s8; -typedef signed short s16; -typedef signed int s32; -typedef signed long long s64; +typedef uintptr_t uptr; +typedef uint8_t u8; +typedef uint16_t u16; +typedef uint32_t u32; +typedef uint64_t u64; +typedef intptr_t sptr; +typedef int8_t s8; +typedef int16_t s16; +typedef int32_t s32; +typedef int64_t s64; // The following two functions have platform specific implementations. void outputRaw(const char *Buffer); diff --git a/standalone/secondary.h b/standalone/secondary.h index 630e64d46ed..aa50fa98b11 100644 --- a/standalone/secondary.h +++ b/standalone/secondary.h @@ -485,7 +485,7 @@ void *MapAllocator::allocate(Options Options, uptr Size, uptr Alignment, FillContentsMode FillContents) { if (Options.get(OptionBit::AddLargeAllocationSlack)) Size += 1UL << SCUDO_MIN_ALIGNMENT_LOG; - Alignment = Max(Alignment, 1UL << SCUDO_MIN_ALIGNMENT_LOG); + Alignment = Max(Alignment, uptr(1U) << SCUDO_MIN_ALIGNMENT_LOG); const uptr PageSize = getPageSizeCached(); uptr RoundedSize = roundUpTo(roundUpTo(Size, Alignment) + LargeBlock::getHeaderSize() + diff --git a/standalone/wrappers_c_checks.h b/standalone/wrappers_c_checks.h index 7fc1a9600e5..ec9c1a104e8 100644 --- a/standalone/wrappers_c_checks.h +++ b/standalone/wrappers_c_checks.h @@ -46,8 +46,10 @@ inline bool checkPosixMemalignAlignment(uptr Alignment) { // builtin supported by recent clang & GCC if it exists, otherwise fallback to a // costly division. inline bool checkForCallocOverflow(uptr Size, uptr N, uptr *Product) { -#if __has_builtin(__builtin_umull_overflow) +#if __has_builtin(__builtin_umull_overflow) && (SCUDO_WORDSIZE == 64U) return __builtin_umull_overflow(Size, N, Product); +#elif __has_builtin(__builtin_umul_overflow) && (SCUDO_WORDSIZE == 32U) + return __builtin_umul_overflow(Size, N, Product); #else *Product = Size * N; if (!Size) -- cgit v1.2.3 From c2e59964f5866acb106d217462c1771afdd24844 Mon Sep 17 00:00:00 2001 From: Kostya Kortchinsky Date: Mon, 16 Aug 2021 15:23:48 -0700 Subject: [scudo] Fix format string specifiers Enable `-Wformat` again, and fix the offending instances. Differential Revision: https://reviews.llvm.org/D108168 GitOrigin-RevId: 5009be2f09ae25654753ee533dbfbc238aaf591c Change-Id: Icd55c53967be36c9df0350a17b21decf295e0223 --- standalone/primary64.h | 4 ++-- standalone/secondary.h | 11 +++++------ standalone/size_class_map.h | 4 ++-- standalone/wrappers_c.inc | 2 +- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/standalone/primary64.h b/standalone/primary64.h index 13420bf3d22..6c1785512c6 100644 --- a/standalone/primary64.h +++ b/standalone/primary64.h @@ -164,9 +164,9 @@ public: PoppedBlocks += Region->Stats.PoppedBlocks; PushedBlocks += Region->Stats.PushedBlocks; } - Str->append("Stats: SizeClassAllocator64: %zuM mapped (%zuM rss) in %zu " + Str->append("Stats: SizeClassAllocator64: %zuM mapped (%uM rss) in %zu " "allocations; remains %zu\n", - TotalMapped >> 20, 0, PoppedBlocks, + TotalMapped >> 20, 0U, PoppedBlocks, PoppedBlocks - PushedBlocks); for (uptr I = 0; I < NumClasses; I++) diff --git a/standalone/secondary.h b/standalone/secondary.h index aa50fa98b11..abb58a2882a 100644 --- a/standalone/secondary.h +++ b/standalone/secondary.h @@ -602,12 +602,11 @@ void MapAllocator::deallocate(Options Options, void *Ptr) { template void MapAllocator::getStats(ScopedString *Str) const { - Str->append( - "Stats: MapAllocator: allocated %zu times (%zuK), freed %zu times " - "(%zuK), remains %zu (%zuK) max %zuM\n", - NumberOfAllocs, AllocatedBytes >> 10, NumberOfFrees, FreedBytes >> 10, - NumberOfAllocs - NumberOfFrees, (AllocatedBytes - FreedBytes) >> 10, - LargestSize >> 20); + Str->append("Stats: MapAllocator: allocated %u times (%zuK), freed %u times " + "(%zuK), remains %u (%zuK) max %zuM\n", + NumberOfAllocs, AllocatedBytes >> 10, NumberOfFrees, + FreedBytes >> 10, NumberOfAllocs - NumberOfFrees, + (AllocatedBytes - FreedBytes) >> 10, LargestSize >> 20); } } // namespace scudo diff --git a/standalone/size_class_map.h b/standalone/size_class_map.h index ba0f78453bc..28b16d976e5 100644 --- a/standalone/size_class_map.h +++ b/standalone/size_class_map.h @@ -335,8 +335,8 @@ template inline void printMap() { const uptr L = S ? getMostSignificantSetBitIndex(S) : 0; const uptr Cached = SCMap::getMaxCachedHint(S) * S; Buffer.append( - "C%02zu => S: %zu diff: +%zu %02zu%% L %zu Cached: %zu %zu; id %zu\n", - I, S, D, P, L, SCMap::getMaxCachedHint(S), Cached, + "C%02zu => S: %zu diff: +%zu %02zu%% L %zu Cached: %u %zu; id %zu\n", I, + S, D, P, L, SCMap::getMaxCachedHint(S), Cached, SCMap::getClassIdBySize(S)); TotalCached += Cached; PrevS = S; diff --git a/standalone/wrappers_c.inc b/standalone/wrappers_c.inc index 43efb02cb86..6c6bcb6783a 100644 --- a/standalone/wrappers_c.inc +++ b/standalone/wrappers_c.inc @@ -226,7 +226,7 @@ INTERFACE WEAK int SCUDO_PREFIX(malloc_info)(UNUSED int options, FILE *stream) { fputs("\n", stream); for (scudo::uptr i = 0; i != max_size; ++i) if (sizes[i]) - fprintf(stream, "\n", i, sizes[i]); + fprintf(stream, "\n", i, sizes[i]); fputs("\n", stream); SCUDO_PREFIX(free)(sizes); return 0; -- cgit v1.2.3