This is an archive of the discontinued LLVM Phabricator instance.

[asan] Provide an interface to update an allocation stack trace.
ClosedPublic

Authored by eugenis on Oct 18 2019, 5:21 PM.

Details

Summary

Sometimes an allocation stack trace is not very informative. Provide a
way to replace it with a stack trace of the user's choice.

Diff Detail

Event Timeline

eugenis created this revision.Oct 18 2019, 5:21 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 18 2019, 5:21 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
dvyukov added a subscriber: dvyukov.

FWIW for kernel sanitizers we want an ability to memorize few _additional_ stacks per heap object:
https://bugzilla.kernel.org/show_bug.cgi?id=198437
That's very useful for any kind of asynchronous processing environments that involves tasks, callbacks, thread pools, etc (read - almost all large C/C++ software today) because allocation/free stack may be meaningless and/or other stacks may be crucially important.

kcc added inline comments.Oct 21 2019, 11:01 AM
compiler-rt/include/sanitizer/asan_interface.h
320

Add some explanation of when it is going to be successful.
Also, what about threads?
This API is racey

eugenis marked an inline comment as done.Oct 21 2019, 11:36 AM
eugenis added inline comments.
compiler-rt/include/sanitizer/asan_interface.h
320

Racey, as in someone could be deallocating this memory on another thread?
This API should be used in a situation when this is possible, of course.
It's exactly the same as malloc_usable_size().

kcc added inline comments.Oct 21 2019, 12:44 PM
compiler-rt/include/sanitizer/asan_interface.h
320

malloc_usable_size() only reads the data.
__asan_update_allocation writes the data, so if two threads try to simultaneously call this function, we'll have a race on alloc_context_id

BTW, the name "asan_update_allocation" doesn't seem to reflect the action.
Maybe "
asan_update_allocation_context"?

eugenis updated this revision to Diff 226732.Oct 28 2019, 12:55 PM

Use atomic to update the allocation stack.
Rename the function.

eugenis marked an inline comment as done.Oct 28 2019, 12:59 PM

FWIW for kernel sanitizers we want an ability to memorize few _additional_ stacks per heap object:
https://bugzilla.kernel.org/show_bug.cgi?id=198437
That's very useful for any kind of asynchronous processing environments that involves tasks, callbacks, thread pools, etc (read - almost all large C/C++ software today) because allocation/free stack may be meaningless and/or other stacks may be crucially important.

Agreed. We don't have space in the chunk header to fit any additional stack traces for free though. And the kernel has its own runtime implementation anyway.

compiler-rt/include/sanitizer/asan_interface.h
320

OK, I made the store atomic. It could still race with deallocation, but the same is true for malloc_usable_size.

Changed the name to asan_update_allocation_context

Btw, we can implement this really nicely in MSan with origin tracking.
And if we really wanted, we could pull the chained origin stack depot code to sanitizer_common and use it in asan for the extra stack traces.
But this sounds like a lot of work for a fringe feature.

kcc accepted this revision.Oct 30 2019, 4:26 PM

LGTM

This revision is now accepted and ready to land.Oct 30 2019, 4:26 PM
This revision was automatically updated to reflect the committed changes.