Hi Maintainers,
This patch adds pointer mangling support for Aarch64. Regressed by cross testing glibc tests on V8 Foundation model running a linaro open embedded image. Ok for trunk? Please provide your feedback. ChangeLog.aarch64: ----------------------------- 2013-12-26 Venkataramanan Kumar <[hidden email]> * sysdeps/aarch64/__longjmp.S (__longjmp): Demangle sp and lr when restoring register values. * sysdeps/aarch64/setjmp.S (__sigsetjmp): Mangle sp and lr before storing register values. * sysdeps/arm/jmpbuf-unwind.h (_jmpbuf_sp): Remove. * sysdeps/aarch64/jmpbuf-offsets.h (_jmpbuf_sp): Add. (JB_FRAME_ADDRESS): call _jmpbuf_sp. * sysdeps/aarch64/sysdep.h (LDST_PCREL) : New macros. (LDST_GLOBAL): Likewise. * sysdeps/unix/sysv/linux/aarch64/sysdep.h (PTR_MANGLE): New macro. (PTR_DEMANGLE): Likewise. (PTR_MANGLE2): Likewise. (PTR_DEMANGLE2): Likewise. regards Venkat. |
On 12/26/2013 04:25 AM, Venkataramanan Kumar wrote:
> +#ifdef PTR_DEMANGLE > ldp x29, x30, [x0, #JB_X29<<3] > - > + PTR_DEMANGLE (x4, x30, x3, x2) > + mov x30, x4 > +#else Why not load into x4 and then demangle into x30 directly? I.e. ldp x29, x4, [x0, #JB_X29<<3] PTR_DEMANGLE (x30, x4, x3, x2) r~ |
Hi Richard,
On 27 December 2013 01:03, Richard Henderson <[hidden email]> wrote: > On 12/26/2013 04:25 AM, Venkataramanan Kumar wrote: >> +#ifdef PTR_DEMANGLE >> ldp x29, x30, [x0, #JB_X29<<3] >> - >> + PTR_DEMANGLE (x4, x30, x3, x2) >> + mov x30, x4 >> +#else > > Why not load into x4 and then demangle into x30 directly? I.e. > > ldp x29, x4, [x0, #JB_X29<<3] > PTR_DEMANGLE (x30, x4, x3, x2) > > > r~ pair instruction. Attached is the revised patch. I will do a cross test on V8 foundation model again. ChangeLog.aarch64: ----------------------------- 2013-12-26 Venkataramanan Kumar <[hidden email]> * sysdeps/aarch64/__longjmp.S (__longjmp): Demangle sp and lr when restoring register values. * sysdeps/aarch64/setjmp.S (__sigsetjmp): Mangle sp and lr before storing register values. * sysdeps/arm/jmpbuf-unwind.h (_jmpbuf_sp): Remove. * sysdeps/aarch64/jmpbuf-offsets.h (_jmpbuf_sp): Add. (JB_FRAME_ADDRESS): call _jmpbuf_sp. * sysdeps/aarch64/sysdep.h (LDST_PCREL) : New macros. (LDST_GLOBAL): Likewise. * sysdeps/unix/sysv/linux/aarch64/sysdep.h (PTR_MANGLE): New macro. (PTR_DEMANGLE): Likewise. (PTR_MANGLE2): Likewise. (PTR_DEMANGLE2): Likewise. regards Venkat. |
On 12/26/2013 11:48 PM, Venkataramanan Kumar wrote:
> +/* Load or store to/from a pc-relative EXPR into/from R, using T. */ > +#define LDST_PCREL(OP, R, T, EXPR)\ > + adrp T, EXPR; \ > + add T, T, #:lo12:EXPR;\ > + OP R, [T]; Is T supposed to be live after the macro? Otherwise the whole point of the 12-bit offset is that it can be used inside the memory operation offset. As you in fact do with the LDST_GLOBAL macro just below. r~ |
Hi Richard,
Attached is the revised patch with the changes you suggested. Regressed with glibc tests on ARMV8 Foundation model running open embedded image. ChangeLog.aarch64: ----------------------------- 2013-12-26 Venkataramanan Kumar <[hidden email]> * sysdeps/aarch64/__longjmp.S (__longjmp): Demangle sp and lr when restoring register values. * sysdeps/aarch64/setjmp.S (__sigsetjmp): Mangle sp and lr before storing register values. * sysdeps/arm/jmpbuf-unwind.h (_jmpbuf_sp): Remove. * sysdeps/aarch64/jmpbuf- offsets.h (_jmpbuf_sp): Add. (JB_FRAME_ADDRESS): call _jmpbuf_sp. * sysdeps/aarch64/sysdep.h (LDST_PCREL) : New macros. (LDST_GLOBAL): Likewise. * sysdeps/unix/sysv/linux/aarch64/sysdep.h (PTR_MANGLE): New macro. (PTR_DEMANGLE): Likewise. (PTR_MANGLE2): Likewise. (PTR_DEMANGLE2): Likewise. On 27 December 2013 20:39, Richard Henderson <[hidden email]> wrote: > On 12/26/2013 11:48 PM, Venkataramanan Kumar wrote: >> +/* Load or store to/from a pc-relative EXPR into/from R, using T. */ >> +#define LDST_PCREL(OP, R, T, EXPR)\ >> + adrp T, EXPR; \ >> + add T, T, #:lo12:EXPR;\ >> + OP R, [T]; > > Is T supposed to be live after the macro? Otherwise the whole point of the > 12-bit offset is that it can be used inside the memory operation offset. > > As you in fact do with the LDST_GLOBAL macro just below. > > > r~ regards, Venkat. |
Looks fine to me.
r~ |
Hi Richard,
Thank you for reviewing the patch . I don't have write access. So I will wait for someone to commit on my behalf. regards, Venkat. On 30 December 2013 20:14, Richard Henderson <[hidden email]> wrote: > Looks fine to me. > > > r~ |
In reply to this post by Venkataramanan Kumar-3
Hi, Couple of nits:
On 30 December 2013 05:00, Venkataramanan Kumar <[hidden email]> wrote: +# define PTR_MANGLE(dst, src, guard, tmp) \ + LDST_PCREL(ldr, guard, tmp, C_SYMBOL_NAME(__pointer_chk_guard_local)); \ + PTR_MANGLE2(dst, src, guard) Space before ( in macro invocations. +#ifdef PTR_DEMANGLE + PTR_DEMANGLE (sp); +#endif Nested #if should have indentation between # and if. +/* Pointer mangling is support for AArch64. */ support -> supported Otherwise OK /Marcus |
Hi Marcus,
Thank you for reviewing the patch. I have incorporated your review comments. (Snip) +#ifdef PTR_DEMANGLE + PTR_DEMANGLE (sp); +#endif Nested #if should have indentation between # and if. (Snip) In ports/sysdeps/aarch64/jmpbuf-offsets.h, should I need to add space between # and if? Reading the glibc coding style, I understood it as outer #ifndef __ASSEMBLER__ will not increase indentation level. if it is Ok, can you please commit on my behalf since I don't have write access. ChangeLog.aarch64: ----------------------------- 2013-12-31 Venkataramanan Kumar <[hidden email]> * sysdeps/aarch64/__longjmp.S (__longjmp): Demangle sp and lr when restoring register values. * sysdeps/aarch64/setjmp.S (__sigsetjmp): Mangle sp and lr before storing register values. * sysdeps/arm/jmpbuf-unwind.h (_jmpbuf_sp): Remove. * sysdeps/aarch64/jmpbuf-offsets.h (_jmpbuf_sp): Add. (JB_FRAME_ADDRESS): call _jmpbuf_sp. * sysdeps/aarch64/sysdep.h (LDST_PCREL) : New macros. (LDST_GLOBAL): Likewise. * sysdeps/unix/sysv/linux/aarch64/sysdep.h (PTR_MANGLE): New macro. (PTR_DEMANGLE): Likewise. (PTR_MANGLE2): Likewise. (PTR_DEMANGLE2): Likewise. regards, Venkat. On 31 December 2013 03:50, Marcus Shawcroft <[hidden email]> wrote: > Hi, Couple of nits: > > On 30 December 2013 05:00, Venkataramanan Kumar > <[hidden email]> wrote: > > +# define PTR_MANGLE(dst, src, guard, tmp) \ > + LDST_PCREL(ldr, guard, tmp, C_SYMBOL_NAME(__pointer_chk_guard_local)); \ > + PTR_MANGLE2(dst, src, guard) > > Space before ( in macro invocations. > > +#ifdef PTR_DEMANGLE > + PTR_DEMANGLE (sp); > +#endif > > Nested #if should have indentation between # and if. > > +/* Pointer mangling is support for AArch64. */ > > support -> supported > > Otherwise OK > /Marcus |
On 31 December 2013 05:15, Venkataramanan Kumar
<[hidden email]> wrote: > Hi Marcus, > > Thank you for reviewing the patch. > I have incorporated your review comments. > > (Snip) > +#ifdef PTR_DEMANGLE > + PTR_DEMANGLE (sp); > +#endif > Nested #if should have indentation between # and if. > (Snip) > > In ports/sysdeps/aarch64/jmpbuf-offsets.h, should I need to add space > between # and if? > Reading the glibc coding style, I understood it as outer #ifndef > __ASSEMBLER__ will not increase indentation level. > > if it is Ok, can you please commit on my behalf since I don't have write access. You are right. I'll commit this for you shortly. /Marcus |
In reply to this post by Venkataramanan Kumar-3
>>>>> ">" == Venkataramanan Kumar <[hidden email]> writes:
>> This patch adds pointer mangling support for Aarch64. IIRC the Aarch64 longjmp code doesn't have the sdt.h probes in place. If you add pointer mangling without also adding the sdt.h probes, then gdb's longjmp support will break. You should be able to see this easily in the gdb test suite. Adding the probes is quite easy to do, so I encourage you to add that to the patch. thanks, Tom |
Hi,
The mangling patch is already up streamed. So I will include this addition as a separate patch. I looked at x86 and powerpc ports. So for Aarch64 longjmp/longjmp_target probe expects * first argument 8@address of jmpbuf in x0. * second argument -4@return val in x1. * third argument 8@ address of target in LR/X30. The patch below does that. (--snip--) --- a/ports/sysdeps/aarch64/__longjmp.S +++ b/ports/sysdeps/aarch64/__longjmp.S @@ -18,6 +18,7 @@ #include <sysdep.h> #include <jmpbuf-offsets.h> +#include <stap-probe.h> /* __longjmp(jmpbuf, val) */ @@ -56,6 +57,7 @@ ENTRY (__longjmp) #else ldp x29, x30, [x0, #JB_X29<<3] #endif + LIBC_PROBE (longjmp, 3, 8@x0, -4@x1, 8@x30) ldp d8, d9, [x0, #JB_D8<<3] ldp d10, d11, [x0, #JB_D10<<3] ldp d12, d13, [x0, #JB_D12<<3] @@ -97,6 +99,7 @@ ENTRY (__longjmp) #else ldr x5, [x0, #JB_SP<<3] #endif + LIBC_PROBE (longjmp_target, 3, 8@x0, -4@x1, 8@x30) mov sp, x5 cmp x1, #0 mov x0, #1 (--snip--) Please let me know if this is fine, I will start doing the testing. regards, Venkat. On 6 January 2014 23:40, Tom Tromey <[hidden email]> wrote: >>>>>> ">" == Venkataramanan Kumar <[hidden email]> writes: > >>> This patch adds pointer mangling support for Aarch64. > > IIRC the Aarch64 longjmp code doesn't have the sdt.h probes in place. > If you add pointer mangling without also adding the sdt.h probes, then > gdb's longjmp support will break. You should be able to see this easily > in the gdb test suite. > > Adding the probes is quite easy to do, so I encourage you to add that > to the patch. > > thanks, > Tom |
On 7 January 2014 11:05, Venkataramanan Kumar
<[hidden email]> wrote: > I looked at x86 and powerpc ports. So for Aarch64 > longjmp/longjmp_target probe expects > * first argument 8@address of jmpbuf in x0. > * second argument -4@return val in x1. > * third argument 8@ address of target in LR/X30. Hi, I'm not familiar with the inner workings of STAP probes, can you explain what the arguments of longjmp_target should be at a semantic level rather than the proposed location to get the values from? Do we need probes in setjmp aswell? Cheers /Marcus |
>>>>> "Marcus" == Marcus Shawcroft <[hidden email]> writes:
Marcus> Hi, I'm not familiar with the inner workings of STAP probes, can you Marcus> explain what the arguments of longjmp_target should be at a semantic Marcus> level rather than the proposed location to get the values from? I was going to say manual/probes.texi but I see the longjmp probes aren't documented. Currently gdb only uses the third argument, which is the target PC. Marcus> Do we need probes in setjmp aswell? I think it's nice for users if the probes are the same across ports. That said, gdb itself only uses the longjmp and rtld probes from glibc. gdb users (or SystemTap users) can refer to the other probes though. Tom |
In reply to this post by Marcus
Hi Marcus,
On 7 January 2014 17:47, Marcus Shawcroft <[hidden email]> wrote: > On 7 January 2014 11:05, Venkataramanan Kumar > <[hidden email]> wrote: > >> I looked at x86 and powerpc ports. So for Aarch64 >> longjmp/longjmp_target probe expects >> * first argument 8@address of jmpbuf in x0. >> * second argument -4@return val in x1. >> * third argument 8@ address of target in LR/X30. > > Hi, I'm not familiar with the inner workings of STAP probes, can you > explain what the arguments of longjmp_target should be at a semantic > level rather than the proposed location to get the values from? I checked below link and found the argument format. https://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation (Snip) For compiler-generated code, each argument will be of the form N@OP. For hand-written assembly, or for inline assembly in C or C++, the initial N@ may be missing. If N is present, it describes the size of the argument. It will be one of: 1 8 bits unsigned -1 8 bits signed 2 16 bits unsigned -2 16 bits signed 4 32 bits unsigned -4 32 bits signed 8 64 bits unsigned -8 64 bits signed (Snip) Also looking at x86 and powerpc ports. They pass argument 1 - the address of jump buffer argument 2 - return value of __longjmp argument 3 - PC address or the jump target For Aarch64, the longjmp/longjmp_target probe passed as below. * first argument - 8@address of jmpbuf in x0. * second argument- -4@return val in x1. * third argument- 8@ PC address or jump target target in LR/X30. is that fine? > > Do we need probes in setjmp aswell? Ok, I will add that as well. I have not yet tested, the patch by enabling --enable-systemtap as it needs std.h. This is missing in my cross build system. > > Cheers > /Marcus |
In reply to this post by Tom Tromey
On 7 January 2014 18:10, Tom Tromey <[hidden email]> wrote:
Hi Tom, > Marcus> Hi, I'm not familiar with the inner workings of STAP probes, can you > Marcus> explain what the arguments of longjmp_target should be at a semantic > Marcus> level rather than the proposed location to get the values from? > > I was going to say manual/probes.texi but I see the longjmp probes > aren't documented. > > Currently gdb only uses the third argument, which is the target PC. > > Marcus> Do we need probes in setjmp aswell? > > I think it's nice for users if the probes are the same across ports. > > That said, gdb itself only uses the longjmp and rtld probes from glibc. > gdb users (or SystemTap users) can refer to the other probes though. Do you know if it is documented anywhere what the difference is between the longjmp and longjmp_target probes? It looks like on i386 the registers are restored in between longjmp/longjmp_target but on powerpc half the registers seem to have been restored by the time the longjmp probe is executed. AFAICT gdb does not use the longjmp_target probe either... Thanks, -- Will Newton Toolchain Working Group, Linaro |
In reply to this post by Venkataramanan Kumar-3
On 8 January 2014 10:32, Venkataramanan Kumar
<[hidden email]> wrote: > Also looking at x86 and powerpc ports. > They pass argument 1 - the address of jump buffer > argument 2 - return value of __longjmp > argument 3 - PC address or the jump target > For Aarch64, the longjmp/longjmp_target probe passed as below. > * first argument - 8@address of jmpbuf in x0. > * second argument- -4@return val in x1. > * third argument- 8@ PC address or jump target target in LR/X30. OK, so the arguments to the LIBC_PROBES in your patch look sensible. > is that fine? I'd like to see follow up to Will's question regarding placement of the two probes. > >> >> Do we need probes in setjmp aswell? > > Ok, I will add that as well. > I have not yet tested, the patch by enabling --enable-systemtap as it The patch needs to be tested ;-) Cheers /Marcus |
In reply to this post by Tom Tromey
On 7 January 2014 18:10, Tom Tromey <[hidden email]> wrote:
Hi Tom, > Marcus> Hi, I'm not familiar with the inner workings of STAP probes, can you > Marcus> explain what the arguments of longjmp_target should be at a semantic > Marcus> level rather than the proposed location to get the values from? > > I was going to say manual/probes.texi but I see the longjmp probes > aren't documented. > > Currently gdb only uses the third argument, which is the target PC. > > Marcus> Do we need probes in setjmp aswell? > > I think it's nice for users if the probes are the same across ports. > > That said, gdb itself only uses the longjmp and rtld probes from glibc. > gdb users (or SystemTap users) can refer to the other probes though. Can you confirm that gdb does not use the longjmp_target probe? It doesn't appear to use it as far as I can tell. Are you aware of what consumers there are for this probe point? Thanks, -- Will Newton Toolchain Working Group, Linaro |
>>>>> "Will" == Will Newton <[hidden email]> writes:
Will> Can you confirm that gdb does not use the longjmp_target probe? It Will> doesn't appear to use it as far as I can tell. Are you aware of what Will> consumers there are for this probe point? Yeah, gdb uses the "longjmp" probe, not "longjmp_target". I don't know what uses the latter; though it's worth noting that all probes are available to gdb and systemtap users, so I wouldn't necessarily know whether someone is using it. Tom |
On 20 January 2014 16:44, Tom Tromey <[hidden email]> wrote:
>>>>>> "Will" == Will Newton <[hidden email]> writes: > > Will> Can you confirm that gdb does not use the longjmp_target probe? It > Will> doesn't appear to use it as far as I can tell. Are you aware of what > Will> consumers there are for this probe point? > > Yeah, gdb uses the "longjmp" probe, not "longjmp_target". > I don't know what uses the latter; though it's worth noting that all > probes are available to gdb and systemtap users, so I wouldn't > necessarily know whether someone is using it. It's not clear to me the difference between the two - they take the same arguments as far as I can tell (longjmp arguments 1 and 2, and the target PC) but longjmp_target occurs at some later point which seems implementation dependant. For that reason I would propose new ports (or ports lacking these probes entirely such as ARM) not implement longjmp_target unless there is a compelling reason to do so. -- Will Newton Toolchain Working Group, Linaro |
Free forum by Nabble | Edit this page |