From mboxrd@z Thu Jan 1 00:00:00 1970 From: Efraim Flashner Subject: Re: [PATCH 3/6] daemon: On aarch64, use increments of 16 on the stack. Date: Sat, 5 Aug 2017 21:24:02 +0300 Message-ID: <20170805182401.GA2458@macbook42.flashner.co.il> 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: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="J2SCkAp4GZ/dPZZf" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:55894) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1de3kC-0001kF-Go for guix-devel@gnu.org; Sat, 05 Aug 2017 14:24:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1de3k8-000644-L0 for guix-devel@gnu.org; Sat, 05 Aug 2017 14:24:12 -0400 Content-Disposition: inline In-Reply-To: <874ltm5ybg.fsf@netris.org> 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 --J2SCkAp4GZ/dPZZf Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Aug 05, 2017 at 02:21:55AM -0400, Mark H Weaver wrote: > Reviving a very old thread... >=20 > ludo@gnu.org (Ludovic Court=C3=A8s) writes: >=20 > > Efraim Flashner skribis: > > > >> man2 clone: EINVAL: ... on aarch64, child_stack must be a multiple of = 16. > >> > >> * nix/libstore/build.cc (DerivationGoal::startBuilder): When on aarch6= 4, > >> 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_NEWU= TS | 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 > > #include > > #include > > @@ -2008,7 +2009,11 @@ 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); > > + > > + /* 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 >=20 > 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. >=20 > 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. >=20 > I reverted this commit. Let's try again to find a proper fix for this > issue on aarch64. >=20 > Thanks, > Mark How about doubling the size of the stack to [32 * 1024 * 2] and changing the clone location to 'stack + sizeof(stack) - 16', does that work for mips64el? With this revert the daemon is completely broken on aarch64. --=20 Efraim Flashner =D7=90=D7=A4=D7=A8=D7=99=D7=9D = =D7=A4=D7=9C=D7=A9=D7=A0=D7=A8 GPG key =3D A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351 Confidentiality cannot be guaranteed on emails sent or received unencrypted --J2SCkAp4GZ/dPZZf Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEoov0DD5VE3JmLRT3Qarn3Mo9g1EFAlmGDTMACgkQQarn3Mo9 g1Gx7Q/8CX/bfm/JoISJCXy319LIHo3CdR2ShRYePLoOhlhoZStm79/37svqTbhe P31rqqQCTgSIYsKm3INZjSgKRWz7HjlPS+hRoYOqUVB4ag6LcJPEkfB1YMAZwHWm LTcitiiK6oy7A3IdBUILtLdDP9CzvOZ3M8ndr/IvwiYSD/K3BG3hl4XkQmIF1JXQ SR552pOCuA3Ptc8ruYnYOBpJoy0Fj1jjE6XtHbXrZFf8nKSLcIk7HWupclqBm/G7 MidEPfWekVEeiDymzYhva4JOfAciehI8/hc3RVTziCQJr1iS7L1ORlDgjVv3cabL ecfrTdgvb9DwP/lwWsr/E+261FIW+lNm/cEc4hRcCyaRT6+GErzQ6SRPF1W8W6KW EJza3QclXcqxH2Koo3OLECiOROl+QxzOBd/bVrhHoXfibpp0RBB1NQB74D/cAdkv QHfKcCNOwyq3TNwADwrGkOIt2steZT4Hq8X7/GQVMHcaFSOqS1ebf4DGT2Lzj/3u khHGNiAQdjvdtALcKjhv07EZtVgKwqKrJzy/6O/3XAhAnGrhv7mV27ZTFG6p7xpM 5Jgp9j3TvW4Bc5Luu7FbU9pWzFo84mTw7wSy5A/HVvbGovllnnzkLscLwAsHpMoh 5zReCnELLwU9hVYO1ltChyl3qsLuigFqVXce4H/L/CGSTDm5MQ4= =nckG -----END PGP SIGNATURE----- --J2SCkAp4GZ/dPZZf--