Re: [PATCH 3/6] daemon: On aarch64, use increments of 16 on the stack.

[Efraim Flashner]

On Sat, Aug 05, 2017 at 05:41:50PM -0400, Mark H Weaver wrote:
> I wrote:
> > Can you try the following patch on aarch64 and report back?
> 
> Actually, the last patch was not quite right.  C/C++ makes it rather
> difficult to avoid edge cases in arithmetic.  Can you try this one
> instead?
> 
>     Thanks,
>       Mark
> 
> --8<---------------cut here---------------start------------->8---
> diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
> index 693fa70c8..63540ddfc 100644
> --- a/nix/libstore/build.cc
> +++ b/nix/libstore/build.cc
> @@ -26,6 +26,7 @@
>  #include <errno.h>
>  #include <stdio.h>
>  #include <cstring>
> +#include <stdint.h>
>  
>  #include <pwd.h>
>  #include <grp.h>
> @@ -2008,11 +2009,11 @@ void DerivationGoal::startBuilder()
>       char stack[32 * 1024];
>       int flags = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | 
> SIGCHLD;
>       if (!fixedOutput) flags |= CLONE_NEWNET;
> -#ifdef __aarch64__
> -         pid = clone(childEntry, stack + sizeof(stack) - 16, flags, this);
> -#else
> -         pid = clone(childEntry, stack + sizeof(stack) - 8, flags, this);
> -#endif
> +     /* Ensure proper alignment on the stack.  On aarch64, it has to be 16
> +        bytes.  */
> +     pid = clone(childEntry,
> +                 (char *)(((uintptr_t)stack + sizeof(stack) - 8) & 
> ~(uintptr_t)0xf),
> +                 flags, this);
>       if (pid == -1)
>           throw SysError("cloning builder process");
>      } else
> --8<---------------cut here---------------end--------------->8---

The aarch64 machine that I tested this on built packages without any
problems. Looks good to me.

-- 
Efraim Flashner   <address@hidden>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

signature.asc
Description: PGP signature