Changeset 80179 in webkit

Timestamp:

Mar 2, 2011 4:00:17 PM (13 years ago)

Author:

oliver@apple.com

Message:

2011-03-02 Oliver Hunt <​oliver@apple.com>

Reviewed by Gavin Barraclough.

Remove "register slot" concept from PropertySlot
​https://bugs.webkit.org/show_bug.cgi?id=55621

PropertySlot had already stopped storing Register "slots"
so this patch is simply removing that api entirely.
This exposed a problem in the ProgramNode constructor for
BytecodeGenerator where it reads from the registerfile
before it has initialised it.

This bug wasn't a problem before as we were merely testing
for property existence rather than the actual value, and
used to work because setRegisterSlot didn't check that the
provided slot contained an initialised value.

To get around this issue we now use symbolTableHasProperty
to do the symbol table check without trying to read the
RegisterFile.

• JavaScriptCore.xcodeproj/project.pbxproj:
• bytecompiler/BytecodeGenerator.cpp: (JSC::BytecodeGenerator::BytecodeGenerator):
• runtime/Arguments.cpp: (JSC::Arguments::getOwnPropertySlot):
• runtime/JSActivation.cpp: (JSC::JSActivation::symbolTableGet):
• runtime/JSGlobalObject.h: (JSC::JSGlobalObject::symbolTableHasProperty):
• runtime/JSVariableObject.h: (JSC::JSVariableObject::symbolTableGet):
• runtime/PropertySlot.h:

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• JavaScriptCore.xcodeproj/project.pbxproj (modified) (1 diff)
• bytecompiler/BytecodeGenerator.cpp (modified) (2 diffs)
• runtime/Arguments.cpp (modified) (2 diffs)
• runtime/JSActivation.cpp (modified) (1 diff)
• runtime/JSGlobalObject.h (modified) (2 diffs)
• runtime/JSVariableObject.h (modified) (2 diffs)
• runtime/PropertySlot.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2011-03-02  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Gavin Barraclough.
+
+        Remove "register slot" concept from PropertySlot
+        https://bugs.webkit.org/show_bug.cgi?id=55621
+
+        PropertySlot had already stopped storing Register "slots"
+        so this patch is simply removing that api entirely.
+        This exposed a problem in the ProgramNode constructor for
+        BytecodeGenerator where it reads from the registerfile
+        before it has initialised it.
+
+        This bug wasn't a problem before as we were merely testing
+        for property existence rather than the actual value, and
+        used to work because setRegisterSlot didn't check that the
+        provided slot contained an initialised value.
+
+        To get around this issue we now use symbolTableHasProperty
+        to do the symbol table check without trying to read the
+        RegisterFile.
+
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::BytecodeGenerator):
+        * runtime/Arguments.cpp:
+        (JSC::Arguments::getOwnPropertySlot):
+        * runtime/JSActivation.cpp:
+        (JSC::JSActivation::symbolTableGet):
+        * runtime/JSGlobalObject.h:
+        (JSC::JSGlobalObject::symbolTableHasProperty):
+        * runtime/JSVariableObject.h:
+        (JSC::JSVariableObject::symbolTableGet):
+        * runtime/PropertySlot.h:
+
 2011-03-02  Daniel Cheng  <dcheng@chromium.org>
 

trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

@@ -2540 +2540 @@
                         buildConfigurationList = 149C277108902AFE008A9EFC /* Build configuration list for PBXProject "JavaScriptCore" */;
                         compatibilityVersion = "Xcode 3.1";
+                        developmentRegion = English;
                         hasScannedForEncodings = 1;
                         knownRegions = (

trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

@@ -271 +271 @@
         Vector<RegisterID*, 32> newVars;
         for (size_t i = 0; i < varStack.size(); ++i) {
-            if (!globalObject->hasProperty(exec, *varStack[i].first))
-                newVars.append(addGlobalVar(*varStack[i].first, varStack[i].second & DeclarationStacks::IsConstant));
+            if (globalObject->symbolTableHasProperty(*varStack[i].first) || globalObject->hasProperty(exec, *varStack[i].first))
+                continue;
+            newVars.append(addGlobalVar(*varStack[i].first, varStack[i].second & DeclarationStacks::IsConstant));
         }
 
@@ -287 +288 @@
         }
         for (size_t i = 0; i < varStack.size(); ++i) {
-            if (globalObject->hasProperty(exec, *varStack[i].first))
+            if (globalObject->symbolTableHasProperty(*varStack[i].first) || globalObject->hasProperty(exec, *varStack[i].first))
                 continue;
             int attributes = DontDelete;

trunk/Source/JavaScriptCore/runtime/Arguments.cpp

@@ -146 +146 @@
     if (i < d->numArguments && (!d->deletedArguments || !d->deletedArguments[i])) {
         if (i < d->numParameters) {
-            slot.setRegisterSlot(&d->registers[d->firstParameterIndex + i]);
+            slot.setValue(d->registers[d->firstParameterIndex + i].jsValue());
         } else
             slot.setValue(d->extraArguments[i - d->numParameters].jsValue());
@@ -185 +185 @@
     if (isArrayIndex && i < d->numArguments && (!d->deletedArguments || !d->deletedArguments[i])) {
         if (i < d->numParameters) {
-            slot.setRegisterSlot(&d->registers[d->firstParameterIndex + i]);
+            slot.setValue(d->registers[d->firstParameterIndex + i].jsValue());
         } else
             slot.setValue(d->extraArguments[i - d->numParameters].jsValue());

trunk/Source/JavaScriptCore/runtime/JSActivation.cpp

@@ -76 +76 @@
     if (!entry.isNull()) {
         ASSERT(entry.getIndex() < static_cast<int>(d()->functionExecutable->capturedVariableCount()));
-        slot.setRegisterSlot(&registerAt(entry.getIndex()));
+        slot.setValue(registerAt(entry.getIndex()).jsValue());
         return true;
     }

trunk/Source/JavaScriptCore/runtime/JSGlobalObject.h

@@ -178 +178 @@
         virtual void defineSetter(ExecState*, const Identifier& propertyName, JSObject* setterFunc, unsigned attributes);
 
+        // We use this in the code generator as we perform symbol table
+        // lookups prior to initializing the properties
+        bool symbolTableHasProperty(const Identifier& propertyName);
+
         // The following accessors return pristine values, even if a script 
         // replaces the global object's associated property.
@@ -352 +356 @@
     }
 
+    inline bool JSGlobalObject::symbolTableHasProperty(const Identifier& propertyName)
+    {
+        SymbolTableEntry entry = symbolTable().inlineGet(propertyName.impl());
+        return !entry.isNull();
+    }
+
     inline JSValue Structure::prototypeForLookup(ExecState* exec) const
     {

trunk/Source/JavaScriptCore/runtime/JSVariableObject.h

@@ -106 +106 @@
         SymbolTableEntry entry = symbolTable().inlineGet(propertyName.impl());
         if (!entry.isNull()) {
-            slot.setRegisterSlot(&registerAt(entry.getIndex()));
+            slot.setValue(registerAt(entry.getIndex()).jsValue());
             return true;
         }
@@ -116 +116 @@
         SymbolTableEntry entry = symbolTable().inlineGet(propertyName.impl());
         if (!entry.isNull()) {
-            slot.setRegisterSlot(&registerAt(entry.getIndex()));
+            slot.setValue(registerAt(entry.getIndex()).jsValue());
             slotIsWriteable = !entry.isReadOnly();
             return true;

trunk/Source/JavaScriptCore/runtime/PropertySlot.h

@@ -124 +124 @@
         }
 
-        void setRegisterSlot(Register* registerSlot)
-        {
-            ASSERT(registerSlot);
-            clearBase();
-            clearOffset();
-            m_getValue = JSC_VALUE_MARKER;
-            m_value = registerSlot->jsValue();
-        }
-
         void setCustom(JSValue slotBase, GetValueFunc getValue)
         {