[asan] Assert in __sanitizer_ptr_{sub,cmp} if one of the pointers was freed.

[filcab]

Summary

This (partially) implements the check mentioned at
http://kristerw.blogspot.co.uk/2016/04/dangling-pointers-and-undefined-behavior.html
(via John Regehr)

Quoting:
"That the behavior is undefined follows from C11 6.2.4 "Storage
durations of objects"

The lifetime of an object is the portion of program execution during
which storage is guaranteed to be reserved for it. An object exists, has
a constant address, and retains its last-stored value throughout its
lifetime. If an object is referred to outside of its lifetime, the
behavior is undefined. The value of a pointer becomes indeterminate when
the object it points to (or just past) reaches the end of its lifetime.

and 7.22.3 "Memory management functions" that says that free ends the
lifetime of objects

The lifetime of an allocated object extends from the allocation until
the deallocation.

"

We can probably implement this for stack variables too, but I think this
is a good start to see if there's interest in this check.
We can also hide this behind a flag, too.

[kcc]

test/asan/TestCases/invalid-pointer-pairs.cc

1 ↗

(On Diff #55491)

there is no CHECK in this file, is that right?

[filcab]

Updated with a proper test case.

test/asan/TestCases/invalid-pointer-pairs.cc

1 ↗

(On Diff #55491)

Errr, no. Sorry.
I forgot to add. I'm submitting a new version with the proper test.

[kcc]

LGTM

compiler-rt/trunk/lib/asan/asan_allocator.h

 
 void InitializeAllocator(const AllocatorOptions &options);
 void ReInitializeAllocator(const AllocatorOptions &options);
 void GetAllocatorOptions(AllocatorOptions *options);
 
 class AsanChunkView {
  public:
   explicit AsanChunkView(AsanChunk *chunk) : chunk_(chunk) {}
-  bool IsValid();   // Checks if AsanChunkView points to a valid allocated
+  bool IsValid();        // Checks if AsanChunkView points to a valid allocated
                          // or quarantined chunk.
-  uptr Beg();       // First byte of user memory.
-  uptr End();       // Last byte of user memory.
-  uptr UsedSize();  // Size requested by the user.
+  bool IsAllocated();    // Checks if the memory is currently allocated.
+  uptr Beg();            // First byte of user memory.
+  uptr End();            // Last byte of user memory.
+  uptr UsedSize();       // Size requested by the user.
   uptr AllocTid();
   uptr FreeTid();
   bool Eq(const AsanChunkView &c) const { return chunk_ == c.chunk_; }
   StackTrace GetAllocStack();
   StackTrace GetFreeStack();
   bool AddrIsInside(uptr addr, uptr access_size, sptr *offset) {
     if (addr >= Beg() && (addr + access_size) <= End()) {
       *offset = addr - Beg();

compiler-rt/trunk/lib/asan/asan_allocator.cc

 
 static AsanAllocator &get_allocator() {
   return instance.allocator;
 }
 
 bool AsanChunkView::IsValid() {
   return chunk_ && chunk_->chunk_state != CHUNK_AVAILABLE;
 }
+bool AsanChunkView::IsAllocated() {
+  return chunk_ && chunk_->chunk_state == CHUNK_ALLOCATED;
+}
 uptr AsanChunkView::Beg() { return chunk_->Beg(); }
 uptr AsanChunkView::End() { return Beg() + UsedSize(); }
 uptr AsanChunkView::UsedSize() { return chunk_->UsedSize(); }
 uptr AsanChunkView::AllocTid() { return chunk_->alloc_tid; }
 uptr AsanChunkView::FreeTid() { return chunk_->free_tid; }
 
 static StackTrace GetStackTraceFromId(u32 id) {
   CHECK(id);

compiler-rt/trunk/lib/asan/asan_report.cc

 }
 
 static INLINE void CheckForInvalidPointerPair(void *p1, void *p2) {
   if (!flags()->detect_invalid_pointer_pairs) return;
   uptr a1 = reinterpret_cast<uptr>(p1);
   uptr a2 = reinterpret_cast<uptr>(p2);
   AsanChunkView chunk1 = FindHeapChunkByAddress(a1);
   AsanChunkView chunk2 = FindHeapChunkByAddress(a2);
-  bool valid1 = chunk1.IsValid();
-  bool valid2 = chunk2.IsValid();
-  if ((valid1 != valid2) || (valid1 && valid2 && !chunk1.Eq(chunk2))) {
-    GET_CALLER_PC_BP_SP;                                              \
+  bool valid1 = chunk1.IsAllocated();
+  bool valid2 = chunk2.IsAllocated();
+  if (!valid1 || !valid2 || !chunk1.Eq(chunk2)) {
+    GET_CALLER_PC_BP_SP;
     return ReportInvalidPointerPair(pc, bp, sp, a1, a2);
   }
 }
 // ----------------------- Mac-specific reports ----------------- {{{1
 
 void ReportMacMzReallocUnknown(uptr addr, uptr zone_ptr, const char *zone_name,
                                BufferedStackTrace *stack) {
   ScopedInErrorReport in_report;

compiler-rt/trunk/test/asan/TestCases/invalid-pointer-pairs.cc

+// RUN: %clangxx_asan -O0 %s -o %t -mllvm -asan-detect-invalid-pointer-pair
+
+// RUN: %env_asan_opts=detect_invalid_pointer_pairs=1 %run %t k 2>&1 | FileCheck %s -check-prefix=OK -allow-empty
+// RUN: %env_asan_opts=detect_invalid_pointer_pairs=1 not %run %t g 2>&1 | FileCheck %s -check-prefix=CMP -check-prefix=ALL-ERRORS
+// RUN: %env_asan_opts=detect_invalid_pointer_pairs=1 not %run %t s 2>&1 | FileCheck %s -check-prefix=SUB -check-prefix=ALL-ERRORS
+// RUN: %env_asan_opts=detect_invalid_pointer_pairs=1 not %run %t f 2>&1 | FileCheck %s -check-prefix=FREE -check-prefix=ALL-ERRORS
+
+#include <assert.h>
+#include <stdlib.h>
+
+int main(int argc, char **argv) {
+  // ALL-ERRORS: ERROR: AddressSanitizer: invalid-pointer-pair
+  // [[PTR1:0x[0-9a-f]+]] [[PTR2:0x[0-9a-f]+]]
+  assert(argc >= 2);
+  char *p = (char *)malloc(42);
+  char *q = (char *)malloc(42);
+  switch (argv[1][0]) {
+  case 'g':
+    // CMP: #0 {{.*}} in main {{.*}}invalid-pointer-pairs.cc:[[@LINE+1]]:14
+    return p > q;
+  case 's':
+    // SUB: #0 {{.*}} in main {{.*}}invalid-pointer-pairs.cc:[[@LINE+1]]:14
+    return p - q;
+  case 'k': {
+    // OK-NOT: ERROR
+    char *p2 = p + 20;
+    return p > p2;
+  }
+  case 'f': {
+    char *p3 = p + 20;
+    free(p);
+    // FREE: #0 {{.*}} in main {{.*}}invalid-pointer-pairs.cc:[[@LINE+2]]:14
+    // FREE: freed by thread
+    return p < p3;
+  }
+  }
+}