[Lldb-commits] [PATCH] D21221: Fix for PrintStackTraces

Tue Jun 14 22:04:30 PDT 2016

jasonmolenda added a comment.

Thanks Ravi, I see the problem here and I agree that lldb should use eh_frame to unwind from this function - that's the only way this is going to work.  I'm surprised that there is accurate eh_frame instructions for this function, it's great to see it.

I'm wondering if we can come up with a way to detect this scenario - where we're unwinding from a function that doesn't conform to the ABI.  I know it's a little special-case-y but what if we modified FuncUnwinders::GetUnwindPlanAtNonCallSite to:

Get the assembly unwind plan, eh_frame unwind plan, the ABI ArchitectureDefaultAtFunctionEntry unwind plan, and the ArchitectureDefault unwind plan.

If the eh_frame's row 0 (initial set of unwind instructions) has a pc/lr restore rule that doesn't match any of the other three unwind plans, then we assume that the eh_frame instructions have knowledge that the others do not have, and we use eh_frame for this function.

I'll write this up tomorrow and see if it works on mac (although we've almost completely stopped using eh_frame on our platform so I'm not a very good test bed at all) - I can imagine how the code would look and I think it's possible to do.

If eh_frame accurately describes the prologue instructions (I think I mentioned in the lldb-dev emails a couple weeks ago -- it is not required to describe the prologue), it the pc unwind rule will be the same as the ABI's ArchitectureDefaultAtFunctionEntry.  Or it will be the same as the assembly unwind rule.  If the eh_frame doesn't describe the prologue -- if the 0th row is the register state after the prologue has executed -- it will match the ABI ArchitectureDefault's unwind rule to restore the pc.

The main oddity here are architectures like arm where the restore rule for pc may be in terms of the pc, or it may be in terms of the link register ("return address register" in lldb's terminology).  Different UnwindPlans may specify how to restore the pc register value, or how to restore the ra register value -- they're inconsistent -- and that needs to be taken into account.

I know this isn't the approach you took with the patch, but I think it would be an improvement to lldb (especially if systems are getting eh_frame like this) and it will solve the problem I believe.  What do you think?  As I said, I'm happy to write up what I'm suggesting here, I don't want to ask you to do extra work to try out this idea.  I'm in and out of the office this week because of an apple conference but I'll try to get something together over the next few days.

As for writing test cases, these are tricky, and I need to write unwind test cases too, it's something I haven't had time to do yet for lldb and we really need these test cases, the unwinder is VERY hard to work on without a rich testsuite.

They end up being architecture (x86_64) and platform (mac) specific... you write a top-level driver in C which calls hand-written assembly functions (either inlined or a .s file) and the little details / ABI vary from platform to platform, I'm not sure if it's worth trying to make them portable - we can rely on buildbots to run them on the matrix of systems if nothing else.  The only alternative are core files of processes in the correct stopped state - but then you have to store gigantic core files and possibly binaries along with them and I don't think we're going to head down that route any time soon.

http://reviews.llvm.org/D21221