Page MenuHomePhabricator

Fix _Unwind_Backtrace for libc++abi built with libgcc.
ClosedPublic

Authored by logan on Jan 17 2015, 12:05 AM.

Details

Summary

Implement an undocumented _US_FORCE_UNWIND state for force
unwinding.

Diff Detail

Event Timeline

logan updated this revision to Diff 18338.Jan 17 2015, 12:05 AM
logan retitled this revision from to Fix _Unwind_Backtrace for libc++abi built with libgcc..
logan updated this object.
logan edited the test plan for this revision. (Show Details)
logan added reviewers: danalbert, rengolin, jroelofs.
logan added a subscriber: Unknown Object (MLST).
rengolin added inline comments.Jan 20 2015, 6:11 AM
src/cxa_personality.cpp
1107

Why do you need to clear the forced flag?

jroelofs added inline comments.Jan 20 2015, 6:29 AM
src/cxa_personality.cpp
1105

Why not add this to the public interface in include/unwind.h? If we're matching the behavior between gcc's unwinder, then perhaps it makes sense to 'advertise' that fact.

1111

can the state ever be set to just _US_FORCE_UNWIND? Also, continue_unwind() takes three parameters now... can you rebase this patch?

jroelofs accepted this revision.Jan 20 2015, 7:03 AM
jroelofs edited edge metadata.

LGTM.

src/cxa_personality.cpp
1107

The _US_* things are a bitmask. Not clearing it means that the switch below won't match it.

1111

can the state ever be set to just _US_FORCE_UNWIND?

Nevermind about that part.

This revision is now accepted and ready to land.Jan 20 2015, 7:03 AM
jroelofs added inline comments.Jan 20 2015, 8:58 AM
src/cxa_personality.cpp
1111

Also, nevermind about the rebase. I see that you've changed the signature of this function in a different patch in the series.

danalbert added inline comments.Jan 20 2015, 9:59 AM
src/cxa_personality.cpp
1106

It's already a uint32_t, what do you need the cast for? If you really do need a cast, use something explicitly sized.

1107

Would be more intention revealing to clear the flag for the switch condition itself.

logan updated this revision to Diff 18529.Jan 21 2015, 9:23 AM
logan edited edge metadata.

Address the review comments.

logan added inline comments.Jan 21 2015, 9:28 AM
src/cxa_personality.cpp
1105

Thanks. I have moved this flag to unwind.h.

1106

Thanks. Removed.

1107

Some explaination on this: Although _Unwind_Backtrace() only uses one of the case, it is required to implement the personality for both _US_VIRTUAL_UNWIND_FRAME and _US_UNWIND_FRAME_STARTING cases.

p.s. The later case is used by _Unwind_ForcedUnwind(). However, it is not working yet. I am debugging at the moment.

logan updated this revision to Diff 18531.Jan 21 2015, 9:53 AM

Make Unwind-EHABI.h become C compatible.

logan updated this revision to Diff 18610.Jan 22 2015, 5:33 AM

Recover the previous patch set.

This differential is incorrectly covered by incorrect patch set.

logan closed this revision.Jan 22 2015, 5:34 AM

Thanks. Committed as rL226820.