Bug 1438151 - Fix race in CodeGenerator::visitLambda in debug builds. r=arai
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 02 Mar 2018 10:56:32 +0100
changeset 461305 4780be3c03b490e2f1d7d0acb4f998cb729901dd
parent 461304 1a74aee7c5498625d7f6ddc9676bf931a225503b
child 461306 f1b56646984161b5209383cb48bb82c6e2111be8
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai
bugs1438151
milestone60.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 1438151 - Fix race in CodeGenerator::visitLambda in debug builds. r=arai
js/src/jit/CodeGenerator.cpp
js/src/jit/MIR.h
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -2830,40 +2830,38 @@ CodeGenerator::visitClassConstructor(LCl
 
 typedef JSObject* (*LambdaFn)(JSContext*, HandleFunction, HandleObject);
 static const VMFunction LambdaInfo = FunctionInfo<LambdaFn>(js::Lambda, "Lambda");
 
 void
 CodeGenerator::visitLambdaForSingleton(LLambdaForSingleton* lir)
 {
     pushArg(ToRegister(lir->environmentChain()));
-    pushArg(ImmGCPtr(lir->mir()->info().fun));
+    pushArg(ImmGCPtr(lir->mir()->info().funUnsafe()));
     callVM(LambdaInfo, lir);
 }
 
 void
 CodeGenerator::visitLambda(LLambda* lir)
 {
     Register envChain = ToRegister(lir->environmentChain());
     Register output = ToRegister(lir->output());
     Register tempReg = ToRegister(lir->temp());
     const LambdaFunctionInfo& info = lir->mir()->info();
 
-    OutOfLineCode* ool = oolCallVM(LambdaInfo, lir, ArgList(ImmGCPtr(info.fun), envChain),
+    OutOfLineCode* ool = oolCallVM(LambdaInfo, lir, ArgList(ImmGCPtr(info.funUnsafe()), envChain),
                                    StoreRegisterTo(output));
 
     MOZ_ASSERT(!info.singletonType);
 
-    masm.createGCObject(output, tempReg, info.fun, gc::DefaultHeap, ool->entry());
+    masm.createGCObject(output, tempReg, info.funUnsafe(), gc::DefaultHeap, ool->entry());
 
     emitLambdaInit(output, envChain, info);
 
     if (info.flags & JSFunction::EXTENDED) {
-        MOZ_ASSERT(info.fun->allowSuperProperty() || info.fun->isSelfHostedBuiltin() ||
-                   info.fun->isAsync());
         static_assert(FunctionExtended::NUM_EXTENDED_SLOTS == 2, "All slots must be initialized");
         masm.storeValue(UndefinedValue(), Address(output, FunctionExtended::offsetOfExtendedSlot(0)));
         masm.storeValue(UndefinedValue(), Address(output, FunctionExtended::offsetOfExtendedSlot(1)));
     }
 
     masm.bind(ool->rejoin());
 }
 
@@ -2903,17 +2901,17 @@ CodeGenerator::visitOutOfLineLambdaArrow
     masm.pop(newTarget.scratchReg());
 
     masm.bind(ool->entryNoPop());
 
     saveLive(ool->lir);
 
     pushArg(newTarget);
     pushArg(envChain);
-    pushArg(ImmGCPtr(info.fun));
+    pushArg(ImmGCPtr(info.funUnsafe()));
 
     callVM(LambdaArrowInfo, ool->lir);
     StoreRegisterTo(output).generate(this);
 
     restoreLiveIgnore(ool->lir, StoreRegisterTo(output).clobbered());
 
     masm.jump(ool->rejoin());
 }
@@ -2940,17 +2938,17 @@ CodeGenerator::visitLambdaArrow(LLambdaA
     }
 
     // There's not enough registers on x86 with the profiler enabled to request
     // a temp. Instead, spill part of one of the values, being prepared to
     // restore it if necessary on the out of line path.
     Register tempReg = newTarget.scratchReg();
     masm.push(newTarget.scratchReg());
 
-    masm.createGCObject(output, tempReg, info.fun, gc::DefaultHeap, ool->entry());
+    masm.createGCObject(output, tempReg, info.funUnsafe(), gc::DefaultHeap, ool->entry());
 
     masm.pop(newTarget.scratchReg());
 
     emitLambdaInit(output, envChain, info);
 
     // Initialize extended slots. Lexical |this| is stored in the first one.
     MOZ_ASSERT(info.flags & JSFunction::EXTENDED);
     static_assert(FunctionExtended::NUM_EXTENDED_SLOTS == 2, "All slots must be initialized");
@@ -2981,17 +2979,18 @@ CodeGenerator::emitLambdaInit(Register o
     static_assert(JSFunction::offsetOfFlags() == JSFunction::offsetOfNargs() + 2,
                   "the code below needs to be adapted");
     masm.store32(Imm32(u.word), Address(output, JSFunction::offsetOfNargs()));
     masm.storePtr(ImmGCPtr(info.scriptOrLazyScript),
                   Address(output, JSFunction::offsetOfScriptOrLazyScript()));
     masm.storePtr(envChain, Address(output, JSFunction::offsetOfEnvironment()));
     // No post barrier needed because output is guaranteed to be allocated in
     // the nursery.
-    masm.storePtr(ImmGCPtr(info.fun->displayAtom()), Address(output, JSFunction::offsetOfAtom()));
+    masm.storePtr(ImmGCPtr(info.funUnsafe()->displayAtom()),
+                  Address(output, JSFunction::offsetOfAtom()));
 }
 
 typedef bool (*SetFunNameFn)(JSContext*, HandleFunction, HandleValue, FunctionPrefixKind);
 static const VMFunction SetFunNameInfo =
     FunctionInfo<SetFunNameFn>(js::SetFunctionNameIfNoOwnName, "SetFunName");
 
 void
 CodeGenerator::visitSetFunName(LSetFunName* lir)
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -8793,38 +8793,58 @@ class MClassConstructor : public MNullar
     }
 };
 
 struct LambdaFunctionInfo
 {
     // The functions used in lambdas are the canonical original function in
     // the script, and are immutable except for delazification. Record this
     // information while still on the active thread to avoid races.
-    CompilerFunction fun;
+  private:
+    CompilerFunction fun_;
+
+  public:
     uint16_t flags;
     uint16_t nargs;
     gc::Cell* scriptOrLazyScript;
     bool singletonType;
     bool useSingletonForClone;
 
     explicit LambdaFunctionInfo(JSFunction* fun)
-      : fun(fun), flags(fun->flags()), nargs(fun->nargs()),
+      : fun_(fun), flags(fun->flags()), nargs(fun->nargs()),
         scriptOrLazyScript(fun->hasScript()
                            ? (gc::Cell*) fun->nonLazyScript()
                            : (gc::Cell*) fun->lazyScript()),
         singletonType(fun->isSingleton()),
         useSingletonForClone(ObjectGroup::useSingletonForClone(fun))
-    {}
+    {
+        // If this assert fails, make sure CodeGenerator::visitLambda does the
+        // right thing. We can't assert this off-thread in CodeGenerator,
+        // because fun->isAsync() accesses the script/lazyScript and can race
+        // with delazification on the main thread.
+        MOZ_ASSERT_IF(flags & JSFunction::EXTENDED,
+                      fun->isArrow() ||
+                      fun->allowSuperProperty() ||
+                      fun->isSelfHostedBuiltin() ||
+                      fun->isAsync());
+    }
+
+    // Be careful when calling this off-thread. Don't call any JSFunction*
+    // methods that depend on script/lazyScript - this can race with
+    // delazification on the main thread.
+    JSFunction* funUnsafe() const {
+        return fun_;
+    }
 
     bool appendRoots(MRootList& roots) const {
-        if (!roots.append(fun))
-            return false;
-        if (fun->hasScript())
-            return roots.append(fun->nonLazyScript());
-        return roots.append(fun->lazyScript());
+        if (!roots.append(fun_))
+            return false;
+        if (fun_->hasScript())
+            return roots.append(fun_->nonLazyScript());
+        return roots.append(fun_->lazyScript());
     }
 
   private:
     LambdaFunctionInfo(const LambdaFunctionInfo&) = delete;
     void operator=(const LambdaFunctionInfo&) = delete;
 };
 
 class MLambda
@@ -8834,18 +8854,19 @@ class MLambda
     const LambdaFunctionInfo info_;
 
     MLambda(TempAllocator& alloc, CompilerConstraintList* constraints, MDefinition* envChain,
             MConstant* cst)
       : MBinaryInstruction(classOpcode, envChain, cst),
         info_(&cst->toObject().as<JSFunction>())
     {
         setResultType(MIRType::Object);
-        if (!info().fun->isSingleton() && !ObjectGroup::useSingletonForClone(info().fun))
-            setResultTypeSet(MakeSingletonTypeSet(alloc, constraints, info().fun));
+        JSFunction* fun = info().funUnsafe();
+        if (!fun->isSingleton() && !ObjectGroup::useSingletonForClone(fun))
+            setResultTypeSet(MakeSingletonTypeSet(alloc, constraints, fun));
     }
 
   public:
     INSTRUCTION_HEADER(Lambda)
     TRIVIAL_NEW_WRAPPERS_WITH_ALLOC
     NAMED_OPERANDS((0, environmentChain))
 
     MConstant* functionOperand() const {
@@ -8870,19 +8891,20 @@ class MLambdaArrow
     const LambdaFunctionInfo info_;
 
     MLambdaArrow(TempAllocator& alloc, CompilerConstraintList* constraints, MDefinition* envChain,
                  MDefinition* newTarget, MConstant* cst)
       : MTernaryInstruction(classOpcode, envChain, newTarget, cst),
         info_(&cst->toObject().as<JSFunction>())
     {
         setResultType(MIRType::Object);
-        MOZ_ASSERT(!ObjectGroup::useSingletonForClone(info().fun));
-        if (!info().fun->isSingleton())
-            setResultTypeSet(MakeSingletonTypeSet(alloc, constraints, info().fun));
+        JSFunction* fun = info().funUnsafe();
+        MOZ_ASSERT(!ObjectGroup::useSingletonForClone(fun));
+        if (!fun->isSingleton())
+            setResultTypeSet(MakeSingletonTypeSet(alloc, constraints, fun));
     }
 
   public:
     INSTRUCTION_HEADER(LambdaArrow)
     TRIVIAL_NEW_WRAPPERS_WITH_ALLOC
     NAMED_OPERANDS((0, environmentChain), (1, newTargetDef))
 
     MConstant* functionOperand() const {