Bug 1447372 - Cleanup LifoAlloc usage for BytecodeCompiler r=jorendorff

Make off-thread and main-thread parsing more consistent by using the
JSContext tempLifoAlloc always. This also makes BytecodeCompiler APIs
more consistent.

Remove BytecodeCompiler::alloc which was obscuring lifetimes and
conflicts of allocator.

Differential Revision: https://phabricator.services.mozilla.com/D9978

--HG--
extra : moz-landing-system : lando

js/src/builtin/Eval.cpp

@@ -322,9 +322,7 @@ EvalKernel(JSContext* cx, HandleValue v, EvalType evalType, AbstractFramePtr cal
                                                   ? SourceBufferHolder::GiveOwnership
                                                   : SourceBufferHolder::NoOwnership;
         SourceBufferHolder srcBuf(chars, linearStr->length(), ownership);
-        JSScript* compiled = frontend::CompileEvalScript(cx, cx->tempLifoAlloc(),
-                                                         env, enclosing,
-                                                         options, srcBuf);
+        JSScript* compiled = frontend::CompileEvalScript(cx, env, enclosing, options, srcBuf);
         if (!compiled) {
             return false;
         }
@@ -408,9 +406,7 @@ js::DirectEvalStringFromIon(JSContext* cx,
                                                   ? SourceBufferHolder::GiveOwnership
                                                   : SourceBufferHolder::NoOwnership;
         SourceBufferHolder srcBuf(chars, linearStr->length(), ownership);
-        JSScript* compiled = frontend::CompileEvalScript(cx, cx->tempLifoAlloc(),
-                                                         env, enclosing,
-                                                         options, srcBuf);
+        JSScript* compiled = frontend::CompileEvalScript(cx, env, enclosing, options, srcBuf);
         if (!compiled) {
             return false;
         }

js/src/frontend/BytecodeCompiler.cpp

@@ -45,7 +45,6 @@ class MOZ_STACK_CLASS BytecodeCompiler
   public:
     // Construct an object passing mandatory arguments.
     BytecodeCompiler(JSContext* cx,
-                     LifoAlloc& alloc,
                      const ReadOnlyCompileOptions& options,
                      SourceBufferHolder& sourceBuffer,
                      HandleScope enclosingScope);
@@ -83,7 +82,6 @@ class MOZ_STACK_CLASS BytecodeCompiler
     AutoKeepAtoms keepAtoms;
 
     JSContext* cx;
-    LifoAlloc& alloc;
     const ReadOnlyCompileOptions& options;
     SourceBufferHolder& sourceBuffer;
 
@@ -154,13 +152,11 @@ AutoFrontendTraceLog::AutoFrontendTraceLog(JSContext* cx, const TraceLoggerTextI
 #endif
 
 BytecodeCompiler::BytecodeCompiler(JSContext* cx,
-                                   LifoAlloc& alloc,
                                    const ReadOnlyCompileOptions& options,
                                    SourceBufferHolder& sourceBuffer,
                                    HandleScope enclosingScope)
   : keepAtoms(cx),
     cx(cx),
-    alloc(alloc),
     options(options),
     sourceBuffer(sourceBuffer),
     enclosingScope(cx, enclosingScope),
@@ -233,15 +229,15 @@ BytecodeCompiler::createParser(ParseGoal goal)
     usedNames.emplace(cx);
 
     if (canLazilyParse()) {
-        syntaxParser.emplace(cx, alloc, options, sourceBuffer.get(), sourceBuffer.length(),
-                             /* foldConstants = */ false, *usedNames, nullptr, nullptr,
-                             sourceObject, goal);
+        syntaxParser.emplace(cx, cx->tempLifoAlloc(), options, sourceBuffer.get(),
+                             sourceBuffer.length(), /* foldConstants = */ false,
+                             *usedNames, nullptr, nullptr, sourceObject, goal);
         if (!syntaxParser->checkOptions()) {
             return false;
         }
     }
 
-    parser.emplace(cx, alloc, options, sourceBuffer.get(), sourceBuffer.length(),
+    parser.emplace(cx, cx->tempLifoAlloc(), options, sourceBuffer.get(), sourceBuffer.length(),
                    /* foldConstants = */ true, *usedNames, syntaxParser.ptrOr(nullptr), nullptr,
                    sourceObject, goal);
     parser->ss = scriptSource;
@@ -643,14 +639,14 @@ class MOZ_RAII AutoAssertReportedException
 };
 
 JSScript*
-frontend::CompileGlobalScript(JSContext* cx, LifoAlloc& alloc, ScopeKind scopeKind,
+frontend::CompileGlobalScript(JSContext* cx, ScopeKind scopeKind,
                               const ReadOnlyCompileOptions& options,
                               SourceBufferHolder& srcBuf,
                               ScriptSourceObject** sourceObjectOut)
 {
     MOZ_ASSERT(scopeKind == ScopeKind::Global || scopeKind == ScopeKind::NonSyntactic);
     AutoAssertReportedException assertException(cx);
-    BytecodeCompiler compiler(cx, alloc, options, srcBuf, /* enclosingScope = */ nullptr);
+    BytecodeCompiler compiler(cx, options, srcBuf, /* enclosingScope = */ nullptr);
     AutoInitializeSourceObject autoSSO(compiler, sourceObjectOut);
     JSScript* script = compiler.compileGlobalScript(scopeKind);
     if (!script) {
@@ -724,14 +720,14 @@ frontend::CompileGlobalBinASTScript(JSContext* cx, LifoAlloc& alloc, const ReadO
 #endif // JS_BUILD_BINAST
 
 JSScript*
-frontend::CompileEvalScript(JSContext* cx, LifoAlloc& alloc,
-                            HandleObject environment, HandleScope enclosingScope,
+frontend::CompileEvalScript(JSContext* cx, HandleObject environment,
+                            HandleScope enclosingScope,
                             const ReadOnlyCompileOptions& options,
                             SourceBufferHolder& srcBuf,
                             ScriptSourceObject** sourceObjectOut)
 {
     AutoAssertReportedException assertException(cx);
-    BytecodeCompiler compiler(cx, alloc, options, srcBuf, enclosingScope);
+    BytecodeCompiler compiler(cx, options, srcBuf, enclosingScope);
     AutoInitializeSourceObject autoSSO(compiler, sourceObjectOut);
     JSScript* script = compiler.compileEvalScript(environment, enclosingScope);
     if (!script) {
@@ -744,7 +740,7 @@ frontend::CompileEvalScript(JSContext* cx, LifoAlloc& alloc,
 
 ModuleObject*
 frontend::CompileModule(JSContext* cx, const ReadOnlyCompileOptions& optionsInput,
-                        SourceBufferHolder& srcBuf, LifoAlloc& alloc,
+                        SourceBufferHolder& srcBuf,
                         ScriptSourceObject** sourceObjectOut)
 {
     MOZ_ASSERT(srcBuf.get());
@@ -758,7 +754,7 @@ frontend::CompileModule(JSContext* cx, const ReadOnlyCompileOptions& optionsInpu
     options.allowHTMLComments = false;
 
     RootedScope emptyGlobalScope(cx, &cx->global()->emptyGlobalScope());
-    BytecodeCompiler compiler(cx, alloc, options, srcBuf, emptyGlobalScope);
+    BytecodeCompiler compiler(cx, options, srcBuf, emptyGlobalScope);
     AutoInitializeSourceObject autoSSO(compiler, sourceObjectOut);
     ModuleObject* module = compiler.compileModule();
     if (!module) {
@@ -779,8 +775,7 @@ frontend::CompileModule(JSContext* cx, const JS::ReadOnlyCompileOptions& options
         return nullptr;
     }
 
-    LifoAlloc& alloc = cx->tempLifoAlloc();
-    RootedModuleObject module(cx, CompileModule(cx, options, srcBuf, alloc));
+    RootedModuleObject module(cx, CompileModule(cx, options, srcBuf, nullptr));
     if (!module) {
         return nullptr;
     }
@@ -1011,7 +1006,7 @@ frontend::CompileStandaloneFunction(JSContext* cx, MutableHandleFunction fun,
         scope = &cx->global()->emptyGlobalScope();
     }
 
-    BytecodeCompiler compiler(cx, cx->tempLifoAlloc(), options, srcBuf, scope);
+    BytecodeCompiler compiler(cx, options, srcBuf, scope);
     if (!compiler.compileStandaloneFunction(fun, GeneratorKind::NotGenerator,
                                             FunctionAsyncKind::SyncFunction,
                                             parameterListEnd))
@@ -1033,7 +1028,7 @@ frontend::CompileStandaloneGenerator(JSContext* cx, MutableHandleFunction fun,
 
     RootedScope emptyGlobalScope(cx, &cx->global()->emptyGlobalScope());
 
-    BytecodeCompiler compiler(cx, cx->tempLifoAlloc(), options, srcBuf, emptyGlobalScope);
+    BytecodeCompiler compiler(cx, options, srcBuf, emptyGlobalScope);
     if (!compiler.compileStandaloneFunction(fun, GeneratorKind::Generator,
                                             FunctionAsyncKind::SyncFunction,
                                             parameterListEnd))
@@ -1055,7 +1050,7 @@ frontend::CompileStandaloneAsyncFunction(JSContext* cx, MutableHandleFunction fu
 
     RootedScope emptyGlobalScope(cx, &cx->global()->emptyGlobalScope());
 
-    BytecodeCompiler compiler(cx, cx->tempLifoAlloc(), options, srcBuf, emptyGlobalScope);
+    BytecodeCompiler compiler(cx, options, srcBuf, emptyGlobalScope);
     if (!compiler.compileStandaloneFunction(fun, GeneratorKind::NotGenerator,
                                             FunctionAsyncKind::AsyncFunction,
                                             parameterListEnd))
@@ -1077,7 +1072,7 @@ frontend::CompileStandaloneAsyncGenerator(JSContext* cx, MutableHandleFunction f
 
     RootedScope emptyGlobalScope(cx, &cx->global()->emptyGlobalScope());
 
-    BytecodeCompiler compiler(cx, cx->tempLifoAlloc(), options, srcBuf, emptyGlobalScope);
+    BytecodeCompiler compiler(cx, options, srcBuf, emptyGlobalScope);
     if (!compiler.compileStandaloneFunction(fun, GeneratorKind::Generator,
                                             FunctionAsyncKind::AsyncFunction,
                                             parameterListEnd))

js/src/frontend/BytecodeCompiler.h

@@ -20,7 +20,6 @@ class JSLinearString;
 namespace js {
 
 class LazyScript;
-class LifoAlloc;
 class ModuleObject;
 class ScriptSourceObject;
 
@@ -31,7 +30,7 @@ class FunctionBox;
 class ParseNode;
 
 JSScript*
-CompileGlobalScript(JSContext* cx, LifoAlloc& alloc, ScopeKind scopeKind,
+CompileGlobalScript(JSContext* cx, ScopeKind scopeKind,
                     const JS::ReadOnlyCompileOptions& options,
                     JS::SourceBufferHolder& srcBuf,
                     ScriptSourceObject** sourceObjectOut = nullptr);
@@ -50,8 +49,8 @@ CompileLazyBinASTFunction(JSContext* cx, Handle<LazyScript*> lazy, const uint8_t
 #endif // JS_BUILD_BINAST
 
 JSScript*
-CompileEvalScript(JSContext* cx, LifoAlloc& alloc,
-                  HandleObject scopeChain, HandleScope enclosingScope,
+CompileEvalScript(JSContext* cx, HandleObject environment,
+                  HandleScope enclosingScope,
                   const JS::ReadOnlyCompileOptions& options,
                   JS::SourceBufferHolder& srcBuf,
                   ScriptSourceObject** sourceObjectOut = nullptr);
@@ -62,8 +61,8 @@ CompileModule(JSContext* cx, const JS::ReadOnlyCompileOptions& options,
 
 ModuleObject*
 CompileModule(JSContext* cx, const JS::ReadOnlyCompileOptions& options,
-              JS::SourceBufferHolder& srcBuf, LifoAlloc& alloc,
-              ScriptSourceObject** sourceObjectOut = nullptr);
+              JS::SourceBufferHolder& srcBuf,
+              ScriptSourceObject** sourceObjectOut);
 
 MOZ_MUST_USE bool
 CompileLazyFunction(JSContext* cx, Handle<LazyScript*> lazy, const char16_t* chars, size_t length);

js/src/vm/CompilationAndEvaluation.cpp

@@ -51,7 +51,7 @@ CompileSourceBuffer(JSContext* cx, const ReadOnlyCompileOptions& options,
     AssertHeapIsIdle();
     CHECK_THREAD(cx);
 
-    script.set(frontend::CompileGlobalScript(cx, cx->tempLifoAlloc(), scopeKind, options, srcBuf));
+    script.set(frontend::CompileGlobalScript(cx, scopeKind, options, srcBuf));
     return !!script;
 }
 
@@ -491,8 +491,7 @@ Evaluate(JSContext* cx, ScopeKind scopeKind, HandleObject env,
     MOZ_ASSERT_IF(!IsGlobalLexicalEnvironment(env), scopeKind == ScopeKind::NonSyntactic);
 
     options.setIsRunOnce(true);
-    RootedScript script(cx, frontend::CompileGlobalScript(cx, cx->tempLifoAlloc(),
-                                                          scopeKind, options, srcBuf));
+    RootedScript script(cx, frontend::CompileGlobalScript(cx, scopeKind, options, srcBuf));
     if (!script) {
         return false;
     }

js/src/vm/Debugger.cpp

@@ -8903,8 +8903,7 @@ EvaluateInEnv(JSContext* cx, Handle<Env*> env, AbstractFramePtr frame,
         if (!scope) {
             return false;
         }
-        script = frontend::CompileEvalScript(cx, cx->tempLifoAlloc(), env, scope,
-                                             options, srcBuf);
+        script = frontend::CompileEvalScript(cx, env, scope, options, srcBuf);
         if (script) {
             script->setActiveEval();
         }
@@ -8914,8 +8913,7 @@ EvaluateInEnv(JSContext* cx, Handle<Env*> env, AbstractFramePtr frame,
         // circumvent the fresh lexical scope that all eval have, so that the
         // users of executeInGlobal, like the web console, may add new bindings to
         // the global scope.
-        script = frontend::CompileGlobalScript(cx, cx->tempLifoAlloc(), scopeKind, options,
-                                               srcBuf);
+        script = frontend::CompileGlobalScript(cx, scopeKind, options, srcBuf);
     }
 
     if (!script) {

js/src/vm/HelperThreads.cpp

@@ -568,8 +568,7 @@ ScriptParseTask::parse(JSContext* cx)
 
     ScopeKind scopeKind = options.nonSyntacticScope ? ScopeKind::NonSyntactic : ScopeKind::Global;
 
-    JSScript* script = frontend::CompileGlobalScript(cx, cx->tempLifoAlloc(), scopeKind,
-                                                     options, data,
+    JSScript* script = frontend::CompileGlobalScript(cx, scopeKind, options, data,
                                                      /* sourceObjectOut = */ &sourceObject.get());
     if (script) {
         scripts.infallibleAppend(script);
@@ -592,7 +591,7 @@ ModuleParseTask::parse(JSContext* cx)
 
     Rooted<ScriptSourceObject*> sourceObject(cx);
 
-    ModuleObject* module = frontend::CompileModule(cx, options, data, cx->tempLifoAlloc(), &sourceObject.get());
+    ModuleObject* module = frontend::CompileModule(cx, options, data, &sourceObject.get());
     if (module) {
         scripts.infallibleAppend(module->script());
         if (sourceObject) {
@@ -615,8 +614,7 @@ ScriptDecodeTask::parse(JSContext* cx)
     RootedScript resultScript(cx);
     Rooted<ScriptSourceObject*> sourceObject(cx);
 
-    XDROffThreadDecoder decoder(cx, cx->tempLifoAlloc(), &options,
-                                /* sourceObjectOut = */ &sourceObject.get(), range);
+    XDROffThreadDecoder decoder(cx, &options, /* sourceObjectOut = */ &sourceObject.get(), range);
     XDRResult res = decoder.codeScript(&resultScript);
     MOZ_ASSERT(bool(resultScript) == res.isOk());
     if (res.isOk()) {
@@ -681,7 +679,7 @@ MultiScriptsDecodeTask::parse(JSContext* cx)
         RootedScript resultScript(cx);
         Rooted<ScriptSourceObject*> sourceObject(cx);
 
-        XDROffThreadDecoder decoder(cx, cx->tempLifoAlloc(), &opts, &sourceObject.get(),
+        XDROffThreadDecoder decoder(cx, &opts, &sourceObject.get(),
                                     source.range);
         XDRResult res = decoder.codeScript(&resultScript);
         MOZ_ASSERT(bool(resultScript) == res.isOk());
@@ -2310,6 +2308,8 @@ HelperThread::handleParseWorkload(AutoLockHelperThreadState& locked)
 
         task->parse(cx);
 
+        MOZ_ASSERT(cx->tempLifoAlloc().isEmpty());
+        cx->tempLifoAlloc().freeAll();
         cx->frontendCollectionPool().purge();
         cx->atomsZoneFreeLists().clear();
     }

js/src/vm/Xdr.cpp

@@ -30,12 +30,6 @@ using namespace js;
 using mozilla::ArrayEqual;
 using mozilla::Utf8Unit;
 
-template<XDRMode mode>
-LifoAlloc&
-XDRState<mode>::lifoAlloc() const {
-    return buf.cx()->tempLifoAlloc();
-}
-
 #ifdef DEBUG
 bool
 XDRCoderBase::validateResultCode(JSContext* cx, JS::TranscodeResult code) const

js/src/vm/Xdr.h

@@ -254,7 +254,6 @@ class XDRState : public XDRCoderBase
     JSContext* cx() const {
         return buf.cx();
     }
-    virtual LifoAlloc& lifoAlloc() const;
 
     virtual bool hasOptions() const { return false; }
     virtual const JS::ReadOnlyCompileOptions& options() {
@@ -508,7 +507,6 @@ class XDROffThreadDecoder : public XDRDecoder
 {
     const JS::ReadOnlyCompileOptions* options_;
     ScriptSourceObject** sourceObjectOut_;
-    LifoAlloc& alloc_;
 
   public:
     // Note, when providing an JSContext, where isJSContext is false,
@@ -519,24 +517,19 @@ class XDROffThreadDecoder : public XDRDecoder
     //
     // When providing a sourceObjectOut pointer, you have to ensure that it is
     // marked by the GC to avoid dangling pointers.
-    XDROffThreadDecoder(JSContext* cx, LifoAlloc& alloc,
+    XDROffThreadDecoder(JSContext* cx,
                         const JS::ReadOnlyCompileOptions* options,
                         ScriptSourceObject** sourceObjectOut,
                         const JS::TranscodeRange& range)
       : XDRDecoder(cx, range),
         options_(options),
-        sourceObjectOut_(sourceObjectOut),
-        alloc_(alloc)
+        sourceObjectOut_(sourceObjectOut)
     {
         MOZ_ASSERT(options);
         MOZ_ASSERT(sourceObjectOut);
         MOZ_ASSERT(*sourceObjectOut == nullptr);
     }
 
-    LifoAlloc& lifoAlloc() const override {
-        return alloc_;
-    }
-
     bool hasOptions() const override { return true; }
     const JS::ReadOnlyCompileOptions& options() override {
         return *options_;