Bug#902961: mozjs52: FTBFS in recent unstable: ReserveProcessExecutableMemory() fails during tests
Control: tags -1 = ftbfs patch pending On Wed, 04 Jul 2018 at 00:51:05 +0100, Simon McVittie wrote: > Running it under gdb reveals that this is because > ReserveProcessExecutableMemory() in js/src/jit/ProcessExecutableMemory.cpp > fails during startup, causing JS_Init() to fail. On closer inspection, debian/patches/ia64-support.patch #ifdefs out the final "return p" on non-IA64 architectures, so the practical result of that function ends up being whatever happens to be in the register or stack location that should have held the return value, and it's not surprising that this is often (although perhaps not always!) NULL. The attached revision of that patch seems to work, and passes tests on barriere. If you are interested in portability to ia64, please either take this change upstream (probably best done via Firefox), or fix ia64 kernels' mmap() implementation to behave as documented in mmap(2) (with addr being merely a hint, as it apparently is on other architectures). This was a good example of a portability patch breaking architectures other than the one for which the patch was applied, demonstrating that adding patches for the benefit of non-release architectures is not necessarily harmless to release architectures. > The changes between 52.3.1-7 and what I'm compiling all seem > to be specific to non-amd64 architectures ... except inasmuch as they affect architectures other than their target by adding more #ifdef complexity. > A hack that works to avoid this is to retry the mmap() without the address > hint, like in Jason Duerstock's debian/patches/ia64-support.patch, > but for non-IA64 architectures as well; see attached. I suspect this > might be the wrong solution This worked, but not for the reason I initially thought it did: it removed the #ifdefs, and hence the bug. smcv From: Jason Duerstock Date: Sun, 29 Apr 2018 09:16:20 -0400 Subject: On ia64, retry failed mmap without address hint [smcv: Move the #endif so we still return a defined value on non-ia64] Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=897178 Last-Updated: 2018-07-10 --- js/src/jit/ProcessExecutableMemory.cpp | 11 +++ 1 file changed, 11 insertions(+) diff --git a/js/src/jit/ProcessExecutableMemory.cpp b/js/src/jit/ProcessExecutableMemory.cpp index 71c2ab0..89b5dfe 100644 --- a/js/src/jit/ProcessExecutableMemory.cpp +++ b/js/src/jit/ProcessExecutableMemory.cpp @@ -290,8 +290,19 @@ ReserveProcessExecutableMemory(size_t bytes) void* randomAddr = ComputeRandomAllocationAddress(); void* p = MozTaggedAnonymousMmap(randomAddr, bytes, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0, "js-executable-memory"); + +#ifndef __ia64__ if (p == MAP_FAILED) return nullptr; +#else +if (p == MAP_FAILED) { +// the comment above appears to be incorrect on ia64, so retry without the hint +p = MozTaggedAnonymousMmap(NULL, bytes, PROT_NONE, MAP_PRIVATE | MAP_ANON, + -1, 0, "js-executable-memory"); +if (p == MAP_FAILED) +return nullptr; +} +#endif return p; }
Processed: Re: Bug#902961: mozjs52: FTBFS in recent unstable: ReserveProcessExecutableMemory() fails during tests
Processing control commands: > tags -1 = ftbfs patch pending Bug #902961 [src:mozjs52] mozjs52: FTBFS in recent unstable: ReserveProcessExecutableMemory() fails during tests Added tag(s) patch and pending; removed tag(s) help. -- 902961: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=902961 Debian Bug Tracking System Contact ow...@bugs.debian.org with problems
Bug#902961: mozjs52: FTBFS in recent unstable: ReserveProcessExecutableMemory() fails during tests
On Fri, 06 Jul 2018 at 23:43:57 -0400, Jason Duerstock wrote: > I suspect the "right" solution to this is to define the "base" and > "mask" variables in ComputeRandomAllocationAddress() based on what > works for each architecture Perhaps, but I'd be surprised if the values provided by upstream weren't right for at least amd64... and the code seems to be the same in the latest Firefox, which runs fine on my laptop. smcv
Bug#902961: mozjs52: FTBFS in recent unstable: ReserveProcessExecutableMemory() fails during tests
I suspect the "right" solution to this is to define the "base" and "mask" variables in ComputeRandomAllocationAddress() based on what works for each architecture, but my enthusiasm for gathering that information was pretty low at the time I ran into this, and hasn't gone up much since then. On Tue, Jul 3, 2018 at 7:54 PM Simon McVittie wrote: > > Source: mozjs52 > Version: 52.3.1-8 > Severity: serious > Justification: fails to build from source (but built successfully in the past) > > (X-Debbugs-Cc to contributors to #897178, to which this seems related, > and #902197) > > After fixing #902197, the mozjs52 build-time tests failed on the amd64 > Debian porterbox barriere. Specifically, the smoke-test in debian/test.sh > that's run before trying the actual test suite: > > "$SRCDIR/js/src/js" -e 'print("Hello, world")' > > fails, with js exiting 1. > > Running it under gdb reveals that this is because > ReserveProcessExecutableMemory() in js/src/jit/ProcessExecutableMemory.cpp > fails during startup, causing JS_Init() to fail. js helpfully has no > diagnostics at all for this, it just silently exits with EXIT_FAILURE. > (Ignore the first implementation of ReserveProcessExecutableMemory(), > which is for Windows; the Unix implementation is around line 300.) > > That function gets a random address, and passes it as the first argument > of mmap(., 1 gigabyte, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0) on > 64-bit machines, or the same but with 128M instead of 1G on 32-bit. > This now seems to be failing and returning NULL. > > I was surprised to find that on the same machine, if I install gjs (a > GNOME wrapper around mozjs) and the slightly older mozjs 52.3.1-7 that it > depends on, gjs-console -c 'print("hello")' works OK. (We don't package > mozjs' own command-line tool 'js' any more, so no precompiled copy is > available.) The changes between 52.3.1-7 and what I'm compiling all seem > to be specific to non-amd64 architectures, so maybe this is about the > environment in which mozjs was built, rather than the source code used? > > A hack that works to avoid this is to retry the mmap() without the address > hint, like in Jason Duerstock's debian/patches/ia64-support.patch, > but for non-IA64 architectures as well; see attached. I suspect this > might be the wrong solution, but perhaps it points someone who knows > this better than I do in the direction of a better solution... > > This also makes me wonder whether the failure addressed by > ia64-support.patch was in fact a generic issue with newer $something > (kernel? compiler? dpkg-buildflags?), which happened to only be seen > by ia64 porters because everyone else was compiling and testing mozjs52 > with an older $something? > > smcv
Bug#902961: mozjs52: FTBFS in recent unstable: ReserveProcessExecutableMemory() fails during tests
Source: mozjs52 Version: 52.3.1-8 Severity: serious Justification: fails to build from source (but built successfully in the past) (X-Debbugs-Cc to contributors to #897178, to which this seems related, and #902197) After fixing #902197, the mozjs52 build-time tests failed on the amd64 Debian porterbox barriere. Specifically, the smoke-test in debian/test.sh that's run before trying the actual test suite: "$SRCDIR/js/src/js" -e 'print("Hello, world")' fails, with js exiting 1. Running it under gdb reveals that this is because ReserveProcessExecutableMemory() in js/src/jit/ProcessExecutableMemory.cpp fails during startup, causing JS_Init() to fail. js helpfully has no diagnostics at all for this, it just silently exits with EXIT_FAILURE. (Ignore the first implementation of ReserveProcessExecutableMemory(), which is for Windows; the Unix implementation is around line 300.) That function gets a random address, and passes it as the first argument of mmap(., 1 gigabyte, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0) on 64-bit machines, or the same but with 128M instead of 1G on 32-bit. This now seems to be failing and returning NULL. I was surprised to find that on the same machine, if I install gjs (a GNOME wrapper around mozjs) and the slightly older mozjs 52.3.1-7 that it depends on, gjs-console -c 'print("hello")' works OK. (We don't package mozjs' own command-line tool 'js' any more, so no precompiled copy is available.) The changes between 52.3.1-7 and what I'm compiling all seem to be specific to non-amd64 architectures, so maybe this is about the environment in which mozjs was built, rather than the source code used? A hack that works to avoid this is to retry the mmap() without the address hint, like in Jason Duerstock's debian/patches/ia64-support.patch, but for non-IA64 architectures as well; see attached. I suspect this might be the wrong solution, but perhaps it points someone who knows this better than I do in the direction of a better solution... This also makes me wonder whether the failure addressed by ia64-support.patch was in fact a generic issue with newer $something (kernel? compiler? dpkg-buildflags?), which happened to only be seen by ia64 porters because everyone else was compiling and testing mozjs52 with an older $something? smcv From: Simon McVittie Date: Tue, 3 Jul 2018 23:47:20 +0100 Subject: If executable memory can't be mapped, retry without randomization on all architectures --- js/src/jit/ProcessExecutableMemory.cpp | 6 -- 1 file changed, 6 deletions(-) diff --git a/js/src/jit/ProcessExecutableMemory.cpp b/js/src/jit/ProcessExecutableMemory.cpp index 31836f1..5e7ea5c 100644 --- a/js/src/jit/ProcessExecutableMemory.cpp +++ b/js/src/jit/ProcessExecutableMemory.cpp @@ -291,19 +291,13 @@ ReserveProcessExecutableMemory(size_t bytes) void* p = MozTaggedAnonymousMmap(randomAddr, bytes, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0, "js-executable-memory"); -#ifndef __ia64__ -if (p == MAP_FAILED) -return nullptr; -#else if (p == MAP_FAILED) { -// the comment above appears to be incorrect on ia64, so retry without the hint p = MozTaggedAnonymousMmap(NULL, bytes, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0, "js-executable-memory"); if (p == MAP_FAILED) return nullptr; } return p; -#endif } static void