sigaltstack and ar.bspstore

Currently when using an alternate stack for signal handling, the trampoline code switches to the new register stack without saving bspstore. When returning to the original register stack, it calculates the bspstore as:

bspstore0 = rse_skip_regs(bsp0, -rse_num_regs(bsp1 - (loadrs >> 19), bsp1)); where bsp0 is sc->sc_ar_bsp and bsp1 is the current bsp

Presumably the result should be the same as the original bspstore. However, it isn't in the case when the original bspstore is pointing to slot 63 (the RNAT word). The new bspstore comes out pointing to slot 0 of the next group, and the RNAT bits never get written. This happens more often then you'd expect, since it seems that current implementations make the optimisation of writing the RNAT word together with the first word of the new group instead of the last word of the old group (sensible because it potentially saves a write).

I'd propose that the trampoline code should save bspstore in the sigcontext structure and restore it from there rather than recalculating it. I can send a patch, but first is there any reason why it shouldn't be done that way?

Matt

Matt> Presumably the result should be the same as the original Matt> bspstore. However, it isn't in the case when the original Matt> bspstore is pointing to slot 63 (the RNAT word). The new Matt> bspstore comes out pointing to slot 0 of the next group, and Matt> the RNAT bits never get written. This happens more often then Matt> you'd expect, since it seems that current implementations make Matt> the optimisation of writing the RNAT word together with the Matt> first word of the new group instead of the last word of the Matt> old group (sensible because it potentially saves a write).

Yes, I noticed the same in libunwind. ;-/

Matt> I'd propose that the trampoline code should save bspstore in Matt> the sigcontext structure and restore it from there rather than Matt> recalculating it. I can send a patch, but first is there any Matt> reason why it shouldn't be done that way?

My preference would be to fix it in a way that preserves binary compatibility for the sigcontext structure.

	--david

I wasn't suggesting changing the layout, I was suggesting using one of the reserved slots. Unless you're referring to software that changes sc_ar_bsp...

I don't think there's currently enough information available upon return from the signal handler to decide whether to point bspstore at slot 63 or 0. It wouldn't be safe to always go for slot 63 because it might be beyond the start of the RBS.

Matt

Matt> I wasn't suggesting changing the layout, I was suggesting Matt> using one of the reserved slots. Unless you're referring to Matt> software that changes sc_ar_bsp...

Yes, that's what I was referring to.

Matt> I don't think there's currently enough information available Matt> upon return from the signal handler to decide whether to point Matt> bspstore at slot 63 or 0. It wouldn't be safe to always go Matt> for slot 63 because it might be beyond the start of the RBS.

I'll have to think about this some more. Especially to make sure that there aren't any other places were we (I) made the assumption that bspstore never points to an RNaT slot. But it will take me a while before I get to it (in case I forget or you come up with a great solution, please ping me... ;-).

	--david

Matt> Currently when using an alternate stack for signal handling, Matt> the trampoline code switches to the new register stack without Matt> saving bspstore. When returning to the original register Matt> stack, it calculates the bspstore as:

Matt> bspstore0 = rse_skip_regs(bsp0, -rse_num_regs(bsp1 - (loadrs
Matt> >> 19), bsp1)); where bsp0 is sc->sc_ar_bsp and bsp1 is the Matt> current bsp

Matt> Presumably the result should be the same as the original Matt> bspstore. However, it isn't in the case when the original Matt> bspstore is pointing to slot 63 (the RNAT word). The new Matt> bspstore comes out pointing to slot 0 of the next group, and Matt> the RNAT bits never get written.

Yes, this is a problem. Can you try the attached patch? Rather than storing an additional sc_bspstore member, the backing-store switching code checks whether bspstore points to an RNaT slot and, if so, stores the RNaT manually. This effectively "normalizes" the backing-store such that ar.bspstore never points to an RNaT slot.

Note that binary compatibility is the main reason for this solution. Perhaps some day there will be an opportunity to redo the sigcontext structure and at that point we could store the bspstore/loadrs pair (as we do on kernel entry) instead of the bsp/loadrs pair that we currently store in sigcontext.

	--david

=== arch/ia64/kernel/gate.S 1.17 vs edited ==--- 1.17/arch/ia64/kernel/gate.S Tue Jun 17 23:50:16 2003
+++ edited/arch/ia64/kernel/gate.S Fri Jun 20 22:26:34 2003
@@ -199,7 +199,7 @@
cmp.ne p8,p0=r15,r0 // do we need to switch the rbs?
mov.m r9=ar.bsp // fetch ar.bsp
.spillsp.p p8, ar.rnat, RNAT_OFF+SIGCONTEXT_OFF
-(p8) br.cond.spnt setup_rbs // yup -> (clobbers r14, r15, and r16)
+(p8) br.cond.spnt setup_rbs // yup -> (clobbers p8, r14-r16, and r18-r20)
back_from_setup_rbs:
alloc r8=ar.pfs,0,0,3,0
ld8 out0=[base0],16 // load arg0 (signum)
@@ -268,26 +268,30 @@
setup_rbs:
mov ar.rsc=0 // put RSE into enforced lazy mode
;;
- .save ar.rnat, r16
- mov r16=ar.rnat // save RNaT before switching backing store area
+ .save ar.rnat, r19
+ mov r19=ar.rnat // save RNaT before switching backing store area
adds r14=(RNAT_OFF+SIGCONTEXT_OFF),sp

+ mov r18=ar.bspstore
mov ar.bspstore=r15 // switch over to new register backing store area
;;
+
.spillsp ar.rnat, RNAT_OFF+SIGCONTEXT_OFF
- st8 [r14]=r16 // save sc_ar_rnat
+ st8 [r14]=r19 // save sc_ar_rnat
.body
- adds r14=(LOADRS_OFF+SIGCONTEXT_OFF),sp
-
mov.m r16=ar.bsp // sc_loadrs <- (new bsp - new bspstore) << 16
+ adds r14=(LOADRS_OFF+SIGCONTEXT_OFF),sp
;;
invala
sub r15=r16,r15
+ extr.u r20=r18,3,6
;;
+ mov ar.rsc=0xf // set RSE into eager mode, pl 3
+ cmp.eq p8,p0c,r20
shl r15=r15,16
;;
st8 [r14]=r15 // save sc_loadrs
- mov ar.rsc=0xf // set RSE into eager mode, pl 3
+(p8) st8 [r18]=r19 // if bspstore points at RNaT slot, store RNaT there now
.restore sp // pop .prologue
br.cond.sptk back_from_setup_rbs

_______________________________________________
Linux-IA64 mailing list
[email protected]
http://lists.linuxia64.org/lists/listinfo/linux-ia64

Ah, that's a good way of doing it (given the constraints of backwards compatibility). I've tested your patch and it works well for me.

Thanks!

Matt

_______________________________________________
Linux-IA64 mailing list
[email protected]
http://lists.linuxia64.org/lists/listinfo/linux-ia64

Matt> I've tested your patch and it works well for me.

Great! Thanks for testing it.

	--david

_______________________________________________
Linux-IA64 mailing list
[email protected]
http://lists.linuxia64.org/lists/listinfo/linux-ia64