Do not rely on that subject of ErrorAllocTypeMismatch is a heap address.

[marxin]

Summary

When delete operator is called for a stack memory address, what can happen is that:

#0  __asan::Allocator::AtomicallySetQuarantineFlagIfAllocated (this=0x7ffff7fa62e1 <typeinfo name for std::locale::facet+1>, m=0x7ffff7cb04b0 <__sanitizer::theDepot+7184496>, ptr=0x12400, stack=0x7ffff75d6440 <__sanitizer::theDepot>)
    at /home/marxin/Programming/gcc/libsanitizer/asan/asan_allocator.cc:548
#1  0x00007ffff73e3ff8 in __asan::Allocator::Deallocate (this=0x7ffff7530e80 <__asan::instance>, ptr=0x7fffffffdab0, delete_size=16, delete_alignment=0, stack=0x7fffffffd1d0, alloc_type=__asan::FROM_NEW)
    at /home/marxin/Programming/gcc/libsanitizer/asan/asan_allocator.cc:629
#2  0x00007ffff73dfd9c in __asan::asan_delete (ptr=0x7fffffffdab0, size=16, alignment=0, stack=0x7fffffffd1d0, alloc_type=__asan::FROM_NEW) at /home/marxin/Programming/gcc/libsanitizer/asan/asan_allocator.cc:870
#3  0x00007ffff74b5aca in operator delete (ptr=0x7fffffffdab0, size=16) at /home/marxin/Programming/gcc/libsanitizer/asan/asan_new_delete.cc:177
#4  0x0000000000401554 in Baz::~Baz (this=0x7fffffffda70, __in_chrg=<optimized out>) at pr86229-v2.cpp:25
#5  0x00000000004013bf in main () at pr86229-v2.cpp:32

but luckily:

(gdb) p m->chunk_state
$4 = 2

thus AtomicallySetQuarantineFlagIfAllocated does not call ReportInvalidFree.
But then we correctly call ReportAllocTypeMismatch, but currently only a heap address is expected:

GetHeapAddressInformation(addr, 1, &addr_description);

That however immediately bails out because it's not a heap address. And thus we end up with uninitialized
addr_description. That then triggers a crash in Print function.

[kcc]

Vitaly, please review.
Martin, is a test possible here?

[vitalybuka]

lib/asan/asan_errors.h

114

Please use approach similar to ErrorFreeNotMalloced with "AddressDescription addr_description;"

123

please clang-format the patch
e.g

git clang-format -f --style=file HEAD^

[marxin]

Updated patch, I also found a test-case that fails for clang before my patch:

cat pr86229-bad.cpp
#include <iostream>

struct FooBar {
  int kii;
  int lii;
  int lii3;
};

struct Baz {
  FooBar *foo;

  Baz(FooBar *f) : foo(f) {}

  Baz() {
    foo = new FooBar();
  }

  ~Baz() {
    delete foo;
  }
};

void setme(volatile int *p)
{
  *p = 2;
}

int main() {
  int a;
  setme (&a);

  FooBar f;
  __builtin_printf ("&f: %p\n", &f);
  Baz baz(&f);
  return 0;
}

$ clang++ pr86229-bad.cpp -fsanitize=address && ./a.out
&f: 0x7fffffffda90
=================================================================
==20122==ERROR: AddressSanitizer: alloc-dealloc-mismatch (INVALID vs operator delete) on 0x000000009000
    #0 0x52bda8 in operator delete(void*) /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:167:3
    #1 0x52e6a1 in Baz::~Baz() (/home/marxin/Programming/testcases/a.out+0x52e6a1)
    #2 0x52e523 in main (/home/marxin/Programming/testcases/a.out+0x52e523)
    #3 0x7ffff7061fea in __libc_start_main /usr/src/debug/glibc-2.27-6.1.x86_64/csu/../csu/libc-start.c:308:16
    #4 0x41e379 in _start /home/abuild/rpmbuild/BUILD/glibc-2.27/csu/../sysdeps/x86_64/start.S:120

0x000000001000 is located 5537600 bytes to the left of 140737488345936-byte region [0x000000576c20,0x800000574770)
==20122==AddressSanitizer CHECK failed: /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_descriptions.cc:178 "((res.trace)) != (0)" (0x0, 0x0)
    #0 0x4fc38b in __asan::AsanCheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_rtl.cc:74:5
    #1 0x514f69 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_termination.cc:79:24
    #2 0x42c74c in GetStackTraceFromId /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_descriptions.cc:178:3
    #3 0x42c74c in __asan::HeapAddressDescription::Print() const /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_descriptions.cc:418:62
    #4 0x42d682 in __asan::ErrorAllocTypeMismatch::Print() /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_errors.cc:136:25
    #5 0x4f6515 in __asan::ErrorDescription::Print() /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_errors.h:423:7
    #6 0x4f6515 in _ZN6__asan19ScopedInErrorReportD4Ev /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_report.cc:142:55
    #7 0x4f6515 in __asan::ReportAllocTypeMismatch(unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType, __asan::AllocType) /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_report.cc:241:23
    #8 0x4273d6 in __asan::Allocator::Deallocate(void*, unsigned long, unsigned long, __sanitizer::BufferedStackTrace*, __asan::AllocType) /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_allocator.cc:635:32
    #9 0x52bd82 in operator delete(void*) /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:167:3
    #10 0x52e6a1 in Baz::~Baz() (/home/marxin/Programming/testcases/a.out+0x52e6a1)
    #11 0x52e523 in main (/home/marxin/Programming/testcases/a.out+0x52e523)
    #12 0x7ffff7061fea in __libc_start_main /usr/src/debug/glibc-2.27-6.1.x86_64/csu/../csu/libc-start.c:308:16
    #13 0x41e379 in _start /home/abuild/rpmbuild/BUILD/glibc-2.27/csu/../sysdeps/x86_64/start.S:120

with the patch applied I see:

$ clang++ pr86229-bad.cpp -fsanitize=address && ./a.out
&f: 0x7fffffffda90
=================================================================
==24415==ERROR: AddressSanitizer: alloc-dealloc-mismatch (INVALID vs operator delete) on 0x7fffffffda90
    #0 0x52bef8 in operator delete(void*) /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:167:3
    #1 0x52e7f1 in Baz::~Baz() (/home/marxin/Programming/testcases/a.out+0x52e7f1)
    #2 0x52e673 in main (/home/marxin/Programming/testcases/a.out+0x52e673)
    #3 0x7ffff7061fea in __libc_start_main /usr/src/debug/glibc-2.27-6.1.x86_64/csu/../csu/libc-start.c:308:16
    #4 0x41e379 in _start /home/abuild/rpmbuild/BUILD/glibc-2.27/csu/../sysdeps/x86_64/start.S:120

Address 0x7fffffffda90 is located in stack of thread T0 at offset 48 in frame
    #0 0x52e52f in main (/home/marxin/Programming/testcases/a.out+0x52e52f)

  This frame has 3 object(s):
    [32, 36) 'a'
    [48, 60) 'f' <== Memory access at offset 48 is inside this variable
    [80, 88) 'baz'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: alloc-dealloc-mismatch /home/marxin/BIG/Programming/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:167:3 in operator delete(void*)
==24415==HINT: if you don't care about these errors you may set ASAN_OPTIONS=alloc_dealloc_mismatch=0
==24415==ABORTING

Note that I have issues with running asan test due to i386 target that cmake configures, but then linkage fails.
I would appreciate if you adapt the test, thanks.

[marxin]

@vitalybuka: I modified the patch based on your notes.

[kcc]

could you please actually add the test to the patch?
(somewhere to compiler-rt/test/asan/TestCases/Linux, probably)

[vitalybuka]

Thanks, code LGTM

In D54856#1309991, @kcc wrote:

could you please actually add the test to the patch?
(somewhere to compiler-rt/test/asan/TestCases/Linux, probably)

I'd recommend a tests which check reports for all possible new types of pointers.
Similar example is TestCases/Linux/new_delete_mismatch.cc.
You can use argv[1] to test different types in a single test.

[marxin]

Updated patch that introduces 2 new tests.

[vitalybuka]

lib/asan/asan_errors.h

122–123

/asan_errors.h:122:9: warning: field 'dealloc_type' will be initialized after field 'addr_description' [-Wreorder]

test/asan/TestCases/Linux/new_delete_mismatch_stack.cc

17 ↗

(On Diff #175666)

Fails on 32 bit

/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/test/asan/TestCases/Linux/new_delete_mismatch_stack.cc:17:11: error: CHECK: expected string not found in input
// CHECK: 'a'{{.*}} <== Memory access at offset 32 is inside this variable
          ^
<stdin>:8:63: note: scanning from here
Address 0xff911490 is located in stack of thread T0 at offset 16 in frame
                                                              ^
<stdin>:12:17: note: possible intended match here
 [16, 26) 'a' (line 10) <== Memory access at offset 16 is inside this variable
                ^

--

********************
Testing: 0 .. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
Testing Time: 26.45s
********************
Failing Tests (1):
    AddressSanitizer-i386-linux :: TestCases/Linux/new_delete_mismatch_stack.cc

[marxin]

Thanks for testing Vitaly. I must confirm that i386 is broken on my openSUSE, thus I can't test that.

[marxin]

Updated version of the patch.

[marxin]

The right patch, previous one belongs to GCC :)

[vitalybuka]

Thanks,
LGTM

[marxin]

It's installed.

lib/asan/asan_descriptions.h

struct HeapAddressDescription {
   ChunkAccess chunk_access;
 
   void Print() const;
 };
 
 bool GetHeapAddressInformation(uptr addr, uptr access_size,
                                HeapAddressDescription *descr);
 bool DescribeAddressIfHeap(uptr addr, uptr access_size = 1);
+bool DescribeAddressIfStack(uptr addr, uptr access_size = 1);
 
 struct StackAddressDescription {
   uptr addr;
   uptr tid;
   uptr offset;
   uptr frame_pc;
   uptr access_size;
   const char *frame_descr;

lib/asan/asan_errors.h

ErrorFreeNotMalloced(u32 tid, BufferedStackTrace *stack, uptr addr)
       : ErrorBase(tid, 40, "bad-free"),
         free_stack(stack),
         addr_description(addr, /*shouldLockThreadRegistry=*/false) {}
   void Print();
 };
 
 struct ErrorAllocTypeMismatch : ErrorBase {
   const BufferedStackTrace *dealloc_stack;
-  HeapAddressDescription addr_description;
   AllocType alloc_type, dealloc_type;
+  uptr addr;
 
   ErrorAllocTypeMismatch() = default;  // (*)
-  ErrorAllocTypeMismatch(u32 tid, BufferedStackTrace *stack, uptr addr,
+  ErrorAllocTypeMismatch(u32 tid, BufferedStackTrace *stack, uptr addr_,
                          AllocType alloc_type_, AllocType dealloc_type_)
       : ErrorBase(tid, 10, "alloc-dealloc-mismatch"),
         dealloc_stack(stack),
         alloc_type(alloc_type_),
-        dealloc_type(dealloc_type_) {
-    GetHeapAddressInformation(addr, 1, &addr_description);
-  };
+        dealloc_type(dealloc_type_),
+        addr (addr_) {}
   void Print();
 };
 
 struct ErrorMallocUsableSizeNotOwned : ErrorBase {
   const BufferedStackTrace *stack;
   AddressDescription addr_description;
 
   ErrorMallocUsableSizeNotOwned() = default;  // (*)

lib/asan/asan_errors.cc

void ErrorAllocTypeMismatch::Print() {
   static const char *dealloc_names[] = {"INVALID", "free", "operator delete",
                                         "operator delete []"};
   CHECK_NE(alloc_type, dealloc_type);
   Decorator d;
   Printf("%s", d.Error());
   Report("ERROR: AddressSanitizer: %s (%s vs %s) on %p\n",
          scariness.GetDescription(),
          alloc_names[alloc_type], dealloc_names[dealloc_type],
-         addr_description.addr);
+         addr);
   Printf("%s", d.Default());
   CHECK_GT(dealloc_stack->size, 0);
   scariness.Print();
   GET_STACK_TRACE_FATAL(dealloc_stack->trace[0], dealloc_stack->top_frame_bp);
   stack.Print();
-  addr_description.Print();
+  if (!DescribeAddressIfHeap (addr)) {
+    if (!DescribeAddressIfStack (addr))
+        DescribeAddressIfGlobal (addr, 1, scariness.GetDescription ());
+  }
   ReportErrorSummary(scariness.GetDescription(), &stack);
   Report(
       "HINT: if you don't care about these errors you may set "
       "ASAN_OPTIONS=alloc_dealloc_mismatch=0\n");
 }
 
 void ErrorMallocUsableSizeNotOwned::Print() {
   Decorator d;