Changeset 33438 in webkit

Timestamp:

May 14, 2008 5:14:56 AM (16 years ago)

Author:

oliver@apple.com

Message:

Bug 19024: SQUIRRELFISH: ASSERTION FAILED: activation->isActivationObject() in Machine::unwindCallFrame
<​https://bugs.webkit.org/show_bug.cgi?id=19024>

Reviewed by Maciej

This fixes a number of issues. The most important is that we now check every register
file for tainting rather than just looking for function register files as that was
insufficient. Additionally guarded against implicit re-entry into Eval code.

Also added a few additional assertions to reduce the amout of time between something
going wrong and us seeing the error.

Location:

branches/squirrelfish

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/VM/Machine.cpp (modified) (4 diffs)
• JavaScriptCore/VM/RegisterFile.cpp (modified) (2 diffs)
• JavaScriptCore/VM/RegisterFileStack.cpp (modified) (3 diffs)
• JavaScriptCore/VM/RegisterFileStack.h (modified) (3 diffs)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/js/implicit-global-to-global-reentry-expected.txt (added)
• LayoutTests/fast/js/implicit-global-to-global-reentry.html (added)

branches/squirrelfish/JavaScriptCore/ChangeLog

@@ +1 @@
+2008-05-14  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Maciej.
+
+        Bug 19024: SQUIRRELFISH: ASSERTION FAILED: activation->isActivationObject() in Machine::unwindCallFrame
+        <https://bugs.webkit.org/show_bug.cgi?id=19024>
+
+        This fixes a number of issues.  The most important is that we now check every register
+        file for tainting rather than just looking for function register files as that was
+        insufficient. Additionally guarded against implicit re-entry into Eval code.
+
+        Also added a few additional assertions to reduce the amout of time between something
+        going wrong and us seeing the error.
+
+        * VM/Machine.cpp:
+        (KJS::Machine::execute):
+        (KJS::Machine::privateExecute):
+        * VM/RegisterFile.cpp:
+        (KJS::RegisterFile::growBuffer):
+        (KJS::RegisterFile::addGlobalSlots):
+        * VM/RegisterFileStack.cpp:
+        (KJS::RegisterFileStack::pushGlobalRegisterFile):
+        (KJS::RegisterFileStack::pushFunctionRegisterFile):
+        * VM/RegisterFileStack.h:
+        (KJS::RegisterFileStack::inImplicitCall):
+
 2008-05-14  Geoffrey Garen  <ggaren@apple.com>
 

branches/squirrelfish/JavaScriptCore/VM/Machine.cpp

@@ -724 +724 @@
 {
     RegisterFile* registerFile = registerFileStack->current();
-    return Machine::execute(evalNode, exec, thisObj, registerFile, registerFile->size(), scopeChain, exception);
+    if (registerFile->safeForReentry())
+        return Machine::execute(evalNode, exec, thisObj, registerFile, registerFile->size(), scopeChain, exception);
+    registerFile = registerFileStack->pushFunctionRegisterFile();
+    JSValue* result = Machine::execute(evalNode, exec, thisObj, registerFile, registerFile->size(), scopeChain, exception);
+    registerFileStack->popFunctionRegisterFile();
+    return result;
 }
 
@@ -1422 +1427 @@
         int base = (++vPC)->u.operand;
         int property = (++vPC)->u.operand;
-
+#ifndef NDEBUG
+        int registerOffset = r - (*registerBase);
+#endif
         JSObject* baseObj = r[base].u.jsValue->toObject(exec);
 
         Identifier& ident = codeBlock->identifiers[property];
         JSValue *result = baseObj->get(exec, ident);
+        ASSERT(registerOffset == (r - (*registerBase)));
         VM_CHECK_EXCEPTION();
         r[dst].u.jsValue = result;
@@ -1444 +1452 @@
         int property = (++vPC)->u.operand;
         int value = (++vPC)->u.operand;
+#ifndef NDEBUG
+        int registerOffset = r - (*registerBase);
+#endif
 
         JSObject* baseObj = r[base].u.jsValue->toObject(exec);
@@ -1449 +1460 @@
         Identifier& ident = codeBlock->identifiers[property];
         baseObj->put(exec, ident, r[value].u.jsValue);
+        ASSERT(registerOffset == (r - (*registerBase)));
         
         VM_CHECK_EXCEPTION();

branches/squirrelfish/JavaScriptCore/VM/RegisterFile.cpp

@@ -53 +53 @@
     if (minCapacity > m_maxSize)
         return false;
+
     size_t numGlobalSlots = this->numGlobalSlots();
     size_t size = m_size + numGlobalSlots;
@@ -69 +70 @@
     if (!count)
         return;
-
+    ASSERT(safeForReentry());
     size_t numGlobalSlots = this->numGlobalSlots();
     size_t size = m_size + numGlobalSlots;

branches/squirrelfish/JavaScriptCore/VM/RegisterFileStack.cpp

@@ -45 +45 @@
 
     // Common case: Existing register file is not in use: re-use it.
-    if (!current->size())
+    if (!current->size()) {
+        current->setSafeForReentry(true);
         return current;
+    }
 
     // Slow case: Existing register file is in use: Create a nested
@@ -87 +89 @@
 RegisterFile* RegisterFileStack::pushFunctionRegisterFile()
 {
-    m_functionStackDepth++;
     return allocateRegisterFile(current()->maxSize() - current()->size());
 }
@@ -93 +94 @@
 void RegisterFileStack::popFunctionRegisterFile()
 {
-    m_functionStackDepth--;
     delete m_stack.last();
     m_stack.removeLast();

branches/squirrelfish/JavaScriptCore/VM/RegisterFileStack.h

@@ -39 +39 @@
         RegisterFileStack()
             : m_globalBase(0)
-            , m_functionStackDepth(0)
         {
             allocateRegisterFile(RegisterFile::DefaultRegisterFileSize, this);
@@ -70 +69 @@
         }
 
-        bool inImplicitCall() { return m_functionStackDepth > 0; }
+        bool inImplicitCall() {
+            for (size_t i = 0; i < m_stack.size(); ++i) { 
+                if (!m_stack[i]->safeForReentry())
+                    return true;
+            }
+            return false;
+        }
+
     private:
         typedef Vector<RegisterFile*, 4> Stack;
@@ -91 +97 @@
         Stack m_stack;
         Register* m_globalBase;
-        int m_functionStackDepth;
     };
 

branches/squirrelfish/LayoutTests/ChangeLog

@@ +1 @@
+2008-05-14  Oliver Hunt  <oliver@apple.com>
+
+        Reviewed by Maciej.
+
+        Bug 19024: SQUIRRELFISH: ASSERTION FAILED: activation->isActivationObject() in Machine::unwindCallFrame
+        <https://bugs.webkit.org/show_bug.cgi?id=19024>
+
+        Make sure we handled tainted global RegisterFiles properly.
+
+        * fast/js/implicit-global-to-global-reentry-expected.txt: Added.
+        * fast/js/implicit-global-to-global-reentry.html: Added.
+
 2008-05-14  Oliver Hunt  <oliver@apple.com>