Changeset 249577 in webkit for trunk/Source/JavaScriptCore/interpreter/ShadowChicken.cpp

Timestamp:

Sep 6, 2019, 10:03:28 AM (6 years ago)

Author:

Ryan Haddad

Message:

Unreviewed, rolling out r249566.

Causes inspector layout test crashes under GuardMalloc

Reverted changeset:

"Tail Deleted Frames shown in Web Inspector are sometimes
incorrect (Shadow Chicken)"
​https://bugs.webkit.org/show_bug.cgi?id=201366
​https://trac.webkit.org/changeset/249566

File:

• trunk/Source/JavaScriptCore/interpreter/ShadowChicken.cpp (modified) (10 diffs)

trunk/Source/JavaScriptCore/interpreter/ShadowChicken.cpp

@@ -46 +46 @@
     
     if (isPrologue()) {
-        String name = "?"_s;
-        if (auto* function = jsDynamicCast<JSFunction*>(callee->vm(), callee)) {
-            name = function->name(callee->vm());
-            if (name.isEmpty())
-                name = "?"_s;
-        }
-
         out.print(
             "{callee = ", RawPointer(callee), ", frame = ", RawPointer(frame), ", callerFrame = ",
-            RawPointer(callerFrame), ", name = ", name, "}");
+            RawPointer(callerFrame), "}");
         return;
     }
@@ -70 +63 @@
 void ShadowChicken::Frame::dump(PrintStream& out) const
 {
-    String name = "?"_s;
-    if (auto* function = jsDynamicCast<JSFunction*>(callee->vm(), callee)) {
-        name = function->name(callee->vm());
-        if (name.isEmpty())
-            name = "?"_s;
-    }
-
     out.print(
-        "{callee = ", *callee, ", frame = ", RawPointer(frame), ", isTailDeleted = ",
-        isTailDeleted, ", name = ", name, "}");
+        "{callee = ", RawPointer(callee), ", frame = ", RawPointer(frame), ", isTailDeleted = ",
+        isTailDeleted, "}");
 }
 
@@ -85 +71 @@
     : m_logSize(Options::shadowChickenLogSize())
 {
-    // Allow one additional packet beyond m_logEnd. This is useful for the moment we
-    // log a packet when the log is full and force an update. At that moment the packet
-    // that is being logged should be included in the update because it may be
-    // a critical prologue needed to rationalize the current machine stack with the
-    // shadow stack.
-    m_log = static_cast<Packet*>(fastZeroedMalloc(sizeof(Packet) * m_logSize + 1));
+    m_log = static_cast<Packet*>(fastZeroedMalloc(sizeof(Packet) * m_logSize));
     m_logCursor = m_log;
     m_logEnd = m_log + m_logSize;
@@ -102 +83 @@
 void ShadowChicken::log(VM& vm, ExecState* exec, const Packet& packet)
 {
-    // This write is allowed because we construct the log with space for 1 additional packet.
+    update(vm, exec);
     *m_logCursor++ = packet;
-    update(vm, exec);
 }
 
@@ -163 +143 @@
     }
 
+    
     if (ShadowChickenInternal::verbose)
         dataLog("    Revised stack: ", listDump(m_stack), "\n");
@@ -308 +289 @@
 
             CallFrame* callFrame = visitor->callFrame();
-            if (ShadowChickenInternal::verbose) {
-                dataLog("    Examining callFrame:", RawPointer(callFrame), ", callee:", RawPointer(callFrame->jsCallee()), ", callerFrame:", RawPointer(callFrame->callerFrame()), "\n");
-                JSObject* callee = callFrame->jsCallee();
-                if (auto* function = jsDynamicCast<JSFunction*>(callee->vm(), callee))
-                    dataLog("      Function = ", function->name(callee->vm()), "\n");
-            }
-
+            if (ShadowChickenInternal::verbose)
+                dataLog("    Examining ", RawPointer(callFrame), "\n");
             if (callFrame == highestPointSinceLastTime) {
                 if (ShadowChickenInternal::verbose)
-                    dataLog("    Bailing at ", RawPointer(callFrame), " because it's the highest point since last time\n");
-
-                // FIXME: At this point the shadow stack may still have tail deleted frames
-                // that do not run into the current call frame but are left in the shadow stack.
-                // Those tail deleted frames should be validated somehow.
-
+                    dataLog("    Bailing at ", RawPointer(callFrame), " because it's the highest point since last time.\n");
                 return StackVisitor::Done;
             }
@@ -348 +319 @@
                 && m_log[indexInLog].frame == toPush.last().frame) {
                 if (ShadowChickenInternal::verbose)
-                    dataLog("    Going to loop through to find tail deleted frames using ", RawPointer(callFrame), " with indexInLog = ", indexInLog, " and push-stack top = ", toPush.last(), "\n");
+                    dataLog("    Going to loop through to find tail deleted frames with indexInLog = ", indexInLog, " and push-stack top = ", toPush.last(), "\n");
                 for (;;) {
                     ASSERT(m_log[indexInLog].frame == toPush.last().frame);
@@ -370 +341 @@
                     }
                     indexInLog--; // Skip over the tail packet.
-
-                    // FIXME: After a few iterations the tail packet referenced frame may not be the
-                    // same as the original callFrame for the real stack frame we started with.
-                    // It is unclear when we should break.
                     
                     if (!advanceIndexInLogTo(tailPacket.frame, nullptr, nullptr)) {
@@ -413 +380 @@
 
     if (ShadowChickenInternal::verbose)
-        dataLog("    After pushing: ", listDump(m_stack), "\n");
+        dataLog("    After pushing: ", *this, "\n");
 
     // Remove tail frames until the number of tail deleted frames is small enough.
@@ -481 +448 @@
     out.print("\n");
     for (unsigned i = 0; i < limit; ++i)
-        out.print("\t", comma, "[", i, "] ", m_log[i], "\n");
+        out.print("\t", comma, m_log[i], "\n");
     out.print("]}");
 }