From: Kostya Kortchinsky Date: Wed, 23 Sep 2020 17:22:15 +0000 (-0700) Subject: [scudo][standalone] Fix tests under ASan/UBSan X-Git-Tag: llvmorg-13-init~11126 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2efc09c90914a6c887cb772130d6b375a1713472;p=platform%2Fupstream%2Fllvm.git [scudo][standalone] Fix tests under ASan/UBSan Fix a potential UB in `appendSignedDecimal` (with -INT64_MIN) by making it a special case. Fix the terrible test cases for `isOwned`: I was pretty sloppy on those and used some stack & static variables, but since `isOwned` accesses memory prior to the pointer to check for the validity of the Scudo header, it ended up being detected as some global and stack buffer out of bounds accesses. So not I am using buffers with enough room so that the test will not access memory prior to the variables. With those fixes, the tests pass on the ASan+UBSan Fuchsia build. Thanks to Roland for pointing those out! Differential Revision: https://reviews.llvm.org/D88170 --- diff --git a/compiler-rt/lib/scudo/standalone/string_utils.cpp b/compiler-rt/lib/scudo/standalone/string_utils.cpp index 5de8b57..7578123 100644 --- a/compiler-rt/lib/scudo/standalone/string_utils.cpp +++ b/compiler-rt/lib/scudo/standalone/string_utils.cpp @@ -78,10 +78,11 @@ static int appendUnsigned(char **Buffer, const char *BufferEnd, u64 Num, static int appendSignedDecimal(char **Buffer, const char *BufferEnd, s64 Num, u8 MinNumberLength, bool PadWithZero) { const bool Negative = (Num < 0); - return appendNumber(Buffer, BufferEnd, - static_cast(Negative ? -Num : Num), 10, - MinNumberLength, PadWithZero, Negative, - /*Upper=*/false); + const u64 UnsignedNum = (Num == INT64_MIN) + ? static_cast(INT64_MAX) + 1 + : static_cast(Negative ? -Num : Num); + return appendNumber(Buffer, BufferEnd, UnsignedNum, 10, MinNumberLength, + PadWithZero, Negative, /*Upper=*/false); } // Use the fact that explicitly requesting 0 Width (%0s) results in UB and @@ -158,16 +159,18 @@ int formatString(char *Buffer, uptr BufferLength, const char *Format, CHECK(!((Precision >= 0 || LeftJustified) && *Cur != 's')); switch (*Cur) { case 'd': { - DVal = HaveLL ? va_arg(Args, s64) - : HaveZ ? va_arg(Args, sptr) : va_arg(Args, int); + DVal = HaveLL ? va_arg(Args, s64) + : HaveZ ? va_arg(Args, sptr) + : va_arg(Args, int); Res += appendSignedDecimal(&Buffer, BufferEnd, DVal, Width, PadWithZero); break; } case 'u': case 'x': case 'X': { - UVal = HaveLL ? va_arg(Args, u64) - : HaveZ ? va_arg(Args, uptr) : va_arg(Args, unsigned); + UVal = HaveLL ? va_arg(Args, u64) + : HaveZ ? va_arg(Args, uptr) + : va_arg(Args, unsigned); const bool Upper = (*Cur == 'X'); Res += appendUnsigned(&Buffer, BufferEnd, UVal, (*Cur == 'u') ? 10 : 16, Width, PadWithZero, Upper); diff --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp index 9fe7e24..481dfd8 100644 --- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp +++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp @@ -82,11 +82,16 @@ template static void testAllocator() { using AllocatorT = TestAllocator; auto Allocator = std::unique_ptr(new AllocatorT()); - EXPECT_FALSE(Allocator->isOwned(&Mutex)); - EXPECT_FALSE(Allocator->isOwned(&Allocator)); - scudo::u64 StackVariable = 0x42424242U; - EXPECT_FALSE(Allocator->isOwned(&StackVariable)); - EXPECT_EQ(StackVariable, 0x42424242U); + static scudo::u8 StaticBuffer[scudo::Chunk::getHeaderSize() + 1]; + EXPECT_FALSE( + Allocator->isOwned(&StaticBuffer[scudo::Chunk::getHeaderSize()])); + + scudo::u8 StackBuffer[scudo::Chunk::getHeaderSize() + 1]; + for (scudo::uptr I = 0; I < sizeof(StackBuffer); I++) + StackBuffer[I] = 0x42U; + EXPECT_FALSE(Allocator->isOwned(&StackBuffer[scudo::Chunk::getHeaderSize()])); + for (scudo::uptr I = 0; I < sizeof(StackBuffer); I++) + EXPECT_EQ(StackBuffer[I], 0x42U); constexpr scudo::uptr MinAlignLog = FIRST_32_SECOND_64(3U, 4U);