Open GoogleCodeExporter opened 9 years ago
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.
Original comment by ramosian.glider@gmail.com
on 17 Jun 2014 at 4:31
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.
Original comment by ramosian.glider@gmail.com
on 17 Jun 2014 at 4:34
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.
Original comment by samso...@google.com
on 17 Jun 2014 at 4:46
The first solution sounds cleaner...
Original comment by tetra2...@gmail.com
on 17 Jun 2014 at 4:57
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.
Original comment by rnk@google.com
on 17 Jun 2014 at 6:32
>> Another important conclusion is that our tests must be less tolerant to
CHECK failures
Agree. I would add a check for SUMMARY
Original comment by konstant...@gmail.com
on 17 Jun 2014 at 6:45
>> 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.
Original comment by konstant...@gmail.com
on 17 Jun 2014 at 6:48
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.
Original comment by konstant...@gmail.com
on 17 Jun 2014 at 6:54
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.
Original comment by ramosian.glider@gmail.com
on 18 Jun 2014 at 9:38
Original comment by ramosian.glider@gmail.com
on 30 Jul 2015 at 9:05
Adding Project:AddressSanitizer as part of GitHub migration.
Original comment by ramosian.glider@gmail.com
on 30 Jul 2015 at 9:06
Original issue reported on code.google.com by
ramosian.glider@gmail.com
on 17 Jun 2014 at 3:14