Cherry-pick 272387@main (84589d6). https://bugs.webkit.org/show_bug.c…

…gi?id=266720 Unreviewed, reverting 272253@main https://bugs.webkit.org/show_bug.cgi?id=266720 rdar://119946693 Causes stability critical crashes Reverted change: Scoped Arguements needs to alias between named and unnamed accesses and across nested scopes rdar://119594814 https://bugs.webkit.org/show_bug.cgi?id=261934 rdar://114925088 https://commits.webkit.org/272253@main Canonical link: https://commits.webkit.org/272387@main Canonical link: https://commits.webkit.org/266719.379@webkitglib/2.42
WebKit · Mar 14, 2024 · b88b266 · b88b266
1 parent d83eab5
commit b88b266
Show file tree

Hide file tree

Showing 12 changed files with 23 additions and 140 deletions.
diff --git a/JSTests/stress/arrow-function-captured-arguments-aliased.js b/JSTests/stress/arrow-function-captured-arguments-aliased.js
diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
@@ -596,12 +596,7 @@ 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));
-                    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();
+                    iter->value.prepareToWatch();
                     metadata.m_watchpointSet = iter->value.watchpointSet();
                 } else
                     metadata.m_watchpointSet = nullptr;

diff --git a/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
@@ -595,22 +595,17 @@ 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_WARNINGS_BEGIN("dangling-reference")
-                    const Identifier& ident =
-                        static_cast<const BindingNode*>(parameters.at(i).first)->boundProperty();
-IGNORE_WARNINGS_END
-
-                    varOrAnonymous = addConstant(ident);
                 }
-
-                OpPutToScope::emit(this, m_lexicalEnvironmentRegister, varOrAnonymous, virtualRegisterForArgumentIncludingThis(1 + i), GetPutInfo(ThrowIfNotFound, ResolvedClosureVar, InitializationMode::ScopedArgumentInitialization, ecmaMode), SymbolTableOrScopeDepth::symbolTable(VirtualRegister { symbolTableConstantIndex }), offset.offset());
+                OpPutToScope::emit(this, m_lexicalEnvironmentRegister, UINT_MAX, virtualRegisterForArgumentIncludingThis(1 + i), GetPutInfo(ThrowIfNotFound, ResolvedClosureVar, InitializationMode::NotInitialization, ecmaMode), SymbolTableOrScopeDepth::symbolTable(VirtualRegister { symbolTableConstantIndex }), offset.offset());
             }
 
             // This creates a scoped arguments object and copies the overflow arguments into the

diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -8739,9 +8739,6 @@ 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;
             }
 

diff --git a/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm b/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
-        bilt t0, NotInitialization, .noNeedForTDZCheck
+        bineq t0, NotInitialization, .noNeedForTDZCheck
         loadp OpPutToScope::Metadata::m_operand[t5], t0
         loadi TagOffset[t0], t0
         bieq t0, EmptyValueTag, .pDynamic

diff --git a/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm b/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
-        bilt t0, NotInitialization, .noNeedForTDZCheck
+        bineq t0, NotInitialization, .noNeedForTDZCheck
         loadp OpPutToScope::Metadata::m_operand[t5], t0
         loadq [t0], t0
         bqeq t0, ValueEmpty, .pDynamic

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

diff --git a/Source/JavaScriptCore/runtime/ScopedArguments.h b/Source/JavaScriptCore/runtime/ScopedArguments.h
@@ -27,7 +27,6 @@
 
 #include "GenericArguments.h"
 #include "JSLexicalEnvironment.h"
-#include "Watchpoint.h"
 
 namespace JSC {
 
@@ -111,13 +110,9 @@ 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);
-
-            auto* watchpointSet = m_table->getWatchpointSet(i);
-            if (watchpointSet)
-                watchpointSet->touch(vm, "Write to ScopedArgument.");
-        } else
+        else
             storage()[i - namedLength].set(vm, this, value);
     }
 

diff --git a/Source/JavaScriptCore/runtime/ScopedArgumentsTable.cpp b/Source/JavaScriptCore/runtime/ScopedArgumentsTable.cpp
@@ -70,7 +70,6 @@ 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;
 }
 
@@ -81,7 +80,6 @@ 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;
 }
 
@@ -95,18 +93,14 @@ 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;
 }
 
@@ -125,15 +119,6 @@ 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());

diff --git a/Source/JavaScriptCore/runtime/ScopedArgumentsTable.h b/Source/JavaScriptCore/runtime/ScopedArgumentsTable.h
@@ -33,8 +33,6 @@
 
 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
@@ -69,16 +67,14 @@ 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);
@@ -100,7 +96,6 @@ 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
diff --git a/Source/JavaScriptCore/runtime/SymbolTable.cpp b/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,8 +152,9 @@ 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);
 

diff --git a/Source/JavaScriptCore/runtime/SymbolTable.h b/Source/JavaScriptCore/runtime/SymbolTable.h
@@ -646,7 +646,7 @@ class SymbolTable final : public JSCell {
     {
         return m_map.contains(key);
     }
-
+    
     bool contains(UniquedStringImpl* key)
     {
         ConcurrentJSLocker locker(m_lock);
@@ -680,7 +680,6 @@ class SymbolTable final : public JSCell {
                 return false;
             m_arguments.set(vm, this, table);
         }
-
         return true;
     }
 
@@ -692,32 +691,22 @@ class SymbolTable final : public JSCell {
 
     bool trySetArgumentOffset(VM& vm, uint32_t i, ScopeOffset offset)
     {
-        RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(m_arguments);
+        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);