[PATCH] Correct PowerPC VDSO call frame info

[Michael Neuling]

Alan,

Thanks for the patch... A few minor comments...

> Re: [PATCH] Correct PowerPC VDSO call frame info

Our convention is to add powerpc: to the start. ie

  [PATCH] powerpc: Correct VDSO call frame info

or even: 

  [PATCH] powerpc/vdso: Correct call frame info


On Fri, 2018-09-14 at 08:57 +0930, Alan Modra wrote:
> There is control flow in __kernel_clock_gettime that reaches label 99
> without saving lr in r12.  CFI info however is interpreted by the
> unwinder without reference to control flow: It's a simple matter of
> "Execute all the CFI opcodes up to the current address".  That means
> the unwinder thinks r12 contains the return address at label 99.
> Disabuse it of that notion by resetting CFI for the return address at
> label 99.

Can you expand on CFI == Call Frame Information in the commit message.  Not a
common term for us mere kernel developers?

> Note that the ".cfi_restore lr" could have gone anywhere from the
> "mtlr r12" a few instructions earlier to the instruction at label 99.
> I put the CFI as late as possible, because in general that's best
> practice (and if possible grouped with other CFI in order to reduce
> the number of CFI opcodes executed when unwinding).  Using r12 as the
> return address is perfectly fine after the "mtlr r12" since r12 on
> that code path still contains the return address.
> 
> __get_datapage also has a CFI error.  That function temporarily saves
> lr in r0, and reflects that fact with ".cfi_register lr,r0".  A later
> use of r0 means the CFI at that point isn't correct, as r0 no longer
> contains the return address.  Fix that too.

Can you describe the problem this fixes at a high level?  ie This fixes doing a
stack unwind with gdb when in __kernel_clock_gettime.

Mikey

> Signed-off-by: Alan Modra <amodra at gmail.com>
> 
> diff --git a/arch/powerpc/kernel/vdso32/datapage.S
> b/arch/powerpc/kernel/vdso32/datapage.S
> index 3745113fcc65..2a7eb5452aba 100644
> --- a/arch/powerpc/kernel/vdso32/datapage.S
> +++ b/arch/powerpc/kernel/vdso32/datapage.S
> @@ -37,6 +37,7 @@ data_page_branch:
>  	mtlr	r0
>  	addi	r3, r3, __kernel_datapage_offset-data_page_branch
>  	lwz	r0,0(r3)
> +  .cfi_restore lr
>  	add	r3,r0,r3
>  	blr
>    .cfi_endproc
> diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S
> b/arch/powerpc/kernel/vdso32/gettimeofday.S
> index 769c2624e0a6..1e0bc5955a40 100644
> --- a/arch/powerpc/kernel/vdso32/gettimeofday.S
> +++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
> @@ -139,6 +139,7 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
>  	 */
>  99:
>  	li	r0,__NR_clock_gettime
> +  .cfi_restore lr
>  	sc
>  	blr
>    .cfi_endproc
> diff --git a/arch/powerpc/kernel/vdso64/datapage.S
> b/arch/powerpc/kernel/vdso64/datapage.S
> index abf17feffe40..bf9668691511 100644
> --- a/arch/powerpc/kernel/vdso64/datapage.S
> +++ b/arch/powerpc/kernel/vdso64/datapage.S
> @@ -37,6 +37,7 @@ data_page_branch:
>  	mtlr	r0
>  	addi	r3, r3, __kernel_datapage_offset-data_page_branch
>  	lwz	r0,0(r3)
> +  .cfi_restore lr
>  	add	r3,r0,r3
>  	blr
>    .cfi_endproc
> diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S
> b/arch/powerpc/kernel/vdso64/gettimeofday.S
> index c002adcc694c..a4ed9edfd5f0 100644
> --- a/arch/powerpc/kernel/vdso64/gettimeofday.S
> +++ b/arch/powerpc/kernel/vdso64/gettimeofday.S
> @@ -169,6 +169,7 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
>  	 */
>  99:
>  	li	r0,__NR_clock_gettime
> +  .cfi_restore lr
>  	sc
>  	blr
>    .cfi_endproc
>