This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] Add ASan debugging API to get malloc/free stack traces and shadow memory mapping info
ClosedPublic

Authored by kubamracek on Jul 10 2014, 12:32 PM.

Details

Reviewers
kcc
glider
Summary

ASan currently has only __asan_describe_address as a debugging API, which prints out a report in textual form only. This patch is part of an effort to implement a more generic debugging API, as proposed in http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-July/074656.html.

It adds 1) asan_get_alloc_stack and asan_get_free_stack, which can return a stack trace and thread ID for the allocation or free of a specified heap memory address and 2) __asan_get_shadow_mapping which just returns the current scale and offset for the shadow memory mapping.

The idea is that this would allow an LLDB script (or even calling the functions manually) to query various memory addresses and get their malloc/free stack traces and relevant shadow memory addresses. We could then use LLDB's backtrace formatting to show more consistently formatted information (and use LLDB's symbolication) and even do things like "frame select". The __asan_get_shadow_mapping API can be used to create macros like MemToShadow/ShadowToMem in LLDB and we could even have something like "x --shadow 0x100000000" in LLDB to show the relevant shadow memory alongside with the memory contents.

Diff Detail

Event Timeline

kubamracek retitled this revision from to [compiler-rt] Add ASan debugging API to get malloc/free stack traces and shadow memory mapping info.
kubamracek updated this object.
kubamracek edited the test plan for this revision. (Show Details)
kubamracek added a reviewer: kcc.
kubamracek added a subscriber: Unknown Object (MLST).
kcc added inline comments.Jul 11 2014, 1:02 AM
include/sanitizer/asan_interface.h
69

Does your interface need top_frame_bp?

Also, maybe change it like this?

// Store up to 'size' frames into 'trace', return the number of stored frames or 0 on error.
/...
uptr __asan_get_alloc_stack(void *addr, void **trace, size_t size, int *thread)

lib/asan/asan_debugging.cc
25

This is not part of the interface, right?
So, it should be inside the __asan namespace, should be called AsanGetStack, and alloc_stack should be boolean

Removed top_frame_bp from the interface, I don't really need it. Changed the malloc/free stack API return value to be the number of frames returned. Moved the internal function into __asan namespace, fixed the signature of it.

Updated the test case to check the return value (number of frames).

kcc accepted this revision.Jul 14 2014, 2:30 AM
kcc edited edge metadata.

LGTM with two small nits.

lib/asan/asan_debugging.cc
23

move this below, right before the first SANITIZER_INTERFACE_ATTRIBUTE

71

we tend to omit {} in such simple expressions

This revision is now accepted and ready to land.Jul 14 2014, 2:30 AM
glider added a subscriber: glider.Jul 14 2014, 6:18 AM

Looks mostly good.

lib/asan/asan_debugging.cc
61

Can you please write parameter names before constants, e.g.:

AsanGetStack(addr, trace, size, thread_id, /*alloc_stack*/ true);

?

66

ditto

lib/asan/tests/asan_debugging_noinst_test.cc
14 ↗(On Diff #11323)

Please sort the includes alphabetically.

20 ↗(On Diff #11323)

Please move the test for __asan_get_shadow_mapping() to a separate lit test.
The _noinst tests are intended to test only the ASan internals, not the public interfaces, and your test may end up checking something completely different.

test/asan/TestCases/debug_stacks.cc
6

Please keep the includes sorted.

29

Please add a second space before "//" here and below.

39

What's the point in checking for a constant string you have just printed?

kubamracek updated this revision to Diff 11392.Jul 14 2014, 9:59 AM
kubamracek edited edge metadata.

Applying the comments from review http://reviews.llvm.org/D4466.

kubamracek added inline comments.Jul 14 2014, 10:01 AM
test/asan/TestCases/debug_stacks.cc
29

Not sure if you really just wanted another space, so moving the CHECKs to separate lines as done in all other test cases.

glider added inline comments.Jul 15 2014, 1:49 AM
test/asan/TestCases/debug_stacks.cc
29

I was talking about two spaces between the code and the inline comment:

...);  // CHECK: ...

, which is used across the sanitizers codebase.
CHECKs on separate lines are also fine in this case.

glider accepted this revision.Jul 15 2014, 1:54 AM
glider added a reviewer: glider.

LGTM with nits.

lib/asan/asan_debugging.cc
73

Nit: spare newline.
In general it's better when more code fits on the screen, and this function is small enough to comprehend without that newline.

test/asan/TestCases/debug_mapping.cc
2

?

kubamracek updated this revision to Diff 11449.Jul 15 2014, 9:54 AM
kubamracek edited edge metadata.
kubamracek closed this revision.Jul 15 2014, 10:42 AM

Landed in r213080.