[PATCH] D88170: [scudo][standalone] Fix tests under ASan/UBSan

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad created this revision.
cryptoad added reviewers: pcc, hctim, cferris, mcgrathr.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.
cryptoad requested review of this revision.

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!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88170

Files:
  compiler-rt/lib/scudo/standalone/string_utils.cpp
  compiler-rt/lib/scudo/standalone/tests/combined_test.cpp


Index: compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
===================================================================
--- compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -82,11 +82,16 @@
   using AllocatorT = TestAllocator<Config>;
   auto Allocator = std::unique_ptr<AllocatorT>(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);
 
Index: compiler-rt/lib/scudo/standalone/string_utils.cpp
===================================================================
--- compiler-rt/lib/scudo/standalone/string_utils.cpp
+++ compiler-rt/lib/scudo/standalone/string_utils.cpp
@@ -78,10 +78,11 @@
 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<u64>(Negative ? -Num : Num), 10,
-                      MinNumberLength, PadWithZero, Negative,
-                      /*Upper=*/false);
+  const u64 UnsignedNum = (Num == INT64_MIN)
+                              ? static_cast<u64>(INT64_MAX) + 1
+                              : static_cast<u64>(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 @@
     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);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88170.293792.patch
Type: text/x-patch
Size: 3180 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200923/fc8f49d5/attachment.bin>