Re: [PATCH] arm: add RESET_PID in the clone impl
Hi,
I don't think clone on arm is missing RESET_PID if you use the correct
implementation from libpthread/nptl/sysdeps/unix/sysv/linux/arm/clone.S.
Cheers
Carmelo
Inviato con AquaMail per Android
http://www.aqua-mail.com
Il 02 settembre 2014 14:12:24 Wangyufen ha scritto:
From: Wang Yufen
Called getpid() When creating a new process with clone(), getpid() returns
the father_process's value. It should be child_process's value.
The reason is missing a RESET_PID in the arm clone impl.
Signed-off-by: Wang Yufen
---
libc/sysdeps/linux/arm/clone.S | 61 ++-
libc/sysdeps/linux/arm/sysdep.h | 45
2 files changed, 92 insertions(+), 14 deletions(-)
diff --git a/libc/sysdeps/linux/arm/clone.S b/libc/sysdeps/linux/arm/clone.S
index 03cd10e..29045ef 100644
--- a/libc/sysdeps/linux/arm/clone.S
+++ b/libc/sysdeps/linux/arm/clone.S
@@ -19,12 +19,17 @@
/* clone() is even more special than fork() as it mucks with stacks
and invokes a function in the right context after its all over. */
+#include
#define _ERRNO_H
#include
#include
#include
#include
#include
+#include
+
+#define CLONE_VM 0x0100
+#define CLONE_THREAD 0x0001
#if defined(__NR_clone)
/* int clone(int (*fn)(void *arg), void *child_stack, int flags, void *arg); */
@@ -87,6 +92,8 @@ __error:
.pool
#else
__clone:
+.fnstart
+.cantunwind
@ sanity check args
cmp r0, #0
IT(te, ne)
@@ -95,32 +102,58 @@ __clone:
beq __error
@ insert the args onto the new stack
- sub r1, r1, #8
- str r3, [r1, #4]
- @ save the function pointer as the 0th element
- str r0, [r1]
+ str r3, [r1, #-4]!
+ str r0, [r1, #-4]!
@ do the system call
@ get flags
mov r0, r2
+#ifdef RESET_PID
+ mov ip, r2
+#endif
@ new sp is already in r1
- @ load remaining arguments off the stack
- stmfd sp!, {r4}
- ldr r2, [sp, #4]
- ldr r3, [sp, #8]
- ldr r4, [sp, #12]
- DO_CALL (clone)
- movsa1, a1
- IT(t, ne)
- ldmnefd sp!, {r4}
+ push{r4, r7}
+ cfi_adjust_cfa_offset (8)
+ cfi_rel_offset (r4, 0)
+ cfi_rel_offset (r7, 4)
+ ldr r2, [sp, #8]
+ ldr r3, [sp, #12]
+ ldr r4, [sp, #16]
+ ldr r7, =SYS_ify(clone)
+ swi 0x0
+ cfi_endproc
+ cmp r0, #0
+ beq 1f
+ pop {r4, r7}
blt __error
- IT(t, ne)
#if defined(__USE_BX__)
bxnelr
#else
movne pc, lr
#endif
+ cfi_startproc
+.fnend
+PSEUDO_END (__clone)
+
+1:
+ .fnstart
+ .cantunwind
+#ifdef RESET_PID
+ tst ip, #CLONE_THREAD
+ bne 3f
+ GET_TLS (lr)
+ mov r1, r0
+ tst ip, #CLONE_VM
+ ldr r7, =SYS_ify(getpid)
+ ite ne
+ movne r0, #-1
+ swieq 0x0
+ NEGOFF_ADJ_BASE (r1, TID_OFFSET)
+ str r0, NEGOFF_OFF1 (r1, TID_OFFSET)
+ str r0, NEGOFF_OFF2 (r1, PID_OFFSET, TID_OFFSET)
+3:
+#endif
@ pick the function arg and call address off the stack and execute
ldr r0, [sp, #4]
mov lr, pc
diff --git a/libc/sysdeps/linux/arm/sysdep.h b/libc/sysdeps/linux/arm/sysdep.h
index 64f4040..38a131d 100644
--- a/libc/sysdeps/linux/arm/sysdep.h
+++ b/libc/sysdeps/linux/arm/sysdep.h
@@ -213,6 +213,51 @@ __local_syscall_error:
\
sees the right arguments.
*/
+#if __ARM_ARCH > 4 || defined (__ARM_ARCH_4T__)
+# define ARCH_HAS_BX
+#endif
+#if __ARM_ARCH > 4
+# define ARCH_HAS_BLX
+#endif
+#if __ARM_ARCH > 6 || defined (__ARM_ARCH_6K__) || defined (__ARM_ARCH_6ZK__)
+# define ARCH_HAS_HARD_TP
+#endif
+#if __ARM_ARCH > 6 || defined (__ARM_ARCH_6T2__)
+# define ARCH_HAS_T2
+#endif
+
+# ifdef __thumb2__
+# define NEGOFF_ADJ_BASE(R, OFF) add R, R, $OFF
+# define NEGOFF_ADJ_BASE2(D, S, OFF) add D, S, $OFF
+# define NEGOFF_OFF1(R, OFF) [R]
+# define NEGOFF_OFF2(R, OFFA, OFFB) [R, $((OFFA) - (OFFB))]
+# else
+# define NEGOFF_ADJ_BASE(R, OFF)
+# define NEGOFF_ADJ_BASE2(D, S, OFF) mov D, S
+# define NEGOFF_OFF1(R, OFF) [R, $OFF]
+# define NEGOFF_OFF2(R, OFFA, OFFB) [R, $OFFA]
+# endif
+
+# ifdef ARCH_HAS_HARD_TP
+/* If the cpu has cp15 available, use it. */
+# define GET_TLS(TMP) mrc p15, 0, r0, c13, c0, 3
+# else
+/* At this generic level we have no tricks to pull. Call the ABI routine. */
+# define GET_TLS(TMP) \
+ push{ r1, r2, r3, lr }; \
+ cfi_remember_state; \
+ cfi_adjust_cfa_offset (16); \
+ cfi_rel_offset (r1, 0); \
+ cfi_rel_offset (r2, 4); \
+ cfi_rel_offset (r3, 8);
Re: [PATCH] arm: add RESET_PID in the clone impl
Hi Carmelo,
Carmelo Amoroso wrote,
> Hi,
> I don't think clone on arm is missing RESET_PID if you use the
> correct implementation from
> libpthread/nptl/sysdeps/unix/sysv/linux/arm/clone.S.
Can you explain this a little bit more?
cat ./libpthread/nptl/sysdeps/unix/sysv/linux/arm/clone.S
#define RESET_PID
#include
#include "../../../../../../../libc/sysdeps/linux/arm/clone.S"
Do you think the test-suite failures on arm come from a buildsystem
problem?
best regards
Waldemar
> Cheers
> Carmelo
>
> Inviato con AquaMail per Android
> http://www.aqua-mail.com
>
>
> Il 02 settembre 2014 14:12:24 Wangyufen ha scritto:
>
> >From: Wang Yufen
> >
> >Called getpid() When creating a new process with clone(), getpid() returns
> >the father_process's value. It should be child_process's value.
> >The reason is missing a RESET_PID in the arm clone impl.
> >
> >Signed-off-by: Wang Yufen
> >---
> > libc/sysdeps/linux/arm/clone.S | 61
> > ++-
> > libc/sysdeps/linux/arm/sysdep.h | 45
> > 2 files changed, 92 insertions(+), 14 deletions(-)
> >
> >diff --git a/libc/sysdeps/linux/arm/clone.S b/libc/sysdeps/linux/arm/clone.S
> >index 03cd10e..29045ef 100644
> >--- a/libc/sysdeps/linux/arm/clone.S
> >+++ b/libc/sysdeps/linux/arm/clone.S
> >@@ -19,12 +19,17 @@
> > /* clone() is even more special than fork() as it mucks with stacks
> >and invokes a function in the right context after its all over. */
> >
> >+#include
> > #define _ERRNO_H
> > #include
> > #include
> > #include
> > #include
> > #include
> >+#include
> >+
> >+#define CLONE_VM 0x0100
> >+#define CLONE_THREAD 0x0001
> >
> > #if defined(__NR_clone)
> > /* int clone(int (*fn)(void *arg), void *child_stack, int flags, void
> > *arg); */
> >@@ -87,6 +92,8 @@ __error:
> > .pool
> > #else
> > __clone:
> >+.fnstart
> >+.cantunwind
> > @ sanity check args
> > cmp r0, #0
> > IT(te, ne)
> >@@ -95,32 +102,58 @@ __clone:
> > beq __error
> >
> > @ insert the args onto the new stack
> >-sub r1, r1, #8
> >-str r3, [r1, #4]
> >-@ save the function pointer as the 0th element
> >-str r0, [r1]
> >+str r3, [r1, #-4]!
> >+str r0, [r1, #-4]!
> >
> > @ do the system call
> > @ get flags
> > mov r0, r2
> >+#ifdef RESET_PID
> >+mov ip, r2
> >+#endif
> > @ new sp is already in r1
> >-@ load remaining arguments off the stack
> >-stmfd sp!, {r4}
> >-ldr r2, [sp, #4]
> >-ldr r3, [sp, #8]
> >-ldr r4, [sp, #12]
> >-DO_CALL (clone)
> >-movsa1, a1
> >-IT(t, ne)
> >-ldmnefd sp!, {r4}
> >+push{r4, r7}
> >+cfi_adjust_cfa_offset (8)
> >+cfi_rel_offset (r4, 0)
> >+cfi_rel_offset (r7, 4)
> >+ldr r2, [sp, #8]
> >+ldr r3, [sp, #12]
> >+ldr r4, [sp, #16]
> >+ldr r7, =SYS_ify(clone)
> >+swi 0x0
> >+cfi_endproc
> >+cmp r0, #0
> >+beq 1f
> >+pop {r4, r7}
> > blt __error
> >-IT(t, ne)
> > #if defined(__USE_BX__)
> > bxnelr
> > #else
> > movne pc, lr
> > #endif
> >
> >+cfi_startproc
> >+.fnend
> >+PSEUDO_END (__clone)
> >+
> >+1:
> >+.fnstart
> >+.cantunwind
> >+#ifdef RESET_PID
> >+tst ip, #CLONE_THREAD
> >+bne 3f
> >+GET_TLS (lr)
> >+mov r1, r0
> >+tst ip, #CLONE_VM
> >+ldr r7, =SYS_ify(getpid)
> >+ite ne
> >+movne r0, #-1
> >+swieq 0x0
> >+NEGOFF_ADJ_BASE (r1, TID_OFFSET)
> >+str r0, NEGOFF_OFF1 (r1, TID_OFFSET)
> >+str r0, NEGOFF_OFF2 (r1, PID_OFFSET, TID_OFFSET)
> >+3:
> >+#endif
> > @ pick the function arg and call address off the stack and execute
> > ldr r0, [sp, #4]
> > mov lr, pc
> >diff --git a/libc/sysdeps/linux/arm/sysdep.h
> >b/libc/sysdeps/linux/arm/sysdep.h
> >index 64f4040..38a131d 100644
> >--- a/libc/sysdeps/linux/arm/sysdep.h
> >+++ b/libc/sysdeps/linux/arm/sysdep.h
> >@@ -213,6 +213,51 @@ __local_syscall_error:
> >\
> >sees the right arguments.
> >
> > */
> >+#if __ARM_ARCH > 4 || defined (__ARM_ARCH_4T__)
> >+# define ARCH_HAS_BX
> >+#endif
> >+#if __ARM_ARCH > 4
> >+# define ARCH_HAS_BLX
> >+#endif
> >+#if __ARM_ARCH > 6 || defined (__ARM_ARCH_6K__) || defined
> >(__ARM_ARCH_6ZK__)
> >+# define ARCH_HAS_HARD_TP
> >+#endif
> >+#if __ARM_ARCH > 6 || defined (__ARM_ARCH_6T2__)
> >+# define ARCH_HAS_T2
> >+#endif
> >+
> >+# ifdef __thumb2__
> >+# define NEGOFF_ADJ_BASE(R, OFF) add R, R, $OFF
> >+# define NEGOFF_ADJ_BASE2(D, S, OFF) add D, S, $OFF
> >+# define NEGOFF_OFF1(R, OFF) [R]
> >+# define NEGOFF_OFF2(R, OFFA, OFFB)[R, $((OFFA) - (OFFB))]
> >+# else
> >+# define NEGOFF_ADJ_BASE(R, OFF)
> >+# define NEGOFF_ADJ_BASE2(D, S, OFF) mov D, S
> >+# define NEGOFF_OFF1(R, OFF) [R, $OFF]
> >+# define NEGOFF_OFF2
Re: [PATCH] arm: add RESET_PID in the clone impl
On 4 September 2014 00:01, Waldemar Brodkorb wrote: > Hi Carmelo, > Carmelo Amoroso wrote, > >> Hi, >> I don't think clone on arm is missing RESET_PID if you use the >> correct implementation from >> libpthread/nptl/sysdeps/unix/sysv/linux/arm/clone.S. > > Can you explain this a little bit more? > > cat ./libpthread/nptl/sysdeps/unix/sysv/linux/arm/clone.S > #define RESET_PID > #include > #include "../../../../../../../libc/sysdeps/linux/arm/clone.S" > > Do you think the test-suite failures on arm come from a buildsystem > problem? No, the patch is fine and needed. ARCH_HAS_BX and ARCH_HAS_BLX should be deleted though, we handle that via bits/arm_bx.h Can somebody run this through a test-harness for !threads and NPTL on the affected ISA revisions just to be sure? That'd help. TIA, ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] arm: add RESET_PID in the clone impl
Il 04 settembre 2014 00:01:38 Waldemar Brodkorb ha scritto:
Hi Carmelo,
Hello,
Carmelo Amoroso wrote,
> Hi,
> I don't think clone on arm is missing RESET_PID if you use the
> correct implementation from
> libpthread/nptl/sysdeps/unix/sysv/linux/arm/clone.S.
Can you explain this a little bit more?
What I mean to say is that if the root cause is the lack of RESET_PIN
definition it's not the case.
Then, I did not enter into the details of the patch, but at first glance I
could see several other unrelated changes that could eventually have been
split in more patches, but I could be wrong.
On my platform armv7 I don't remember to have such failures.
cat ./libpthread/nptl/sysdeps/unix/sysv/linux/arm/clone.S
#define RESET_PID
#include
#include "../../../../../../../libc/sysdeps/linux/arm/clone.S"
Do you think the test-suite failures on arm come from a buildsystem
problem?
best regards
Waldemar
Carmelo
> Cheers
> Carmelo
>
> Inviato con AquaMail per Android
> http://www.aqua-mail.com
>
>
> Il 02 settembre 2014 14:12:24 Wangyufen ha scritto:
>
> >From: Wang Yufen
> >
> >Called getpid() When creating a new process with clone(), getpid() returns
> >the father_process's value. It should be child_process's value.
> >The reason is missing a RESET_PID in the arm clone impl.
> >
> >Signed-off-by: Wang Yufen
> >---
> > libc/sysdeps/linux/arm/clone.S | 61
++-
> > libc/sysdeps/linux/arm/sysdep.h | 45
> > 2 files changed, 92 insertions(+), 14 deletions(-)
> >
> >diff --git a/libc/sysdeps/linux/arm/clone.S b/libc/sysdeps/linux/arm/clone.S
> >index 03cd10e..29045ef 100644
> >--- a/libc/sysdeps/linux/arm/clone.S
> >+++ b/libc/sysdeps/linux/arm/clone.S
> >@@ -19,12 +19,17 @@
> > /* clone() is even more special than fork() as it mucks with stacks
> >and invokes a function in the right context after its all over. */
> >
> >+#include
> > #define _ERRNO_H
> > #include
> > #include
> > #include
> > #include
> > #include
> >+#include
> >+
> >+#define CLONE_VM 0x0100
> >+#define CLONE_THREAD 0x0001
> >
> > #if defined(__NR_clone)
> > /* int clone(int (*fn)(void *arg), void *child_stack, int flags, void
*arg); */
> >@@ -87,6 +92,8 @@ __error:
> > .pool
> > #else
> > __clone:
> >+.fnstart
> >+.cantunwind
> > @ sanity check args
> > cmp r0, #0
> > IT(te, ne)
> >@@ -95,32 +102,58 @@ __clone:
> > beq __error
> >
> > @ insert the args onto the new stack
> >- sub r1, r1, #8
> >- str r3, [r1, #4]
> >- @ save the function pointer as the 0th element
> >- str r0, [r1]
> >+ str r3, [r1, #-4]!
> >+ str r0, [r1, #-4]!
> >
> > @ do the system call
> > @ get flags
> > mov r0, r2
> >+#ifdef RESET_PID
> >+ mov ip, r2
> >+#endif
> > @ new sp is already in r1
> >- @ load remaining arguments off the stack
> >- stmfd sp!, {r4}
> >- ldr r2, [sp, #4]
> >- ldr r3, [sp, #8]
> >- ldr r4, [sp, #12]
> >- DO_CALL (clone)
> >- movsa1, a1
> >- IT(t, ne)
> >- ldmnefd sp!, {r4}
> >+ push{r4, r7}
> >+ cfi_adjust_cfa_offset (8)
> >+ cfi_rel_offset (r4, 0)
> >+ cfi_rel_offset (r7, 4)
> >+ ldr r2, [sp, #8]
> >+ ldr r3, [sp, #12]
> >+ ldr r4, [sp, #16]
> >+ ldr r7, =SYS_ify(clone)
> >+ swi 0x0
> >+ cfi_endproc
> >+ cmp r0, #0
> >+ beq 1f
> >+ pop {r4, r7}
> > blt __error
> >- IT(t, ne)
> > #if defined(__USE_BX__)
> > bxnelr
> > #else
> > movne pc, lr
> > #endif
> >
> >+ cfi_startproc
> >+.fnend
> >+PSEUDO_END (__clone)
> >+
> >+1:
> >+ .fnstart
> >+ .cantunwind
> >+#ifdef RESET_PID
> >+ tst ip, #CLONE_THREAD
> >+ bne 3f
> >+ GET_TLS (lr)
> >+ mov r1, r0
> >+ tst ip, #CLONE_VM
> >+ ldr r7, =SYS_ify(getpid)
> >+ ite ne
> >+ movne r0, #-1
> >+ swieq 0x0
> >+ NEGOFF_ADJ_BASE (r1, TID_OFFSET)
> >+ str r0, NEGOFF_OFF1 (r1, TID_OFFSET)
> >+ str r0, NEGOFF_OFF2 (r1, PID_OFFSET, TID_OFFSET)
> >+3:
> >+#endif
> > @ pick the function arg and call address off the stack and execute
> > ldr r0, [sp, #4]
> > mov lr, pc
> >diff --git a/libc/sysdeps/linux/arm/sysdep.h
b/libc/sysdeps/linux/arm/sysdep.h
> >index 64f4040..38a131d 100644
> >--- a/libc/sysdeps/linux/arm/sysdep.h
> >+++ b/libc/sysdeps/linux/arm/sysdep.h
> >@@ -213,6 +213,51 @@ __local_syscall_error:
\
> >sees the right arguments.
> >
> > */
> >+#if __ARM_ARCH > 4 || defined (__ARM_ARCH_4T__)
> >+# define ARCH_HAS_BX
> >+#endif
> >+#if __ARM_ARCH > 4
> >+# define ARCH_HAS_BLX
> >+#endif
> >+#if __ARM_ARCH > 6 || defined (__ARM_ARCH_6K__) || defined
(__ARM_ARCH_6ZK__)
> >+# define ARCH_HAS_HARD_TP
>
Re: [PATCH] arm: add RESET_PID in the clone impl
>On 4 September 2014 19:32, Carmelo Amoroso wrote: > > > Il 04 settembre 2014 00:01:38 Waldemar Brodkorb ha > scritto: > >> Hi Carmelo, > > > Hello, > > >> Carmelo Amoroso wrote, >> >> > Hi, >> > I don't think clone on arm is missing RESET_PID if you use the >> > correct implementation from >> > libpthread/nptl/sysdeps/unix/sysv/linux/arm/clone.S. >> >> Can you explain this a little bit more? >> > > What I mean to say is that if the root cause is the lack of RESET_PIN > definition it's not the case. No, it's not the define of RESET_PID but nothing in the arm clone.S actually does anything with RESET_PID, i.e. the actual implementation is missing, AFAICS. > > Then, I did not enter into the details of the patch, but at first glance I > could see several other unrelated changes that could eventually have been > split in more patches, but I could be wrong. > On my platform armv7 I don't remember to have such failures. Curious how you get a correct getpid() on arm then? This whole caching stuff is a pain and should be removed instead. ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] arm: add RESET_PID in the clone impl
Il 04 settembre 2014 22:32:01 Bernhard Reutner-Fischer ha scritto: >On 4 September 2014 19:32, Carmelo Amoroso wrote: > > > Il 04 settembre 2014 00:01:38 Waldemar Brodkorb ha > scritto: > >> Hi Carmelo, > > > Hello, > > >> Carmelo Amoroso wrote, >> >> > Hi, >> > I don't think clone on arm is missing RESET_PID if you use the >> > correct implementation from >> > libpthread/nptl/sysdeps/unix/sysv/linux/arm/clone.S. >> >> Can you explain this a little bit more? >> > > What I mean to say is that if the root cause is the lack of RESET_PIN > definition it's not the case. No, it's not the define of RESET_PID but nothing in the arm clone.S actually does anything with RESET_PID, i.e. the actual implementation is missing, AFAICS. Indeed by looking at the patch better I've noticed the actual fix. > > Then, I did not enter into the details of the patch, but at first glance I > could see several other unrelated changes that could eventually have been > split in more patches, but I could be wrong. > On my platform armv7 I don't remember to have such failures. Curious how you get a correct getpid() on arm then? This whole caching stuff is a pain and should be removed instead. I will test and check it again as soon as back from my bz trip... I could not remember correctly. For sure I did not debug it. Carmelo Inviato con AquaMail per Android http://www.aqua-mail.com ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
Re: [PATCH] arm: add RESET_PID in the clone impl
On 2014/9/4 19:40, Bernhard Reutner-Fischer wrote: > On 4 September 2014 00:01, Waldemar Brodkorb wrote: >> Hi Carmelo, >> Carmelo Amoroso wrote, >> >>> Hi, >>> I don't think clone on arm is missing RESET_PID if you use the >>> correct implementation from >>> libpthread/nptl/sysdeps/unix/sysv/linux/arm/clone.S. >> >> Can you explain this a little bit more? >> >> cat ./libpthread/nptl/sysdeps/unix/sysv/linux/arm/clone.S >> #define RESET_PID >> #include >> #include "../../../../../../../libc/sysdeps/linux/arm/clone.S" >> >> Do you think the test-suite failures on arm come from a buildsystem >> problem? > > No, the patch is fine and needed. > ARCH_HAS_BX and ARCH_HAS_BLX should be deleted though, we handle that > via bits/arm_bx.h Actually，to fix missing RESET_PID， I just need ARCH_HAS_HARD_TP. I'll resend the patch. Regards, Wang > Can somebody run this through a test-harness for !threads and NPTL on > the affected ISA revisions just to be sure? > > That'd help. > TIA, > ___ > uClibc mailing list > uClibc@uclibc.org > http://lists.busybox.net/mailman/listinfo/uclibc > > ___ uClibc mailing list uClibc@uclibc.org http://lists.busybox.net/mailman/listinfo/uclibc
