Bug 1054438 - Tidy up AsmJSValidate.h and some heap-access related code (r=dougc)

js/src/asmjs/AsmJSModule.cpp

@@ -107,7 +107,7 @@ AsmJSModule::AsmJSModule(ScriptSource *scriptSource, uint32_t srcStart, uint32_t
     mozilla::PodZero(&pod);
     pod.funcPtrTableAndExitBytes_ = SIZE_MAX;
     pod.functionBytes_ = UINT32_MAX;
-    pod.minHeapLength_ = AsmJSAllocationGranularity;
+    pod.minHeapLength_ = RoundUpToNextValidAsmJSHeapLength(0);
     pod.strict_ = strict;
     pod.usesSignalHandlers_ = canUseSignalHandlers;
 

js/src/asmjs/AsmJSModule.h

@@ -827,6 +827,7 @@ class AsmJSModule
 
     void requireHeapLengthToBeAtLeast(uint32_t len) {
         JS_ASSERT(isFinishedWithModulePrologue() && !isFinishedWithFunctionBodies());
+        len = RoundUpToNextValidAsmJSHeapLength(len);
         if (len > pod.minHeapLength_)
             pod.minHeapLength_ = len;
     }

js/src/asmjs/AsmJSSignalHandlers.cpp

@@ -483,7 +483,7 @@ HandleException(PEXCEPTION_POINTERS exception)
     // sure we aren't covering up a real bug.
     if (!module.maybeHeap() ||
         faultingAddress < module.maybeHeap() ||
-        faultingAddress >= module.maybeHeap() + AsmJSBufferProtectedSize)
+        faultingAddress >= module.maybeHeap() + AsmJSMappedSize)
     {
         return false;
     }
@@ -691,7 +691,7 @@ HandleMachException(JSRuntime *rt, const ExceptionRequest &request)
     // sure we aren't covering up a real bug.
     if (!module.maybeHeap() ||
         faultingAddress < module.maybeHeap() ||
-        faultingAddress >= module.maybeHeap() + AsmJSBufferProtectedSize)
+        faultingAddress >= module.maybeHeap() + AsmJSMappedSize)
     {
         return false;
     }
@@ -930,7 +930,7 @@ HandleSignal(int signum, siginfo_t *info, void *ctx)
     // sure we aren't covering up a real bug.
     if (!module.maybeHeap() ||
         faultingAddress < module.maybeHeap() ||
-        faultingAddress >= module.maybeHeap() + AsmJSBufferProtectedSize)
+        faultingAddress >= module.maybeHeap() + AsmJSMappedSize)
     {
         return false;
     }

js/src/asmjs/AsmJSValidate.cpp

@@ -1421,8 +1421,6 @@ class MOZ_STACK_CLASS ModuleCompiler
         return exits_.add(p, Move(exitDescriptor), *exitIndex);
     }
 
-    // Note a constraint on the minimum size of the heap.  The heap size is
-    // constrained when linking to be at least the maximum of all such constraints.
     void requireHeapLengthToBeAtLeast(uint32_t len) {
         module_->requireHeapLengthToBeAtLeast(len);
     }
@@ -3429,17 +3427,17 @@ CheckArrayAccess(FunctionCompiler &f, ParseNode *elem, Scalar::Type *viewType,
 
     *viewType = global->viewType();
 
-    uint32_t pointer;
-    if (IsLiteralOrConstInt(f, indexExpr, &pointer)) {
-        if (pointer > (uint32_t(INT32_MAX) >> TypedArrayShift(*viewType)))
+    uint32_t index;
+    if (IsLiteralOrConstInt(f, indexExpr, &index)) {
+        uint64_t byteOffset = uint64_t(index) << TypedArrayShift(*viewType);
+        if (byteOffset > INT32_MAX)
             return f.fail(indexExpr, "constant index out of range");
-        pointer <<= TypedArrayShift(*viewType);
-        // It is adequate to note pointer+1 rather than rounding up to the next
-        // access-size boundary because access is always aligned and the constraint
-        // will be rounded up to a larger alignment later.
-        f.m().requireHeapLengthToBeAtLeast(uint32_t(pointer) + 1);
+
+        unsigned elementSize = 1 << TypedArrayShift(*viewType);
+        f.m().requireHeapLengthToBeAtLeast(byteOffset + elementSize);
+
         *needsBoundsCheck = NO_BOUNDS_CHECK;
-        *def = f.constant(Int32Value(pointer), Type::Int);
+        *def = f.constant(Int32Value(byteOffset), Type::Int);
         return true;
     }
 
@@ -3467,12 +3465,13 @@ CheckArrayAccess(FunctionCompiler &f, ParseNode *elem, Scalar::Type *viewType,
         // Fold a 'literal constant right shifted' now, and skip the bounds check if
         // currently possible. This handles the optimization of many of these uses without
         // the need for range analysis, and saves the generation of a MBitAnd op.
-        if (IsLiteralOrConstInt(f, pointerNode, &pointer) && pointer <= uint32_t(INT32_MAX)) {
+        uint32_t byteOffset;
+        if (IsLiteralOrConstInt(f, pointerNode, &byteOffset) && byteOffset <= uint32_t(INT32_MAX)) {
             // Cases: b[c>>n], and b[(c&m)>>n]
-            pointer &= mask;
-            if (pointer < f.m().minHeapLength())
+            byteOffset &= mask;
+            if (byteOffset < f.m().minHeapLength())
                 *needsBoundsCheck = NO_BOUNDS_CHECK;
-            *def = f.constant(Int32Value(pointer), Type::Int);
+            *def = f.constant(Int32Value(byteOffset), Type::Int);
             return true;
         }
 
@@ -5406,14 +5405,8 @@ CheckFunction(ModuleCompiler &m, LifoAlloc &lifo, MIRGenerator **mir, ModuleComp
 
     m.parser().release(mark);
 
-    // Copy the cumulative minimum heap size constraint to the MIR for use in analysis.  The length
-    // is also constrained to particular lengths, so firstly round up - a larger 'heap required
-    // length' can help range analysis to prove that bounds checks are not needed.
-    uint32_t len = RoundUpToNextValidAsmJSHeapLength(m.minHeapLength());
-    m.requireHeapLengthToBeAtLeast(len);
-
     *mir = f.extractMIR();
-    (*mir)->noteMinAsmJSHeapLength(len);
+    (*mir)->initMinAsmJSHeapLength(m.minHeapLength());
     *funcOut = func;
     return true;
 }
@@ -6934,7 +6927,7 @@ NoExceptionPending(ExclusiveContext *cx)
 }
 
 bool
-js::CompileAsmJS(ExclusiveContext *cx, AsmJSParser &parser, ParseNode *stmtList, bool *validated)
+js::ValidateAsmJS(ExclusiveContext *cx, AsmJSParser &parser, ParseNode *stmtList, bool *validated)
 {
     *validated = false;
 

js/src/asmjs/AsmJSValidate.h

@@ -47,78 +47,56 @@ typedef frontend::ParseContext<frontend::FullParseHandler> AsmJSParseContext;
 // In this case, the parser.tokenStream has been advanced an indeterminate
 // amount and the entire function should be reparsed from the beginning.
 extern bool
-CompileAsmJS(ExclusiveContext *cx, AsmJSParser &parser, frontend::ParseNode *stmtList,
+ValidateAsmJS(ExclusiveContext *cx, AsmJSParser &parser, frontend::ParseNode *stmtList,
              bool *validated);
 
-// The assumed page size; dynamically checked in CompileAsmJS.
+// The assumed page size; dynamically checked in ValidateAsmJS.
 const size_t AsmJSPageSize = 4096;
 
-// The asm.js spec requires that the ArrayBuffer's byteLength be a multiple of 4096.
-static const size_t AsmJSAllocationGranularity = 4096;
-
 #ifdef JS_CODEGEN_X64
 // On x64, the internal ArrayBuffer data array is inflated to 4GiB (only the
 // byteLength portion of which is accessible) so that out-of-bounds accesses
 // (made using a uint32 index) are guaranteed to raise a SIGSEGV.
-static const size_t AsmJSBufferProtectedSize = 4 * 1024ULL * 1024ULL * 1024ULL;
+static const size_t AsmJSMappedSize = 4 * 1024ULL * 1024ULL * 1024ULL;
+#endif
 
-// To avoid dynamically checking bounds on each load/store, asm.js code relies
-// on the SIGSEGV handler in AsmJSSignalHandlers.cpp. However, this only works
-// if we can guarantee that *any* out-of-bounds access generates a fault. This
-// isn't generally true since an out-of-bounds access could land on other
-// Mozilla data. To overcome this on x64, we reserve an entire 4GB space,
-// making only the range [0, byteLength) accessible, and use a 32-bit unsigned
-// index into this space. (x86 and ARM require different tricks.)
-//
-// One complication is that we need to put an ObjectElements struct immediately
-// before the data array (as required by the general JSObject data structure).
-// Thus, we must stick a page before the elements to hold ObjectElements.
-//
-//   |<------------------------------ 4GB + 1 pages --------------------->|
-//           |<--- sizeof --->|<------------------- 4GB ----------------->|
-//
-//   | waste | ObjectElements | data array | inaccessible reserved memory |
-//                            ^            ^                              ^
-//                            |            \                             /
-//                      obj->elements       required to be page boundaries
-//
-static const size_t AsmJSMappedSize = AsmJSPageSize + AsmJSBufferProtectedSize;
-#endif // JS_CODEGEN_X64
-
-// Return whether asm.js optimization is inhibited by the platform or
-// dynamically disabled:
-extern bool
-IsAsmJSCompilationAvailable(JSContext *cx, unsigned argc, JS::Value *vp);
-
-// To succesfully link an asm.js module to an ArrayBuffer heap, the
-// ArrayBuffer's byteLength must be:
-//  - greater or equal to 4096
-//  - either a power of 2 OR a multiple of 16MB
-inline bool
-IsValidAsmJSHeapLength(uint32_t length)
-{
-    if (length < 4096)
-        return false;
-
-    if (IsPowerOfTwo(length))
-        return true;
-
-    return (length & 0x00ffffff) == 0;
-}
+// From the asm.js spec Linking section:
+//  the heap object's byteLength must be either
+//    2^n for n in [12, 24)
+//  or
+//    2^24 * n for n >= 1.
 
 inline uint32_t
 RoundUpToNextValidAsmJSHeapLength(uint32_t length)
 {
-    if (length < 4096)
+    if (length <= 4096)
         return 4096;
 
-    if (length < 16 * 1024 * 1024)
+    if (length <= 16 * 1024 * 1024)
         return mozilla::RoundUpPow2(length);
 
     JS_ASSERT(length <= 0xff000000);
     return (length + 0x00ffffff) & ~0x00ffffff;
 }
 
+inline bool
+IsValidAsmJSHeapLength(uint32_t length)
+{
+    bool valid = length >= 4096 &&
+                 (IsPowerOfTwo(length) ||
+                  (length & 0x00ffffff) == 0);
+
+    JS_ASSERT_IF(valid, length % AsmJSPageSize == 0);
+    JS_ASSERT_IF(valid, length == RoundUpToNextValidAsmJSHeapLength(length));
+
+    return valid;
+}
+
+// Return whether asm.js optimization is inhibited by the platform or
+// dynamically disabled:
+extern bool
+IsAsmJSCompilationAvailable(JSContext *cx, unsigned argc, JS::Value *vp);
+
 } // namespace js
 
 #endif // jit_AsmJS_h

js/src/frontend/Parser.cpp

@@ -2440,7 +2440,7 @@ Parser<FullParseHandler>::asmJS(Node list)
     // function from the beginning. Reparsing is triggered by marking that a
     // new directive has been encountered and returning 'false'.
     bool validated;
-    if (!CompileAsmJS(context, *this, list, &validated))
+    if (!ValidateAsmJS(context, *this, list, &validated))
         return false;
     if (!validated) {
         pc->newDirectives->setAsmJS();

js/src/jit/MIRGenerator.h

@@ -144,7 +144,8 @@ class MIRGenerator
     // Traverses the graph to find if there's any SIMD instruction. Costful but
     // the value is cached, so don't worry about calling it several times.
     bool usesSimd();
-    void noteMinAsmJSHeapLength(uint32_t len) {
+    void initMinAsmJSHeapLength(uint32_t len) {
+        JS_ASSERT(minAsmJSHeapLength_ == 0);
         minAsmJSHeapLength_ = len;
     }
     uint32_t minAsmJSHeapLength() const {

js/src/jit/MIRGraph.cpp

@@ -32,7 +32,7 @@ MIRGenerator::MIRGenerator(CompileCompartment *compartment, const JitCompileOpti
     performsCall_(false),
     usesSimd_(false),
     usesSimdCached_(false),
-    minAsmJSHeapLength_(AsmJSAllocationGranularity),
+    minAsmJSHeapLength_(0),
     modifiesFrameArguments_(false),
     instrumentedProfiling_(false),
     instrumentedProfilingIsCached_(false),

js/src/vm/ArrayBufferObject.cpp

@@ -392,11 +392,6 @@ ArrayBufferObject::changeContents(JSContext *cx, BufferContents newContents)
     }
 }
 
-#if defined(JS_CODEGEN_X64)
-// Refer to comment above AsmJSMappedSize in AsmJS.h.
-JS_STATIC_ASSERT(AsmJSAllocationGranularity == AsmJSPageSize);
-#endif
-
 /* static */ bool
 ArrayBufferObject::prepareForAsmJSNoSignals(JSContext *cx, Handle<ArrayBufferObject*> buffer)
 {
@@ -449,7 +444,7 @@ ArrayBufferObject::prepareForAsmJS(JSContext *cx, Handle<ArrayBufferObject*> buf
 # endif
 
     // Enable access to the valid region.
-    JS_ASSERT(buffer->byteLength() % AsmJSAllocationGranularity == 0);
+    JS_ASSERT(buffer->byteLength() % AsmJSPageSize == 0);
 # ifdef XP_WIN
     if (!VirtualAlloc(data, buffer->byteLength(), MEM_COMMIT, PAGE_READWRITE)) {
         VirtualFree(data, 0, MEM_RELEASE);

js/src/vm/TypedArrayObject.cpp

@@ -28,8 +28,6 @@
 #endif
 #include "jswrapper.h"
 
-#include "asmjs/AsmJSModule.h"
-#include "asmjs/AsmJSValidate.h"
 #include "gc/Barrier.h"
 #include "gc/Marking.h"
 #include "vm/ArrayBufferObject.h"