On Sat, Aug 05, 2017 at 02:21:55AM -0400, Mark H Weaver wrote: > Reviving a very old thread... > > l...@gnu.org (Ludovic Courtès) writes: > > > Efraim Flashner <efr...@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 = CLONE_NEWPID | CLONE_NEWNS | CLONE_NEWIPC | CLONE_NEWUTS | > >> SIGCHLD; > >> if (!fixedOutput) flags |= CLONE_NEWNET; > >> - pid = clone(childEntry, stack + sizeof(stack) - 8, flags, this); > >> +// if statements are hard, fix this > >> +//#if __AARCH64__ > >> + pid = clone(childEntry, stack + sizeof(stack) - 16, flags, this); > >> +//#else > >> +// pid = 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’. > > > > 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 > > 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
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. -- Efraim Flashner <efr...@flashner.co.il> אפרים פלשנר 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