Scoped Arguements needs to alias between named and unnamed accesses a…

…nd across nested scopes

https://bugs.webkit.org/show_bug.cgi?id=261934
rdar://114925088

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: New test.
(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.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.h:

Originally-landed-as: 267815.345@safari-7617-branch (99b8814). rdar://119594814
Canonical link: https://commits.webkit.org/272253@main

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

@@ -587,7 +587,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

@@ -596,17 +596,22 @@ 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);
+
+IGNORE_GCC_WARNINGS_BEGIN("dangling-reference")
+                    const Identifier& ident =
+                        static_cast<const BindingNode*>(parameters.at(i).first)->boundProperty();
+IGNORE_GCC_WARNINGS_END
+                    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

@@ -8826,6 +8826,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

@@ -2853,7 +2853,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

@@ -3092,7 +3092,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.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);
+        m_watchpointSets.fill(nullptr, newLength);
         return this;
     }
 
     ScopedArgumentsTable* result = tryCreate(vm, newLength);
     if (UNLIKELY(!result))
         return nullptr;
-    for (unsigned i = std::min(m_length, newLength); i--;)
+    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,16 @@ 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);
+
     DECLARE_INFO;
 
     static Structure* createStructure(VM&, JSGlobalObject*, JSValue prototype);
@@ -96,6 +100,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.cpp

@@ -117,15 +117,15 @@ const SymbolTable::LocalToEntryVec& SymbolTable::localToEntry(const ConcurrentJS
             if (offset.isScope())
                 size = std::max(size, offset.scopeOffset().offset() + 1);
         }
-    
+
         m_localToEntry = makeUnique<LocalToEntryVec>(size, nullptr);
         for (auto& entry : m_map) {
             VarOffset offset = entry.value.varOffset();
             if (offset.isScope())
                 m_localToEntry->at(offset.scopeOffset().offset()) = &entry.value;
         }
     }
-    
+
     return *m_localToEntry;
 }
 
@@ -140,7 +140,7 @@ SymbolTableEntry* SymbolTable::entryFor(const ConcurrentJSLocker& locker, ScopeO
 SymbolTable* SymbolTable::cloneScopePart(VM& vm)
 {
     SymbolTable* result = SymbolTable::create(vm);
-    
+
     result->m_usesSloppyEval = m_usesSloppyEval;
     result->m_nestedLexicalScope = m_nestedLexicalScope;
     result->m_scopeType = m_scopeType;
@@ -152,9 +152,8 @@ SymbolTable* SymbolTable::cloneScopePart(VM& vm)
             iter->key,
             SymbolTableEntry(iter->value.varOffset(), iter->value.getAttributes()));
     }
-
     result->m_maxScopeOffset = m_maxScopeOffset;
-    
+
     if (ScopedArgumentsTable* arguments = this->arguments())
         result->m_arguments.set(vm, result, arguments);
 

Source/JavaScriptCore/runtime/SymbolTable.h

@@ -643,7 +643,7 @@ class SymbolTable final : public JSCell {
     {
         return m_map.contains(key);
     }
-    
+
     bool contains(UniquedStringImpl* key)
     {
         ConcurrentJSLocker locker(m_lock);
@@ -677,6 +677,7 @@ class SymbolTable final : public JSCell {
                 return false;
             m_arguments.set(vm, this, table);
         }
+
         return true;
     }
 
@@ -688,22 +689,32 @@ class SymbolTable final : public JSCell {
 
     bool trySetArgumentOffset(VM& vm, uint32_t i, ScopeOffset offset)
     {
-        ASSERT_WITH_SECURITY_IMPLICATION(m_arguments);
+        RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(m_arguments);
         auto* maybeCloned = m_arguments->trySet(vm, i, offset);
         if (!maybeCloned)
             return false;
         m_arguments.set(vm, this, maybeCloned);
         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)
             return nullptr;
         m_arguments->lock();
         return m_arguments.get();
     }
-    
+
     const LocalToEntryVec& localToEntry(const ConcurrentJSLocker&);
     SymbolTableEntry* entryFor(const ConcurrentJSLocker&, ScopeOffset);