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

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

On 04-12-2013 11:58, Adhemerval Zanella wrote:
> 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.
>
>
Pushed as eb5ad6b9bcf579f1cb5c67ca4650ee4a0cf1b4b1. I also pushed a
c6bb4f23b036ab3ae972583bf1186a6131ce4968 because I wrongly pushed a
wrong commit along with this one.