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 <jason.duerst...@gmail.com>
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;
 }
 

Reply via email to