Page MenuHomePhabricator

[Sanitizer] On Darwin `__sanitizer_print_stack_trace` only prints topmost frame

Authored by yln on Feb 12 2019, 4:26 PM.



In compiler-rt we have the notion of a fast and a slow stack
unwinder. Darwin currently only supports the fast unwinder.

From reading the code, my understanding is that
BufferedStackTrace::Unwind can be called with `bp=0, stack_top=0,
stack_bottom=0, request_fast_unwind=false`. If
request_fast_unwind=true, then we alos need to supply bp, stack_top,
and stack_bottom.

However, BufferedStackTrace::Unwind uses
StackTrace::WillUseFastUnwind which will adapt request_fast_unwind
if the requested unwinder is not supported. On Darwin, the result is
that we don't pass actual values for bp, stack_top, and stack_bottom,
but end up using the fast unwinder. The tests then fail because we only
print the topmost stack frame.

This patch adds a check to WillUseFastUnwind at the point of usage to
avoid the mismatch between request_fast_unwind and what Unwind
actually does. I am also interested in cleaning up the
request_fast_unwind machinery in general so this patch is just the simplest thing
possible so I can enable the tests.

Diff Detail


Event Timeline

yln created this revision.Feb 12 2019, 4:26 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 12 2019, 4:26 PM
Herald added subscribers: llvm-commits, Restricted Project, jdoerfert, kubamracek. · View Herald Transcript
yln edited the summary of this revision. (Show Details)Feb 12 2019, 4:35 PM
vsk accepted this revision.Feb 13 2019, 1:08 PM

Thanks, LGTM. Could you wait for another +1? It's been a while since I've looked at this code, and I think another review would help make sure this is safe.

This revision is now accepted and ready to land.Feb 13 2019, 1:08 PM
vitalybuka accepted this revision.Feb 13 2019, 5:06 PM


6 ↗(On Diff #186556)

What is wrong with lsan?

yln marked an inline comment as done.Feb 18 2019, 10:40 AM
yln added inline comments.
6 ↗(On Diff #186556)

I don't know. I will take a quick look and provide a separate patch if it's easy to fix.
Internally, we don't support LSan, so this isn't really a priority for us.

This revision was automatically updated to reflect the committed changes.