Mark H Weaver <m...@netris.org> skribis: > 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.
Arrgh, good catch; my bad! I wonder why this did not cause more problems on other architectures. Ludo’.