bug#21694: 'clone' syscall binding unreliable

[Mark H Weaver]

address@hidden (Ludovic Courtès) writes:

> Looking at (guix build syscalls) though, I see an ABI mismatch between
> our definition and the actual ‘syscall’ C function, and between our
> ‘clone’ definition and the actual C function.

Good catch!  However, please see below.

> This leads to the attached patch, which also fixes the above problem for me.
>
> diff --git a/guix/build/syscalls.scm b/guix/build/syscalls.scm
> index 80b9d00..f931f8d 100644
> --- a/guix/build/syscalls.scm
> +++ b/guix/build/syscalls.scm
> @@ -322,10 +322,16 @@ string TMPL and return its file name.  TMPL must end 
> with 'XXXXXX'."
>  (define CLONE_NEWNET         #x40000000)
>  
>  ;; The libc interface to sys_clone is not useful for Scheme programs, so the
> -;; low-level system call is wrapped instead.
> +;; low-level system call is wrapped instead.  The 'syscall' function is
> +;; declared in <unistd.h> as a variadic function; in practice, it expects 6
> +;; pointer-sized arguments, as shown in, e.g., x86_64/syscall.S.
>  (define clone
>    (let* ((ptr        (dynamic-func "syscall" (dynamic-link)))
> -         (proc       (pointer->procedure int ptr (list int int '*)))
> +         (proc       (pointer->procedure long ptr
> +                                         (list long                   ;sysno
> +                                               unsigned-long          ;flags

'long' and 'unsigned long' might not be the same size as a pointer.
Better to use 'size_t' for both of these.  While not strictly guaranteed
to be the same size as a pointer, in practice they should be the same
except on architectures with segmented memory models.

What do you think?

      Mark

PS: 'intptr_t' and 'uintptr_t' would be best, but they are optional in
    C99 and not in (system foreign).  'ptrdiff_t' would be better, but
    was not available in (system foreign) before guile-2.0.9.