[Lldb-commits] [PATCH] D18977: Add new ABI callback to return CFA offset
Ulrich Weigand via lldb-commits
lldb-commits at lists.llvm.org
Tue Apr 12 02:47:23 PDT 2016
uweigand added a comment.
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.
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.
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.
More information about the lldb-commits