[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D38680#1123018, @joerg wrote:

> After a careful review of newer GCC / libgcc and the assembler annotations from LLVM, I have come to the following conclusions:
>
> (1) The semantics have been somewhat changed by GCC in recent years. There is no actual specification, so we have to go by what behavior actually makes sense.
>  (2) The primary motivation is still that the DW_CFA_GNU_args_size is a call-site specific annotation. It is expected to be applied when the IP is moved by the personality routine to compensate for the call site specific (temporary) adjustment.


Right.

> (3) It is not clear with plain unw_set_ip outside the scope of the Itanium EH handling should have this behavior, so it might need to be split into an internal routine.

I don't know enough about this code to really respond to this.

> (4) LLVM does not produce correct CFA annotation for stdcall and similar cases where the callee removes additional stack space.

Here's what we generate for that case today: https://godbolt.org/g/33cNJy
The important part is:

  .cfi_escape 0x2e, 0x0c
  pushl   $3
  .cfi_adjust_cfa_offset 4
  pushl   $2
  .cfi_adjust_cfa_offset 4
  pushl   $1
  .cfi_adjust_cfa_offset 4
  calll   __Z13may_throw_stdiii at 12
  .cfi_adjust_cfa_offset -12

Are you saying that the runtime will calculate the wrong CFA because it will include the `.cfi_adjust_cfa_offset -12`? As in, adding a nop after the call would fix the glitch? If so, I think the right thing to do would be to fix libunwind to use return_address - 1 when unwinding.


https://reviews.llvm.org/D38680