From mboxrd@z Thu Jan  1 00:00:00 1970
From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=)
Subject: Re: [PATCH 3/6] daemon: On aarch64, use increments of 16 on the stack.
Date: Tue, 14 Feb 2017 09:47:30 +0100
Message-ID: <87r331xiot.fsf@gnu.org>
References: <20170209184510.24200-1-efraim@flashner.co.il>
	<20170209184510.24200-4-efraim@flashner.co.il>
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Return-path: <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
Received: from eggs.gnu.org ([2001:4830:134:3::10]:39350)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1cdYlu-0008SL-9N
	for guix-devel@gnu.org; Tue, 14 Feb 2017 03:47:39 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <ludo@gnu.org>) id 1cdYlr-0004mt-5r
	for guix-devel@gnu.org; Tue, 14 Feb 2017 03:47:38 -0500
In-Reply-To: <20170209184510.24200-4-efraim@flashner.co.il> (Efraim Flashner's
	message of "Thu, 9 Feb 2017 20:45:07 +0200")
List-Id: "Development of GNU Guix and the GNU System distribution."
	<guix-devel.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/archive/html/guix-devel/>
List-Post: <mailto:guix-devel@gnu.org>
List-Help: <mailto:guix-devel-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/guix-devel>,
	<mailto:guix-devel-request@gnu.org?subject=subscribe>
Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org
Sender: "Guix-devel" <guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org>
To: Efraim Flashner <efraim@flashner.co.il>
Cc: guix-devel@gnu.org

--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Efraim Flashner <efraim@flashner.co.il> skribis:

> man2 clone: EINVAL: ... on aarch64, child_stack must be a multiple of 16.
>
> * nix/libstore/build.cc (DerivationGoal::startBuilder): When on aarch64,
> when calling clone(), increment the stack by 16.
> ---
>  nix/libstore/build.cc | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
> index cebc404d1..362b2d91d 100644
> --- a/nix/libstore/build.cc
> +++ b/nix/libstore/build.cc
> @@ -2008,7 +2008,12 @@ void DerivationGoal::startBuilder()
>  	char stack[32 * 1024];
>  	int flags =3D CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS =
| SIGCHLD;
>  	if (!fixedOutput) flags |=3D CLONE_NEWNET;
> -	pid =3D clone(childEntry, stack + sizeof(stack) - 8, flags, this);
> +// if statements are hard, fix this
> +//#if __AARCH64__
> +	pid =3D clone(childEntry, stack + sizeof(stack) - 16, flags, this);
> +//#else
> +//	pid =3D clone(childEntry, stack + sizeof(stack) - 8, flags, this);
> +//#endif

I think we can make it unconditional.  Could you test whether the
attached patch works for aarch64?

Thanks!

Ludo=E2=80=99.


--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline

diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index cebc404d1..9b7bb5391 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,7 +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;
-	pid = clone(childEntry, stack + sizeof(stack) - 8, flags, this);
+
+	/* Ensure proper alignment on the stack.  On aarch64, it has to be 16
+	   bytes.  */
+	pid = clone(childEntry, (char *)(((uintptr_t)stack + 16) & ~0xf),
+		    flags, this);
 	if (pid == -1)
 	    throw SysError("cloning builder process");
     } else

--=-=-=--