Re: [Libunwind-devel] Mixed stack unwinding bug on ppc64le

[Dave Watson]

Hello!

On 08/16/18 09:26 AM, Stephen Roberts wrote:
> I have two candidate solutions, both of which solve the issue but make 
> different compromises. My first solution is to include an "ignore_cfi" flag 
> inside the struct cursor from tdep-ppc64/libunwind_i.h. 
> This flag is initially false but set to true whenever dwarf_step fails with 
> -UNW_ENOINFO. It has the effect of skipping the dwarf_step logic entirely, 
> causing libunwind to stick to using back chain unwinding after it encounters 
> its first frame without CFI. This is a robust and simple solution, but I'm 
> not sure if this is the libunwind way to approach the issue.
> 
> My second solution is a little more invasive, but has the potential to 
> provide more information. The dwarf_step function uses dwarf_apply_registers 
> internally to 1) compute the new Canonical Frame Address (CFA) and 2) update 
> the register values. My fix effectively breaks the logic in 
> dwarf_apply_registers out into two helper functions, something like 
> "dwarf_advance_cfa" and "dwarf_update_registers".
> 
> The dwarf_step function still works exactly as before, but we can now use 
> "dwarf_update_registers" to update registers for a user-specified CFA. Given 
> we have a valid CFA from backchain unwinding, we call 
> "dwarf_update_registers" with our backchain-derived CFA to leave us with some 
> register values even when we've unwound from a non-CFA frame. The next time 
> we unwind then we will *probably* have the registers we need to continue to 
> unwind successfully using CFI. The caveat with this approach is it is 
> possible for compilers to save volatile registers in non-volatile registers 
> rather than on the stack.
> In this case, dwarf_update_registers would fail to retrieve the 
> register-saved value and would fail. This is rare in practice, and is still 
> much better than never being able to unwind. If we wanted we could be 
> extra-safe and use both solutions, with the ignore_cfi flag being set only 
> when dwarf_update_registers failed.

I see registers saved in other registers all the time on x86, so I'm
curious how much it actually helps in practice.  Of course even c code
on x86 has unwind tables, so it doesn't really apply there anyway.

> 
> My first question is this: which approach is more in-keeping with the 
> libunwind philosophy? Would either (or both) of them be accepted upstream? 
> I've tested both approaches and found they both solve my issues on ppc64le, 
> so either way I'm happy.

Both of these sound reasonable, I'd be happy to accept either or both.

> A second question involves writing test cases. I have a test case that I run 
> outside the libunwind infrastructure, but it's proving difficult to make it 
> run within libunwind's own testing infrastructure. This bug only manifests 
> when a frame has no CFI, however the libunwind make system turns on 
> -fexceptions everywhere so this can't be tested easily. I know there are 
> test-specific CFLAGS, however these come first in the compile command so they 
> are clobbered by the global options. I think it makes sense to be able to 
> vary the exception/unwind compiler flags as part of testing as these control 
> which libunwind code paths are executed. Did I miss something? Is there some 
> other mechanism to set these flags for test cases?

No other mechanism that I know of, hopefully switching the local flags
to come second is enough?