Re: [PATCH v5 4/5] x86-64: Add the clone3 wrapper

[Florian Weimer <fweimer@redhat.com>]

* H. J. Lu:

> extern int clone3 (struct clone_args *__cl_args,
> 		   int (*__func) (void *__arg), void *__arg);
> ---
>  sysdeps/unix/sysv/linux/x86_64/clone3.S | 92 +++++++++++++++++++++++++
>  sysdeps/unix/sysv/linux/x86_64/sysdep.h |  2 +
>  2 files changed, 94 insertions(+)
>  create mode 100644 sysdeps/unix/sysv/linux/x86_64/clone3.S
>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/clone3.S b/sysdeps/unix/sysv/linux/x86_64/clone3.S
> new file mode 100644
> index 0000000000..f7d4036a6a

> +        .text
> +ENTRY (__clone3)
> +	/* Sanity check arguments.  */
> +	movq	$-EINVAL, %rax
> +	testq	%rdi, %rdi		/* No NULL cl_args pointer.  */
> +	jz	SYSCALL_ERROR_LABEL
> +	testq	%rsi, %rsi		/* No NULL function pointer.  */
> +	jz	SYSCALL_ERROR_LABEL

I think some of these register aren't x32-compatible.  Isn't the upper
half undefined?

> +	/* Save the function pointer in R8 which is preserved by the
> +	   syscall.  */
> +	movq	%rsi, %r8
> +
> +	/* Put sizeof (struct clone_args) in ESI.  */
> +	movl	$CLONE_ARGS_SIZE , %esi

If this is in preparation of the public wrapper, this should actually be
an argument.  Sorry didn't realize this was the direction.

> +L(thread_start):
> +	cfi_startproc
> +	/* Clearing frame pointer is insufficient, use CFI.  */
> +	cfi_undefined (rip)
> +	/* Clear the frame pointer.  The ABI suggests this be done, to mark
> +	   the outermost frame obviously.  */
> +	xorl	%ebp, %ebp
> +
> +	/* Set up arguments for the function call.  */
> +	movq	%rdx, %rdi	/* Argument.  */
> +	call	*%r8		/* Call function.  */
> +	/* Call exit with return value from function call. */
> +	movq	%rax, %rdi
> +	movl	$SYS_ify(exit), %eax
> +	syscall
> +	cfi_endproc
> +
> +	cfi_startproc
> +PSEUDO_END (__clone3)

If this is a public wrapper, should it round up %rsp to 16 bytes
at the point of the caller, to follow the x86-64 calling convention?

Thanks,
Florian