[parisc-linux] Re: [glibc] tststatic failues, reduced to simple testcase.

[parisc-linux] Re: [glibc] tststatic failues, reduced to simple testcase.

[Carlos O'Donell]

Dave,

I will start by saying that I wasn't "fair" in just dumping the assembly
into an email, falling asleep at my keyboard at 4:30 AM and leaving it
up to you to guess what was _really_ going on :)

I'm sending this to the list so it gets recorded on archive. Our problem
right now it that we don't properly restore r19 after an __asm()
statement even if the clobber contains r19. Or rather gcc doesn't
schedule the restore to occur at the right time.

What does this mean for glibc, well it means that ld.so's first fork
corrupts the PIC register r19 aka LTP, and the subsequent import stub
for a function call fails (SIGSEGV). What is effected in glibc?
The following:
	- INTERNAL_SYSCALL (Macro syscall)
	- INLINE_SYSCALL (Macro syscall)
	- syscall(...) (C version)
	- DO_CALL (Assembly wrapper syscall)

What is not effected:
	- DO_CALL_NOERRNO
	- DO_CALL_ERRVAL
	- PSEUDO (syscall cancellation wrapper)
	
Explicitly storing and loading r19 around the syscall e.g. inside the
__asm() statement works around the problem. I do not want to have to
stw/ldw since it costs a lot in performance, we know now to look at gcc
for help. Perhaps I will use this as a temporary measure to release
glibc 2.3.2 for debian so we keep testing moving.

This problem has a number of interesting heisenbugs:
	- If the kernel decides not to scratch in r19 then you're okay.
	- If the compiler version scheduled r19 restore differently then
          you're okay.

All of these contributed to a lof of head scratching on my part. Since
things worked sometimes, on some boxes, and differently with different
compilers. Needless to say I learned a lot and poked enough other people
that we have had our mmap flush problems fixed, and our -fPIC -static
problems fixed.

See the following for expansion on both:
http://www.ussg.iu.edu/hypermail/linux/kernel/0308.2/1680.html
http://sources.redhat.com/ml/binutils/2003-08/msg00467.html

> I don't see the restore of r19 from r4.  What are the other 10 insns?
> Normally, I would have expected it before

This code is the relocated libpthread.so as viewed without symbols by
tracing through ld.so loading ex14 testcase in glibc. This code is the
beginning of the loader trying to start the child process immediately
after the last few calls in dl-runtime.c. What follows is the whole insn
stream up to the crash from the last call to fixup.

Here it is for posterity.

Breakpoint 1, 0x4100ceb0 in _dl_runtime_resolve () at dl-runtime.c:213
213               value = l->l_addr + sym->st_value;
(gdb) c 21
Will ignore next 20 crossings of breakpoint 1.  Continuing.
                                                                                           
Breakpoint 1, 0x4100ceb0 in _dl_runtime_resolve () at dl-runtime.c:213
213               value = l->l_addr + sym->st_value;
                                                   
si from here forward
                                        
0x4015e3b8:     stw rp,-14(sr0,sp)
0x4015e3bc:     stw,ma r4,40(sr0,sp)
0x4015e3c0:     stw r19,-20(sr0,sp)
0x4015e3c4:     addil 1000,r19,%r1
0x4015e3c8:     copy r1,r21
0x4015e3cc:     ldw 200(sr0,r21),r21
0x4015e3d0:     ldw 6c(sr0,r21),r22
0x4015e3d4:     cmpib,<> 0,r22,0x4015e3e8
0x4015e3d8:     addil 800,r19,%r1
                                                                                           
0x4015e3e8:     ldw 5d8(sr0,r1),r20
0x4015e3ec:     copy r20,r26
0x4015e3f0:     b,l 0x40167e30,r31
0x4015e3f4:     copy r31,rp
                                                                                           
0x40167e30:     b,l 0x40167e38,r1
0x40167e34:     addil 9f800,r1,%r1
0x40167e38:     be,n 218(sr4,r1)
                                                                                           
0x40207850:     bb,>=,n r22,1e,0x40207860
0x40207854:     depwi 0,31,2,r22
0x40207858:     ldw 4(sr0,r22),r19 <---------- r19 = 0x40020718
0x4020785c:     ldw 0(sr0,r22),r22
0x40207860:     bv r0(r22)
0x40207864:     stw rp,-18(sr0,sp)
                                                                                           
0x4000812c:     stw rp,-14(sr0,sp)
0x40008130:     stw,ma r5,40(sr0,sp)
0x40008134:     stw r4,-3c(sr0,sp)
0x40008138:     stw r3,-38(sr0,sp)
0x4000813c:     stw r19,-20(sr0,sp)
0x40008140:     ldw c(sr0,r26),r20
0x40008144:     copy r26,r3
0x40008148:     cmpib,<< 3,r20,0x40008184
0x4000814c:     ldi 16,ret0
0x40008150:     blr r20,r0
0x40008154:     nop
                                                                                           
0x40008160:     b,l 0x400081a8,r0
0x40008164:     ldw 8(sr0,r26),r20
                                                                                           
0x400081a8:     ldi 0,ret0
0x400081ac:     ldo 10(r26),r26
0x400081b0:     mfctl tr3,r5
0x400081b4:     cmpb,=,n r5,r20,0x400081cc
0x400081b8:     b,l 0x4000b29c,rp
0x400081bc:     copy r5,r25
                                                                                           
0x4000b29c:     stw rp,-14(sr0,sp)
0x4000b2a0:     stw,ma r4,40(sr0,sp)
0x4000b2a4:     stw r19,-20(sr0,sp)
0x4000b2a8:     b,l 0x4000b930,rp
0x4000b2ac:     ldo 10(r26),r26
                                                                                           
0x4000b930:     stw rp,-14(sr0,sp)
0x4000b934:     ldo 80(sp),sp
0x4000b938:     stw r7,-68(sr0,sp)
0x4000b93c:     stw r6,-64(sr0,sp)
0x4000b940:     stw r5,-60(sr0,sp)
0x4000b944:     stw r4,-5c(sr0,sp)
0x4000b948:     stw r3,-58(sr0,sp)
0x4000b94c:     stw r19,-20(sr0,sp)
0x4000b950:     copy r26,r5
0x4000b954:     ldi 0,r3
0x4000b958:     ldil 1e8000,r20
0x4000b95c:     ldi 31,r6
0x4000b960:     ldo 481(r20),r7
0x4000b964:     stw r5,-70(sr0,sp)
0x4000b968:     ldw -70(sr0,sp),r20
0x4000b96c:     depwi 0,31,4,r20
0x4000b970:     cmpb,<<=,n r5,r20,0x4000b984
                                                                                           
0x4000b984:     ldw -70(sr0,sp),r20
0x4000b988:     ldcw 0(sr0,r20),r20
0x4000b98c:     cmpib,<> 0,r20,0x4000b9d0
0x4000b990:     ldw -94(sr0,sp),rp
                                                                                           
0x4000b9d0:     ldw -68(sr0,sp),r7
0x4000b9d4:     ldw -64(sr0,sp),r6
0x4000b9d8:     ldw -60(sr0,sp),r5
0x4000b9dc:     ldw -5c(sr0,sp),r4
0x4000b9e0:     ldw -58(sr0,sp),r3
0x4000b9e4:     bv r0(rp)
0x4000b9e8:     ldo -80(sp),sp
                                                                                           
0x4000b2b0:     ldw -54(sr0,sp),rp
0x4000b2b4:     bv r0(rp)
0x4000b2b8:     ldw,mb -40(sr0,sp),r4
                                                                                           
0x400081c0:     stw r0,4(sr0,r3)
0x400081c4:     b,l 0x40008180,r0
0x400081c8:     stw r5,8(sr0,r3)
                                                                                           
0x40008180:     ldi 0,ret0
0x40008184:     ldw -54(sr0,sp),rp
0x40008188:     ldw -3c(sr0,sp),r4
0x4000818c:     ldw -38(sr0,sp),r3
0x40008190:     bv r0(rp)
0x40008194:     ldw,mb -40(sr0,sp),r5
                                                                                           
0x4015e3f8:     b,l 0x4015e3e0,r0
0x4015e3fc:     ldw -54(sr0,sp),rp
                                                                                           
0x4015e3e0:     bv r0(rp)
0x4015e3e4:     ldw,mb -40(sr0,sp),r4

0x40008838:     copy r4,r19  <---Restore----- r19 = 0x40020718
__asm(
0x4000883c:     be,l 100(sr2,r0),%sr0,%r31
0x40008840:     ldi 2,r20                    !! FORK !!
);
0x40008844:     ldi -1000,r20 <--Corrupted--- r19 = 0x10106368
0x40008848:     cmpb,>>= r20,ret0,0x40008868
0x4000884c:     copy ret0,r6
                                                                                           
0x40008868:     cmpib,<> 0,r6,0x400088e4
0x4000886c:     copy r19,r4
                                                                                           
0x400088e4:     b,l 0x4000a9d4,rp
0x400088e8:     ldo 38(r7),r5
                                                                                           
0x4000a9d4:     stw rp,-14(sr0,sp)
0x4000a9d8:     ldo 40(sp),sp
0x4000a9dc:     stw r19,-20(sr0,sp)
0x4000a9e0:     ldw -54(sr0,sp),rp
0x4000a9e4:     b,l 0x40005440,r0
0x4000a9e8:     ldo -40(sp),sp

No scheduled r19 restore yet.  
                                                                                         
0x40005440:     addil -800,r19,%r1
0x40005444:     ldw 55c(sr0,r1),r21 <-- Not quote boom, probably wrong.
0x40005448:     bv r0(r21)
0x4000544c:     ldw 560(sr0,r1),r19 <-- *Boom*

> If the restore is not there, please send preprocessed source and
> compilation details.  BOOM appears to be in an import stub (i.e.,
> there must be a call in the 10).  Calls use r19 in pic code (i.e.,
> in the import stub), so it's not obvious why the restore wouldn't be
> there.

The restore is not there. Placing r19 into the __asm(syscall) clobber 
list doesn't fix the issue. Nothing short of an explicity stw/ldw inside 
the __asm statement saves r19 from corruption.

> Scheduling can reorder instructions, so the pic restore doesn't have
> to immediately follow a call.  "FORK" isn't a GCC generated call
> (we never use sr2).  Calls are tricky and the procedure for generating
> them has been revised several times.  Now, we don't split out the save
> and restore of the pic register until after reload.  Reload can introduce
> new uses of the pic register.  When not using exceptions, register
> copies following a call are part of an "in call group" that keeps the
> restore in the same basic block as the call for scheduling purposes.
> However, when exceptions are enabled, the basic block ends at the
> call.  If the restore is split out from the call before reload,
> it will be scheduled in a different basic block from the call.  As
> a result, scheduling may move another instruction which has an
> implicit dependence on the pic register forward past the restore.
> Then, BOOM.

Preprocessed source for ptfork.c at:
http://www.baldric.uwo.ca/~carlos/ptfork.E
You'll see the INLINE_SYSCALL in __pthread_fork on line 8137.

I would like to not that I might have made _many_ errors, but the simple
stw/ldw r19 fix passes all the glib thread tests so I think its a step
in the right direction.

Thanks for the help!
c.

[parisc-linux] Re: [glibc] tststatic failues, reduced to simple testcase.

[John David Anglin]

> I'm sending this to the list so it gets recorded on archive. Our problem
> right now it that we don't properly restore r19 after an __asm()
> statement even if the clobber contains r19. Or rather gcc doesn't
> schedule the restore to occur at the right time.

An asm will not cause the restoration of %r19.  The compiler treats
r19 as a fixed register when generating pic code.  The saving and
restoring of the pic register across calls is something that the pa
port does itself in the backend.  Clobbering r19 won't cause it
to be restored.

In the case of syscalls, the compiler has no way of telling that
you are making a "call".

> What does this mean for glibc, well it means that ld.so's first fork
> corrupts the PIC register r19 aka LTP, and the subsequent import stub
> for a function call fails (SIGSEGV). What is effected in glibc?
> The following:
> 	- INTERNAL_SYSCALL (Macro syscall)
> 	- INLINE_SYSCALL (Macro syscall)
> 	- syscall(...) (C version)
> 	- DO_CALL (Assembly wrapper syscall)
> Explicitly storing and loading r19 around the syscall e.g. inside the
> __asm() statement works around the problem. I do not want to have to
> stw/ldw since it costs a lot in performance, we know now to look at gcc
> for help. Perhaps I will use this as a temporary measure to release
> glibc 2.3.2 for debian so we keep testing moving.
> 
> This problem has a number of interesting heisenbugs:
> 	- If the kernel decides not to scratch in r19 then you're okay.
> 	- If the compiler version scheduled r19 restore differently then
>           you're okay.

> 0x40008838:     copy r4,r19  <---Restore----- r19 = 0x40020718

This is a restore after a call.

> __asm(
> 0x4000883c:     be,l 100(sr2,r0),%sr0,%r31
> 0x40008840:     ldi 2,r20                    !! FORK !!
> );
> 0x40008844:     ldi -1000,r20 <--Corrupted--- r19 = 0x10106368

There is no save and restore around the syscall as GCC doesn't know
the asm does a call.  Clobbering r19 isn't an option in PIC code.
Thus, either the syscall has to save and restore r19, or the kernel
has to avoid clobbering r19.

> Preprocessed source for ptfork.c at:
> http://www.baldric.uwo.ca/~carlos/ptfork.E
> You'll see the INLINE_SYSCALL in __pthread_fork on line 8137.
> 
> I would like to not that I might have made _many_ errors, but the simple
> stw/ldw r19 fix passes all the glib thread tests so I think its a step
> in the right direction.

You should be able to use a register.  The problem is how to do this
in a nice way so that you don't have to save and restore r19 in non-pic
code.

Ouch, I am surprised that this hasn't had a bigger effect.

Dave

[parisc-linux] Re: [glibc] tststatic failues, reduced to simple testcase.

[John David Anglin]

> > __asm(
> > 0x4000883c:     be,l 100(sr2,r0),%sr0,%r31
> > 0x40008840:     ldi 2,r20                    !! FORK !!
> > );
> > 0x40008844:     ldi -1000,r20 <--Corrupted--- r19 = 0x10106368

Looking at the kernel syscall code, it seems at first glance that
r19 is saved and restored.  Thus, the problem may be specific to fork.

Dave

Re: [parisc-linux] Re: [glibc] tststatic failues, reduced to simple testcase.

[Carlos O'Donell]

Dave,

> An asm will not cause the restoration of %r19.  The compiler treats
> r19 as a fixed register when generating pic code.  The saving and
> restoring of the pic register across calls is something that the pa
> port does itself in the backend.  Clobbering r19 won't cause it
> to be restored.

I thought that listing it in the clobber would be the solution.

> In the case of syscalls, the compiler has no way of telling that
> you are making a "call".

Yes, glibc has begun using inlined macros rather than incuring the
penalty of a function call to get at the syscall. I _could_ cheat and
make the inlined code call a 'C' function which would force a restore,
but we then have _many_ more insn on the fast path (rather than just a
stw and ldw).
 
> > 0x40008838:     copy r4,r19  <---Restore----- r19 = 0x40020718
> 
> This is a restore after a call.

Correct. This is a delayed restore from a previous call.
 
> > __asm(
> > 0x4000883c:     be,l 100(sr2,r0),%sr0,%r31
> > 0x40008840:     ldi 2,r20                    !! FORK !!
> > );
> > 0x40008844:     ldi -1000,r20 <--Corrupted--- r19 = 0x10106368
> 
> There is no save and restore around the syscall as GCC doesn't know
> the asm does a call.  Clobbering r19 isn't an option in PIC code.
> Thus, either the syscall has to save and restore r19, or the kernel
> has to avoid clobbering r19.

I would vote for having the kernel avoid trashing r19, though that means
that the kernel has to do the stw/ldw, and wouldn't it be more
beneficial to do this in userspace and allow the kernel an extra
register to generate code with?
 
> You should be able to use a register.  The problem is how to do this
> in a nice way so that you don't have to save and restore r19 in non-pic
> code.

Glibc tells me if I'm in PIC code by setting -DPIC on the command line.
I'm planning on using "ifdef PIC" to optimize away the save and restore.
The issue really is that most programs are dynamically linked against
libc, and thus all their syscalls have to save and restore r19.
 
> Ouch, I am surprised that this hasn't had a bigger effect.

Me too. I really see this as a "We should have been doing this anyway"
and we aren't going to get away from this.

c.

[parisc-linux] Re: [glibc] tststatic failues, reduced to simple testcase.

[Carlos O'Donell]

On Fri, Aug 29, 2003 at 06:44:03PM -0400, John David Anglin wrote:
> > > __asm(
> > > 0x4000883c:     be,l 100(sr2,r0),%sr0,%r31
> > > 0x40008840:     ldi 2,r20                    !! FORK !!
> > > );
> > > 0x40008844:     ldi -1000,r20 <--Corrupted--- r19 = 0x10106368
> 
> Looking at the kernel syscall code, it seems at first glance that
> r19 is saved and restored.  Thus, the problem may be specific to fork.

The first place I went to was syscall.S and entry.S to see if r19 was
saved and restored. It is infact saved and restored, _but_ there seems
to be a case in the sys_fork_wrapper where r19 is written back as a temp
slot (PT_XX struct).

linux-2.4/arch/parisc/kernel/entry.S

   2004         /* These are call-clobbered registers and therefore
   2005            also syscall-clobbered (we hope). */
   2006         STREG   %r2,PT_GR19(%r1)        /* save for child */
   2007         STREG   %r30,PT_GR21(%r1)

This is done just before the call to 'sys_clone', but it's never used
anywhere. The comment indicates that the author believed he had all
right to use caller saves registers, and they should.

Aflicted: sys_fork_wrapper, sys_clone_wrapper, sys_vfork_wrapper

I'm tempted to remove the store and load of call-clobbered registers
from our syscall path, push them into the glibc wrappers, and see what 
happens :)

c.

[parisc-linux] Re: [glibc] tststatic failues, reduced to simple testcase.

[John David Anglin]

> On Fri, Aug 29, 2003 at 06:44:03PM -0400, John David Anglin wrote:
> > > > __asm(
> > > > 0x4000883c:     be,l 100(sr2,r0),%sr0,%r31
> > > > 0x40008840:     ldi 2,r20                    !! FORK !!
> > > > );
> > > > 0x40008844:     ldi -1000,r20 <--Corrupted--- r19 = 0x10106368
> > 
> > Looking at the kernel syscall code, it seems at first glance that
> > r19 is saved and restored.  Thus, the problem may be specific to fork.
> 
> The first place I went to was syscall.S and entry.S to see if r19 was
> saved and restored. It is infact saved and restored, _but_ there seems
> to be a case in the sys_fork_wrapper where r19 is written back as a temp
> slot (PT_XX struct).
> 
> linux-2.4/arch/parisc/kernel/entry.S
> 
>    2004         /* These are call-clobbered registers and therefore
>    2005            also syscall-clobbered (we hope). */
>    2006         STREG   %r2,PT_GR19(%r1)        /* save for child */
>    2007         STREG   %r30,PT_GR21(%r1)
> 
> This is done just before the call to 'sys_clone', but it's never used
> anywhere. The comment indicates that the author believed he had all
> right to use caller saves registers, and they should.

I don't fully understand this code but possibly PT_GR20 might be used
to save r2.  In the fork call for example, we know that this location
contains  __NR_fork.  This value gets restored to r20 in wrapper_exit.
I think the valued saved above is loaded into r2 here:

child_return:
	bl      schedule_tail, %r2
	nop

	LDREG   TASK_PT_GR19-TASK_SZ_ALGN-FRAME_SIZE-FRAME_SIZE(%r30),%r2
	b       wrapper_exit
	copy    %r0,%r28

I don't see where the %r30 value saved in PT_GR21 is used.

If a syscall is going to clobber registers, the appropriate clobbers
need to be added to the asm used for the syscall so that gcc doesn't
try to use these registers over the syscall.  Use of PT_GR19 appears
to have been a bad choice because of its special use in pic code.

Dave

Re: [parisc-linux] Re: [glibc] tststatic failues, reduced to simple testcase.

[Carlos O'Donell]

> 
> Aflicted: sys_fork_wrapper, sys_clone_wrapper, sys_vfork_wrapper
> 
> I'm tempted to remove the store and load of call-clobbered registers
> from our syscall path, push them into the glibc wrappers, and see what 
> happens :)

What happens when your stack changes on the route back from the
syscall?

	stw r19, -32(sp)
	/* clone */
	ldw -32(sp), r19

Obviously I could add a "if parent then ldw -32(sp),r19", but the child,
not having the same stack would be hard pressed if r19 changed during
the syscall. Although, I think I see that in glibc the child's function
is called via $$dyncall and I assume that might fixup r19 for the child.

Do any other syscalls change the stack on return? I can only really
think of all the fork-ish type syscalls doing that sort of stuff.

c.

Re: [parisc-linux] Re: [glibc] tststatic failues, reduced to simple testcase.

[Carlos O'Donell]

> I don't fully understand this code but possibly PT_GR20 might be used
> to save r2.  In the fork call for example, we know that this location
> contains  __NR_fork.  This value gets restored to r20 in wrapper_exit.
> I think the valued saved above is loaded into r2 here:

So you say perhaps instead of PT_GR19 we use PT_GR20 and
TASK_PT_GR20? I think we can't though, because tracing the syscall
requires that GR20 continue to retain the syscall value. I think we can
use _any_ other caller-saves registers (including the last input for the
fork-ish calls e.g. GR21 or GR22?).
 
> child_return:
> 	bl      schedule_tail, %r2
> 	nop
> 
> 	LDREG   TASK_PT_GR19-TASK_SZ_ALGN-FRAME_SIZE-FRAME_SIZE(%r30),%r2
> 	b       wrapper_exit
> 	copy    %r0,%r28
> 
> I don't see where the %r30 value saved in PT_GR21 is used.

Me neither. Perhaps it's superfluous and can be used to retrieve r2 and
thus keep r19 safe.

> If a syscall is going to clobber registers, the appropriate clobbers
> need to be added to the asm used for the syscall so that gcc doesn't
> try to use these registers over the syscall.  Use of PT_GR19 appears
> to have been a bad choice because of its special use in pic code.

I've added all the caller-saves registers to our clobber lists when
making syscalls. Though, r19 being special, I've had to add "STW_PIC"
and "LDW_PIC" that do the saving and restore _only_ if we are compiled
PIC.

c.

Re: [parisc-linux] Re: [glibc] tststatic failues, reduced to simp le testcase.

[John David Anglin]

> > I don't fully understand this code but possibly PT_GR20 might be used
> > to save r2.  In the fork call for example, we know that this location
> > contains  __NR_fork.  This value gets restored to r20 in wrapper_exit.
> > I think the valued saved above is loaded into r2 here:
> 
> So you say perhaps instead of PT_GR19 we use PT_GR20 and
> TASK_PT_GR20? I think we can't though, because tracing the syscall
> requires that GR20 continue to retain the syscall value. I think we can
> use _any_ other caller-saves registers (including the last input for the
> fork-ish calls e.g. GR21 or GR22?).

Isn't the restoration of the value in wrapper_exit enough or is strace
looking in the task struct?

> Me neither. Perhaps it's superfluous and can be used to retrieve r2 and
> thus keep r19 safe.

It's worth a try.

> > If a syscall is going to clobber registers, the appropriate clobbers
> > need to be added to the asm used for the syscall so that gcc doesn't
> > try to use these registers over the syscall.  Use of PT_GR19 appears
> > to have been a bad choice because of its special use in pic code.
> 
> I've added all the caller-saves registers to our clobber lists when
> making syscalls. Though, r19 being special, I've had to add "STW_PIC"
> and "LDW_PIC" that do the saving and restore _only_ if we are compiled
> PIC.

That seems overly pessimistic and will mean a lot more register saves
in routines that do syscalls.  It appears the system saves the caller-save
registers in all syscalls except the fork related calls.  It would
be best to keep the number of clobbers to a minimum.  I wonder, there
seems to be a slot in the task struct for r1, but it's never saved in
a syscall, and r1 is clobbered.  Maybe the r1 slot could be used to save
r2.  This might mean that we don't need to use GR21 or GR22 at all.
It would avoid having to have special pic code.

Dave