| Message ID | 1391594162-22269-1-git-send-email-will.newton@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 6 February 2014 16:26, Carlos O'Donell <carlos@redhat.com> wrote: > On 02/05/2014 04:56 AM, Will Newton wrote: >> 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 points 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. > > This looks good to me. > > If it's considered a bug, please check this in immediately and CC Allan > to keep him in the loop for these last minute bug fixes. > > We are about to freeze so the 2.19 branch can be cut. If there is anything > else like this please bring it to his attention immediately. Sorry I should have been clearer, I am proposing this patch for 2.20. It is a regression of functionality to add pointer encryption without this patch as gdb now cannot deal with longjmp calls as well as it could previously but I agree with Jospeh that it's not appropriate to push a change of this nature so late in the freeze.
On 6 February 2014 22:11, Roland McGrath <roland@hack.frob.com> wrote: > Can you clarify why you want to change the jmp_buf layout? > I don't see why any change you might want to make would necessitate that. In order to have the correct registers live I need to free up a temp register which involves loading lr and sp earlier than the call-preserved register set. I could do this by offsetting into the jmpbuf but it seems more natural just to move lr and sp to the front of the jmp_buf and load registers in ascending order. As far as I am aware the layout of jmp_buf is completely opaque so I don't see why this would cause any problems?
On 7 February 2014 15:27, Jonathan S. Shapiro <shap@eros-os.org> wrote: > Yes. The layout of jmp_buf is part of the ABI. And not just the libc ABI, > but for some platforms it's part of the platform standard (e.g. SVID > specifies it). In consequence, changing the structure layout isn't an option > in either the current libc release or any foreseeable future libc release. > The patch point needs to be reworked so as to operate given the current > structure layout. The layout has changed in previous releases so I am not convinced this is true. > On the bright side, reworking the code to fit the current layout means that > (a) this can be integrated sooner, and (b) we don't have to accept a small > regression in GDB functionality in order to implement encrypted pointers. The GDB functionality regression is orthogonal to the layout change. The gdb issue is caused by the fact that the saved lr in the jmp_buf is now no longer a straightforward code pointer. The longjmp SystemTap probe added by this patch unencrypts lr and hands it to gdb so gdb can set a break there.
diff --git a/ports/sysdeps/arm/__longjmp.S b/ports/sysdeps/arm/__longjmp.S index 27c57a1..08521e5 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,31 +26,35 @@ 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) - mov sp, a4 - ldr a4, [ip], #4 - PTR_DEMANGLE2 (lr, a4, a3) + PTR_DEMANGLE (a4, a4, a3, r4) + cfi_undefined (r4) + ldr r4, [ip], #4 + PTR_DEMANGLE2 (r4, r4, a3) #else - ldr sp, [ip], #4 - ldr lr, [ip], #4 + ldr a4, [ip], #4 + ldr r4, [ip], #4 + cfi_undefined (r4) #endif + /* longjmp probe expects longjmp first argument (4@r0), second + argument (-4@r1), and target address (4@r4), respectively. */ + LIBC_PROBE (longjmp, 3, 4@r0, -4@r1, 4@r4) + mov sp, a4 + mov lr, r4 + sfi_sp sfi_breg ip, \ + ldmia \B!, JMP_BUF_REGLIST cfi_restore (v1) cfi_restore (v2) cfi_restore (v3) @@ -67,27 +72,27 @@ ENTRY (__longjmp) #ifdef NEED_HWCAP # ifdef IS_IN_rtld - ldr a2, 1f + ldr a4, 1f ldr a3, .Lrtld_local_ro -0: add a2, pc, a2 - add a2, a2, a3 - ldr a2, [a2, #RTLD_GLOBAL_RO_DL_HWCAP_OFFSET] +0: add a4, pc, a4 + add a4, a4, a3 + ldr a4, [a4, #RTLD_GLOBAL_RO_DL_HWCAP_OFFSET] # else # ifdef PIC - ldr a2, 1f + ldr a4, 1f ldr a3, .Lrtld_global_ro -0: add a2, pc, a2 - ldr a2, [a2, a3] - ldr a2, [a2, #RTLD_GLOBAL_RO_DL_HWCAP_OFFSET] +0: add a4, pc, a4 + ldr a4, [a4, a3] + ldr a4, [a4, #RTLD_GLOBAL_RO_DL_HWCAP_OFFSET] # else - ldr a2, .Lhwcap - ldr a2, [a2, #0] + ldr a4, .Lhwcap + ldr a4, [a4, #0] # endif # endif #endif #ifdef __SOFTFP__ - tst a2, #HWCAP_ARM_VFP + tst a4, #HWCAP_ARM_VFP beq .Lno_vfp #endif @@ -98,7 +103,7 @@ ENTRY (__longjmp) .Lno_vfp: #ifndef ARM_ASSUME_NO_IWMMXT - tst a2, #HWCAP_ARM_IWMMXT + tst a4, #HWCAP_ARM_IWMMXT beq .Lno_iwmmxt /* Restore the call-preserved iWMMXt registers. */ @@ -118,6 +123,14 @@ ENTRY (__longjmp) .Lno_iwmmxt: #endif + /* longjmp_target probe expects longjmp first argument (4@r0), second + argument (-4@r1), and target address (4@r14), respectively. */ + LIBC_PROBE (longjmp_target, 3, 4@r0, -4@r1, 4@r14) + + movs r0, r1 /* get the return value in place */ + it eq + moveq r0, #1 /* can't let setjmp() return zero! */ + DO_RET(lr) #ifdef NEED_HWCAP 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