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

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

Alan Modra wrote:
> On Tue, May 17, 2022 at 10:32:09PM +1000, 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.
> 
> Yes.
> 
>> So we could use adjust here, but we'd need to adjust by -(2 * PPC_MIN_STKFRM).
> 
> Yes.
> 
>> It seemed clearer to just set the offset back to 0, which is what it is
>> at the start of the function.
> 
> Yes.  In detail, both .cfi_def_cfa_offset and .cfi_adjust_cfa_offset
> are interpreteted by the assembler into DW_CFA_def_cfa_offset byte
> codes, so you should get the same .eh_frame contents if using Naveen's
> suggestion.  It boils down to style really, and the most common style
> is to use ".cfi_def_cfa_offset 0" here.

Thank you, that clarifies things.


- Naveen