[PATCH] ARM: Add SystemTap probes to longjmp and setjmp.

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] ARM: Add SystemTap probes to longjmp and setjmp.

Will Newton
Now the ARM port implements pointer encryption for jmpbufs, gdb needs
a SystemTap probe point in longjmp to determine the target PC of
a call to longjmp. This patch implements the probe point in longjmp
and a similar probe point in setjmp.

In order to have all the appropriate registers available to pass to the
probe this reorders the layout of jmpbuf, putting the sp and lr registers
at the start rather than the end.

Tested on armv7, no new failures in the glibc testsuite and confirmed
that this fixes the gdb.base/longjmp.exp failures in the gdb testsuite.

ports/ChangeLog.arm:

2014-01-27  Will Newton  <[hidden email]>

        * sysdeps/arm/__longjmp.S: Include stap-probe.h.
        (__longjmp): Restore sp and lr before restoring callee
        saved registers.  Add longjmp SystemTap probe point.
        * sysdeps/arm/include/bits/setjmp.h (__JMP_BUF_SP):
        Define to zero to match jmpbuf layout.
        * sysdeps/arm/setjmp.S: Include stap-probe.h.
        (__sigsetjmp): Save sp and lr before saving callee
        saved registers.  Add setjmp SystemTap probe point.
---
 ports/sysdeps/arm/__longjmp.S           | 21 ++++++++++++++-------
 ports/sysdeps/arm/include/bits/setjmp.h |  2 +-
 ports/sysdeps/arm/setjmp.S              | 12 +++++++++---
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/ports/sysdeps/arm/__longjmp.S b/ports/sysdeps/arm/__longjmp.S
index 27c57a1..ad5fa9f 100644
--- a/ports/sysdeps/arm/__longjmp.S
+++ b/ports/sysdeps/arm/__longjmp.S
@@ -17,6 +17,7 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <sysdep.h>
+#include <stap-probe.h>
 #include <bits/setjmp.h>
 #include <rtld-global-offsets.h>
 #include <arm-features.h>
@@ -25,24 +26,21 @@
 
 ENTRY (__longjmp)
  mov ip, r0
- movs r0, r1 /* get the return value in place */
- it eq
- moveq r0, #1 /* can't let setjmp() return zero! */
 
 #ifdef CHECK_SP
  sfi_breg ip, \
- ldr r4, [\B, #32] /* jmpbuf's sp */
+ ldr r4, [\B] /* jmpbuf's sp */
  cfi_undefined (r4)
 #ifdef PTR_DEMANGLE
  PTR_DEMANGLE (r4, r4, a3, a4)
 #endif
  CHECK_SP (r4)
 #endif
- sfi_sp sfi_breg ip, \
- ldmia \B!, JMP_BUF_REGLIST
+
 #ifdef PTR_DEMANGLE
  ldr a4, [ip], #4
- PTR_DEMANGLE (a4, a4, a3, a2)
+ PTR_DEMANGLE (a4, a4, a3, r4)
+ cfi_undefined (r4)
  mov sp, a4
  ldr a4, [ip], #4
  PTR_DEMANGLE2 (lr, a4, a3)
@@ -50,6 +48,11 @@ ENTRY (__longjmp)
  ldr sp, [ip], #4
  ldr lr, [ip], #4
 #endif
+ /* longjmp probe expects longjmp first argument (4@r0), second
+   argument (-4@r1), and target address (4@r14), respectively.  */
+ LIBC_PROBE (longjmp, 3, 4@r0, -4@r1, 4@r14)
+ sfi_sp sfi_breg ip, \
+ ldmia \B!, JMP_BUF_REGLIST
  cfi_restore (v1)
  cfi_restore (v2)
  cfi_restore (v3)
@@ -61,6 +64,10 @@ ENTRY (__longjmp)
  cfi_restore (sp)
  cfi_restore (lr)
 
+ movs r0, r1 /* get the return value in place */
+ it eq
+ moveq r0, #1 /* can't let setjmp() return zero! */
+
 #if !defined ARM_ASSUME_NO_IWMMXT || defined __SOFTFP__
 # define NEED_HWCAP 1
 #endif
diff --git a/ports/sysdeps/arm/include/bits/setjmp.h b/ports/sysdeps/arm/include/bits/setjmp.h
index 220dfe8..5877c1f 100644
--- a/ports/sysdeps/arm/include/bits/setjmp.h
+++ b/ports/sysdeps/arm/include/bits/setjmp.h
@@ -30,7 +30,7 @@
 # define JMP_BUF_REGLIST {v1-v6, sl, fp}
 
 /* Index of __jmp_buf where the sp register resides.  */
-# define __JMP_BUF_SP 8
+# define __JMP_BUF_SP 0
 #endif
 
 #endif  /* include/bits/setjmp.h */
diff --git a/ports/sysdeps/arm/setjmp.S b/ports/sysdeps/arm/setjmp.S
index b0b45ed..5e55ca5 100644
--- a/ports/sysdeps/arm/setjmp.S
+++ b/ports/sysdeps/arm/setjmp.S
@@ -17,6 +17,7 @@
    <http://www.gnu.org/licenses/>.  */
 
 #include <sysdep.h>
+#include <stap-probe.h>
 #include <bits/setjmp.h>
 #include <rtld-global-offsets.h>
 #include <arm-features.h>
@@ -27,9 +28,11 @@ ENTRY (__sigsetjmp)
 #endif
  mov ip, r0
 
- /* Save registers */
- sfi_breg ip, \
- stmia \B!, JMP_BUF_REGLIST
+ /* setjmp probe expects sigsetjmp first argument (4@r0), second
+   argument (-4@r1), and target address (4@r14), respectively.  */
+ LIBC_PROBE (setjmp, 3, 4@r0, -4@r1, 4@r14)
+
+ /* Save sp and lr */
 #ifdef PTR_MANGLE
  mov a4, sp
  PTR_MANGLE2 (a4, a4, a3)
@@ -40,6 +43,9 @@ ENTRY (__sigsetjmp)
  str sp, [ip], #4
  str lr, [ip], #4
 #endif
+ /* Save registers */
+ sfi_breg ip, \
+ stmia \B!, JMP_BUF_REGLIST
 
 #if !defined ARM_ASSUME_NO_IWMMXT || defined __SOFTFP__
 # define NEED_HWCAP 1
--
1.8.1.4

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ARM: Add SystemTap probes to longjmp and setjmp.

Joseph Myers
On Mon, 27 Jan 2014, Will Newton wrote:

> + /* longjmp probe expects longjmp first argument (4@r0), second
> +   argument (-4@r1), and target address (4@r14), respectively.  */
> + LIBC_PROBE (longjmp, 3, 4@r0, -4@r1, 4@r14)

I don't know what 4@r0 etc. mean - where is this documented (I don't see
it in include/stap-probe.h)?  Also, where are the semantics of these
probes documented (I don't see them in manual/probes.texi)?

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ARM: Add SystemTap probes to longjmp and setjmp.

Will Newton
On 27 January 2014 18:22, Joseph S. Myers <[hidden email]> wrote:
> On Mon, 27 Jan 2014, Will Newton wrote:
>
>> +     /* longjmp probe expects longjmp first argument (4@r0), second
>> +        argument (-4@r1), and target address (4@r14), respectively.  */
>> +     LIBC_PROBE (longjmp, 3, 4@r0, -4@r1, 4@r14)
>
> I don't know what 4@r0 etc. mean - where is this documented (I don't see
> it in include/stap-probe.h)?  Also, where are the semantics of these
> probes documented (I don't see them in manual/probes.texi)?

None of this seems particularly well-documented as far as I can tell.

The SystemTap probe syntax is covered here:

https://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation

But there's no mention of ARM specifically, and the gdb source seems
to be the place where the syntax is parsed so I wrote to match that.
For example, lr does not seem to be a supported register name but r14
is.

The semantics of the probes is not documented at all as far as I can
tell. Again gdb is the consumer so I looked at the sources there as
well as the comments in the powerpc port of glibc (thanks powerpc
maintainers!). Only the third argument is used by gdb, which is the
target pc.

It would be great if someone who actually knew the details of this for
sure added some docs (i.e. see discussion of longjmp_target probe that
other architectures implement).

--
Will Newton
Toolchain Working Group, Linaro
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ARM: Add SystemTap probes to longjmp and setjmp.

Joseph Myers
On Mon, 27 Jan 2014, Will Newton wrote:

> The SystemTap probe syntax is covered here:
>
> https://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation

Perhaps you could add a comment to stap-probe.h pointing to this page as
the explanation of what the arguments mean?

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ARM: Add SystemTap probes to longjmp and setjmp.

Joseph Myers
In reply to this post by Will Newton
On Mon, 27 Jan 2014, Will Newton wrote:

> @@ -50,6 +48,11 @@ ENTRY (__longjmp)
>   ldr sp, [ip], #4
>   ldr lr, [ip], #4
>  #endif
> + /* longjmp probe expects longjmp first argument (4@r0), second
> +   argument (-4@r1), and target address (4@r14), respectively.  */
> + LIBC_PROBE (longjmp, 3, 4@r0, -4@r1, 4@r14)

The discussion on libc-alpha said this probe should be before unwinding;
shouldn't that mean before sp and lr are restored, so that at probe time
it's still possible to unwind through the caller of longjmp?

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ARM: Add SystemTap probes to longjmp and setjmp.

Roland McGrath-4
> On Mon, 27 Jan 2014, Will Newton wrote:
>
> > @@ -50,6 +48,11 @@ ENTRY (__longjmp)
> >   ldr sp, [ip], #4
> >   ldr lr, [ip], #4
> >  #endif
> > + /* longjmp probe expects longjmp first argument (4@r0), second
> > +   argument (-4@r1), and target address (4@r14), respectively.  */
> > + LIBC_PROBE (longjmp, 3, 4@r0, -4@r1, 4@r14)
>
> The discussion on libc-alpha said this probe should be before unwinding;
> shouldn't that mean before sp and lr are restored, so that at probe time
> it's still possible to unwind through the caller of longjmp?

Yes, the longjmp probe is conceptually "at entry to longjmp".  So it needs
to be just far enough in that you have demangled the pointers to make them
available as the probe's arguments.  The longmp_target probe is the one
that should be as late as possible, so all the registers except the PC
itself (or as close as the particular machine's implementation makes
possible) reflect the state being restored.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ARM: Add SystemTap probes to longjmp and setjmp.

Will Newton
In reply to this post by Joseph Myers
On 28 January 2014 21:35, Joseph S. Myers <[hidden email]> wrote:

> On Mon, 27 Jan 2014, Will Newton wrote:
>
>> @@ -50,6 +48,11 @@ ENTRY (__longjmp)
>>       ldr     sp, [ip], #4
>>       ldr     lr, [ip], #4
>>  #endif
>> +     /* longjmp probe expects longjmp first argument (4@r0), second
>> +        argument (-4@r1), and target address (4@r14), respectively.  */
>> +     LIBC_PROBE (longjmp, 3, 4@r0, -4@r1, 4@r14)
>
> The discussion on libc-alpha said this probe should be before unwinding;
> shouldn't that mean before sp and lr are restored, so that at probe time
> it's still possible to unwind through the caller of longjmp?

Yep, I'm working on an updated patch.

--
Will Newton
Toolchain Working Group, Linaro