[PATCH] [AARCH64]: Pointer mangling support for Aarch64

[Venkataramanan Kumar-3]

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.

[Richard Henderson]

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~

[Venkataramanan Kumar-3]

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~

Yes this can be done. Also I changed the mangling code to use store
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.

[Richard Henderson]

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~

[Venkataramanan Kumar-3]

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.

[Richard Henderson]

Looks fine to me.


r~

[Venkataramanan Kumar-3]

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~

[Marcus]

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

[Venkataramanan Kumar-3]

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

[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

[Tom Tromey]

>>>>> ">" == 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

[Venkataramanan Kumar-3]

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

[Marcus]

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

[Tom Tromey]

>>>>> "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

[Venkataramanan Kumar-3]

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

[Will Newton]

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

[Marcus]

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

[Will Newton]

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

[Tom Tromey]

>>>>> "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

[Will Newton]

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