Bug 1054438 - Tidy up AsmJSValidate.h and some heap-access related code (r=dougc)
authorLuke Wagner <luke@mozilla.com>
Thu, 21 Aug 2014 11:27:48 -0500
changeset 200851 880bd2e2b616fb5fe8761735d2cbfe8b894fb9c0
parent 200850 36ffb9b24c56e71062bd621036c75a5c4911d576
child 200852 57c6d6fd6bb1fbda360aba892d9b60c1c70c7778
push id48015
push userlwagner@mozilla.com
push dateThu, 21 Aug 2014 16:50:26 +0000
treeherdermozilla-inbound@6b9c89464dc6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdougc
bugs1054438
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1054438 - Tidy up AsmJSValidate.h and some heap-access related code (r=dougc)
js/src/asmjs/AsmJSModule.cpp
js/src/asmjs/AsmJSModule.h
js/src/asmjs/AsmJSSignalHandlers.cpp
js/src/asmjs/AsmJSValidate.cpp
js/src/asmjs/AsmJSValidate.h
js/src/frontend/Parser.cpp
js/src/jit/MIRGenerator.h
js/src/jit/MIRGraph.cpp
js/src/vm/ArrayBufferObject.cpp
js/src/vm/TypedArrayObject.cpp
--- a/js/src/asmjs/AsmJSModule.cpp
+++ b/js/src/asmjs/AsmJSModule.cpp
@@ -102,17 +102,17 @@ AsmJSModule::AsmJSModule(ScriptSource *s
     dynamicallyLinked_(false),
     loadedFromCache_(false),
     profilingEnabled_(false),
     codeIsProtected_(false)
 {
     mozilla::PodZero(&pod);
     pod.funcPtrTableAndExitBytes_ = SIZE_MAX;
     pod.functionBytes_ = UINT32_MAX;
-    pod.minHeapLength_ = AsmJSAllocationGranularity;
+    pod.minHeapLength_ = RoundUpToNextValidAsmJSHeapLength(0);
     pod.strict_ = strict;
     pod.usesSignalHandlers_ = canUseSignalHandlers;
 
     scriptSource_->incref();
 }
 
 AsmJSModule::~AsmJSModule()
 {
--- a/js/src/asmjs/AsmJSModule.h
+++ b/js/src/asmjs/AsmJSModule.h
@@ -822,16 +822,17 @@ class AsmJSModule
         JS_ASSERT(isFinishedWithModulePrologue());
     }
 
     /*************************************************************************/
     // These functions are called while parsing/compiling function bodies:
 
     void requireHeapLengthToBeAtLeast(uint32_t len) {
         JS_ASSERT(isFinishedWithModulePrologue() && !isFinishedWithFunctionBodies());
+        len = RoundUpToNextValidAsmJSHeapLength(len);
         if (len > pod.minHeapLength_)
             pod.minHeapLength_ = len;
     }
     bool addCodeRange(CodeRange::Kind kind, uint32_t begin, uint32_t end) {
         return codeRanges_.append(CodeRange(kind, begin, end));
     }
     bool addCodeRange(CodeRange::Kind kind, uint32_t begin, uint32_t pret, uint32_t end) {
         return codeRanges_.append(CodeRange(kind, begin, pret, end));
--- a/js/src/asmjs/AsmJSSignalHandlers.cpp
+++ b/js/src/asmjs/AsmJSSignalHandlers.cpp
@@ -478,17 +478,17 @@ HandleException(PEXCEPTION_POINTERS exce
         return true;
     }
 
 # if defined(JS_CODEGEN_X64)
     // These checks aren't necessary, but, since we can, check anyway to make
     // 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;
     }
 
     const AsmJSHeapAccess *heapAccess = module.lookupHeapAccess(pc);
     if (!heapAccess)
         return false;
 
@@ -686,17 +686,17 @@ HandleMachException(JSRuntime *rt, const
         return kret == KERN_SUCCESS;
     }
 
 # if defined(JS_CODEGEN_X64)
     // These checks aren't necessary, but, since we can, check anyway to make
     // 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;
     }
 
     const AsmJSHeapAccess *heapAccess = module.lookupHeapAccess(pc);
     if (!heapAccess)
         return false;
 
@@ -925,17 +925,17 @@ HandleSignal(int signum, siginfo_t *info
         return true;
     }
 
 # if defined(JS_CODEGEN_X64)
     // These checks aren't necessary, but, since we can, check anyway to make
     // 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;
     }
 
     const AsmJSHeapAccess *heapAccess = module.lookupHeapAccess(pc);
     if (!heapAccess)
         return false;
 
--- a/js/src/asmjs/AsmJSValidate.cpp
+++ b/js/src/asmjs/AsmJSValidate.cpp
@@ -1416,18 +1416,16 @@ class MOZ_STACK_CLASS ModuleCompiler
             *exitIndex = p->value();
             return true;
         }
         if (!module_->addExit(ffiIndex, exitIndex))
             return false;
         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);
     }
     uint32_t minHeapLength() const {
         return module_->minHeapLength();
     }
     LifoAlloc &lifo() {
         return moduleLifo_;
@@ -3424,27 +3422,27 @@ CheckArrayAccess(FunctionCompiler &f, Pa
         return f.fail(viewName, "base of array access must be a typed array view name");
 
     const ModuleCompiler::Global *global = f.lookupGlobal(viewName->name());
     if (!global || global->which() != ModuleCompiler::Global::ArrayView)
         return f.fail(viewName, "base of array access must be a typed array view name");
 
     *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;
     }
 
     // Mask off the low bits to account for the clearing effect of a right shift
     // followed by the left shift implicit in the array access. E.g., H32[i>>2]
     // loses the low two bits.
     int32_t mask = ~((uint32_t(1) << TypedArrayShift(*viewType)) - 1);
 
@@ -3462,22 +3460,23 @@ CheckArrayAccess(FunctionCompiler &f, Pa
             return f.failf(shiftNode, "shift amount must be %u", requiredShift);
 
         if (pointerNode->isKind(PNK_BITAND))
             FoldMaskedArrayIndex(f, &pointerNode, &mask, needsBoundsCheck);
 
         // 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;
         }
 
         Type pointerType;
         if (!CheckExpr(f, pointerNode, &pointerDef, &pointerType))
             return false;
 
         if (!pointerType.isIntish())
@@ -5401,24 +5400,18 @@ CheckFunction(ModuleCompiler &m, LifoAll
     if (func->defined())
         return m.failName(fn, "function '%s' already defined", FunctionName(fn));
 
     func->define(m, fn);
     func->accumulateCompileTime((PRMJ_Now() - before) / PRMJ_USEC_PER_MSEC);
 
     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;
 }
 
 static bool
 GenerateCode(ModuleCompiler &m, ModuleCompiler::Func &func, MIRGenerator &mir, LIRGraph &lir)
 {
     int64_t before = PRMJ_Now();
@@ -6929,17 +6922,17 @@ EstablishPreconditions(ExclusiveContext 
 
 static bool
 NoExceptionPending(ExclusiveContext *cx)
 {
     return !cx->isJSContext() || !cx->asJSContext()->isExceptionPending();
 }
 
 bool
-js::CompileAsmJS(ExclusiveContext *cx, AsmJSParser &parser, ParseNode *stmtList, bool *validated)
+js::ValidateAsmJS(ExclusiveContext *cx, AsmJSParser &parser, ParseNode *stmtList, bool *validated)
 {
     *validated = false;
 
     if (!EstablishPreconditions(cx, parser))
         return NoExceptionPending(cx);
 
     ScopedJSFreePtr<char> compilationTimeReport;
     ScopedJSDeletePtr<AsmJSModule> module;
--- a/js/src/asmjs/AsmJSValidate.h
+++ b/js/src/asmjs/AsmJSValidate.h
@@ -42,83 +42,61 @@ typedef frontend::Parser<frontend::FullP
 typedef frontend::ParseContext<frontend::FullParseHandler> AsmJSParseContext;
 
 // Takes over parsing of a function starting with "use asm". The return value
 // indicates whether an error was reported which the caller should propagate.
 // If no error was reported, the function may still fail to validate as asm.js.
 // 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
+
+// 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)
+        return 4096;
 
-// 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
+    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);
 
-// 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;
-}
-
-inline uint32_t
-RoundUpToNextValidAsmJSHeapLength(uint32_t length)
-{
-    if (length < 4096)
-        return 4096;
-
-    if (length < 16 * 1024 * 1024)
-        return mozilla::RoundUpPow2(length);
-
-    JS_ASSERT(length <= 0xff000000);
-    return (length + 0x00ffffff) & ~0x00ffffff;
-}
-
 } // namespace js
 
 #endif // jit_AsmJS_h
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -2435,17 +2435,17 @@ Parser<FullParseHandler>::asmJS(Node lis
     pc->sc->asFunctionBox()->useAsm = true;
 
     // Attempt to validate and compile this asm.js module. On success, the
     // tokenStream has been advanced to the closing }. On failure, the
     // tokenStream is in an indeterminate state and we must reparse the
     // 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();
         return false;
     }
 
     return true;
 }
--- a/js/src/jit/MIRGenerator.h
+++ b/js/src/jit/MIRGenerator.h
@@ -139,17 +139,18 @@ class MIRGenerator
         performsCall_ = true;
     }
     bool performsCall() const {
         return performsCall_;
     }
     // 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 {
         return minAsmJSHeapLength_;
     }
 
     bool modifiesFrameArguments() const {
         return modifiesFrameArguments_;
--- a/js/src/jit/MIRGraph.cpp
+++ b/js/src/jit/MIRGraph.cpp
@@ -27,17 +27,17 @@ MIRGenerator::MIRGenerator(CompileCompar
     abortReason_(AbortReason_NoAbort),
     error_(false),
     pauseBuild_(nullptr),
     cancelBuild_(false),
     maxAsmJSStackArgBytes_(0),
     performsCall_(false),
     usesSimd_(false),
     usesSimdCached_(false),
-    minAsmJSHeapLength_(AsmJSAllocationGranularity),
+    minAsmJSHeapLength_(0),
     modifiesFrameArguments_(false),
     instrumentedProfiling_(false),
     instrumentedProfilingIsCached_(false),
     options(options)
 { }
 
 bool
 MIRGenerator::usesSimd()
--- a/js/src/vm/ArrayBufferObject.cpp
+++ b/js/src/vm/ArrayBufferObject.cpp
@@ -387,21 +387,16 @@ ArrayBufferObject::changeContents(JSCont
             view->setPrivate(viewDataPointer);
         }
 
         // Notify compiled jit code that the base pointer has moved.
         MarkObjectStateChange(cx, view);
     }
 }
 
-#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)
 {
     if (buffer->isAsmJSArrayBuffer())
         return true;
 
     if (buffer->isSharedArrayBuffer())
         return true;
@@ -444,17 +439,17 @@ ArrayBufferObject::prepareForAsmJS(JSCon
         return false;
 # else
     data = MozTaggedAnonymousMmap(nullptr, AsmJSMappedSize, PROT_NONE, MAP_PRIVATE | MAP_ANON, -1, 0, "asm-js-reserved");
     if (data == MAP_FAILED)
         return false;
 # 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);
         return false;
     }
 # else
     size_t validLength = buffer->byteLength();
     if (mprotect(data, validLength, PROT_READ | PROT_WRITE)) {
--- a/js/src/vm/TypedArrayObject.cpp
+++ b/js/src/vm/TypedArrayObject.cpp
@@ -23,18 +23,16 @@
 #include "jsobj.h"
 #include "jstypes.h"
 #include "jsutil.h"
 #ifdef XP_WIN
 # include "jswin.h"
 #endif
 #include "jswrapper.h"
 
-#include "asmjs/AsmJSModule.h"
-#include "asmjs/AsmJSValidate.h"
 #include "gc/Barrier.h"
 #include "gc/Marking.h"
 #include "vm/ArrayBufferObject.h"
 #include "vm/GlobalObject.h"
 #include "vm/Interpreter.h"
 #include "vm/NumericConversions.h"
 #include "vm/SharedArrayObject.h"
 #include "vm/WrapperObject.h"