[PATCH] D15807: [asan] Fix internal CHECK failure on double free in recovery mode.

[Maxim Ostapenko via llvm-commits]

m.ostapenko added a reviewer: kcc.
m.ostapenko removed a project: lld.
m.ostapenko updated this revision to Diff 45986.
m.ostapenko added a comment.

Added testcase for the bug. I didn't updated the patch itself because we still want continue execution after double free was detected. Can this approach be acceptable for recovery mode?


Repository:
  rL LLVM

http://reviews.llvm.org/D15807

Files:
  lib/asan/asan_allocator.cc
  test/asan/TestCases/double-free.cc

Index: test/asan/TestCases/double-free.cc
===================================================================
--- test/asan/TestCases/double-free.cc
+++ test/asan/TestCases/double-free.cc
@@ -4,6 +4,10 @@
 // Also works if no malloc context is available.
 // RUN: %env_asan_opts=malloc_context_size=0:fast_unwind_on_malloc=0 not %run %t 2>&1 | FileCheck %s
 // RUN: %env_asan_opts=malloc_context_size=0:fast_unwind_on_malloc=1 not %run %t 2>&1 | FileCheck %s
+
+// RUN: %clangxx_asan -O0 -fsanitize-recover=address %s -o %t 2>&1
+// RUN: %env_asan_opts=halt_on_error=false %run %t 2>&1 | FileCheck %s --check-prefix CHECK-RECOVER
+
 // XFAIL: arm-linux-gnueabi
 // XFAIL: armv7l-unknown-linux-gnueabihf
 
@@ -23,5 +27,7 @@
   // MALLOC-CTX: #1 0x{{.*}} in main {{.*}}double-free.cc:[[@LINE-7]]
   // CHECK: allocated by thread T0 here:
   // MALLOC-CTX: double-free.cc:[[@LINE-12]]
+  // CHECK-RECOVER: AddressSanitizer: attempting double-free{{.*}}in thread T0
+  // CHECK-RECOVER-NOT: AddressSanitizer CHECK failed:
   return res;
 }
Index: lib/asan/asan_allocator.cc
===================================================================
--- lib/asan/asan_allocator.cc
+++ lib/asan/asan_allocator.cc
@@ -457,18 +457,25 @@
     return res;
   }
 
-  void AtomicallySetQuarantineFlag(AsanChunk *m, void *ptr,
+  // Set quarantine flag if chunk is allocated, issue ASan error report on
+  // available and quarantined ones. Return true on success, false otherwise.
+  bool AtomicallySetQuarantineFlagIfAllocated(AsanChunk *m, void *ptr,
                                    BufferedStackTrace *stack) {
     u8 old_chunk_state = CHUNK_ALLOCATED;
     // Flip the chunk_state atomically to avoid race on double-free.
-    if (!atomic_compare_exchange_strong((atomic_uint8_t*)m, &old_chunk_state,
-                                        CHUNK_QUARANTINE, memory_order_acquire))
+    if (!atomic_compare_exchange_strong((atomic_uint8_t *)m, &old_chunk_state,
+                                        CHUNK_QUARANTINE,
+                                        memory_order_acquire)) {
       ReportInvalidFree(ptr, old_chunk_state, stack);
+      // It's not safe to push a chunk in quarantine on invalid free.
+      return false;
+    }
     CHECK_EQ(CHUNK_ALLOCATED, old_chunk_state);
+    return true;
   }
 
   // Expects the chunk to already be marked as quarantined by using
-  // AtomicallySetQuarantineFlag.
+  // AtomicallySetQuarantineFlagIfAllocated.
   void QuarantineChunk(AsanChunk *m, void *ptr, BufferedStackTrace *stack,
                        AllocType alloc_type) {
     CHECK_EQ(m->chunk_state, CHUNK_QUARANTINE);
@@ -522,7 +529,8 @@
     }
     ASAN_FREE_HOOK(ptr);
     // Must mark the chunk as quarantined before any changes to its metadata.
-    AtomicallySetQuarantineFlag(m, ptr, stack);
+    // Do not quarantine given chunk if we failed to set CHUNK_QUARANTINE flag.
+    if (!AtomicallySetQuarantineFlagIfAllocated(m, ptr, stack)) return;
     QuarantineChunk(m, ptr, stack, alloc_type);
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15807.45986.patch
Type: text/x-patch
Size: 3020 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160126/16e76a35/attachment.bin>