Diff
Modified: trunk/JSTests/ChangeLog (250183 => 250184)
--- trunk/JSTests/ChangeLog 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/JSTests/ChangeLog 2019-09-21 18:30:29 UTC (rev 250184)
@@ -1,3 +1,15 @@
+2019-09-21 Tadeu Zagallo <tzaga...@apple.com>
+
+ AccessCase should strongly visit its dependencies while on stack
+ https://bugs.webkit.org/show_bug.cgi?id=201986
+ <rdar://problem/55521953>
+
+ Reviewed by Saam Barati and Yusuke Suzuki.
+
+ * stress/ftl-put-by-id-setter-exception-interesting-live-state-2.js: Added.
+ (foo):
+ (warmup):
+
2019-09-20 Saam Barati <sbar...@apple.com>
Unreviewed. Make toctou-having-a-bad-time-new-array.js run for less time because it's timing out on the debug bots.
Added: trunk/JSTests/stress/ftl-put-by-id-setter-exception-interesting-live-state-2.js (0 => 250184)
--- trunk/JSTests/stress/ftl-put-by-id-setter-exception-interesting-live-state-2.js (rev 0)
+++ trunk/JSTests/stress/ftl-put-by-id-setter-exception-interesting-live-state-2.js 2019-09-21 18:30:29 UTC (rev 250184)
@@ -0,0 +1,40 @@
+function foo(o, p) {
+ var x = 100;
+ var result = 101;
+ var pf = p.g;
+ try {
+ x = 102;
+ pf++;
+ o.f = x + pf;
+ o = 104;
+ pf++;
+ x = 106;
+ } catch (e) {
+ return {outcome: "exception", values: [o, pf, x]};
+ }
+ return {outcome: "return", values: [o, pf, x]};
+}
+
+noInline(foo);
+
+function warmup() {
+ var o = {};
+ o.__defineSetter__("f", function(value) {
+ this._f = value;
+ });
+ if (i & 1)
+ o["i" + i] = i; // Make it polymorphic.
+ var result = foo(o, {g:200});
+}
+noInline(warmup);
+
+// Warm up foo() with polymorphic objects and getters.
+for (var i = 0; i < 100000; ++i) {
+ warmup();
+}
+
+var o = {};
+o.__defineSetter__("f", function() {
+ throw "Error42";
+});
+var result = foo(o, {g:300});
Modified: trunk/Source/_javascript_Core/ChangeLog (250183 => 250184)
--- trunk/Source/_javascript_Core/ChangeLog 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-09-21 18:30:29 UTC (rev 250184)
@@ -1,3 +1,56 @@
+2019-09-21 Tadeu Zagallo <tzaga...@apple.com>
+
+ AccessCase should strongly visit its dependencies while on stack
+ https://bugs.webkit.org/show_bug.cgi?id=201986
+ <rdar://problem/55521953>
+
+ Reviewed by Saam Barati and Yusuke Suzuki.
+
+ AccessCase::doesCalls is responsible for specifying the cells it depends on, so that
+ MarkingGCAwareJITStubRoutine can strongly visit them while the stub is on stack. However,
+ it was missing most of its dependencies, which led to it being collected while on stack.
+ This manifested in the flaky test stress/ftl-put-by-id-setter-exception-interesting-live-state.js
+ as the PolymorphicAccess being collected and removing its exception handler from the code
+ block, which led to exception propagating past the try/catch.
+
+ In order to fix this, we abstract the dependency gathering logic from AccessCase into
+ forEachDependentCell and use it to implement visitWeak as well as doesCalls in order to
+ guarantee that their implementation is consistent.
+
+ * bytecode/AccessCase.cpp:
+ (JSC::AccessCase::forEachDependentCell const):
+ (JSC::AccessCase::doesCalls const):
+ (JSC::AccessCase::visitWeak const):
+ * bytecode/AccessCase.h:
+ * bytecode/CallLinkInfo.cpp:
+ (JSC::CallLinkInfo::lastSeenCallee const):
+ (JSC::CallLinkInfo::haveLastSeenCallee const):
+ (JSC::CallLinkInfo::lastSeenCallee): Deleted.
+ (JSC::CallLinkInfo::haveLastSeenCallee): Deleted.
+ * bytecode/CallLinkInfo.h:
+ (JSC::CallLinkInfo::isDirect const):
+ (JSC::CallLinkInfo::isLinked const):
+ (JSC::CallLinkInfo::stub const):
+ (JSC::CallLinkInfo::forEachDependentCell const):
+ (JSC::CallLinkInfo::isLinked): Deleted.
+ (JSC::CallLinkInfo::stub): Deleted.
+ * bytecode/ObjectPropertyCondition.cpp:
+ (JSC::ObjectPropertyCondition::isStillLive const):
+ * bytecode/ObjectPropertyCondition.h:
+ (JSC::ObjectPropertyCondition::forEachDependentCell const):
+ * bytecode/ObjectPropertyConditionSet.cpp:
+ (JSC::ObjectPropertyConditionSet::areStillLive const):
+ * bytecode/ObjectPropertyConditionSet.h:
+ (JSC::ObjectPropertyConditionSet::forEachDependentCell const):
+ * bytecode/PropertyCondition.cpp:
+ (JSC::PropertyCondition::isStillLive const):
+ * bytecode/PropertyCondition.h:
+ (JSC::PropertyCondition::forEachDependentCell const):
+ * jit/PolymorphicCallStubRoutine.cpp:
+ (JSC::PolymorphicCallStubRoutine::visitWeak):
+ * jit/PolymorphicCallStubRoutine.h:
+ (JSC::PolymorphicCallStubRoutine::forEachDependentCell):
+
2019-09-21 David Kilzer <ddkil...@apple.com>
clang-tidy: Fix unnecessary copy/ref churn of for loop variables in WTF/_javascript_Core
Modified: trunk/Source/_javascript_Core/bytecode/AccessCase.cpp (250183 => 250184)
--- trunk/Source/_javascript_Core/bytecode/AccessCase.cpp 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/bytecode/AccessCase.cpp 2019-09-21 18:30:29 UTC (rev 250184)
@@ -203,27 +203,109 @@
}
}
-bool AccessCase::doesCalls(Vector<JSCell*>* cellsToMark) const
+template<typename Functor>
+void AccessCase::forEachDependentCell(const Functor& functor) const
{
+ m_conditionSet.forEachDependentCell(functor);
+ if (m_structure)
+ functor(m_structure.get());
+ if (m_polyProtoAccessChain) {
+ for (Structure* structure : m_polyProtoAccessChain->chain())
+ functor(structure);
+ }
+
switch (type()) {
case Getter:
+ case Setter: {
+ auto& accessor = this->as<GetterSetterAccessCase>();
+ if (accessor.callLinkInfo())
+ accessor.callLinkInfo()->forEachDependentCell(functor);
+ break;
+ }
+ case CustomValueGetter:
+ case CustomValueSetter: {
+ auto& accessor = this->as<GetterSetterAccessCase>();
+ if (accessor.customSlotBase())
+ functor(accessor.customSlotBase());
+ break;
+ }
+ case IntrinsicGetter: {
+ auto& intrinsic = this->as<IntrinsicGetterAccessCase>();
+ if (intrinsic.intrinsicFunction())
+ functor(intrinsic.intrinsicFunction());
+ break;
+ }
+ case ModuleNamespaceLoad: {
+ auto& accessCase = this->as<ModuleNamespaceAccessCase>();
+ if (accessCase.moduleNamespaceObject())
+ functor(accessCase.moduleNamespaceObject());
+ if (accessCase.moduleEnvironment())
+ functor(accessCase.moduleEnvironment());
+ break;
+ }
+ case InstanceOfHit:
+ case InstanceOfMiss:
+ if (as<InstanceOfAccessCase>().prototype())
+ functor(as<InstanceOfAccessCase>().prototype());
+ break;
+ case CustomAccessorGetter:
+ case CustomAccessorSetter:
+ case Load:
+ case Transition:
+ case Replace:
+ case Miss:
+ case GetGetter:
+ case InHit:
+ case InMiss:
+ case ArrayLength:
+ case StringLength:
+ case DirectArgumentsLength:
+ case ScopedArgumentsLength:
+ case InstanceOfGeneric:
+ break;
+ }
+}
+
+bool AccessCase::doesCalls(Vector<JSCell*>* cellsToMarkIfDoesCalls) const
+{
+ bool doesCalls;
+ switch (type()) {
+ case Transition:
+ doesCalls = newStructure()->outOfLineCapacity() != structure()->outOfLineCapacity() && structure()->couldHaveIndexingHeader();
+ break;
+ case Getter:
case Setter:
case CustomValueGetter:
case CustomAccessorGetter:
case CustomValueSetter:
case CustomAccessorSetter:
- return true;
- case Transition:
- if (newStructure()->outOfLineCapacity() != structure()->outOfLineCapacity()
- && structure()->couldHaveIndexingHeader()) {
- if (cellsToMark)
- cellsToMark->append(newStructure());
- return true;
- }
- return false;
- default:
- return false;
+ doesCalls = true;
+ break;
+ case Load:
+ case Replace:
+ case Miss:
+ case GetGetter:
+ case IntrinsicGetter:
+ case InHit:
+ case InMiss:
+ case ArrayLength:
+ case StringLength:
+ case DirectArgumentsLength:
+ case ScopedArgumentsLength:
+ case ModuleNamespaceLoad:
+ case InstanceOfHit:
+ case InstanceOfMiss:
+ case InstanceOfGeneric:
+ doesCalls = false;
+ break;
}
+
+ if (doesCalls && cellsToMarkIfDoesCalls) {
+ forEachDependentCell([&](JSCell* cell) {
+ cellsToMarkIfDoesCalls->append(cell);
+ });
+ }
+ return doesCalls;
}
bool AccessCase::couldStillSucceed() const
@@ -321,38 +403,17 @@
bool AccessCase::visitWeak(VM& vm) const
{
- if (m_structure && !vm.heap.isMarked(m_structure.get()))
- return false;
- if (m_polyProtoAccessChain) {
- for (Structure* structure : m_polyProtoAccessChain->chain()) {
- if (!vm.heap.isMarked(structure))
- return false;
- }
- }
- if (!m_conditionSet.areStillLive(vm))
- return false;
if (isAccessor()) {
auto& accessor = this->as<GetterSetterAccessCase>();
if (accessor.callLinkInfo())
accessor.callLinkInfo()->visitWeak(vm);
- if (accessor.customSlotBase() && !vm.heap.isMarked(accessor.customSlotBase()))
- return false;
- } else if (type() == IntrinsicGetter) {
- auto& intrinsic = this->as<IntrinsicGetterAccessCase>();
- if (intrinsic.intrinsicFunction() && !vm.heap.isMarked(intrinsic.intrinsicFunction()))
- return false;
- } else if (type() == ModuleNamespaceLoad) {
- auto& accessCase = this->as<ModuleNamespaceAccessCase>();
- if (accessCase.moduleNamespaceObject() && !vm.heap.isMarked(accessCase.moduleNamespaceObject()))
- return false;
- if (accessCase.moduleEnvironment() && !vm.heap.isMarked(accessCase.moduleEnvironment()))
- return false;
- } else if (type() == InstanceOfHit || type() == InstanceOfMiss) {
- if (as<InstanceOfAccessCase>().prototype() && !vm.heap.isMarked(as<InstanceOfAccessCase>().prototype()))
- return false;
}
- return true;
+ bool isValid = true;
+ forEachDependentCell([&](JSCell* cell) {
+ isValid &= vm.heap.isMarked(cell);
+ });
+ return isValid;
}
bool AccessCase::propagateTransitions(SlotVisitor& visitor) const
Modified: trunk/Source/_javascript_Core/bytecode/AccessCase.h (250183 => 250184)
--- trunk/Source/_javascript_Core/bytecode/AccessCase.h 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/bytecode/AccessCase.h 2019-09-21 18:30:29 UTC (rev 250184)
@@ -219,6 +219,9 @@
friend class CodeBlock;
friend class PolymorphicAccess;
+ template<typename Functor>
+ void forEachDependentCell(const Functor&) const;
+
bool visitWeak(VM&) const;
bool propagateTransitions(SlotVisitor&) const;
Modified: trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp (250183 => 250184)
--- trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/bytecode/CallLinkInfo.cpp 2019-09-21 18:30:29 UTC (rev 250184)
@@ -166,13 +166,13 @@
m_lastSeenCalleeOrExecutable.clear();
}
-JSObject* CallLinkInfo::lastSeenCallee()
+JSObject* CallLinkInfo::lastSeenCallee() const
{
RELEASE_ASSERT(!isDirect());
return jsCast<JSObject*>(m_lastSeenCalleeOrExecutable.get());
}
-bool CallLinkInfo::haveLastSeenCallee()
+bool CallLinkInfo::haveLastSeenCallee() const
{
RELEASE_ASSERT(!isDirect());
return !!m_lastSeenCalleeOrExecutable;
Modified: trunk/Source/_javascript_Core/bytecode/CallLinkInfo.h (250183 => 250184)
--- trunk/Source/_javascript_Core/bytecode/CallLinkInfo.h 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/bytecode/CallLinkInfo.h 2019-09-21 18:30:29 UTC (rev 250184)
@@ -134,7 +134,7 @@
return callModeFor(static_cast<CallType>(m_callType));
}
- bool isDirect()
+ bool isDirect() const
{
return isDirect(static_cast<CallType>(m_callType));
}
@@ -154,7 +154,7 @@
return isVarargsCallType(static_cast<CallType>(m_callType));
}
- bool isLinked() { return m_stub || m_calleeOrCodeBlock; }
+ bool isLinked() const { return m_stub || m_calleeOrCodeBlock; }
void unlink(VM&);
void setUpCall(CallType callType, CodeOrigin codeOrigin, GPRReg calleeGPR)
@@ -201,8 +201,8 @@
void setLastSeenCallee(VM&, const JSCell* owner, JSObject* callee);
void clearLastSeenCallee();
- JSObject* lastSeenCallee();
- bool haveLastSeenCallee();
+ JSObject* lastSeenCallee() const;
+ bool haveLastSeenCallee() const;
void setExecutableDuringCompilation(ExecutableBase*);
ExecutableBase* executable();
@@ -215,7 +215,7 @@
void clearStub();
- PolymorphicCallStubRoutine* stub()
+ PolymorphicCallStubRoutine* stub() const
{
return m_stub.get();
}
@@ -332,6 +332,22 @@
return m_codeOrigin;
}
+ template<typename Functor>
+ void forEachDependentCell(const Functor& functor) const
+ {
+ if (isLinked()) {
+ if (stub())
+ stub()->forEachDependentCell(functor);
+ else {
+ functor(m_calleeOrCodeBlock.get());
+ if (isDirect())
+ functor(m_lastSeenCalleeOrExecutable.get());
+ }
+ }
+ if (!isDirect() && haveLastSeenCallee())
+ functor(lastSeenCallee());
+ }
+
void visitWeak(VM&);
void setFrameShuffleData(const CallFrameShuffleData&);
Modified: trunk/Source/_javascript_Core/bytecode/ObjectPropertyCondition.cpp (250183 => 250184)
--- trunk/Source/_javascript_Core/bytecode/ObjectPropertyCondition.cpp 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/bytecode/ObjectPropertyCondition.cpp 2019-09-21 18:30:29 UTC (rev 250184)
@@ -147,10 +147,11 @@
if (!*this)
return false;
- if (!vm.heap.isMarked(m_object))
- return false;
-
- return m_condition.isStillLive(vm);
+ bool isStillLive = true;
+ forEachDependentCell([&](JSCell* cell) {
+ isStillLive &= vm.heap.isMarked(cell);
+ });
+ return isStillLive;
}
void ObjectPropertyCondition::validateReferences(const TrackedReferences& tracked) const
Modified: trunk/Source/_javascript_Core/bytecode/ObjectPropertyCondition.h (250183 => 250184)
--- trunk/Source/_javascript_Core/bytecode/ObjectPropertyCondition.h 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/bytecode/ObjectPropertyCondition.h 2019-09-21 18:30:29 UTC (rev 250184)
@@ -242,6 +242,13 @@
{
return condition().watchingRequiresReplacementWatchpoint();
}
+
+ template<typename Functor>
+ void forEachDependentCell(const Functor& functor) const
+ {
+ functor(m_object);
+ m_condition.forEachDependentCell(functor);
+ }
// This means that the objects involved in this are still live.
bool isStillLive(VM&) const;
Modified: trunk/Source/_javascript_Core/bytecode/ObjectPropertyConditionSet.cpp (250183 => 250184)
--- trunk/Source/_javascript_Core/bytecode/ObjectPropertyConditionSet.cpp 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/bytecode/ObjectPropertyConditionSet.cpp 2019-09-21 18:30:29 UTC (rev 250184)
@@ -143,11 +143,11 @@
bool ObjectPropertyConditionSet::areStillLive(VM& vm) const
{
- for (const ObjectPropertyCondition& condition : *this) {
- if (!condition.isStillLive(vm))
- return false;
- }
- return true;
+ bool stillLive = true;
+ forEachDependentCell([&](JSCell* cell) {
+ stillLive &= vm.heap.isMarked(cell);
+ });
+ return stillLive;
}
void ObjectPropertyConditionSet::dumpInContext(PrintStream& out, DumpContext* context) const
Modified: trunk/Source/_javascript_Core/bytecode/ObjectPropertyConditionSet.h (250183 => 250184)
--- trunk/Source/_javascript_Core/bytecode/ObjectPropertyConditionSet.h 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/bytecode/ObjectPropertyConditionSet.h 2019-09-21 18:30:29 UTC (rev 250184)
@@ -111,6 +111,14 @@
bool structuresEnsureValidityAssumingImpurePropertyWatchpoint() const;
bool needImpurePropertyWatchpoint() const;
+
+ template<typename Functor>
+ void forEachDependentCell(const Functor& functor) const
+ {
+ for (const ObjectPropertyCondition& condition : *this)
+ condition.forEachDependentCell(functor);
+ }
+
bool areStillLive(VM&) const;
void dumpInContext(PrintStream&, DumpContext*) const;
Modified: trunk/Source/_javascript_Core/bytecode/PropertyCondition.cpp (250183 => 250184)
--- trunk/Source/_javascript_Core/bytecode/PropertyCondition.cpp 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/bytecode/PropertyCondition.cpp 2019-09-21 18:30:29 UTC (rev 250184)
@@ -351,20 +351,6 @@
&& isWatchableWhenValid(structure, effort);
}
-bool PropertyCondition::isStillLive(VM& vm) const
-{
- if (hasPrototype() && prototype() && !vm.heap.isMarked(prototype()))
- return false;
-
- if (hasRequiredValue()
- && requiredValue()
- && requiredValue().isCell()
- && !vm.heap.isMarked(requiredValue().asCell()))
- return false;
-
- return true;
-}
-
void PropertyCondition::validateReferences(const TrackedReferences& tracked) const
{
if (hasPrototype())
Modified: trunk/Source/_javascript_Core/bytecode/PropertyCondition.h (250183 => 250184)
--- trunk/Source/_javascript_Core/bytecode/PropertyCondition.h 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/bytecode/PropertyCondition.h 2019-09-21 18:30:29 UTC (rev 250184)
@@ -295,10 +295,17 @@
{
return !!*this && m_header.type() == Equivalence;
}
+
+ template<typename Functor>
+ void forEachDependentCell(const Functor& functor) const
+ {
+ if (hasPrototype() && prototype())
+ functor(prototype());
+
+ if (hasRequiredValue() && requiredValue() && requiredValue().isCell())
+ functor(requiredValue().asCell());
+ }
- // This means that the objects involved in this are still live.
- bool isStillLive(VM&) const;
-
void validateReferences(const TrackedReferences&) const;
static bool isValidValueForAttributes(VM&, JSValue, unsigned attributes);
Modified: trunk/Source/_javascript_Core/jit/PolymorphicCallStubRoutine.cpp (250183 => 250184)
--- trunk/Source/_javascript_Core/jit/PolymorphicCallStubRoutine.cpp 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/jit/PolymorphicCallStubRoutine.cpp 2019-09-21 18:30:29 UTC (rev 250184)
@@ -129,11 +129,11 @@
bool PolymorphicCallStubRoutine::visitWeak(VM& vm)
{
- for (auto& variant : m_variants) {
- if (!vm.heap.isMarked(variant.get()))
- return false;
- }
- return true;
+ bool isStillLive = true;
+ forEachDependentCell([&](JSCell* cell) {
+ isStillLive &= vm.heap.isMarked(cell);
+ });
+ return isStillLive;
}
void PolymorphicCallStubRoutine::markRequiredObjectsInternal(SlotVisitor& visitor)
Modified: trunk/Source/_javascript_Core/jit/PolymorphicCallStubRoutine.h (250183 => 250184)
--- trunk/Source/_javascript_Core/jit/PolymorphicCallStubRoutine.h 2019-09-21 16:08:34 UTC (rev 250183)
+++ trunk/Source/_javascript_Core/jit/PolymorphicCallStubRoutine.h 2019-09-21 18:30:29 UTC (rev 250184)
@@ -95,6 +95,13 @@
void clearCallNodesFor(CallLinkInfo*);
+ template<typename Functor>
+ void forEachDependentCell(const Functor& functor)
+ {
+ for (auto& variant : m_variants)
+ functor(variant.get());
+ }
+
bool visitWeak(VM&) override;
protected:
