clayborg added a comment. 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"? > 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. 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... > I've implemented the second method for LLDB since it actually seemed a bit > more general to me (describes more precisely what actually happens on the > platform), and also it seemed to fit nicely into the ABI scheme. > > If you prefer, I can implement the first method instead. However, since we > cannot unconditionally disable the special case of setting SP to CFA (other > platforms depend on that -- which is arguably the real hack to begin with), > we would still need some ABI method to tell the unwinder whether or not the > special case is needed. > > There's no real way to "fix the unwind info", since this is not a new > platform and binaries containing this unwind info have been in the field for > more than 15 years. In any case, it is not clear that there is anything to > "fix" -- a binary containing no unwind info for SP to indicate that SP does > not change isn't really "wrong" IMO. So I think implementing the default rule for unspecified registers more completely will help us solve this problem correctly. Let me know what you think? http://reviews.llvm.org/D18977 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
