CHECK failed: ... "((trace)) != (0)" (0x0, 0x0)

[ramosian-glider]

Originally reported on Google Code with ID 321

$ bin/clang++ ../projects/compiler-rt/test/asan/TestCases/time_interceptor.cc -fsanitize=address
 ./a.out 
=================================================================
==27297==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200000eff0 at
pc 0x43cb36 bp 0x7fff3688ed10 sp 0x7fff3688e4d0
WRITE of size 8 at 0x60200000eff0 thread T0
    #0 0x43cb35 in __interceptor_time.part.54 /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:544
    #1 0x4b34c5 in main (/usr/local/google/ssd/llvm/llvm_cmake_build/a.out+0x4b34c5)
    #2 0x7fb9d32d976c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
    #3 0x4b325c in _start (/usr/local/google/ssd/llvm/llvm_cmake_build/a.out+0x4b325c)

0x60200000eff0 is located 0 bytes inside of 8-byte region [0x60200000eff0,0x60200000eff8)
freed by thread T0 here:
==27297==AddressSanitizer CHECK failed: /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:205
"((trace)) != (0)" (0x0, 0x0)
    #0 0x49b98e in __asan::AsanCheckFailed(char const*, int, char const*, unsigned
long long, unsigned long long) /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_rtl.cc:70
    #1 0x49ff23 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned
long long, unsigned long long) /usr/local/google/ssd/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common.cc:76
    #2 0x41c6ab in GetStackTraceFromId /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:205
    #3 0x41c6ab in __asan::AsanChunkView::GetFreeStack(__sanitizer::StackTrace*) /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_allocator2.cc:214
    #4 0x4989bf in __asan::DescribeHeapAddress(unsigned long, unsigned long) /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_report.cc:462
    #5 0x49a617 in __asan_report_error /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/asan_report.cc:888
    #6 0x43cb56 in __interceptor_time.part.54 /usr/local/google/ssd/llvm/projects/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:544
    #7 0x4b34c5 in main (/usr/local/google/ssd/llvm/llvm_cmake_build/a.out+0x4b34c5)
    #8 0x7fb9d32d976c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226
    #9 0x4b325c in _start (/usr/local/google/ssd/llvm/llvm_cmake_build/a.out+0x4b325c)

Reported by ramosian.glider on 2014-06-17 15:14:09

[ramosian-glider]

This happens because a call to REAL(time) overwrites the stack ID in the freed memory
region:

 539 INTERCEPTOR(unsigned long, time, unsigned long *t) {
 540   void *ctx;
 541   COMMON_INTERCEPTOR_ENTER(ctx, time, t);
 542   unsigned long res = REAL(time)(t);
 543   if (t && res != (unsigned long)-1) {
 544     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, t, sizeof(*t));
 545   }
 546   return res;
 547 }

There is a plenty of other places in sanitizer_common_interceptors.inc where an interceptor
calls the real functions before checking the pointers for being addressable.
Is it possible to store the stack ID somewhere else?

If not, we must fix all the interceptors that may write to heap-allocated memory:
1. Upon INTERCEPTOR(fname) entry, copy the chunk header of every pointer that fname()
can potentially overwrite (most of the time there's only one such pointer);
2. Call REAL(fname)(ptr)
3. Check the return value of the function to determine which pointers were overwritten.
4. For each such pointer check whether it was addressable. If it was not, restore the
chunk header and bail out.

It's still theoretically possible that the memory passed to INTERCEPTOR(fname) is unmapped
already (although ASan allocator doesn't do that), and we get a SEGV before knowing
that it's unaddressable.

Reported by ramosian.glider on 2014-06-17 16:31:24

[ramosian-glider]

Another important conclusion is that our tests must be less tolerant to CHECK failures.
I'd suggest somehow inserting a 'CHECK-NOT: CHECK failed' into each lit test by default.

Reported by ramosian.glider on 2014-06-17 16:34:27

[ramosian-glider]

Re: the CHECK-failed problems - we can make ASan optionally call _exit(0) on CHECK-failure,
instead of _exit(1).
Then "not /path/to/binary" in lit-tests will fail on CHECK-failure, and we will notice
the error.

Reported by samsonov@google.com on 2014-06-17 16:46:47

[ramosian-glider]

The first solution sounds cleaner...

Reported by tetra2005 on 2014-06-17 16:57:53

[ramosian-glider]

Why not exit with a signal code on CHECK failure?  The 'not' utility will return non-zero
if the subprocess exits due to a signal instead of a normal non-zero exit code.  This
makes it so that mainline LLVM tests using not fail properly when an assertion fails.
 Calling abort() is probably inappropriate in the sanitizer, but maybe some other approach
is OK.

Reported by rnk@google.com on 2014-06-17 18:32:28

[ramosian-glider]

>> Another important conclusion is that our tests must be less tolerant to CHECK failures
Agree. I would add a check for SUMMARY

Reported by konstantin.s.serebryany on 2014-06-17 18:45:50

[ramosian-glider]

>> Is it possible to store the stack ID somewhere else?
We can use allocator's metadata (currently, not used in asan at all), 
but it will consume extra RAM == bad. 




>> 1. Upon INTERCEPTOR(fname) entry, copy the chunk header of every pointer that fname()
can potentially overwrite

Too expensive. 
You will have to check if this is a heap (what if not?)
then if it is small heap or larger, then get the header.

Reported by konstantin.s.serebryany on 2014-06-17 18:48:11

[ramosian-glider]

I would fix these issues lazily one-by-one for each interceptor. 
This one, e.g. like this: 

 INTERCEPTOR(unsigned long, time, unsigned long *t) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, time, t);
   long tmp;
   unsigned long res = REAL(time)(&tmp);
   if (t && res != (unsigned long)-1) {
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, t, sizeof(*t));
   }
   if (t)
      *t = tmp;
   return res;
 }

I admit this may not be possible for all the cases. 

Reported by konstantin.s.serebryany on 2014-06-17 18:54:55

[ramosian-glider]

Per an offline discussion we've decided to act in a lazy fashion.
I've fixed the interceptors for time() and frexp() in r211153 and added comments to
other interceptors that may potentially corrupt the stack IDs.
Kostya talked me out of raising a signal on CHECK(), because this is subtle and may
break someone somewhere.

Reported by ramosian.glider on 2014-06-18 09:38:09

[ramosian-glider]

Reported by ramosian.glider on 2015-07-30 09:05:33

• Labels added: ProjectAddressSanitizer