[compiler-rt][asan] Make wild-pointer crash error more useful

[oontvoo]

Summary

Right now, when you have an invalid memory address, asan would just crash and does not offer much useful info.
This patch attempted to give a bit more detail on the access.

[oontvoo]

diff

[vitalybuka]

compiler-rt/lib/asan/asan_descriptions.cpp

473

I guess this math is confusing and applies only to
__asan_region_is_poisoned
if (!AddrIsInMem(end)) return end;

Even __asan_region_is_poisoned can return a different offset from the range, not sure what is going to happen with other callers.

e.g. if we change change the test to
memmove(dest + 0x4567890123456789, p + 0x4567890123456789, 0x4567890123456789);
Address 0x4567e92123456799 is a wild pointer - originating from address 0x602000000010, offset = 0x4567890123456789
which is not true any more

We can't get access begin from this addr/size in general.
I propose just to print only bad address and access size to avoid confusion.

[oontvoo]

compiler-rt/lib/asan/asan_descriptions.cpp

473

Good point. Thanks!

[oontvoo]

updated diff

[vitalybuka]

compiler-rt/lib/asan/asan_descriptions.cpp

470–472

maybe

[oontvoo]

compiler-rt/test/asan/TestCases/wild_pointer.cpp

12

@vitalybuka : btw, do you know why this inclusion doesn't work here? (ie., if I uncomment it, it'd cause "file not found" error)
Am I missing something obvious? Other tests (eg., lsan/TestCases/use_registers.cpp) use this helper just fine.

[vitalybuka]

compiler-rt/test/asan/TestCases/wild_pointer.cpp

12

lsan probably somehow adds include dir
but I don't see why we need it here? (and in lsan as well)

Why not just printf("Expected bad addr: %p", p+offset)

[oontvoo]

updated diff

[vitalybuka]

compiler-rt/test/asan/TestCases/wild_pointer.cpp

1–3

-m64 is not enough it tests are being executed on 32bit platform

[oontvoo]

updated diff

compiler-rt/test/asan/TestCases/wild_pointer.cpp

1–3

Cool! I didn't know asan-64-bits was a thing!

[thakis]

This makes clang crash when building the adam runtime O_o: http://45.33.8.238/linux/41534/step_4.txt

That's a clang bug, but it als

[thakis]

o breaks the build. So I'm afraid you'll have to revert this until that clang crash is fixed :/

[oontvoo]

In D98280#2621150, @thakis wrote:

o breaks the build. So I'm afraid you'll have to revert this until that clang crash is fixed :/

Thanks. I've reverted it. Will wait and reland later

[morehouse]

Broke the test on Windows: https://lab.llvm.org/buildbot/#/builders/127/builds/7495

C:\b\slave\sanitizer-windows\llvm-project\compiler-rt\test\asan\TestCases\wild_pointer.cpp:20:11: error: CHECK: expected string not found in input
// CHECK: Expected bad addr: [[ADDR:0x[0-9,a-f]+]]
          ^
<stdin>:1:1: note: scanning from here
Expected bad addr: 4567998FA7CF6839

I guess Windows has a different printf format.

Please fix.

[oontvoo]

In D98280#2623697, @morehouse wrote:

Broke the test on Windows: https://lab.llvm.org/buildbot/#/builders/127/builds/7495

C:\b\slave\sanitizer-windows\llvm-project\compiler-rt\test\asan\TestCases\wild_pointer.cpp:20:11: error: CHECK: expected string not found in input
// CHECK: Expected bad addr: [[ADDR:0x[0-9,a-f]+]]
          ^
<stdin>:1:1: note: scanning from here
Expected bad addr: 4567998FA7CF6839

I guess Windows has a different printf format.

Please fix.

I've sent https://reviews.llvm.org/D98570

(Dont have a windows to test it, but I *think* it should work. PTAL Thanks!)

compiler-rt/lib/asan/asan_descriptions.h

struct StackAddressDescription {
   const char *frame_descr;
 
   void Print() const;
 };
 
 bool GetStackAddressInformation(uptr addr, uptr access_size,
                                 StackAddressDescription *descr);
 
+struct WildAddressDescription {
+  uptr addr;
+  uptr access_size;
+
+  void Print() const;
+};
+
 struct GlobalAddressDescription {
   uptr addr;
   // Assume address is close to at most four globals.
   static const int kMaxGlobals = 4;
   __asan_global globals[kMaxGlobals];
   u32 reg_sites[kMaxGlobals];
   uptr access_size;
   u8 size;

 class AddressDescription {
   struct AddressDescriptionData {
     AddressKind kind;
     union {
       ShadowAddressDescription shadow;
       HeapAddressDescription heap;
       StackAddressDescription stack;
       GlobalAddressDescription global;
-      uptr addr;
+      WildAddressDescription wild;
     };
   };
 
   AddressDescriptionData data;
 
  public:
   AddressDescription() = default;
   // shouldLockThreadRegistry allows us to skip locking if we're sure we already
   // have done it.
   explicit AddressDescription(uptr addr, bool shouldLockThreadRegistry = true)
       : AddressDescription(addr, 1, shouldLockThreadRegistry) {}
   AddressDescription(uptr addr, uptr access_size,
                      bool shouldLockThreadRegistry = true);
 
   uptr Address() const {
     switch (data.kind) {
       case kAddressKindWild:
-        return data.addr;
+        return data.wild.addr;
       case kAddressKindShadow:
         return data.shadow.addr;
       case kAddressKindHeap:
         return data.heap.addr;
       case kAddressKindStack:
         return data.stack.addr;
       case kAddressKindGlobal:
         return data.global.addr;
     }
     UNREACHABLE("AddressInformation kind is invalid");
   }
   void Print(const char *bug_descr = nullptr) const {
     switch (data.kind) {
       case kAddressKindWild:
-        Printf("Address %p is a wild pointer.\n", data.addr);
+        data.wild.Print();
         return;
       case kAddressKindShadow:
         return data.shadow.Print();
       case kAddressKindHeap:
         return data.heap.Print();
       case kAddressKindStack:
         return data.stack.Print();
       case kAddressKindGlobal:

compiler-rt/lib/asan/asan_descriptions.cpp

static bool GetShadowKind(uptr addr, ShadowKind *shadow_kind) {
   CHECK(!AddrIsInMem(addr));
   if (AddrIsInShadowGap(addr)) {
     *shadow_kind = kShadowKindGap;
   } else if (AddrIsInHighShadow(addr)) {
     *shadow_kind = kShadowKindHigh;
   } else if (AddrIsInLowShadow(addr)) {
     *shadow_kind = kShadowKindLow;
   } else {
-    CHECK(0 && "Address is not in memory and not in shadow?");
     return false;
   }
   return true;
 }
 
 bool DescribeAddressIfShadow(uptr addr) {
   ShadowAddressDescription descr;
   if (!GetShadowAddressInformation(addr, &descr)) return false;
if (isStackMemory) {
     return;
   }
 
   if (GetGlobalAddressInformation(addr, access_size, &data.global)) {
     data.kind = kAddressKindGlobal;
     return;
   }
   data.kind = kAddressKindWild;
-  addr = 0;
+  data.wild.addr = addr;
+  data.wild.access_size = access_size;
+}
+
+void WildAddressDescription::Print() const {
+  Printf("Address %p is a wild pointer inside of access range of size %p.\n",
+         addr, access_size);
 }
 
 void PrintAddressDescription(uptr addr, uptr access_size,
                              const char *bug_type) {
   ShadowAddressDescription shadow_descr;
   if (GetShadowAddressInformation(addr, &shadow_descr)) {
     shadow_descr.Print();
     return;
   }

compiler-rt/test/asan/TestCases/wild_pointer.cpp

+// Needs -m64 because the tested address is too large for m32
+// RUN: %clangxx_asan %s -o %t -m64
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+#include <stdarg.h>
+#include <stdio.h>
+#include <string.h>
+
+int main() {
+  char *p = new char;
+  char *dest = new char;
+  const size_t offset = 0x4567890123456789;
+  // Flush it so the output came out before the asan report.
+  fprintf(stderr, "Expected bad addr: %p\n", p + offset);
+  fflush(stderr);
+  memmove(dest, p, offset);
+  return 0;
+}
+
+// CHECK: Expected bad addr: [[ADDR:0x[0-9,a-f]+]]
+// CHECK: AddressSanitizer: unknown-crash on address [[ADDR]]
+// CHECK: Address [[ADDR]] is a wild pointer inside of access range of size 0x4567890123456789