Re: [PATCH] PowerPC: Add systemtap static probe points in setjmp/longjmp

[Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>]

On 03-12-2013 17:29, Will Schmidt wrote:
> On Thu, 2013-10-31 at 21:42 -0200, Adhemerval Zanella wrote:
> For ease of review, this could probably have been broken apart into two
> patches.  1 - cosmetic/renaming from __longjmp to __longjmp_symbol, and
> 2 - the actual addition of the probes.  
Indeed this patch ended up begin large and I didn't think about split because
I had to change the symbol name after the initial fixing/testing. Anyway, thanks
for the review.

>
> diff --git a/sysdeps/powerpc/powerpc32/__longjmp-common.S b/sysdeps/powerpc/powerpc32/__longjmp-common.S
> index df1d519..08eff37 100644
> --- a/sysdeps/powerpc/powerpc32/__longjmp-common.S
> +++ b/sysdeps/powerpc/powerpc32/__longjmp-common.S
> @@ -17,6 +17,7 @@
>     <http://www.gnu.org/licenses/>.  */
>
>  #include <sysdep.h>
> +#include <stap-probe.h>
>  #define _ASM
>  #ifdef __NO_VMX__
>  # include <novmxsetjmp.h>
> @@ -30,7 +31,7 @@
>  # define LOAD_GP(N)	lwz r##N,((JB_GPRS+(N)-14)*4)(r3)
>  #endif
>
> -ENTRY (__longjmp)
> +ENTRY (__longjmp_symbol)
>
>  #if defined PTR_DEMANGLE || defined CHECK_SP
>  	lwz r24,(JB_GPR1*4)(r3)
> @@ -58,20 +59,22 @@ ENTRY (__longjmp)
>  # endif
>  	PTR_DEMANGLE2 (r0, r25)
>  #endif
> +	LIBC_PROBE (longjmp, 3, 4@3, -4@4, 4@0)
>  	mtlr r0
>  	LOAD_GP (21)
>  	LOAD_GP (22)
> -	lwz r0,(JB_CR*4)(r3)
> +	lwz r5,(JB_CR*4)(r3)
>  	LOAD_GP (23)
>  	LOAD_GP (24)
>  	LOAD_GP (25)
> -	mtcrf 0xFF,r0
> +	mtcrf 0xFF,r5
>  	LOAD_GP (26)
>  	LOAD_GP (27)
>  	LOAD_GP (28)
>  	LOAD_GP (29)
>  	LOAD_GP (30)
>  	LOAD_GP (31)
> +	LIBC_PROBE (longjmp_target, 3, 8@3, -4@4, 8@0)
> Two questions here. 
> Can you explain the use of r5 instead of r0 in the code above?  I'd
> guess the probe wants something specific in R5, but want to confirm
> that.
> What are the 8@3, -4@4, 8@0 mapping to?  The values vary slightly
> throughout, and may be worth a comment on the LIBC_PROBE call to clarify
> the intent.    from a peek at LIBC_PROBE(), there is some _VA_ARGS_
> activity on the other side of the macro.

Right, the LIBC_PROBE macro is in fact wrapper for the systemtap STAP_PROBE*
macros and although it appears to be a vaargs activity, it will turn out to create
an asm directive with a string with instruction to systemtap to probe the correct
values. The current probe is used for GDB to correct get the target address for the
longjmp target.

And that's why I had to change the r0 to r5: at the time of LIBC_PROBE call I need to
have the LR in some register, which was being holding in r0 and then clobbering in the
'lwz'. Since the 'r0' in this context is meant to use as a scratch register and I 
need its value being preserved until the probe I chose 'r5' to be used as a temporary.

I will add a comment with the intend for each field in the LIBC_PROBE.