Cherry-pick 272448.5@safari-7618-branch (9789469). https://bugs.webki…

…t.org/show_bug.cgi?id=261934

    Scoped Arguments needs to alias between named and unnamed accesses and across nested scopes
    https://bugs.webkit.org/show_bug.cgi?id=261934
    rdar://114925088
    rdar://117838992

    Reviewed by Yusuke Suzuki.

    Fixed issue where an access to a named argument and a seperate access via its argument[i] counterpart weren't recognized throughout
    all JIT tiers as accesses to the same scoped value.  The DFG bytecode parser can unknowingly constant fold the read access.
    Added aliasing via the SymbolTable and its ScopedArgumentsTable for both types of accesses of such values.
    related objects

    Added watchpoints for scoped arguments, and shared the watchpoint from the SymbolTableEntry for the named parameter with the
    ScopedArgument entry for the matching index.  Tagged op_put_to_scope bytecodes with a new ScopedArgumentInitialization
    initialization type in GetPutInfo to signify this shared watchpoint case.  Since currently all tiers write to scoped arguments
    via ScopedArguments::setIndexQuickly(), that is where we fire its watchpoint.

    Added a new test.

    * JSTests/stress/arrow-function-captured-arguments-aliased.js: Added.
    (createOptAll):
    (createOpt500):
    (createOpt2000):
    (createOpt5000):
    (main):
    * Source/JavaScriptCore/bytecode/CodeBlock.cpp:
    (JSC::CodeBlock::finishCreation):
    * Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
    (JSC::BytecodeGenerator::BytecodeGenerator):
    * Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:
    (JSC::DFG::ByteCodeParser::parseBlock):
    * Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:
    * Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:
    * Source/JavaScriptCore/runtime/GetPutInfo.h:
    (JSC::initializationModeName):
    (JSC::isInitialization):
    * Source/JavaScriptCore/runtime/ScopedArguments.cpp:
    (JSC::ScopedArguments::unmapArgument):
    * Source/JavaScriptCore/runtime/ScopedArguments.h:
    * Source/JavaScriptCore/runtime/ScopedArgumentsTable.cpp:
    (JSC::ScopedArgumentsTable::tryCreate):
    (JSC::ScopedArgumentsTable::tryClone):
    (JSC::ScopedArgumentsTable::trySetLength):
    (JSC::ScopedArgumentsTable::trySetWatchpointSet):
    * Source/JavaScriptCore/runtime/ScopedArgumentsTable.h:
    * Source/JavaScriptCore/runtime/SymbolTable.cpp:
    (JSC::SymbolTable::cloneScopePart):
    * Source/JavaScriptCore/runtime/SymbolTable.h:

    Canonical link: https://commits.webkit.org/272448.5@safari-7618-branch

Canonical link: https://commits.webkit.org/266719.380@webkitglib/2.42

JSTests/stress/arrow-function-captured-arguments-aliased.js

@@ -0,0 +1,66 @@
+function createOptAll(count) {
+    count = 0;
+
+    return () => {
+        arguments[0]++;
+
+        return count;
+    };
+}
+
+function createOpt500(count) {
+    count = 0;
+
+    return () => {
+        arguments[0]++;
+
+        if (arguments[0] < 500)
+            return arguments[0];
+
+        return count;
+    };
+}
+
+function createOpt2000(count) {
+    count = 0;
+
+    return () => {
+        arguments[0]++;
+
+        if (arguments[0] < 2000)
+            return arguments[0];
+
+        return count;
+    };
+}
+
+function createOpt5000(count) {
+    count = 0;
+
+    return () => {
+        arguments[0]++;
+
+        if (arguments[0] < 5000)
+            return arguments[0];
+
+        return count;
+    };
+}
+
+function main() {
+    const testCount = 10000;
+    const createOptFuncs = [createOptAll, createOpt500, createOpt2000, createOpt5000];
+    for (createOptFunc of createOptFuncs) {
+    const opt = createOptFunc(0);
+        for (let i = 0; i < testCount; i++) {
+            opt();
+        }
+
+        const expectedResult = testCount+1;
+        let actualResult = opt();
+        if (actualResult != expectedResult)
+            print("Expected " + expectedResult + ", got " + actualResult);
+    }
+}
+
+main();

Source/JavaScriptCore/bytecode/CodeBlock.cpp

@@ -596,7 +596,12 @@ bool CodeBlock::finishCreation(VM& vm, ScriptExecutable* ownerExecutable, Unlink
                     ConcurrentJSLocker locker(symbolTable->m_lock);
                     auto iter = symbolTable->find(locker, ident.impl());
                     ASSERT(iter != symbolTable->end(locker));
-                    iter->value.prepareToWatch();
+                    if (bytecode.m_getPutInfo.initializationMode() == InitializationMode::ScopedArgumentInitialization) {
+                        ASSERT(bytecode.m_value.isArgument());
+                        unsigned argumentIndex = bytecode.m_value.toArgument() - 1;
+                        symbolTable->prepareToWatchScopedArgument(iter->value, argumentIndex);
+                    } else
+                        iter->value.prepareToWatch();
                     metadata.m_watchpointSet = iter->value.watchpointSet();
                 } else
                     metadata.m_watchpointSet = nullptr;

Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -595,17 +595,21 @@ BytecodeGenerator::BytecodeGenerator(VM& vm, FunctionNode* functionNode, Unlinke
                     m_outOfMemoryDuringConstruction = true;
                     return;
                 }
+
+                unsigned varOrAnonymous = UINT_MAX;
+
                 if (UniquedStringImpl* name = visibleNameForParameter(parameters.at(i).first)) {
                     VarOffset varOffset(offset);
                     SymbolTableEntry entry(varOffset);
-                    // Stores to these variables via the ScopedArguments object will not do
-                    // notifyWrite(), since that would be cumbersome. Also, watching formal
-                    // parameters when "arguments" is in play is unlikely to be super profitable.
-                    // So, we just disable it.
-                    entry.disableWatching(m_vm);
                     functionSymbolTable->set(NoLockingNecessary, name, entry);
+
+                    const Identifier& ident =
+                        static_cast<const BindingNode*>(parameters.at(i).first)->boundProperty();
+
+                    varOrAnonymous = addConstant(ident);
                 }
-                OpPutToScope::emit(this, m_lexicalEnvironmentRegister, UINT_MAX, virtualRegisterForArgumentIncludingThis(1 + i), GetPutInfo(ThrowIfNotFound, ResolvedClosureVar, InitializationMode::NotInitialization, ecmaMode), SymbolTableOrScopeDepth::symbolTable(VirtualRegister { symbolTableConstantIndex }), offset.offset());
+
+                OpPutToScope::emit(this, m_lexicalEnvironmentRegister, varOrAnonymous, virtualRegisterForArgumentIncludingThis(1 + i), GetPutInfo(ThrowIfNotFound, ResolvedClosureVar, InitializationMode::ScopedArgumentInitialization, ecmaMode), SymbolTableOrScopeDepth::symbolTable(VirtualRegister { symbolTableConstantIndex }), offset.offset());
             }
 
             // This creates a scoped arguments object and copies the overflow arguments into the

Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

@@ -8739,6 +8739,9 @@ void ByteCodeParser::parseBlock(unsigned limit)
                     // Must happen after the store. See comment for GetGlobalVar.
                     addToGraph(NotifyWrite, OpInfo(watchpoints));
                 }
+
+                // Keep scope alive until after put.
+                addToGraph(Phantom, scopeNode);
                 break;
             }
 

Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm

@@ -2872,7 +2872,7 @@ llintOpWithMetadata(op_put_to_scope, OpPutToScope, macro (size, get, dispatch, m
         loadi OpPutToScope::Metadata::m_getPutInfo + GetPutInfo::m_operand[t5], t0
         andi InitializationModeMask, t0
         rshifti InitializationModeShift, t0
-        bineq t0, NotInitialization, .noNeedForTDZCheck
+        bilt t0, NotInitialization, .noNeedForTDZCheck
         loadp OpPutToScope::Metadata::m_operand[t5], t0
         loadi TagOffset[t0], t0
         bieq t0, EmptyValueTag, .pDynamic

Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

@@ -3069,7 +3069,7 @@ llintOpWithMetadata(op_put_to_scope, OpPutToScope, macro (size, get, dispatch, m
         loadi OpPutToScope::Metadata::m_getPutInfo + GetPutInfo::m_operand[t5], t0
         andi InitializationModeMask, t0
         rshifti InitializationModeShift, t0
-        bineq t0, NotInitialization, .noNeedForTDZCheck
+        bilt t0, NotInitialization, .noNeedForTDZCheck
         loadp OpPutToScope::Metadata::m_operand[t5], t0
         loadq [t0], t0
         bqeq t0, ValueEmpty, .pDynamic

Source/JavaScriptCore/runtime/GetPutInfo.h

@@ -83,7 +83,8 @@ enum ResolveType : unsigned {
 enum class InitializationMode : unsigned {
     Initialization,      // "let x = 20;"
     ConstInitialization, // "const x = 20;"
-    NotInitialization    // "x = 20;"
+    NotInitialization,   // "x = 20;"
+    ScopedArgumentInitialization // Assign to scoped argument, which also has NotInitialization semantics.
 };
 
 ALWAYS_INLINE const char* resolveModeName(ResolveMode resolveMode)
@@ -120,7 +121,8 @@ ALWAYS_INLINE const char* initializationModeName(InitializationMode initializati
     static const char* const names[] = {
         "Initialization",
         "ConstInitialization",
-        "NotInitialization"
+        "NotInitialization",
+        "ScopedArgumentInitialization"
     };
     return names[static_cast<unsigned>(initializationMode)];
 }
@@ -132,6 +134,7 @@ ALWAYS_INLINE bool isInitialization(InitializationMode initializationMode)
     case InitializationMode::ConstInitialization:
         return true;
     case InitializationMode::NotInitialization:
+    case InitializationMode::ScopedArgumentInitialization:
         return false;
     }
     ASSERT_NOT_REACHED();

Source/JavaScriptCore/runtime/ScopedArguments.cpp

@@ -150,6 +150,7 @@ void ScopedArguments::unmapArgument(JSGlobalObject* globalObject, uint32_t i)
             return;
         }
         m_table.set(vm, this, maybeCloned);
+        m_table->clearWatchpointSet(i);
     } else
         storage()[i - namedLength].clear();
 }

Source/JavaScriptCore/runtime/ScopedArguments.h

@@ -27,6 +27,7 @@
 
 #include "GenericArguments.h"
 #include "JSLexicalEnvironment.h"
+#include "Watchpoint.h"
 
 namespace JSC {
 
@@ -110,9 +111,13 @@ class ScopedArguments final : public GenericArguments<ScopedArguments> {
     {
         ASSERT_WITH_SECURITY_IMPLICATION(isMappedArgument(i));
         unsigned namedLength = m_table->length();
-        if (i < namedLength)
+        if (i < namedLength) {
             m_scope->variableAt(m_table->get(i)).set(vm, m_scope.get(), value);
-        else
+
+            auto* watchpointSet = m_table->getWatchpointSet(i);
+            if (watchpointSet)
+                watchpointSet->touch(vm, "Write to ScopedArgument.");
+        } else
             storage()[i - namedLength].set(vm, this, value);
     }
 

Source/JavaScriptCore/runtime/ScopedArgumentsTable.cpp

@@ -70,6 +70,7 @@ ScopedArgumentsTable* ScopedArgumentsTable::tryCreate(VM& vm, uint32_t length)
     result->m_arguments = ArgumentsPtr::tryCreate(length);
     if (UNLIKELY(!result->m_arguments))
         return nullptr;
+    result->m_watchpointSets.fill(nullptr, length);
     return result;
 }
 
@@ -80,6 +81,7 @@ ScopedArgumentsTable* ScopedArgumentsTable::tryClone(VM& vm)
         return nullptr;
     for (unsigned i = m_length; i--;)
         result->at(i) = this->at(i);
+    result->m_watchpointSets = this->m_watchpointSets;
     return result;
 }
 
@@ -93,14 +95,18 @@ ScopedArgumentsTable* ScopedArgumentsTable::trySetLength(VM& vm, uint32_t newLen
             newArguments.at(i, newLength) = this->at(i);
         m_length = newLength;
         m_arguments = WTFMove(newArguments);
+        m_watchpointSets.resize(newLength);
         return this;
     }
 
     ScopedArgumentsTable* result = tryCreate(vm, newLength);
     if (UNLIKELY(!result))
         return nullptr;
-    for (unsigned i = std::min(m_length, newLength); i--;)
+    m_watchpointSets.resize(newLength);
+    for (unsigned i = std::min(m_length, newLength); i--;) {
         result->at(i) = this->at(i);
+        result->m_watchpointSets[i] = this->m_watchpointSets[i];
+    }
     return result;
 }
 
@@ -119,6 +125,15 @@ ScopedArgumentsTable* ScopedArgumentsTable::trySet(VM& vm, uint32_t i, ScopeOffs
     return result;
 }
 
+void ScopedArgumentsTable::trySetWatchpointSet(uint32_t i, WatchpointSet* watchpoints)
+{
+    ASSERT(watchpoints);
+    if (i >= m_watchpointSets.size())
+        return;
+
+    m_watchpointSets[i] = watchpoints;
+}
+
 Structure* ScopedArgumentsTable::createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
 {
     return Structure::create(vm, globalObject, prototype, TypeInfo(CellType, StructureFlags), info());

Source/JavaScriptCore/runtime/ScopedArgumentsTable.h

@@ -33,6 +33,8 @@
 
 namespace JSC {
 
+class WatchpointSet;
+
 // This class's only job is to hold onto the list of ScopeOffsets for each argument that a
 // function has. Most of the time, the BytecodeGenerator will create one of these and it will
 // never be modified subsequently. There is a rare case where a ScopedArguments object is created
@@ -67,14 +69,17 @@ class ScopedArgumentsTable final : public JSCell {
     ScopedArgumentsTable* trySetLength(VM&, uint32_t newLength);
 
     ScopeOffset get(uint32_t i) const { return at(i); }
+    WatchpointSet* getWatchpointSet(uint32_t i) const { return m_watchpointSets.at(i); }
 
     void lock()
     {
         m_locked = true;
     }
 
     ScopedArgumentsTable* trySet(VM&, uint32_t index, ScopeOffset);
-
+    void trySetWatchpointSet(uint32_t index, WatchpointSet* watchpoints);
+    void clearWatchpointSet(uint32_t index) { m_watchpointSets[index] = nullptr; }
+
     DECLARE_INFO;
 
     static Structure* createStructure(VM&, JSGlobalObject*, JSValue prototype);
@@ -96,6 +101,7 @@ class ScopedArgumentsTable final : public JSCell {
     uint32_t m_length;
     bool m_locked; // Being locked means that there are multiple references to this object and none of them expect to see the others' modifications. This means that modifications need to make a copy first.
     ArgumentsPtr m_arguments;
+    Vector<WatchpointSet*> m_watchpointSets;
 };
 
 } // namespace JSC

Source/JavaScriptCore/runtime/SymbolTable.h

@@ -680,6 +680,7 @@ class SymbolTable final : public JSCell {
                 return false;
             m_arguments.set(vm, this, table);
         }
+
         return true;
     }
 
@@ -699,6 +700,16 @@ class SymbolTable final : public JSCell {
         return true;
     }
 
+    void prepareToWatchScopedArgument(SymbolTableEntry& entry, uint32_t i)
+    {
+        entry.prepareToWatch();
+        if (!m_arguments)
+            return;
+
+        WatchpointSet* watchpoints = entry.watchpointSet();
+        m_arguments->trySetWatchpointSet(i, watchpoints);
+    }
+
     ScopedArgumentsTable* arguments() const
     {
         if (!m_arguments)