From 2b223cce089bb8cbfb1a463fdd42e09eee63c7b2 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Thu, 5 Sep 2019 13:03:09 +0200 Subject: Use the correct group for JIT constraints. This fixes a rare crash/CTD in JS. This adds information about the constraints to a new RAII class so we can finish all constraints at the end. Based on changes in BZ 1568397 --- js/src/jit/IonAnalysis.cpp | 32 ++++++++++++----- js/src/jit/IonAnalysis.h | 7 ++-- js/src/vm/TypeInference.cpp | 84 +++++++++++++++++++++++++++++++++++++++++---- js/src/vm/TypeInference.h | 59 ++++++++++++++++++++++++++++++- 4 files changed, 164 insertions(+), 18 deletions(-) diff --git a/js/src/jit/IonAnalysis.cpp b/js/src/jit/IonAnalysis.cpp index 5fc624fb1..ace6cd81e 100644 --- a/js/src/jit/IonAnalysis.cpp +++ b/js/src/jit/IonAnalysis.cpp @@ -4005,7 +4005,7 @@ jit::ConvertLinearInequality(TempAllocator& alloc, MBasicBlock* block, const Lin } static bool -AnalyzePoppedThis(JSContext* cx, ObjectGroup* group, +AnalyzePoppedThis(JSContext* cx, DPAConstraintInfo& constraintInfo, ObjectGroup* group, MDefinition* thisValue, MInstruction* ins, bool definitelyExecuted, HandlePlainObject baseobj, Vector* initializerList, @@ -4046,7 +4046,12 @@ AnalyzePoppedThis(JSContext* cx, ObjectGroup* group, return true; RootedId id(cx, NameToId(setprop->name())); - if (!AddClearDefiniteGetterSetterForPrototypeChain(cx, group, id)) { + bool added = false; + if (!AddClearDefiniteGetterSetterForPrototypeChain(cx, constraintInfo, + group, id, &added)) { + return false; + } + if (!added) { // The prototype chain already contains a getter/setter for this // property, or type information is too imprecise. return true; @@ -4106,7 +4111,12 @@ AnalyzePoppedThis(JSContext* cx, ObjectGroup* group, if (!baseobj->lookup(cx, id) && !accessedProperties->append(get->name())) return false; - if (!AddClearDefiniteGetterSetterForPrototypeChain(cx, group, id)) { + bool added = false; + if (!AddClearDefiniteGetterSetterForPrototypeChain(cx, constraintInfo, + group, id, &added)) { + return false; + } + if (!added) { // The |this| value can escape if any property reads it does go // through a getter. return true; @@ -4132,8 +4142,11 @@ CmpInstructions(const void* a, const void* b) } bool -jit::AnalyzeNewScriptDefiniteProperties(JSContext* cx, HandleFunction fun, - ObjectGroup* group, HandlePlainObject baseobj, +jit::AnalyzeNewScriptDefiniteProperties(JSContext* cx, + DPAConstraintInfo& constraintInfo, + HandleFunction fun, + ObjectGroup* group, + HandlePlainObject baseobj, Vector* initializerList) { MOZ_ASSERT(cx->zone()->types.activeAnalysis); @@ -4293,7 +4306,7 @@ jit::AnalyzeNewScriptDefiniteProperties(JSContext* cx, HandleFunction fun, bool handled = false; size_t slotSpan = baseobj->slotSpan(); - if (!AnalyzePoppedThis(cx, group, thisValue, ins, definitelyExecuted, + if (!AnalyzePoppedThis(cx, constraintInfo, group, thisValue, ins, definitelyExecuted, baseobj, initializerList, &accessedProperties, &handled)) { return false; @@ -4312,7 +4325,6 @@ jit::AnalyzeNewScriptDefiniteProperties(JSContext* cx, HandleFunction fun, // contingent on the correct frames being inlined. Add constraints to // invalidate the definite properties if additional functions could be // called at the inline frame sites. - Vector exitBlocks(cx); for (MBasicBlockIterator block(graph.begin()); block != graph.end(); block++) { // Inlining decisions made after the last new property was added to // the object don't need to be frozen. @@ -4320,9 +4332,11 @@ jit::AnalyzeNewScriptDefiniteProperties(JSContext* cx, HandleFunction fun, break; if (MResumePoint* rp = block->callerResumePoint()) { if (block->numPredecessors() == 1 && block->getPredecessor(0) == rp->block()) { - JSScript* script = rp->block()->info().script(); - if (!AddClearDefiniteFunctionUsesInScript(cx, group, script, block->info().script())) + JSScript* caller = rp->block()->info().script(); + JSScript* callee = block->info().script(); + if (!constraintInfo.addInliningConstraint(caller, callee)) { return false; + } } } } diff --git a/js/src/jit/IonAnalysis.h b/js/src/jit/IonAnalysis.h index efd31415b..49bc0b591 100644 --- a/js/src/jit/IonAnalysis.h +++ b/js/src/jit/IonAnalysis.h @@ -196,8 +196,11 @@ MCompare* ConvertLinearInequality(TempAllocator& alloc, MBasicBlock* block, const LinearSum& sum); MOZ_MUST_USE bool -AnalyzeNewScriptDefiniteProperties(JSContext* cx, HandleFunction fun, - ObjectGroup* group, HandlePlainObject baseobj, +AnalyzeNewScriptDefiniteProperties(JSContext* cx, + DPAConstraintInfo& constraintInfo, + HandleFunction fun, + ObjectGroup* group, + HandlePlainObject baseobj, Vector* initializerList); MOZ_MUST_USE bool diff --git a/js/src/vm/TypeInference.cpp b/js/src/vm/TypeInference.cpp index 7c2c0194e..88327b47e 100644 --- a/js/src/vm/TypeInference.cpp +++ b/js/src/vm/TypeInference.cpp @@ -3084,29 +3084,39 @@ class TypeConstraintClearDefiniteGetterSetter : public TypeConstraint }; bool -js::AddClearDefiniteGetterSetterForPrototypeChain(JSContext* cx, ObjectGroup* group, HandleId id) +js::AddClearDefiniteGetterSetterForPrototypeChain(JSContext* cx, + DPAConstraintInfo& constraintInfo, + ObjectGroup* group, + HandleId id, + bool* added) { /* * Ensure that if the properties named here could have a getter, setter or * a permanent property in any transitive prototype, the definite * properties get cleared from the group. */ + + *added = false; + RootedObject proto(cx, group->proto().toObjectOrNull()); while (proto) { ObjectGroup* protoGroup = JSObject::getGroup(cx, proto); if (!protoGroup) { - cx->recoverFromOutOfMemory(); return false; } if (protoGroup->unknownProperties()) - return false; + return true; HeapTypeSet* protoTypes = protoGroup->getProperty(cx, proto, id); - if (!protoTypes || protoTypes->nonDataProperty() || protoTypes->nonWritableProperty()) + if (!protoTypes) return false; - if (!protoTypes->addConstraint(cx, cx->typeLifoAlloc().new_(group))) + if (protoTypes->nonDataProperty() || protoTypes->nonWritableProperty()) + return true; + if (!constraintInfo.addProtoConstraint(proto, id)) return false; proto = proto->staticPrototype(); } + + *added = true; return true; } @@ -3612,6 +3622,43 @@ struct DestroyTypeNewScript } // namespace +bool DPAConstraintInfo::finishConstraints(JSContext* cx, ObjectGroup* group) { + for (const ProtoConstraint& constraint : protoConstraints_) { + ObjectGroup* protoGroup = constraint.proto->group(); + + // Note: we rely on the group's type information being unchanged since + // AddClearDefiniteGetterSetterForPrototypeChain. + + bool unknownProperties = protoGroup->unknownProperties(); + MOZ_RELEASE_ASSERT(!unknownProperties); + + HeapTypeSet* protoTypes = + protoGroup->getProperty(cx, constraint.proto, constraint.id); + MOZ_RELEASE_ASSERT(protoTypes); + + MOZ_ASSERT(!protoTypes->nonDataProperty()); + MOZ_ASSERT(!protoTypes->nonWritableProperty()); + + if (!protoTypes->addConstraint( + cx, + cx->typeLifoAlloc().new_( + group))) { + ReportOutOfMemory(cx); + return false; + } + } + + for (const InliningConstraint& constraint : inliningConstraints_) { + if (!AddClearDefiniteFunctionUsesInScript(cx, group, constraint.caller, + constraint.callee)) { + ReportOutOfMemory(cx); + return false; + } + } + + return true; +} + bool TypeNewScript::maybeAnalyze(JSContext* cx, ObjectGroup* group, bool* regenerate, bool force) { @@ -3715,10 +3762,17 @@ TypeNewScript::maybeAnalyze(JSContext* cx, ObjectGroup* group, bool* regenerate, return false; Vector initializerVector(cx); + + DPAConstraintInfo constraintInfo(cx); RootedPlainObject templateRoot(cx, templateObject()); RootedFunction fun(cx, function()); - if (!jit::AnalyzeNewScriptDefiniteProperties(cx, fun, group, templateRoot, &initializerVector)) + if (!jit::AnalyzeNewScriptDefiniteProperties(cx, + constraintInfo, + fun, + group, + templateRoot, + &initializerVector)) return false; if (!group->newScript()) @@ -3772,6 +3826,14 @@ TypeNewScript::maybeAnalyze(JSContext* cx, ObjectGroup* group, bool* regenerate, // The definite properties analysis found exactly the properties that // are held in common by the preliminary objects. No further analysis // is needed. + + if (!constraintInfo.finishConstraints(cx, group)) { + return false; + } + if (!group->newScript()) { + return true; + } + group->addDefiniteProperties(cx, templateObject()->lastProperty()); destroyNewScript.group = nullptr; @@ -3792,6 +3854,16 @@ TypeNewScript::maybeAnalyze(JSContext* cx, ObjectGroup* group, bool* regenerate, initialFlags); if (!initialGroup) return false; + + // Add the constraints. Use the initialGroup as group referenced by the + // constraints because that's the group that will have the TypeNewScript + // associated with it. See the detachNewScript and setNewScript calls below. + if (!constraintInfo.finishConstraints(cx, initialGroup)) { + return false; + } + if (!group->newScript()) { + return true; + } initialGroup->addDefiniteProperties(cx, templateObject()->lastProperty()); group->addDefiniteProperties(cx, prefixShape); diff --git a/js/src/vm/TypeInference.h b/js/src/vm/TypeInference.h index 94ce7e871..fd021fc96 100644 --- a/js/src/vm/TypeInference.h +++ b/js/src/vm/TypeInference.h @@ -789,8 +789,65 @@ class TemporaryTypeSet : public TypeSet TypedArraySharedness* sharedness); }; +// Stack class to record information about constraints that need to be added +// after finishing the Definite Properties Analysis. When the analysis succeeds, +// the |finishConstraints| method must be called to add the constraints to the +// TypeSets. +// +// There are two constraint types managed here: +// +// 1. Proto constraints for HeapTypeSets, to guard against things like getters +// and setters on the proto chain. +// +// 2. Inlining constraints for StackTypeSets, to invalidate when additional +// functions could be called at call sites where we inlined a function. +// +// This class uses bare GC-thing pointers because GC is suppressed when the +// analysis runs. +class MOZ_RAII DPAConstraintInfo { + struct ProtoConstraint { + JSObject* proto; + jsid id; + ProtoConstraint(JSObject* proto, jsid id) : proto(proto), id(id) {} + }; + struct InliningConstraint { + JSScript* caller; + JSScript* callee; + InliningConstraint(JSScript* caller, JSScript* callee) + : caller(caller), callee(callee) {} + }; + + JS::AutoCheckCannotGC nogc_; + Vector protoConstraints_; + Vector inliningConstraints_; + +public: + explicit DPAConstraintInfo(JSContext* cx) + : nogc_(cx) + , protoConstraints_(cx) + , inliningConstraints_(cx) + { + } + + DPAConstraintInfo(const DPAConstraintInfo&) = delete; + void operator=(const DPAConstraintInfo&) = delete; + + MOZ_MUST_USE bool addProtoConstraint(JSObject* proto, jsid id) { + return protoConstraints_.emplaceBack(proto, id); + } + MOZ_MUST_USE bool addInliningConstraint(JSScript* caller, JSScript* callee) { + return inliningConstraints_.emplaceBack(caller, callee); + } + + MOZ_MUST_USE bool finishConstraints(JSContext* cx, ObjectGroup* group); +}; + bool -AddClearDefiniteGetterSetterForPrototypeChain(JSContext* cx, ObjectGroup* group, HandleId id); +AddClearDefiniteGetterSetterForPrototypeChain(JSContext* cx, + DPAConstraintInfo& constraintInfo, + ObjectGroup* group, + HandleId id, + bool* added); bool AddClearDefiniteFunctionUsesInScript(JSContext* cx, ObjectGroup* group, -- cgit v1.2.3