[Libunwind-devel] cfi execution issue

[Yichao Yu]

Hi,

I've hit some issue trying to unwind a clang compiled program with fpo
enabled on x86. I've found a fix that I would have cleaned up and
submitted a pull request if not because I'm partially confused by the
comment there.

The issue is very similar to the one worked around in
https://github.com/libunwind/libunwind/commit/3d9a694de85f2ba10368b4fbc2aff1c6b8b76f58
and happens to me when there's a cfi right after the call instruction
of a noreturn function. GCC usually emits a `restore_state` in that
case which is what the above patch works around. However, that is
definately not mandated and clang actually emits normal
`def_cfi_offset`s which cause libunwind to mess up badly....

A few notes about the code that reproduces the issue,
FPO plays a role since otherwise the cfa is ebp and doesn't need
changing all the time. X86 is especially susceptible to the problem
since basically all calls need to adjust the sp (for arguments). The
program to reproduce is attached (unw.c) and can be compiled with `
-O3 -lunwind -fomit-frame-pointer -fasynchronous-unwind-tables -fno-P
IE -g -Wl,--export-dynamic -fno-stack-protector
` options with gcc to see the expected behavior and with clang to see the issue.

The patch above claims that the signal frame handling can be broken
when fixing this issue which I can reproduce and although I didn't
track it doesn directly I guess it's caused by `fetch_proc_info`
updating the c->use_prev_instr to that for the next frame too early. I
could reproduce the failure with the naive fix and I can fix that
failure by delaying that update (however, those tests doesn't seems to
be particularly reliable and the main difference is how often it
fails. The difference may or may not be statistically significant...).

That leave the final question of what the comment around the old logic
mean. The comment

  /* Process everything up to and including the current 'end_ip',
     including all the DW_CFA_advance_loc instructions.  See
     'c->use_prev_instr' use in 'fetch_proc_info' for details. */

and

     For execution resume, we need to do the exact opposite and look
     up using the current 'ip' value.  That is where execution will
     continue, and it's important we get this right, as 'ip' could be
     right at the function entry and hence FDE edge, or at instruction
     that manipulates CFA (push/pop). */

Suggests that this is intended. I can see it's reasoning when applying
for unwinding the stack for continuing execution of the program (more
like early return?) and I guess for callee cleanup calling convention
the call instruction can indeed affect the stack. However, I don't
think that argument applies for obtaining back traces and since I only
use libunwind for stack traces I'll need some input of if this fix
actually affect that or what will be a better fix.

The patch is also attached.

0001-.patch
Description: Text Data

unw.c
Description: Text Data