[Lldb-commits] [PATCH] D18977: Add new ABI callback to return CFA offset

Tue Apr 12 11:09:26 PDT 2016

uweigand added a comment.

In http://reviews.llvm.org/D18977#398557, @clayborg wrote:

> In http://reviews.llvm.org/D18977#398157, @uweigand wrote:
>
> > In http://reviews.llvm.org/D18977#397626, @clayborg wrote:
> >
> > > I am not sure why this offset of 160 isn't represented in the unwind info. I guess the OS unwinder that uses the EH frame info just knows that it must adjust the CFA? That seems like a hack. It seems like the unwind info should be fixed and the OS level unwinder should be fixed so that the info can be used correctly without every debugger or tool that has to parse this know about such a hack? Do you have control over the OS and are you able to fix this? That seems like the better fix.
> >
> >
> > The offset of 160 is represented in the unwind info when computing the CFA itself.   This change is about unwinding the SP register when no explicit unwind info for SP is present.  In this case, the LLDB unwinder assumes it should set SP to the CFA, which is where the problem comes in.
>
>
> This is usually represented by the CIE (Common Information Entry) which is pointed to from the FDE (Frame Description Entry). The CIE has the initial state that all FDEs can share. Seems like there should be an entry for the SP that says the rule to unwind it is "CFA+160"?

Well, there could be, just like there could be a default "same register" rule for call-saved registers so we wouldn't have to apply ABI knowledge.  However, none of that has been done historically on any DWARF platform I know of, mostly because this would duplicate information known via the ABI into every object file and thus waste space ...   In any case, the current state is as it is, we have to handle object files that currently exist.

> > Now, on s390, most frames actually have explicit unwind instructions for SP, so this special case is not even triggered.  Only for leaf functions without a stack frame can we ever see the scenario that there are no SP unwind instructions.  In those cases, the correct result on s390 is to simply leave SP unchanged (since the function did not in fact allocate a frame).

> 

> > 

> 

> > However, due to the special case in the LLDB unwinder, SP is not left unchanged, but set to the CFA (i.e. SP + 160 in this case).  This is wrong.   There are two possible ways to fix this:

> 

> > 

> 

> > - Disable the special case of setting SP to CFA on s390.   Instead, when there is no unwind info for SP, just leave it unchanged (just like any other call-saved register).   This solution is implemented in the platform unwinder (libgcc) and also in GDB.

> 

> > - Fix the special case of setting SP to CFA by actually taking the CFA bias into account.  This is what this patch implements for LLDB.

> 

> 

> There seems to be magic happening here. It seems like the CIE or FDE should describe how to unwind the SP when things are tricky. We do have the notion that registers that have no rule can have a default rule applied to them. Currently this is only used for callee saved registers and for any such registers they rule defaults to "the registers is in the register itself". For volatile registers, their default rule is "the register is not available". This is the part where I get fuzzy with respect to how the OS unwinder works: does it have this same notion of default rules for registers? If so, we should be implementing the same thing for LLDB's EH frame parser.

Actually, there are three different default rules: the ones for call-saved and call-clobbered registers you mention, and the special rule for the SP.   The platform unwinder does not care about volatile registers (because when you dispatch an exception, the catch point by definition must not care about the contents of those), but it does implement the "same register" rule for call-saved registers, and it does implement the special rule for SP on the platforms that need it.

> So a third solution is to allow the ABI plug-in to specify the default rules for registers when they aren't specified. I think the ABI plug-in is where we get the fact that a register is volatile or not...

Yes, that would be a solution, in fact this is the exact solution I implemented for GDB back in the days ...

Right now, if RegisterContextLLDB::SavedLocationForRegister finds no unwind info for a register, it implements (itself) the three default rules, in this order: if it is SP, use the CFA rule; otherwise, if the ABI says the register is volatile, use the "not available rule"; otherwise, say "register not found", which the caller interprets as "same as in the next frame".

One option to fix the s390x problem would be to instead of hard-coding the SP rule and the volatile check here, always call an ABI callback to define a "fallback register rule" by returning a UnwindPlan::Row::RegisterLocation.  The ABI could return a "isCFAPlusOffset" fallback rule for the SP if appropriate, or it could return a "undefined" fallback rule for volatile registers.

Is this what you were thinking about?  I can try to come up with a patch along those lines.

http://reviews.llvm.org/D18977