* [parisc-linux] hppa glibc clone() patch proposal (3/3)
@ 2006-03-24 10:18 Joel Soete
2006-03-24 16:51 ` Grant Grundler
[not found] ` <119aab440603241148k755eb8a1m10982b0538fc2cf3@mail.gmail.com>
0 siblings, 2 replies; 8+ messages in thread
From: Joel Soete @ 2006-03-24 10:18 UTC (permalink / raw)
To: Kyle McMartin, Parisc List, carlos, Randolph Chung
Hello Mike, pa*,
And this last step is my actual fix proposal:
--- glibc-2.3.6/sysdeps/unix/sysv/linux/hppa/clone.S.orig2 2006-03-2=
4
10:11:27.000000000 +0100
+++ glibc-2.3.6/sysdeps/unix/sysv/linux/hppa/clone.S 2006-03-24
10:13:12.000000000 +0100
@@ -45,6 +45,13 @@
.text
ENTRY(__clone)
+ /* this proto SAVE_RP: stw rp, -20(sp) */
+
+ /* Sanity check arguments. */
+ comib,=3D 0, %arg0, .Larg_error /* no NULL function poi=
nters */
+ ldi -EINVAL, %ret0
+ comib,=3D 0, %arg1, .Larg_error /* no NULL stack pointe=
rs */
+ nop
/* Save the fn ptr and arg on the new stack. */
stwm %arg0, 64(%arg1)
@@ -60,8 +67,8 @@
/* Create frame for function */
copy %sp, %r21
+ stw %r21, 60(%sp)
stwm %r3, 64(%sp)
- stw %r21, -4(%sp)
/* Save the PIC register. */
#ifdef PIC
@@ -94,13 +101,35 @@
#endif
/* Set errno */
copy %ret0, %r3
- b __syscall_error
+ bl __syscall_error, %rp
sub %r0, %ret0, %arg0
copy %r3, %ret0
/* Return after setting errno */
+ /* Restore rp from ENTRY() */
+ ldw -84(%sp), %rp
bv %r0(%rp)
ldwm -64(%sp), %r3
+.Larg_error:
+
+ /* Save arg0: the fn ptr. */
+ stw %arg0, -36(%sp)
+ /* Save the PIC register? */
+ stwm %r3, 64(%sp)
+
+ /* Set errno */
+ copy %ret0, %r3
+ bl __syscall_error, %rp
+ sub %r0, %ret0, %arg0
+
+ /* Restore arg0: the fn ptr. */
+ ldw -100(%sp), %arg0
+ /* Return after setting errno */
+ /* Restore rp from ENTRY() */
+ ldw -84(%sp), %rp
+ bv %r0(%rp)
+ ldwm -64(%sp), %r3
+
thread_start:
/* Load up the arguments. */
=3D=3D=3D=3D<>=3D=3D=3D=3D
Description:
1/ first hunk resurect the 'arg sanity check'
2/ 2d is just of my taste ;-)
3/ branching change was suggested to me by subsequent code added by C=
.
4/ I did find yet a elegant way to merge the 2 .Lerror_ hunk
That said that bring me more questions then answer:
1/ what's the role of this mistery r3? (comment in last hunk)
2/ can I get rid of save/redtore of arg0?
Thanks for your attention,
Joel
PS1: clone04 test on a chroot disk works fine:
# ./clone04
clone04 1 PASS : expected failure; Got EINVAL
PS2: already build 6 time kernel in a loop with 2.6.16-pa4 64bit on a b2k=
PS3: no regression versus debian stock pkg 2.3.6-4 :-)
grep Err Glibc-2.6.3-4.builld.debian.org.txt
make[3]: ***
[/build/buildd/glibc-2.3.6/build-tree/hppa-libc/localedata/sort-test.out]=
Error 1
make[2]: *** [localedata/tests] Error 2
make[3]: ***
[/build/buildd/glibc-2.3.6/build-tree/hppa-libc/math/test-float.out] Erro=
r 1
make[3]: ***
[/build/buildd/glibc-2.3.6/build-tree/hppa-libc/math/test-double.out] Err=
or 1
make[3]: ***
[/build/buildd/glibc-2.3.6/build-tree/hppa-libc/math/test-idouble.out] Er=
ror 1
make[2]: *** [math/tests] Error 2
make[3]: [/build/buildd/glibc-2.3.6/build-tree/hppa-libc/posix/annexc.out=
]
Error 1 (ignored)
make[3]: ***
[/build/buildd/glibc-2.3.6/build-tree/hppa-libc/posix/tst-rxspencer.out] =
Error 139
make[2]: *** [posix/tests] Error 2
make[3]: ***
[/build/buildd/glibc-2.3.6/build-tree/hppa-libc/linuxthreads/tst-attr1.ou=
t]
Error 1
make[2]: *** [linuxthreads/tests] Error 2
make[3]: ***
[/build/buildd/glibc-2.3.6/build-tree/hppa-libc/elf/tst-tls13.out] Error =
1
make[2]: *** [elf/tests] Error 2
make[1]: *** [check] Error 2
My build:
# tail -f glibc-2.3.6-4.1 | grep Err
make[3]: ***
[/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/locale=
data/sort-test.out]
Error 1
make[2]: *** [localedata/tests] Error 2
make[3]: ***
[/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/math/t=
est-float.out]
Error 1
make[3]: ***
[/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/math/t=
est-double.out]
Error 1
make[3]: ***
[/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/math/t=
est-idouble.out]
Error 1
make[2]: *** [math/tests] Error 2
make[3]: ***
[/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/posix/=
tst-rxspencer.out]
Error 139
make[3]:
[/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/posix/=
annexc.out]
Error 1 (ignored)
make[2]: *** [posix/tests] Error 2
make[3]: ***
[/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/linuxt=
hreads/tst-attr1.out]
Error 1
make[2]: *** [linuxthreads/tests] Error 2
make[3]: ***
[/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/elf/ts=
t-tls13.out]
Error 1
make[2]: *** [elf/tests] Error 2
make[1]: *** [check] Error 2
=0A=0A-------------------------------------------------------=0ANOTE! My =
email address is changing to ... @scarlet.be=0APlease make the necessary =
changes in your address book. =0A=0A
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [parisc-linux] hppa glibc clone() patch proposal (3/3)
2006-03-24 10:18 [parisc-linux] hppa glibc clone() patch proposal (3/3) Joel Soete
@ 2006-03-24 16:51 ` Grant Grundler
[not found] ` <119aab440603241148k755eb8a1m10982b0538fc2cf3@mail.gmail.com>
1 sibling, 0 replies; 8+ messages in thread
From: Grant Grundler @ 2006-03-24 16:51 UTC (permalink / raw)
To: Joel Soete; +Cc: Parisc List, Kyle McMartin
On Fri, Mar 24, 2006 at 11:18:25AM +0100, Joel Soete wrote:
> Hello Mike, pa*,
>
> And this last step is my actual fix proposal:
...
> ENTRY(__clone)
> + /* Sanity check arguments. */
> + comib,= 0, %arg0, .Larg_error /* no NULL function pointers */
> + ldi -EINVAL, %ret0
> + comib,= 0, %arg1, .Larg_error /* no NULL stack pointers */
> + nop
If this is correct, then I would prefer:
/* catch NULL stack or function ptrs */
xor %arg1, %arg0, %ret0
comib,=,n 0, %ret0, .Larg_error
ldi -EINVAL, %ret0
>
> /* Save the fn ptr and arg on the new stack. */
> stwm %arg0, 64(%arg1)
> @@ -60,8 +67,8 @@
>
> /* Create frame for function */
> copy %sp, %r21
> + stw %r21, 60(%sp)
Is %r21 used later again?
If not, then these two ops can become:
stw %sp, 60(%sp)
If yes, then add the copy() *after* the stw.
PA-RISC can only do one store at a time.
PA20 can do the copy in parallel with the store.
hth,
grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
^ permalink raw reply [flat|nested] 8+ messages in thread
* [parisc-linux] Fwd: hppa glibc clone() patch proposal (3/3)
[not found] ` <119aab440603241148k755eb8a1m10982b0538fc2cf3@mail.gmail.com>
@ 2006-03-24 19:50 ` Carlos O'Donell
[not found] ` <442467DF.8060309@tiscali.be>
0 siblings, 1 reply; 8+ messages in thread
From: Carlos O'Donell @ 2006-03-24 19:50 UTC (permalink / raw)
To: parisc-linux
> /* Create frame for function */
> copy %sp, %r21
> + stw %r21, 60(%sp)
> stwm %r3, 64(%sp)
> - stw %r21, -4(%sp)
Bug.
c.
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [parisc-linux] Fwd: hppa glibc clone() patch proposal (3/3)
[not found] ` <119aab440603251126s3a599718n32759d5ed0dcd8c0@mail.gmail.com>
@ 2006-03-26 18:00 ` Joel Soete
2006-03-27 1:33 ` Randolph Chung
0 siblings, 1 reply; 8+ messages in thread
From: Joel Soete @ 2006-03-26 18:00 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: parisc-linux
Carlos O'Donell wrote:
>>PS: when I will revert this hunk, do you think reasonable that I send the patch to Aurelien to insert it in debian pkg?
>
>
> No, post again for review. The whole patch please.
>
No pb here it is:
--- glibc-2.3.6/sysdeps/unix/sysv/linux/hppa/clone.S.orig2 2006-03-24 09:11:27.000000000 +0000
+++ glibc-2.3.6/sysdeps/unix/sysv/linux/hppa/clone.S 2006-03-26 10:20:13.000000000 +0000
@@ -45,6 +45,14 @@
.text
ENTRY(__clone)
+ /* this proto do: stw rp, -20(sp) */
+ /* FIXME: I have no idea how profiling works on hppa. */
+
+ /* Sanity check arguments. */
+ comib,= 0, %arg0, .Larg_error /* no NULL function pointers */
+ ldi -EINVAL, %ret0
+ comib,= 0, %arg1, .Larg_error /* no NULL stack pointers */
+ nop
/* Save the fn ptr and arg on the new stack. */
stwm %arg0, 64(%arg1)
@@ -94,13 +102,35 @@
#endif
/* Set errno */
copy %ret0, %r3
- b __syscall_error
+ bl __syscall_error, %rp
sub %r0, %ret0, %arg0
copy %r3, %ret0
/* Return after setting errno */
+ /* Restore rp from ENTRY() */
+ ldw -84(%sp), %rp
bv %r0(%rp)
ldwm -64(%sp), %r3
+.Larg_error:
+
+ /* Save arg0: the fn ptr. */
+ stw %arg0, -36(%sp)
+ /* Save the PIC register? */
+ stwm %r3, 64(%sp)
+
+ /* Set errno */
+ copy %ret0, %r3
+ bl __syscall_error, %rp
+ sub %r0, %ret0, %arg0
+
+ /* Restore arg0: the fn ptr. */
+ ldw -100(%sp), %arg0
+ /* Return after setting errno */
+ /* Restore rp from ENTRY() */
+ ldw -84(%sp), %rp
+ bv %r0(%rp)
+ ldwm -64(%sp), %r3
+
thread_start:
/* Load up the arguments. */
===<>====
clone04 still works fine ;-)
mmm still have many questions (for so few insn):
1/ what's the role of this mistery r3? (see comment)
2/ is it the right palce to save 'stw %arg0, -36(%sp)' (I grab it from some other of your src)?
3/ though, may be finaly better 'stw %arg0, -100(%sp)' after 'stwm %r3, 64(%sp)'?
3/ can I get rid of save/restore of arg0?
Thanks again for your attention,
Joel
PS: I still got the same 'Error' as buildd:
# tail -f glibc-2.3.6-4.2 | grep Err
make[3]: *** [/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/localedata/sort-test.out] Error 1
make[2]: *** [localedata/tests] Error 2
make[3]: *** [/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/math/test-float.out] Error 1
make[3]: *** [/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/math/test-double.out] Error 1
make[3]: *** [/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/math/test-idouble.out] Error 1
make[2]: *** [math/tests] Error 2
make[3]: *** [/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/posix/tst-rxspencer.out] Error 139
make[3]: [/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/posix/annexc.out] Error 1 (ignored)
make[2]: *** [posix/tests] Error 2
make[3]: *** [/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/linuxthreads/tst-attr1.out] Error 1
make[2]: *** [linuxthreads/tests] Error 2
make[3]: *** [/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/rt/tst-timer.out] Error 137
make[3]: *** [/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/rt/tst-aio4.out] Error 1
make[3]: *** [/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/rt/tst-timer4.out] Error 1
make[2]: *** [rt/tests] Error 2
make[3]: *** [/CAD/parisc-linux/Dpkg/dpkg-work/glibc-2.3.6/build-tree/hppa-libc/elf/tst-tls13.out] Error 1
make[2]: *** [elf/tests] Error 2
make[1]: *** [check] Error 2
So it doesn't seems to introduce regressions?
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [parisc-linux] Fwd: hppa glibc clone() patch proposal (3/3)
2006-03-26 18:00 ` Joel Soete
@ 2006-03-27 1:33 ` Randolph Chung
0 siblings, 0 replies; 8+ messages in thread
From: Randolph Chung @ 2006-03-27 1:33 UTC (permalink / raw)
To: Joel Soete; +Cc: parisc-linux
Joel,
Some comments:
1) I think you are right in fixing the first __syscall_error call. I
don't know why we are doing a b instead of bl now. It seems to be that
the current code will result in an orphaned stack frame if we ever
errored out. However, I see that in the other syscalls, the call to
__syscall_error is inlined. Maybe that would be better.....
2)
> .text
> ENTRY(__clone)
> + /* this proto do: stw rp, -20(sp) */
Yes, you should put "stw %rp, -20(%sp)" here.
> + /* FIXME: I have no idea how profiling works on hppa. */
Let's leave profiling out of this for now....
> + /* Sanity check arguments. */
> + comib,= 0, %arg0, .Larg_error /* no NULL function
> pointers */
> + ldi -EINVAL, %ret0
> + comib,= 0, %arg1, .Larg_error /* no NULL stack
> pointers */
> + nop
There seems to be some confusion here....
You want clone() to return -1, and set errno = -EINVAL. The logic to do
this is in .Larg_error. Probably better here to just detect the two
error conditions and branch to .Larg_error and handle all the logic there.
> /* Save the fn ptr and arg on the new stack. */
> stwm %arg0, 64(%arg1)
> @@ -94,13 +102,35 @@
> #endif
> /* Set errno */
> copy %ret0, %r3
> - b __syscall_error
> + bl __syscall_error, %rp
> sub %r0, %ret0, %arg0
> copy %r3, %ret0
> /* Return after setting errno */
> + /* Restore rp from ENTRY() */
> + ldw -84(%sp), %rp
Right now since you don't store rp, this will give you junk.
> +.Larg_error:
> +
> + /* Save arg0: the fn ptr. */
> + stw %arg0, -36(%sp)
In the error case, you don't invoke the function, so no need to save and
restore arg0.
> + /* Save the PIC register? */
> + stwm %r3, 64(%sp)
r3 is just a caller-save register; it has nothing to do with PIC. You
don't need to use r3 in this path; just create a stack frame with:
ldo 64(%sp), %sp
> + /* Set errno */
> + copy %ret0, %r3
> + bl __syscall_error, %rp
> + sub %r0, %ret0, %arg0
Here, I think you want to do (or inline the __syscall_error logic):
bl __syscall_error, %rp
ldi EINVAL, %arg0
ldi -1, %ret0
> + /* Restore arg0: the fn ptr. */
> + ldw -100(%sp), %arg0
> + /* Return after setting errno */
> + /* Restore rp from ENTRY() */
> + ldw -84(%sp), %rp
> + bv %r0(%rp)
> + ldwm -64(%sp), %r3
and here you can just do:
ldw -84(%sp), %rp
bv %r0(%rp)
ldo -64(%sp), %sp
randolph
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [parisc-linux] Fwd: hppa glibc clone() patch proposal (3/3)
@ 2006-03-27 16:59 Joel Soete
2006-03-27 19:13 ` Carlos O'Donell
0 siblings, 1 reply; 8+ messages in thread
From: Joel Soete @ 2006-03-27 16:59 UTC (permalink / raw)
To: soete.joel; +Cc: parisc-linux, tsg45800
> > Joel,
> >
> > Some comments:
> >
> > 1) I think you are right in fixing the first __syscall_error call. I
> > don't know why we are doing a b instead of bl now. It seems to be tha=
t
> > the current code will result in an orphaned stack frame if we ever
> > errored out. However, I see that in the other syscalls, the call to
> > __syscall_error is inlined. Maybe that would be better.....
> >
> I just lake of knowledge to do it in asm but if you show me another exa=
mple, I
> could try it ;-)
>
> > 2)
> > > .text
> > > ENTRY(__clone)
> > > + /* this proto do: stw rp, -20(sp) */
> >
> > Yes, you should put "stw %rp, -20(%sp)" here.
> >
> well my comment seems to be bad, I would mean:
> /* this Entry() macro do a: stw rp, -20(sp) */
>
> sysdeps/unix/sysv/linux/hppa/sysdep.h
> [snip]
> /* Define an entry point visible from C.
>
> There is currently a bug in gdb which prevents us from specifying
> incomplete stabs information. Fake some entries here which specify
> the current source file. */
> #define ENTRY(name) =
\
> .text ASM_LINE_SEP =
\
> .align ALIGNARG(4) ASM_LINE_SEP =
\
> .export C_SYMBOL_NAME(name) ASM_LINE_SEP =
\
> .type C_SYMBOL_NAME(name),@function ASM_LINE_SEP =
\
> C_LABEL(name) ASM_LINE_SEP =
\
> .PROC ASM_LINE_SEP =
\
> .CALLINFO FRAME=3D64,CALLS,SAVE_RP,ENTRY_GR=3D3 ASM_LINE_SE=
P \
> .ENTRY ASM_LINE_SEP =
\
> /* SAVE_RP says we do */ ASM_LINE_SEP =
\
> stw %rp, -20(%sr0, %sp) ASM_LINE_SEP =
\
> /*FIXME: Call mcount? (carefull with stack!) */
> [snip]
>
> > > + /* FIXME: I have no idea how profiling works on hppa. */
> >
> > Let's leave profiling out of this for now....
> >
> > > + /* Sanity check arguments. */
> > > + comib,=3D 0, %arg0, .Larg_error /* no NULL functi=
on
> > > pointers */
> > > + ldi -EINVAL, %ret0
> > > + comib,=3D 0, %arg1, .Larg_error /* no NULL stack
> > > pointers */
> > > + nop
> >
> > There seems to be some confusion here....
> >
> > You want clone() to return -1, and set errno =3D -EINVAL. The logic t=
o do
> > this is in .Larg_error. Probably better here to just detect the two
> > error conditions and branch to .Larg_error and handle all the logic t=
here.
> >
> Totaly agree, as a first attempt I wonder based that on just reverting =
a
> previous hunk of patch, I will make much effort ;-)
>
> > > /* Save the fn ptr and arg on the new stack. */
> > > stwm %arg0, 64(%arg1)
> > > @@ -94,13 +102,35 @@
> > > #endif
> > > /* Set errno */
> > > copy %ret0, %r3
> > > - b __syscall_error
> > > + bl __syscall_error, %rp
> > > sub %r0, %ret0, %arg0
> > > copy %r3, %ret0
> > > /* Return after setting errno */
> > > + /* Restore rp from ENTRY() */
> > > + ldw -84(%sp), %rp
> >
> > Right now since you don't store rp, this will give you junk.
> >
> > > +.Larg_error:
> > > +
> > > + /* Save arg0: the fn ptr. */
> > > + stw %arg0, -36(%sp)
> >
> > In the error case, you don't invoke the function, so no need to save =
and
> > restore arg0.
> >
> Thanks, I would be sure that gcc will not rely on the availability of t=
his var
> in r26 after the function call.
>
> > > + /* Save the PIC register? */
> > > + stwm %r3, 64(%sp)
> >
> > r3 is just a caller-save register; it has nothing to do with PIC. You=
> > don't need to use r3 in this path; just create a stack frame with:
> > ldo 64(%sp), %sp
> >
> Yes, I just read jda comment about another thread: PIC register is actu=
aly r19.
>
> > > + /* Set errno */
> > > + copy %ret0, %r3
> > > + bl __syscall_error, %rp
> > > + sub %r0, %ret0, %arg0
> >
> > Here, I think you want to do (or inline the __syscall_error logic):
> > bl __syscall_error, %rp
> > ldi EINVAL, %arg0
> > ldi -1, %ret0
> >
> > > + /* Restore arg0: the fn ptr. */
> > > + ldw -100(%sp), %arg0
> > > + /* Return after setting errno */
> > > + /* Restore rp from ENTRY() */
> > > + ldw -84(%sp), %rp
> > > + bv %r0(%rp)
> > > + ldwm -64(%sp), %r3
> >
> > and here you can just do:
> > ldw -84(%sp), %rp
> > bv %r0(%rp)
> > ldo -64(%sp), %sp
> >
> Cool.
>
> Many thanks for all your accurate advises,
> Joel
>
mmm, a bit busy in prod (unfortunately 2 mirrored boot disk was corrupted=
->
hope that ignite tape would help)
please review this next attempt (not yet tested):
# diff -Nau clone.S.orig2 clone.S.New2
--- clone.S.orig2 2006-03-24 10:11:27.000000000 +0100
+++ clone.S.New2 2006-03-27 18:34:54.000000000 +0200
@@ -45,7 +45,27 @@
.text
ENTRY(__clone)
+ /* this ENTRY() macro do: stw rp, -20(sp) */
+ /* Sanity check arguments. */
+ and %arg0, %arg1, %r20 /* no NULL function point=
ers */
+ comib,<>,n 0, %r20, .Lno_arg_error
+
+ /* Create a stack frame */
+ ldo 64(%sp), %sp
+
+ /* Set errno */
+ bl __syscall_error, %rp
+ ldi EINVAL, %arg0
+/* ldi -1, %ret0 */
+
+ /* Return after setting errno */
+ /* Restore rp from ENTRY() */
+ ldw -84(%sp), %rp
+ bv %r0(%rp)
+ ldo -64(%sp), %sp
+
+.Lno_arg_error:
/* Save the fn ptr and arg on the new stack. */
stwm %arg0, 64(%arg1)
stw %arg3, -60(%arg1)
@@ -94,10 +114,12 @@
#endif
/* Set errno */
copy %ret0, %r3
- b __syscall_error
+ bl __syscall_error, %rp
sub %r0, %ret0, %arg0
copy %r3, %ret0
/* Return after setting errno */
+ /* Restore rp from ENTRY() */
+ ldw -84(%sp), %rp
bv %r0(%rp)
ldwm -64(%sp), %r3
=3D=3D=3D=3D<>=3D=3D=3D=3D
Some comments:
1/ I just comment out "ldi -1, %ret0" because imho our __syscall_error(=
) did
it already:
sysdeps/unix/sysv/linux/hppa/sysdep.c:
[snip]
int
__syscall_error (int err_no)
{
__set_errno (err_no);
return -1;
}
[snip]
2/ hope that r20 is a good choice?
Thanks again,
Joel=0A=0A-----------------------------------------------------------=
----=0AA free anti-spam and anti-virus filter on all Scarlet mailboxes=0A=
More info on http://www.scarlet.be/
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [parisc-linux] Fwd: hppa glibc clone() patch proposal (3/3)
2006-03-27 16:59 Joel Soete
@ 2006-03-27 19:13 ` Carlos O'Donell
0 siblings, 0 replies; 8+ messages in thread
From: Carlos O'Donell @ 2006-03-27 19:13 UTC (permalink / raw)
To: Joel Soete; +Cc: parisc-linux, tsg45800
> please review this next attempt (not yet tested):
What source tree is this patch against?
> 2/ hope that r20 is a good choice?
As long as this is a caller-saves register then it doesn't matter.
Cheers,
Carlos.
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [parisc-linux] Fwd: hppa glibc clone() patch proposal (3/3)
@ 2006-03-30 15:08 Joel Soete
0 siblings, 0 replies; 8+ messages in thread
From: Joel Soete @ 2006-03-30 15:08 UTC (permalink / raw)
To: randolph; +Cc: parisc-linux, tsg45800
Hello Randolph, Carlos,
> Joel,
>
> Some comments:
>
> 1) I think you are right in fixing the first __syscall_error call. I
> don't know why we are doing a b instead of bl now. It seems to be that
> the current code will result in an orphaned stack frame if we ever
> errored out. However, I see that in the other syscalls, the call to
> __syscall_error is inlined. Maybe that would be better.....
>
> 2)
> > .text
> > ENTRY(__clone)
> > + /* this proto do: stw rp, -20(sp) */
>
> Yes, you should put "stw %rp, -20(%sp)" here.
>
> > + /* FIXME: I have no idea how profiling works on hppa. */
>
> Let's leave profiling out of this for now....
>
> > + /* Sanity check arguments. */
> > + comib,=3D 0, %arg0, .Larg_error /* no NULL function=
> > pointers */
> > + ldi -EINVAL, %ret0
> > + comib,=3D 0, %arg1, .Larg_error /* no NULL stack
> > pointers */
> > + nop
>
> There seems to be some confusion here....
>
> You want clone() to return -1, and set errno =3D -EINVAL. The logic to =
do
> this is in .Larg_error. Probably better here to just detect the two
> error conditions and branch to .Larg_error and handle all the logic the=
re.
>
> > /* Save the fn ptr and arg on the new stack. */
> > stwm %arg0, 64(%arg1)
> > @@ -94,13 +102,35 @@
> > #endif
> > /* Set errno */
> > copy %ret0, %r3
> > - b __syscall_error
> > + bl __syscall_error, %rp
> > sub %r0, %ret0, %arg0
> > copy %r3, %ret0
> > /* Return after setting errno */
> > + /* Restore rp from ENTRY() */
> > + ldw -84(%sp), %rp
>
> Right now since you don't store rp, this will give you junk.
>
> > +.Larg_error:
> > +
> > + /* Save arg0: the fn ptr. */
> > + stw %arg0, -36(%sp)
>
> In the error case, you don't invoke the function, so no need to save an=
d
> restore arg0.
>
> > + /* Save the PIC register? */
> > + stwm %r3, 64(%sp)
>
> r3 is just a caller-save register; it has nothing to do with PIC. You
> don't need to use r3 in this path; just create a stack frame with:
> ldo 64(%sp), %sp
>
So excepted this above stuff (for which I didn't yet understand the pb) a=
nd
inlining __syscall_error() (not yet enough knowledge to do), ...
> > + /* Set errno */
> > + copy %ret0, %r3
> > + bl __syscall_error, %rp
> > + sub %r0, %ret0, %arg0
>
> Here, I think you want to do (or inline the __syscall_error logic):
> bl __syscall_error, %rp
> ldi EINVAL, %arg0
> ldi -1, %ret0
>
> > + /* Restore arg0: the fn ptr. */
> > + ldw -100(%sp), %arg0
> > + /* Return after setting errno */
> > + /* Restore rp from ENTRY() */
> > + ldw -84(%sp), %rp
> > + bv %r0(%rp)
> > + ldwm -64(%sp), %r3
>
> and here you can just do:
> ldw -84(%sp), %rp
> bv %r0(%rp)
> ldo -64(%sp), %sp
>
here is the last proposal:
--- glibc-2.3.6/sysdeps/unix/sysv/linux/hppa/clone.S.orig2 2006-03-2=
4
10:11:27.000000000 +0100
+++ glibc-2.3.6/sysdeps/unix/sysv/linux/hppa/clone.S 2006-03-30
13:56:56.000000000 +0200
@@ -45,7 +45,30 @@
.text
ENTRY(__clone)
+ /* this ENTRY() macro do: stw rp, -20(sp)
+ and "FIXME: I have no idea how profiling works on hppa." */
+ /* Sanity check arguments. */
+ comib,=3D 0, %arg0, .Larg_error /* no NULL func=
tion
pointers */
+ nop
+ comib,<>,n 0, %arg1, .Lno_arg_error /* no NULL stack
pointers */
+ nop
+
+.Larg_error:
+ /* Create a stack frame */
+ stwm %r3, 64(%sp)
+ /* Set errno */
+ bl __syscall_error, %rp
+ ldi EINVAL, %arg0
+/* ldi -1, %ret0 */
+
+ /* Return after setting errno */
+ /* Restore rp from ENTRY() */
+ ldw -84(%sp), %rp
+ bv %r0(%rp)
+ ldwm -64(%sp), %r3
+
+.Lno_arg_error:
/* Save the fn ptr and arg on the new stack. */
stwm %arg0, 64(%arg1)
stw %arg3, -60(%arg1)
@@ -94,10 +117,12 @@
#endif
/* Set errno */
copy %ret0, %r3
- b __syscall_error
+ bl __syscall_error, %rp
sub %r0, %ret0, %arg0
copy %r3, %ret0
/* Return after setting errno */
+ /* Restore rp from ENTRY() */
+ ldw -84(%sp), %rp
bv %r0(%rp)
ldwm -64(%sp), %r3
=3D=3D=3D=3D<>=3D=3D=3D=3D
May be should I still put the:
+ /* Create a stack frame */
+ stwm %r3, 64(%sp)
in the delay slot of the first cmpib,=3D (on going to test ...)
Thanks,
Joel=0A=0A-------------------------------------------------------=0AN=
OTE! My email address is changing to ... @scarlet.be=0APlease make the ne=
cessary changes in your address book. =0A=0A
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2006-03-30 15:08 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-24 10:18 [parisc-linux] hppa glibc clone() patch proposal (3/3) Joel Soete
2006-03-24 16:51 ` Grant Grundler
[not found] ` <119aab440603241148k755eb8a1m10982b0538fc2cf3@mail.gmail.com>
2006-03-24 19:50 ` [parisc-linux] Fwd: " Carlos O'Donell
[not found] ` <442467DF.8060309@tiscali.be>
[not found] ` <119aab440603241504m5f1f8a97tebe4110185d5ac4c@mail.gmail.com>
[not found] ` <4424F7EE.7020002@tiscali.be>
[not found] ` <119aab440603251126s3a599718n32759d5ed0dcd8c0@mail.gmail.com>
2006-03-26 18:00 ` Joel Soete
2006-03-27 1:33 ` Randolph Chung
-- strict thread matches above, loose matches on Subject: below --
2006-03-27 16:59 Joel Soete
2006-03-27 19:13 ` Carlos O'Donell
2006-03-30 15:08 Joel Soete
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.