This is an archive of the discontinued LLVM Phabricator instance.

[ubsan] Fall back to the fast unwinder when print_stacktrace=1
ClosedPublic

Authored by vsk on Apr 25 2017, 6:06 PM.

Details

Summary

This makes it possible to get stacktrace info when print_stacktrace=1 on
Darwin (where the slow unwinder is not currently supported [1]). This
should not regress any other platforms.

[1] The thread about r300295 has a relatively recent discusion about
this. We should be able to enable the existing slow unwind functionality
for Darwin, but this needs more testing.

Diff Detail

Repository
rL LLVM

Event Timeline

vsk created this revision.Apr 25 2017, 6:06 PM
eugenis added inline comments.Apr 27 2017, 3:10 PM
lib/ubsan/ubsan_diag.cc
39 ↗(On Diff #96662)

This would use fast unwinder on linux, even when the slow one is available.
I think we should prefer slow, as before.

vsk updated this revision to Diff 97123.Apr 28 2017, 10:41 AM
vsk marked an inline comment as done.
vsk retitled this revision from [ubsan] Request the fast unwinder when print_stacktrace=1 to [ubsan] Fall back to the fast unwinder when print_stacktrace=1.
vsk edited the summary of this revision. (Show Details)

Fall back to the fast unwinder, instead of making it the preferred choice.

eugenis added inline comments.May 1 2017, 2:11 PM
lib/ubsan/ubsan_diag.cc
37 ↗(On Diff #97123)

use common_flags()->fast_unwind_on_fatal here

test/ubsan/TestCases/Misc/missing_return.cpp
9 ↗(On Diff #97123)

This is unnecessarily complex. Remove () and (void), we don't really care about the exact function signature.

vsk updated this revision to Diff 97340.May 1 2017, 2:25 PM
vsk marked 2 inline comments as done.
vsk added inline comments.
lib/ubsan/ubsan_diag.cc
37 ↗(On Diff #97123)

Ok. I think it would also make sense to enable the fast unwinder when fast_unwind_on_check is set, then.

eugenis added inline comments.May 1 2017, 2:28 PM
lib/ubsan/ubsan_diag.cc
37 ↗(On Diff #97123)

No, fast_unwind_on_check is for internal CHECK(...) failures.

test/ubsan/TestCases/Misc/missing_return.cpp
9 ↗(On Diff #97123)

And now there is no need to keep 3 separate check lines.

vsk marked 3 inline comments as done.May 1 2017, 2:31 PM

Will update shortly.

lib/ubsan/ubsan_diag.cc
37 ↗(On Diff #97123)

I see, those failures are not surfaced through this function.

vsk updated this revision to Diff 97342.May 1 2017, 2:36 PM
vsk marked an inline comment as done.
eugenis accepted this revision.May 1 2017, 2:42 PM

LGTM

test/ubsan/TestCases/TypeCheck/misaligned.cpp
51 ↗(On Diff #97342)

Here as well, no need for %os.
Will this pass on, say, FreeBSD, without the corresponding CHECK line?

This revision is now accepted and ready to land.May 1 2017, 2:42 PM
vsk marked an inline comment as done.May 1 2017, 2:54 PM
vsk added inline comments.
test/ubsan/TestCases/TypeCheck/misaligned.cpp
51 ↗(On Diff #97342)

I expect this to pass on FreeBSD. I will remove the '%os' stuff in this test before committing.

This revision was automatically updated to reflect the committed changes.