Object allocation sinking phase needs to iterate each scope offset instead of just...
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Nov 2018 20:14:41 +0000 (20:14 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Nov 2018 20:14:41 +0000 (20:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191958
<rdar://problem/46221877>

Reviewed by Yusuke Suzuki.

JSTests:

* stress/object-allocation-sinking-phase-needs-to-write-to-each-scope-offset.js: Added.
(x):
(foo):

Source/JavaScriptCore:

There may be more entries in an activation than unique variables
in a symbol table's hashmap. For example, if you have two parameters
to a function, and they both are the same name, and the function
uses eval, we'll end up with two scope slots, but only a single
entry in the hashmap in the symbol table. Object allocation sinking
phase was previously iterating over the hashmap, assuming these
values were equivalent. This is wrong in the above case. Instead,
we need to iterate over each scope offset.

* dfg/DFGObjectAllocationSinkingPhase.cpp:
* runtime/GenericOffset.h:
(JSC::GenericOffset::operator+=):
(JSC::GenericOffset::operator-=):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238510 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/object-allocation-sinking-phase-needs-to-write-to-each-scope-offset.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGObjectAllocationSinkingPhase.cpp
Source/JavaScriptCore/runtime/GenericOffset.h

index 6a0e763..0d2fdb9 100644 (file)
@@ -1,3 +1,15 @@
+2018-11-26  Saam barati  <sbarati@apple.com>
+
+        Object allocation sinking phase needs to iterate each scope offset instead of just iterating the symbol table's hashmap when handling an activation
+        https://bugs.webkit.org/show_bug.cgi?id=191958
+        <rdar://problem/46221877>
+
+        Reviewed by Yusuke Suzuki.
+
+        * stress/object-allocation-sinking-phase-needs-to-write-to-each-scope-offset.js: Added.
+        (x):
+        (foo):
+
 2018-11-26  Mark Lam  <mark.lam@apple.com>
 
         NaNs read from Wasm code needs to be be purified.
diff --git a/JSTests/stress/object-allocation-sinking-phase-needs-to-write-to-each-scope-offset.js b/JSTests/stress/object-allocation-sinking-phase-needs-to-write-to-each-scope-offset.js
new file mode 100644 (file)
index 0000000..d9eca32
--- /dev/null
@@ -0,0 +1,13 @@
+//@ runDefault("--forceEagerCompilation=1", "--useConcurrentJIT=0")
+
+function foo(a, a) {
+    function x() {
+        eval();
+    }
+}
+foo();
+foo();
+foo();
+foo();
+foo();
+foo(0);
index 418f48b..d7878fb 100644 (file)
@@ -1,3 +1,25 @@
+2018-11-26  Saam barati  <sbarati@apple.com>
+
+        Object allocation sinking phase needs to iterate each scope offset instead of just iterating the symbol table's hashmap when handling an activation
+        https://bugs.webkit.org/show_bug.cgi?id=191958
+        <rdar://problem/46221877>
+
+        Reviewed by Yusuke Suzuki.
+
+        There may be more entries in an activation than unique variables
+        in a symbol table's hashmap. For example, if you have two parameters
+        to a function, and they both are the same name, and the function
+        uses eval, we'll end up with two scope slots, but only a single
+        entry in the hashmap in the symbol table. Object allocation sinking
+        phase was previously iterating over the hashmap, assuming these
+        values were equivalent. This is wrong in the above case. Instead,
+        we need to iterate over each scope offset.
+
+        * dfg/DFGObjectAllocationSinkingPhase.cpp:
+        * runtime/GenericOffset.h:
+        (JSC::GenericOffset::operator+=):
+        (JSC::GenericOffset::operator-=):
+
 2018-11-26  Mark Lam  <mark.lam@apple.com>
 
         NaNs read from Wasm code needs to be be purified.
index ee11fdc..6e0b647 100644 (file)
@@ -877,11 +877,10 @@ private:
             writes.add(ActivationScopePLoc, LazyNode(node->child1().node()));
             {
                 SymbolTable* symbolTable = node->castOperand<SymbolTable*>();
-                ConcurrentJSLocker locker(symbolTable->m_lock);
                 LazyNode initialValue(m_graph.freeze(node->initializationValueForActivation()));
-                for (auto iter = symbolTable->begin(locker), end = symbolTable->end(locker); iter != end; ++iter) {
+                for (ScopeOffset offset { 0 }; offset <= symbolTable->maxScopeOffset(); offset += 1) {
                     writes.add(
-                        PromotedLocationDescriptor(ClosureVarPLoc, iter->value.scopeOffset().offset()),
+                        PromotedLocationDescriptor(ClosureVarPLoc, offset.offset()),
                         initialValue);
                 }
             }
index 45689f4..e071296 100644 (file)
@@ -95,11 +95,11 @@ public:
     }
     T& operator+=(int value)
     {
-        return *this = *this + value;
+        return *static_cast<T*>(this) = *this + value;
     }
     T& operator-=(int value)
     {
-        return *this = *this - value;
+        return *static_cast<T*>(this) = *this - value;
     }
     
 private: