Bug 1236476: Report out of memory in ExpandErrorArgumentsVA; r=jandem draft
authorBenjamin Bouvier <benj@benj.me>
Tue, 05 Jan 2016 10:24:05 +0100
changeset 320513 c4301ef0de9100b83e23f09138c360b874eb97cb
parent 320442 18b1262883cdf2c95293d43fd8e468fe279d697f
child 512755 002da5b8c029c0e5fc7e819af2df76506f16222d
push id9216
push userbenj@benj.me
push dateMon, 11 Jan 2016 15:28:47 +0000
reviewersjandem
bugs1236476
milestone46.0a1
Bug 1236476: Report out of memory in ExpandErrorArgumentsVA; r=jandem
js/src/frontend/TokenStream.cpp
js/src/jit-test/tests/basic/bug1236476.js
js/src/jscntxt.cpp
js/src/jscntxt.h
js/src/vm/HelperThreads.cpp
js/src/vm/HelperThreads.h
--- a/js/src/frontend/TokenStream.cpp
+++ b/js/src/frontend/TokenStream.cpp
@@ -612,17 +612,20 @@ TokenStream::reportCompileErrorNumberVA(
     if (warning && options().werrorOption) {
         flags &= ~JSREPORT_WARNING;
         warning = false;
     }
 
     // On the main thread, report the error immediately. When compiling off
     // thread, save the error so that the main thread can report it later.
     CompileError tempErr;
-    CompileError& err = cx->isJSContext() ? tempErr : cx->addPendingCompileError();
+    CompileError* tempErrPtr = &tempErr;
+    if (!cx->isJSContext() && !cx->addPendingCompileError(&tempErrPtr))
+        return false;
+    CompileError& err = *tempErrPtr;
 
     err.report.flags = flags;
     err.report.errorNumber = errorNumber;
     err.report.filename = filename;
     err.report.isMuted = mutedErrors;
     if (offset == NoOffset) {
         err.report.lineno = 0;
         err.report.column = 0;
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/basic/bug1236476.js
@@ -0,0 +1,16 @@
+// |jit-test| allow-oom; allow-unhandlable-oom
+// 1236476
+
+if (typeof oomTest !== 'function' ||
+    typeof offThreadCompileScript !== 'function' ||
+    typeof runOffThreadScript !== 'function')
+    quit();
+
+oomTest(() => {
+    offThreadCompileScript(`
+      "use asm";
+      return assertEq;
+    `);
+    runOffThreadScript();
+});
+
--- a/js/src/jscntxt.cpp
+++ b/js/src/jscntxt.cpp
@@ -286,17 +286,17 @@ js::ReportOutOfMemory(ExclusiveContext* 
      * OOMs are non-deterministic, especially across different execution modes
      * (e.g. interpreter vs JIT). In more-deterministic builds, print to stderr
      * so that the fuzzers can detect this.
      */
     fprintf(stderr, "ReportOutOfMemory called\n");
 #endif
 
     if (!cxArg->isJSContext())
-        return;
+        return cxArg->addPendingOutOfMemory();
 
     JSContext* cx = cxArg->asJSContext();
     cx->runtime()->hadOutOfMemory = true;
 
     /* Report the oom. */
     if (JS::OutOfMemoryCallback oomCallback = cx->runtime()->oomCallback) {
         AutoSuppressGC suppressGC(cx);
         oomCallback(cx, cx->runtime()->oomCallbackData);
@@ -640,17 +640,16 @@ js::ExpandErrorArgumentsVA(ExclusiveCont
                                  + totalArgsLength;
 
                 /*
                 * Note - the above calculation assumes that each argument
                 * is used once and only once in the expansion !!!
                 */
                 reportp->ucmessage = out = cx->pod_malloc<char16_t>(expandedLength + 1);
                 if (!out) {
-                    ReportOutOfMemory(cx);
                     js_free(buffer);
                     goto error;
                 }
                 while (*fmt) {
                     if (*fmt == '{') {
                         if (isdigit(fmt[1])) {
                             int d = JS7_UNDEC(fmt[1]);
                             MOZ_RELEASE_ASSERT(d < argCount);
@@ -932,23 +931,26 @@ ExclusiveContext::ExclusiveContext(JSRun
     arenas_(nullptr),
     enterCompartmentDepth_(0)
 {
 }
 
 void
 ExclusiveContext::recoverFromOutOfMemory()
 {
-    // If this is not a JSContext, there's nothing to do.
     if (JSContext* maybecx = maybeJSContext()) {
         if (maybecx->isExceptionPending()) {
             MOZ_ASSERT(maybecx->isThrowingOutOfMemory());
             maybecx->clearPendingException();
         }
+        return;
     }
+    // Keep in sync with addPendingOutOfMemory.
+    if (ParseTask* task = helperThread()->parseTask())
+        task->outOfMemory = false;
 }
 
 JSContext::JSContext(JSRuntime* rt)
   : ExclusiveContext(rt, &rt->mainThread, Context_JS),
     throwing(false),
     unwrappedException_(this),
     options_(),
     overRecursed_(false),
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -160,18 +160,20 @@ class ExclusiveContext : public ContextF
     }
 
     template <typename T>
     inline bool isInsideCurrentCompartment(T thing) const {
         return thing->compartment() == compartment_;
     }
 
     void* onOutOfMemory(js::AllocFunction allocFunc, size_t nbytes, void* reallocPtr = nullptr) {
-        if (!isJSContext())
+        if (!isJSContext()) {
+            addPendingOutOfMemory();
             return nullptr;
+        }
         return runtime_->onOutOfMemory(allocFunc, nbytes, reallocPtr, asJSContext());
     }
 
     /* Clear the pending exception (if any) due to OOM. */
     void recoverFromOutOfMemory();
 
     inline void updateMallocCounter(size_t nbytes) {
         // Note: this is racy.
@@ -275,18 +277,19 @@ class ExclusiveContext : public ContextF
     SymbolRegistry& symbolRegistry() {
         return runtime_->symbolRegistry();
     }
     ScriptDataTable& scriptDataTable() {
         return runtime_->scriptDataTable();
     }
 
     // Methods specific to any HelperThread for the context.
-    frontend::CompileError& addPendingCompileError();
+    bool addPendingCompileError(frontend::CompileError** err);
     void addPendingOverRecursed();
+    void addPendingOutOfMemory();
 };
 
 } /* namespace js */
 
 struct JSContext : public js::ExclusiveContext,
                    public mozilla::LinkedListElement<JSContext>
 {
     explicit JSContext(JSRuntime* rt);
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -23,16 +23,17 @@
 #include "jscompartmentinlines.h"
 #include "jsobjinlines.h"
 #include "jsscriptinlines.h"
 
 using namespace js;
 
 using mozilla::ArrayLength;
 using mozilla::DebugOnly;
+using mozilla::UniquePtr;
 
 namespace js {
 
 GlobalHelperThreadState* gHelperThreadState = nullptr;
 
 } // namespace js
 
 bool
@@ -198,17 +199,17 @@ static const JSClass parseTaskGlobalClas
 ParseTask::ParseTask(ExclusiveContext* cx, JSObject* exclusiveContextGlobal, JSContext* initCx,
                      const char16_t* chars, size_t length,
                      JS::OffThreadCompileCallback callback, void* callbackData)
   : cx(cx), options(initCx), chars(chars), length(length),
     alloc(JSRuntime::TEMP_LIFO_ALLOC_PRIMARY_CHUNK_SIZE),
     exclusiveContextGlobal(initCx->runtime(), exclusiveContextGlobal),
     callback(callback), callbackData(callbackData),
     script(initCx->runtime()), sourceObject(initCx->runtime()),
-    errors(cx), overRecursed(false)
+    errors(cx), overRecursed(false), outOfMemory(false)
 {
 }
 
 bool
 ParseTask::init(JSContext* cx, const ReadOnlyCompileOptions& options)
 {
     if (!this->options.copy(cx, options))
         return false;
@@ -1053,16 +1054,22 @@ GlobalHelperThreadState::finishParseTask
     mergeParseTaskCompartment(rt, parseTask, global, cx->compartment());
 
     if (!parseTask->finish(cx))
         return nullptr;
 
     RootedScript script(rt, parseTask->script);
     assertSameCompartment(cx, script);
 
+    // Report out of memory errors eagerly, or errors could be malformed.
+    if (parseTask->outOfMemory) {
+        ReportOutOfMemory(cx);
+        return nullptr;
+    }
+
     // Report any error or warnings generated during the parse, and inform the
     // debugger about the compiled scripts.
     for (size_t i = 0; i < parseTask->errors.length(); i++)
         parseTask->errors[i]->throwError(cx);
     if (parseTask->overRecursed)
         ReportOverRecursed(cx);
     if (cx->isExceptionPending())
         return nullptr;
@@ -1336,34 +1343,44 @@ js::PauseCurrentHelperThread()
 
 void
 ExclusiveContext::setHelperThread(HelperThread* thread)
 {
     helperThread_ = thread;
     perThreadData = thread->threadData.ptr();
 }
 
-frontend::CompileError&
-ExclusiveContext::addPendingCompileError()
+bool
+ExclusiveContext::addPendingCompileError(frontend::CompileError** error)
 {
-    AutoEnterOOMUnsafeRegion oomUnsafe;
-    frontend::CompileError* error = js_new<frontend::CompileError>();
-    if (!error || !helperThread()->parseTask()->errors.append(error))
-        oomUnsafe.crash("ExclusiveContext::addPendingCompileError");
-    return *error;
+    UniquePtr<frontend::CompileError> errorPtr(new_<frontend::CompileError>());
+    if (!errorPtr)
+        return false;
+    if (!helperThread()->parseTask()->errors.append(errorPtr.get()))
+        return false;
+    *error = errorPtr.release();
+    return true;
 }
 
 void
 ExclusiveContext::addPendingOverRecursed()
 {
     if (helperThread()->parseTask())
         helperThread()->parseTask()->overRecursed = true;
 }
 
 void
+ExclusiveContext::addPendingOutOfMemory()
+{
+    // Keep in sync with recoverFromOutOfMemory.
+    if (helperThread()->parseTask())
+        helperThread()->parseTask()->outOfMemory = true;
+}
+
+void
 HelperThread::handleParseWorkload()
 {
     MOZ_ASSERT(HelperThreadState().isLocked());
     MOZ_ASSERT(HelperThreadState().canStartParseTask());
     MOZ_ASSERT(idle());
 
     currentTask.emplace(HelperThreadState().parseWorklist().popCopy());
     ParseTask* task = parseTask();
--- a/js/src/vm/HelperThreads.h
+++ b/js/src/vm/HelperThreads.h
@@ -483,16 +483,17 @@ struct ParseTask
 
     // Holds the ScriptSourceObject generated for the script compilation.
     PersistentRooted<ScriptSourceObject*> sourceObject;
 
     // Any errors or warnings produced during compilation. These are reported
     // when finishing the script.
     Vector<frontend::CompileError*> errors;
     bool overRecursed;
+    bool outOfMemory;
 
     ParseTask(ExclusiveContext* cx, JSObject* exclusiveContextGlobal,
               JSContext* initCx, const char16_t* chars, size_t length,
               JS::OffThreadCompileCallback callback, void* callbackData);
     bool init(JSContext* cx, const ReadOnlyCompileOptions& options);
 
     void activate(JSRuntime* rt);
     bool finish(JSContext* cx);