This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][SH] CFI directives patch
On 2012/5/23 07:35 PM, Thomas Schwinge wrote:
> On Fri, 11 May 2012 17:15:52 +0800, Chung-Lin Tang <chunglin.tang@gmail.com> wrote:
>>>> diff --git a/sysdeps/unix/sh/sysdep.S b/sysdeps/unix/sh/sysdep.S
>>>> index e816575..939eb9a 100644
>>>> --- a/sysdeps/unix/sh/sysdep.S
>>>> +++ b/sysdeps/unix/sh/sysdep.S
>>>> @@ -41,13 +41,15 @@ skip:
>>>> sts.l pr, @-r15
>>>> cfi_adjust_cfa_offset (4)
>>>> cfi_rel_offset (pr, 0)
>>>> + cfi_adjust_cfa_offset (4)
>>>> jsr @r1
>>>> mov.l r0, @-r15
>>>> - cfi_adjust_cfa_offset (4)
>>>
>>> Hmm?
>>
>> I think this was to fix the case of a frame-insn in a call delay slot
>> kind of problem. Imagine unwinding from inside the @r1 function (here
>> it's errno); it will get the stack position one word off.
>
> Hmm, that doesn't match my understanding. The CFI of the caller is of no
> interested to the callee: the CFI is purely a local thing for the
> instructions with addresses between the .cfi_startproc and .cfi_endproc
> directives (to compute the beginning of the frame based on SP's value in
> this case). The callee will have its own CFI to compute the frame
> address based on the value of the SP (which will have been decremented
> before @r1 is executed) and its own CFI between .cfi_startproc and
> .cfi_endproc.
>
>> But reviewing it, I think it's better to move the frame-insn (and CFI)
>> above the call and just fill nop.
>
> With my reasoning above (and please someone tell me if I'm wrong), there
> is no need to do that (and avoid the slight code pessimization).
With a stack adjustment in the call delay slot, the unwinder will be
4-bytes off the correct adjustment when crossing that frame; this
probably is an issue of how program counters map to FDEs (< vs <=).
CCing Richard Henderson here, who's probably the one to answer this.
Richard, I remember seeing related discussion in the archives on this
issue, as well as comments in the current GCC dwarf2cfi.c:scan_trace()
code, can you confirm?
Thanks,
Chung-Lin