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: Sat, 05 Aug 2017 23:12:03 +0200 Message-ID: <878tixlnx8.fsf@gnu.org> References: <20170209184510.24200-1-efraim@flashner.co.il> <20170209184510.24200-4-efraim@flashner.co.il> <87r331xiot.fsf@gnu.org> <874ltm5ybg.fsf@netris.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]:41070) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1de6Mi-0000X6-QW for guix-devel@gnu.org; Sat, 05 Aug 2017 17:12:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1de6Mf-0005DH-Ix for guix-devel@gnu.org; Sat, 05 Aug 2017 17:12:08 -0400 In-Reply-To: <874ltm5ybg.fsf@netris.org> (Mark H. Weaver's message of "Sat, 05 Aug 2017 02:21:55 -0400") 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: Mark H Weaver Cc: guix-devel@gnu.org Mark H Weaver skribis: > 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 1= 6. >>> >>> * 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_NEWUT= S | 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. Arrgh, good catch; my bad! I wonder why this did not cause more problems on other architectures. Ludo=E2=80=99.