Bug 1022773 - Resolve hazard by switching Compile to MutableHandle, r=terrence, a=abillings
☠☠ backed out by ee727fe51f06 ☠ ☠
authorSteve Fink <sfink@mozilla.com>
Wed, 11 Jun 2014 17:38:22 -0700
changeset 207538 5b44fa1cea34cbc7218d590b23128bd03a265899
parent 207537 c96dec196ae61cbf527e9f32b9bb43955106ba96
child 207539 60e8b99055e9da53abf469ece518f6e61af48008
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence, abillings
bugs1022773
milestone32.0a2
Bug 1022773 - Resolve hazard by switching Compile to MutableHandle, r=terrence, a=abillings
content/base/src/nsFrameMessageManager.cpp
content/xul/content/src/nsXULElement.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/xpconnect/loader/mozJSSubScriptLoader.cpp
js/xpconnect/loader/mozJSSubScriptLoader.h
--- a/content/base/src/nsFrameMessageManager.cpp
+++ b/content/base/src/nsFrameMessageManager.cpp
@@ -1526,23 +1526,25 @@ nsFrameScriptExecutor::TryCacheLoadAndCo
     if (global) {
       JSAutoCompartment ac(cx, global);
       JS::CompileOptions options(cx);
       options.setFileAndLine(url.get(), 1);
       JS::Rooted<JSScript*> script(cx);
       JS::Rooted<JSObject*> funobj(cx);
       if (aRunInGlobalScope) {
         options.setNoScriptRval(true);
-        script = JS::Compile(cx, JS::NullPtr(), options, srcBuf);
+        if (!JS::Compile(cx, JS::NullPtr(), options, srcBuf, &script)) {
+          return;
+        }
       } else {
         JS::Rooted<JSFunction *> fun(cx);
-        fun = JS::CompileFunction(cx, JS::NullPtr(), options,
-                                  nullptr, 0, nullptr, /* name, nargs, args */
-                                  srcBuf);
-        if (!fun) {
+        if (!JS::CompileFunction(cx, JS::NullPtr(), options,
+                                 nullptr, 0, nullptr, /* name, nargs, args */
+                                 srcBuf, &fun))
+        {
           return;
         }
         funobj = JS_GetFunctionObject(fun);
       }
 
       if (!script && !funobj) {
         return;
       }
--- a/content/xul/content/src/nsXULElement.cpp
+++ b/content/xul/content/src/nsXULElement.cpp
@@ -2753,18 +2753,18 @@ nsXULPrototypeScript::Compile(JS::Source
                                   aSrcBuf.get(), aSrcBuf.length(),
                                   OffThreadScriptReceiverCallback,
                                   static_cast<void*>(aOffThreadReceiver))) {
             return NS_ERROR_OUT_OF_MEMORY;
         }
         // This reference will be consumed by the NotifyOffThreadScriptCompletedRunnable.
         NS_ADDREF(aOffThreadReceiver);
     } else {
-        JSScript* script = JS::Compile(cx, scope, options, aSrcBuf);
-        if (!script)
+        JS::Rooted<JSScript*> script(cx);
+        if (!JS::Compile(cx, scope, options, aSrcBuf, &script))
             return NS_ERROR_OUT_OF_MEMORY;
         Set(script);
     }
     return NS_OK;
 }
 
 nsresult
 nsXULPrototypeScript::Compile(const char16_t* aText,
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4607,35 +4607,39 @@ JS::CompileOptions::wrap(JSContext *cx, 
     if (introductionScriptRoot) {
         if (introductionScriptRoot->compartment() != compartment)
             introductionScriptRoot = nullptr;
     }
 
     return true;
 }
 
-JSScript *
+bool
 JS::Compile(JSContext *cx, HandleObject obj, const ReadOnlyCompileOptions &options,
-            SourceBufferHolder &srcBuf)
+            SourceBufferHolder &srcBuf, MutableHandleScript script)
 {
     JS_ASSERT(!cx->runtime()->isAtomsCompartment(cx->compartment()));
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj);
     AutoLastFrameCheck lfc(cx);
 
-    return frontend::CompileScript(cx, &cx->tempLifoAlloc(), obj, NullPtr(), options, srcBuf);
+    script.set(frontend::CompileScript(cx, &cx->tempLifoAlloc(), obj, NullPtr(), options, srcBuf));
+    return !!script;
 }
 
 JSScript *
 JS::Compile(JSContext *cx, HandleObject obj, const ReadOnlyCompileOptions &options,
             const jschar *chars, size_t length)
 {
     SourceBufferHolder srcBuf(chars, length, SourceBufferHolder::NoOwnership);
-    return Compile(cx, obj, options, srcBuf);
+    RootedScript script(cx);
+    if (!Compile(cx, obj, options, srcBuf, &script))
+        return nullptr;
+    return script;
 }
 
 JSScript *
 JS::Compile(JSContext *cx, HandleObject obj, const ReadOnlyCompileOptions &options,
             const char *bytes, size_t length)
 {
     jschar *chars;
     if (options.utf8)
@@ -4664,18 +4668,17 @@ JS::Compile(JSContext *cx, HandleObject 
 JSScript *
 JS::Compile(JSContext *cx, HandleObject obj, const ReadOnlyCompileOptions &optionsArg, const char *filename)
 {
     AutoFile file;
     if (!file.open(cx, filename))
         return nullptr;
     CompileOptions options(cx, optionsArg);
     options.setFileAndLine(filename, 1);
-    JSScript *script = Compile(cx, obj, options, file.fp());
-    return script;
+    return Compile(cx, obj, options, file.fp());
 }
 
 JS_PUBLIC_API(bool)
 JS::CanCompileOffThread(JSContext *cx, const ReadOnlyCompileOptions &options, size_t length)
 {
     static const size_t TINY_LENGTH = 1000;
     static const size_t HUGE_LENGTH = 100 * 1000;
 
@@ -4778,66 +4781,69 @@ JS_BufferIsCompilableUnit(JSContext *cx,
 
 JS_PUBLIC_API(JSObject *)
 JS_GetGlobalFromScript(JSScript *script)
 {
     JS_ASSERT(!script->isCachedEval());
     return &script->global();
 }
 
-JS_PUBLIC_API(JSFunction *)
+JS_PUBLIC_API(bool)
 JS::CompileFunction(JSContext *cx, HandleObject obj, const ReadOnlyCompileOptions &options,
                     const char *name, unsigned nargs, const char *const *argnames,
-                    SourceBufferHolder &srcBuf)
+                    SourceBufferHolder &srcBuf, MutableHandleFunction fun)
 {
     JS_ASSERT(!cx->runtime()->isAtomsCompartment(cx->compartment()));
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, obj);
     AutoLastFrameCheck lfc(cx);
 
     RootedAtom funAtom(cx);
     if (name) {
         funAtom = Atomize(cx, name, strlen(name));
         if (!funAtom)
-            return nullptr;
+            return false;
     }
 
     AutoNameVector formals(cx);
     for (unsigned i = 0; i < nargs; i++) {
         RootedAtom argAtom(cx, Atomize(cx, argnames[i], strlen(argnames[i])));
         if (!argAtom || !formals.append(argAtom->asPropertyName()))
-            return nullptr;
+            return false;
     }
 
-    RootedFunction fun(cx, NewFunction(cx, NullPtr(), nullptr, 0, JSFunction::INTERPRETED, obj,
-                                       funAtom, JSFunction::FinalizeKind, TenuredObject));
+    fun.set(NewFunction(cx, NullPtr(), nullptr, 0, JSFunction::INTERPRETED, obj,
+                        funAtom, JSFunction::FinalizeKind, TenuredObject));
     if (!fun)
-        return nullptr;
-
-    if (!frontend::CompileFunctionBody(cx, &fun, options, formals, srcBuf))
-        return nullptr;
+        return false;
+
+    if (!frontend::CompileFunctionBody(cx, fun, options, formals, srcBuf))
+        return false;
 
     if (obj && funAtom && options.defineOnScope) {
         Rooted<jsid> id(cx, AtomToId(funAtom));
         RootedValue value(cx, ObjectValue(*fun));
         if (!JSObject::defineGeneric(cx, obj, id, value, nullptr, nullptr, JSPROP_ENUMERATE))
-            return nullptr;
+            return false;
     }
 
-    return fun;
+    return true;
 }
 
 JS_PUBLIC_API(JSFunction *)
 JS::CompileFunction(JSContext *cx, HandleObject obj, const ReadOnlyCompileOptions &options,
                     const char *name, unsigned nargs, const char *const *argnames,
                     const jschar *chars, size_t length)
 {
-  SourceBufferHolder srcBuf(chars, length, SourceBufferHolder::NoOwnership);
-  return JS::CompileFunction(cx, obj, options, name, nargs, argnames, srcBuf);
+    RootedFunction fun(cx);
+    SourceBufferHolder srcBuf(chars, length, SourceBufferHolder::NoOwnership);
+    if (!JS::CompileFunction(cx, obj, options, name, nargs, argnames, srcBuf, &fun))
+        return nullptr;
+    return fun;
 }
 
 JS_PUBLIC_API(JSFunction *)
 JS::CompileFunction(JSContext *cx, HandleObject obj, const ReadOnlyCompileOptions &options,
                     const char *name, unsigned nargs, const char *const *argnames,
                     const char *bytes, size_t length)
 {
     jschar *chars;
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3742,29 +3742,32 @@ class MOZ_STACK_CLASS JS_FRIEND_API(Comp
     }
 
     virtual bool wrap(JSContext *cx, JSCompartment *compartment) MOZ_OVERRIDE;
 
   private:
     void operator=(const CompileOptions &rhs) MOZ_DELETE;
 };
 
+/*
+ * |script| will always be set. On failure, it will be set to nullptr.
+ */
+extern JS_PUBLIC_API(bool)
+Compile(JSContext *cx, JS::HandleObject obj, const ReadOnlyCompileOptions &options,
+        SourceBufferHolder &srcBuf, JS::MutableHandleScript script);
+
 extern JS_PUBLIC_API(JSScript *)
 Compile(JSContext *cx, JS::HandleObject obj, const ReadOnlyCompileOptions &options,
         const char *bytes, size_t length);
 
 extern JS_PUBLIC_API(JSScript *)
 Compile(JSContext *cx, JS::HandleObject obj, const ReadOnlyCompileOptions &options,
         const jschar *chars, size_t length);
 
 extern JS_PUBLIC_API(JSScript *)
-Compile(JSContext *cx, JS::HandleObject obj, const ReadOnlyCompileOptions &options,
-        SourceBufferHolder &srcBuf);
-
-extern JS_PUBLIC_API(JSScript *)
 Compile(JSContext *cx, JS::HandleObject obj, const ReadOnlyCompileOptions &options, FILE *file);
 
 extern JS_PUBLIC_API(JSScript *)
 Compile(JSContext *cx, JS::HandleObject obj, const ReadOnlyCompileOptions &options, const char *filename);
 
 extern JS_PUBLIC_API(bool)
 CanCompileOffThread(JSContext *cx, const ReadOnlyCompileOptions &options, size_t length);
 
@@ -3787,31 +3790,31 @@ CanCompileOffThread(JSContext *cx, const
 extern JS_PUBLIC_API(bool)
 CompileOffThread(JSContext *cx, const ReadOnlyCompileOptions &options,
                  const jschar *chars, size_t length,
                  OffThreadCompileCallback callback, void *callbackData);
 
 extern JS_PUBLIC_API(JSScript *)
 FinishOffThreadScript(JSContext *maybecx, JSRuntime *rt, void *token);
 
+extern JS_PUBLIC_API(bool)
+CompileFunction(JSContext *cx, JS::HandleObject obj, const ReadOnlyCompileOptions &options,
+                const char *name, unsigned nargs, const char *const *argnames,
+                SourceBufferHolder &srcBuf, JS::MutableHandleFunction fun);
+
 extern JS_PUBLIC_API(JSFunction *)
 CompileFunction(JSContext *cx, JS::HandleObject obj, const ReadOnlyCompileOptions &options,
                 const char *name, unsigned nargs, const char *const *argnames,
                 const char *bytes, size_t length);
 
 extern JS_PUBLIC_API(JSFunction *)
 CompileFunction(JSContext *cx, JS::HandleObject obj, const ReadOnlyCompileOptions &options,
                 const char *name, unsigned nargs, const char *const *argnames,
                 const jschar *chars, size_t length);
 
-extern JS_PUBLIC_API(JSFunction *)
-CompileFunction(JSContext *cx, JS::HandleObject obj, const ReadOnlyCompileOptions &options,
-                const char *name, unsigned nargs, const char *const *argnames,
-                SourceBufferHolder &srcBuf);
-
 } /* namespace JS */
 
 extern JS_PUBLIC_API(JSString *)
 JS_DecompileScript(JSContext *cx, JS::Handle<JSScript*> script, const char *name, unsigned indent);
 
 /*
  * API extension: OR this into indent to avoid pretty-printing the decompiled
  * source resulting from JS_DecompileFunction{,Body}.
--- a/js/xpconnect/loader/mozJSSubScriptLoader.cpp
+++ b/js/xpconnect/loader/mozJSSubScriptLoader.cpp
@@ -90,23 +90,23 @@ ReportError(JSContext *cx, const char *m
     JS_SetPendingException(cx, exn);
     return NS_OK;
 }
 
 nsresult
 mozJSSubScriptLoader::ReadScript(nsIURI *uri, JSContext *cx, JSObject *targetObjArg,
                                  const nsAString &charset, const char *uriStr,
                                  nsIIOService *serv, nsIPrincipal *principal,
-                                 bool reuseGlobal, JSScript **scriptp,
-                                 JSFunction **functionp)
+                                 bool reuseGlobal, JS::MutableHandleScript script,
+                                 JS::MutableHandleFunction function)
 {
     RootedObject target_obj(cx, targetObjArg);
 
-    *scriptp = nullptr;
-    *functionp = nullptr;
+    script.set(nullptr);
+    function.set(nullptr);
 
     // Instead of calling NS_OpenURI, we create the channel ourselves and call
     // SetContentType, to avoid expensive MIME type lookups (bug 632490).
     nsCOMPtr<nsIChannel> chan;
     nsCOMPtr<nsIInputStream> instream;
     nsresult rv = NS_NewChannel(getter_AddRefs(chan), uri, serv,
                                 nullptr, nullptr, nsIRequest::LOAD_NORMAL);
     if (NS_SUCCEEDED(rv)) {
@@ -150,32 +150,33 @@ mozJSSubScriptLoader::ReadScript(nsIURI 
         JS::SourceBufferHolder srcBuf(scriptBuf, scriptLength,
                                       JS::SourceBufferHolder::GiveOwnership);
 
         if (NS_FAILED(rv)) {
             return ReportError(cx, LOAD_ERROR_BADCHARSET);
         }
 
         if (!reuseGlobal) {
-            *scriptp = JS::Compile(cx, target_obj, options, srcBuf);
+            JS::Compile(cx, target_obj, options, srcBuf, script);
         } else {
-            *functionp = JS::CompileFunction(cx, target_obj, options,
-                                             nullptr, 0, nullptr,
-                                             srcBuf);
+            JS::CompileFunction(cx, target_obj, options,
+                                nullptr, 0, nullptr,
+                                srcBuf,
+                                function);
         }
     } else {
         // We only use lazy source when no special encoding is specified because
         // the lazy source loader doesn't know the encoding.
         if (!reuseGlobal) {
             options.setSourceIsLazy(true);
-            *scriptp = JS::Compile(cx, target_obj, options, buf.get(), len);
+            script.set(JS::Compile(cx, target_obj, options, buf.get(), len));
         } else {
-            *functionp = JS::CompileFunction(cx, target_obj, options,
+            function.set(JS::CompileFunction(cx, target_obj, options,
                                              nullptr, 0, nullptr, buf.get(),
-                                             len);
+                                             len));
         }
     }
 
     /* repent for our evil deeds */
     JS_SetErrorReporter(cx, er);
 
     return NS_OK;
 }
@@ -328,17 +329,17 @@ mozJSSubScriptLoader::DoLoadSubScriptWit
 
     RootedFunction function(cx);
     RootedScript script(cx);
     if (cache && !options.ignoreCache)
         rv = ReadCachedScript(cache, cachePath, cx, mSystemPrincipal, &script);
     if (!script) {
         rv = ReadScript(uri, cx, targetObj, options.charset,
                         static_cast<const char*>(uriStr.get()), serv,
-                        principal, reusingGlobal, script.address(), function.address());
+                        principal, reusingGlobal, &script, &function);
         writeScript = !!script;
     }
 
     if (NS_FAILED(rv) || (!script && !function))
         return rv;
 
     if (function) {
         script = JS_GetFunctionScript(cx, function);
--- a/js/xpconnect/loader/mozJSSubScriptLoader.h
+++ b/js/xpconnect/loader/mozJSSubScriptLoader.h
@@ -30,18 +30,18 @@ public:
     // all the interface method declarations...
     NS_DECL_ISUPPORTS
     NS_DECL_MOZIJSSUBSCRIPTLOADER
 
 private:
     nsresult ReadScript(nsIURI *uri, JSContext *cx, JSObject *target_obj,
                         const nsAString &charset, const char *uriStr,
                         nsIIOService *serv, nsIPrincipal *principal,
-                        bool reuseGlobal, JSScript **scriptp,
-                        JSFunction **functionp);
+                        bool reuseGlobal, JS::MutableHandleScript script,
+                        JS::MutableHandleFunction function);
 
     nsresult DoLoadSubScriptWithOptions(const nsAString &url,
                                         LoadSubScriptOptions  &options,
                                         JSContext *cx,
                                         JS::MutableHandle<JS::Value> retval);
 
     nsCOMPtr<nsIPrincipal> mSystemPrincipal;
 };