Changeset 212177 in webkit for trunk/Source/JavaScriptCore/ftl/FTLOperations.cpp

Timestamp:

Feb 10, 2017, 8:05:06 PM (9 years ago)

Author:

[email protected]

Message:

Object allocation sinking phase doesn't properly handle control flow when emitting a PutHint of a materialized object into a PromotedHeapLocation of a still sunken object
​https://bugs.webkit.org/show_bug.cgi?id=168140
<rdar://problem/30205880>

Reviewed by Filip Pizlo.

JSTests:

• stress/allocation-sinking-puthint-control-flow.js: Added.

(e):
(bar):
(let.y):
(else.let.y):
(baz):
(foo):
(catch):

Source/JavaScriptCore:

This patch fixes a bug in allocation sinking phase where
we don't properly handle control flow when materializing
an object and also PutHinting that materialization into
a still sunken object. We were performing the PutHint
for the materialization at the point of materialization,
however, we may have materialized along both edges
of a control flow diamond, in which case, we need to
also PutHint at the join point. Consider this program:

`
bb#0:
b: PhantomActivation()
a: PhantomNewFunction()
c: PutHint(@a, @b, ActivationLoc)
Branch(#1, #2)

bb#1:
d: MaterializeActivation()
e: PutHint(@a, @d, ActivationLoc)
f: Upsilon(@d, ^p)
Jump(#3)

bb#2:
g: MaterializeActivation()
h: PutHint(@a, @g, ActivationLoc)
i: Upsilon(@d, ^p)
Jump(#3)

bb#3:
p: Phi()
What is PromotedHeapLocation(@a, ActivationLoc) here?
What would we do if we exited?
`
Before this patch, we didn't perform a PutHint of the Phi.
However, we need to, otherwise when exit, we won't know
the value of PromotedHeapLocation(@a, ActivationLoc)

The program we need then, for correctness, is this:
`
bb#0:
b: PhantomActivation()
a: PhantomNewFunction()
c: PutHint(@a, @b, ActivationLoc)
Branch(#1, #2)

bb#1:
d: MaterializeActivation()
e: PutHint(@a, @d, ActivationLoc)
f: Upsilon(@d, ^p)
Jump(#3)

bb#2:
g: MaterializeActivation()
h: PutHint(@a, @g, ActivationLoc)
i: Upsilon(@d, ^p)
Jump(#3)

bb#3:
p: Phi()
j: PutHint(@a, @p, ActivationLoc)
`

This patch makes it so that we emit the necessary PutHint at node j.
I've also added more validation to the OSRAvailabilityAnalysisPhase
to catch this problem during validation.

• dfg/DFGOSRAvailabilityAnalysisPhase.cpp:

(JSC::DFG::OSRAvailabilityAnalysisPhase::run):

• dfg/DFGObjectAllocationSinkingPhase.cpp:
• ftl/FTLOperations.cpp:

(JSC::FTL::operationMaterializeObjectInOSR):

File:

• trunk/Source/JavaScriptCore/ftl/FTLOperations.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/ftl/FTLOperations.cpp

@@ -133 +133 @@
                 continue;
 
+            RELEASE_ASSERT(JSValue::decode(values[i]).asCell()->inherits(vm, Structure::info()));
             structure = jsCast<Structure*>(JSValue::decode(values[i]));
             break;
@@ -164 +165 @@
         for (unsigned i = materialization->properties().size(); i--;) {
             const ExitPropertyValue& property = materialization->properties()[i];
-            if (property.location() == PromotedLocationDescriptor(FunctionExecutablePLoc))
+            if (property.location() == PromotedLocationDescriptor(FunctionExecutablePLoc)) {
+                RELEASE_ASSERT(JSValue::decode(values[i]).asCell()->inherits(vm, FunctionExecutable::info()));
                 executable = jsCast<FunctionExecutable*>(JSValue::decode(values[i]));
-            if (property.location() == PromotedLocationDescriptor(FunctionActivationPLoc))
+            }
+            if (property.location() == PromotedLocationDescriptor(FunctionActivationPLoc)) {
+                RELEASE_ASSERT(JSValue::decode(values[i]).asCell()->inherits(vm, JSScope::info()));
                 activation = jsCast<JSScope*>(JSValue::decode(values[i]));
+            }
         }
         RELEASE_ASSERT(executable && activation);
@@ -185 +190 @@
         for (unsigned i = materialization->properties().size(); i--;) {
             const ExitPropertyValue& property = materialization->properties()[i];
-            if (property.location() == PromotedLocationDescriptor(ActivationScopePLoc))
+            if (property.location() == PromotedLocationDescriptor(ActivationScopePLoc)) {
+                RELEASE_ASSERT(JSValue::decode(values[i]).asCell()->inherits(vm, JSScope::info()));
                 scope = jsCast<JSScope*>(JSValue::decode(values[i]));
-            else if (property.location() == PromotedLocationDescriptor(ActivationSymbolTablePLoc))
+            } else if (property.location() == PromotedLocationDescriptor(ActivationSymbolTablePLoc)) {
+                RELEASE_ASSERT(JSValue::decode(values[i]).asCell()->inherits(vm, SymbolTable::info()));
                 table = jsCast<SymbolTable*>(JSValue::decode(values[i]));
+            }
         }
         RELEASE_ASSERT(scope);