From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark H Weaver Subject: Re: [PATCH 3/6] daemon: On aarch64, use increments of 16 on the stack. Date: Sat, 05 Aug 2017 02:21:55 -0400 Message-ID: <874ltm5ybg.fsf@netris.org> References: <20170209184510.24200-1-efraim@flashner.co.il> <20170209184510.24200-4-efraim@flashner.co.il> <87r331xiot.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:33720) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ddsTl-00068f-Vz for guix-devel@gnu.org; Sat, 05 Aug 2017 02:22:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ddsTh-0001Ej-O9 for guix-devel@gnu.org; Sat, 05 Aug 2017 02:22:29 -0400 In-Reply-To: <87r331xiot.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Tue, 14 Feb 2017 09:47:30 +0100") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Ludovic =?utf-8?Q?Court=C3=A8s?= Cc: guix-devel@gnu.org Reviving a very old thread... ludo@gnu.org (Ludovic Court=C3=A8s) writes: > Efraim Flashner 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. > > 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 > #include > #include > +#include >=20=20 > #include > #include > @@ -2008,7 +2009,11 @@ 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); > + > + /* Ensure proper alignment on the stack. On aarch64, it has to be 16 > + bytes. */ > + pid =3D clone(childEntry, (char *)(((uintptr_t)stack + 16) & ~0xf), > + flags, this); > if (pid =3D=3D -1) > throw SysError("cloning builder process"); > } else This patch, applied in February, contains a serious error. The stack address passed to 'clone' is supposed to be near the end of the memory block allocated for the stack, and that's how it was before this patch was applied. Since this patch was applied, it now passes an address very close to the *start* of the memory block. This broke the daemon on mips64el in a subtle way that was rather difficult to debug. After about six months of being too busy with other things to investigate properly, I finally tracked it down to this change. I reverted this commit. Let's try again to find a proper fix for this issue on aarch64. Thanks, Mark