On 20/03/2023 11:12, Amit Daniel Kachhap wrote:
Some of the clone tests use loop check on status flag to determine if the child exited normally. However, this may cause infinite looping if the data handshake does not happen properly between parent and child due to compiler optimization as observed with Morello Gcc.
Although this can be fixed by setting the flag to volatile but here we replace the busy loop with the waitpid() syscall to wait for the child to exit before parent is executed.
Signed-off-by: Amit Daniel Kachhap amit.kachhap@arm.com
tools/testing/selftests/arm64/morello/clone.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/clone.c b/tools/testing/selftests/arm64/morello/clone.c index 0381702ab0fc..6f0ea953848c 100644 --- a/tools/testing/selftests/arm64/morello/clone.c +++ b/tools/testing/selftests/arm64/morello/clone.c @@ -18,9 +18,6 @@ #define in_restricted() \ (!(cheri_perms_get(cheri_pcc_get()) & ARM_CAP_PERMISSION_EXECUTIVE))
-#define MAGIC_NR 0x3562 /* whatever really ..... */
#ifndef MAX_PID_NS_LEVEL #define MAX_PID_NS_LEVEL 32 #endif @@ -36,7 +33,6 @@ #define CLONE_TH_RUSAGE BIT(3) struct test_fixture {
- int status; int flags; void *sp; int result;
@@ -78,7 +74,7 @@ static int clone_base_fn(void *data) if (!tls) { __data->result = CTR_FAILED;
goto done;
return 0;probe_addr_range(tls, STACK_SIZE >> 10, 64); @@ -88,15 +84,13 @@ static int clone_base_fn(void *data) __data->result = !!(__data->flags & CLONE_TH_RESTRICTED) != in_restricted() ? CTR_FAILED : CTR_SUCCESS; -done:
- __data->status = MAGIC_NR; return 0;
} static inline __attribute__((always_inline)) void clone_single(struct test_fixture *data) {
- int ppid = 0, cpid = 0;
- int ppid = 0, cpid = 0, wstatus; int result = -EINVAL;
void *new_stack = mmap_verified(NULL, STACK_SIZE, PROT_READ | PROT_WRITE, @@ -107,7 +101,7 @@ void clone_single(struct test_fixture *data) CAP_LOAD_PERMS | CAP_STORE_PERMS) : NULL;
- int clone_flags = CLONE_VM | CLONE_PARENT_SETTID | CLONE_CHILD_SETTID;
- int clone_flags = CLONE_VM | CLONE_PARENT_SETTID | CLONE_CHILD_SETTID | SIGCHLD;
int (*clone_fn_ptr)(void *) = clone_base_fn; @@ -142,10 +136,9 @@ void clone_single(struct test_fixture *data) goto leave; }
- /* Wait substitute ... */
- while (data->status != MAGIC_NR) {
asm("");- }
- /* Wait for the child to exit */
- result = waitpid(cpid, &wstatus, 0);
- ASSERT_EQ(result, cpid);
This makes me realise we could also get rid of data->result. Instead we can simply return 0/1 from clone_base_fn() and here we can check that WIFEXITED(wstatus) && WEXITSTATUS(wstatus) == 0.
Kevin
/* * CLONE_CHILD_SETTID sets child's thread ID to provided child's * memory but as VM is being shared, it's all good at this point.