Re: [PATCH] powerpc/vdso: Fix incorrect CFI in gettimeofday.S

["Naveen N. Rao" <naveen.n.rao@linux.ibm.com>]

Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.ibm.com> writes:
>> Michael Ellerman wrote:
>>>
>>> diff --git a/arch/powerpc/kernel/vdso/gettimeofday.S b/arch/powerpc/kernel/vdso/gettimeofday.S
>>> index eb9c81e1c218..0aee255e9cbb 100644
>>> --- a/arch/powerpc/kernel/vdso/gettimeofday.S
>>> +++ b/arch/powerpc/kernel/vdso/gettimeofday.S
>>> @@ -22,12 +22,15 @@
>>>  .macro cvdso_call funct call_time=0
>>>    .cfi_startproc
>>>  	PPC_STLU	r1, -PPC_MIN_STKFRM(r1)
>>> +  .cfi_adjust_cfa_offset PPC_MIN_STKFRM
>>>  	mflr		r0
>>> -  .cfi_register lr, r0
>>>  	PPC_STLU	r1, -PPC_MIN_STKFRM(r1)
>>> +  .cfi_adjust_cfa_offset PPC_MIN_STKFRM
>>>  	PPC_STL		r0, PPC_MIN_STKFRM + PPC_LR_STKOFF(r1)
>>
>> <snip>
>>
>>> @@ -46,6 +50,7 @@
>>>  	mtlr		r0
>>>    .cfi_restore lr
>>>  	addi		r1, r1, 2 * PPC_MIN_STKFRM
>>> +  .cfi_def_cfa_offset 0
>>
>> Should this be .cfi_adjust_cfa_offset, given that we used that at the
>> start of the function?
>  
> AIUI "adjust x" is offset += x, whereas "def x" is offset = x.
> 
> So we could use adjust here, but we'd need to adjust by -(2 * PPC_MIN_STKFRM).
> 
> It seemed clearer to just set the offset back to 0, which is what it is
> at the start of the function.

I read the first .cfi_adjust_cfa_offset directive (rather than the 
.cfi_def_cfa_offset directive) in this macro to be intentionally 
retaining the offset to what it was before the VDSO. If that is 
desirable, then setting it to 0 here will change it, I _think_.

> 
> But I'm not a CFI expert at all, so I'll defer to anyone else who has an
> opinion :)

Oh, the above is just my hypothesis. Would be good to get confirmation.


- Naveen