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;
}
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
