Attachment 343761 Details for Bug 187119 – proposed patch.

[patch] proposed patch.

bug-187119.patch (text/plain), 13.36 KB, created by Mark Lam on 2018-06-27 15:22:24 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2018-06-27 15:22:24 PDT

Size: 13.36 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 233280)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,47 @@
>+2018-06-27  Mark Lam  <mark.lam@apple.com>
>+
>+        Fix a bug in $vm.callFrame() and apply previously requested renaming of $vm.println to print.
>+        https://bugs.webkit.org/show_bug.cgi?id=187119
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        $vm.callFrame()'s JSDollarVMCallFrame::finishCreation()
>+        should be checking for codeBlock instead of !codeBlock
>+        before using the codeBlock.
>+
>+        I also renamed some other "print" functions to use "dump" instead
>+        to match their underlying C++ code that they will call e.g.
>+        CodeBlock::dumpSource().
>+
>+        * tools/JSDollarVM.cpp:
>+        (WTF::JSDollarVMCallFrame::finishCreation):
>+        (JSC::functionDumpSourceFor):
>+        (JSC::functionDumpBytecodeFor):
>+        (JSC::doPrint):
>+        (JSC::functionDataLog):
>+        (JSC::functionPrint):
>+        (JSC::functionDumpCallFrame):
>+        (JSC::functionDumpStack):
>+        (JSC::JSDollarVM::finishCreation):
>+        (JSC::functionPrintSourceFor): Deleted.
>+        (JSC::functionPrintBytecodeFor): Deleted.
>+        (JSC::doPrintln): Deleted.
>+        (JSC::functionPrintln): Deleted.
>+        (JSC::functionPrintCallFrame): Deleted.
>+        (JSC::functionPrintStack): Deleted.
>+        * tools/VMInspector.cpp:
>+        (JSC::DumpFrameFunctor::DumpFrameFunctor):
>+        (JSC::DumpFrameFunctor::operator() const):
>+        (JSC::VMInspector::dumpCallFrame):
>+        (JSC::VMInspector::dumpStack):
>+        (JSC::VMInspector::dumpValue):
>+        (JSC::PrintFrameFunctor::PrintFrameFunctor): Deleted.
>+        (JSC::PrintFrameFunctor::operator() const): Deleted.
>+        (JSC::VMInspector::printCallFrame): Deleted.
>+        (JSC::VMInspector::printStack): Deleted.
>+        (JSC::VMInspector::printValue): Deleted.
>+        * tools/VMInspector.h:
>+
> 2018-06-27  Keith Miller  <keith_miller@apple.com>
> 
>         Add logging to try to diagnose where we get a null structure.
>Index: Source/JavaScriptCore/tools/JSDollarVM.cpp
>===================================================================
>--- Source/JavaScriptCore/tools/JSDollarVM.cpp	(revision 233275)
>+++ Source/JavaScriptCore/tools/JSDollarVM.cpp	(working copy)
>@@ -99,7 +99,7 @@ public:
>                 addProperty(vm, "callee", visitor->callee().asCell());
> 
>             CodeBlock* codeBlock = visitor->codeBlock();
>-            if (!codeBlock) {
>+            if (codeBlock) {
>                 addProperty(vm, "codeBlock", codeBlock);
>                 addProperty(vm, "unlinkedCodeBlock", codeBlock->unlinkedCodeBlock());
>                 addProperty(vm, "executable", codeBlock->ownerExecutable());
>@@ -1316,6 +1316,17 @@ static EncodedJSValue JSC_HOST_CALL func
> // Gets a JSDollarVMCallFrame for a specified frame index.
> // Usage: var callFrame = $vm.callFrame(0) // frame 0 is the top frame.
> // Usage: var callFrame = $vm.callFrame() // implies frame 0 i.e. current frame.
>+//
>+// This gives you the ability to query the following:
>+//    callFrame.valid; // false if we asked for a frame beyond the end of the stack, else true.
>+//    callFrame.callee;
>+//    callFrame.codeBlock;
>+//    callFrame.unlinkedCodeBlock;
>+//    callFrame.executable;
>+//
>+// Note: you cannot toString() a codeBlock, unlinkedCodeBlock, or executable because
>+// there are internal objects and not a JS object. Hence, you cannot do string
>+// concatenation with them.
> static EncodedJSValue JSC_HOST_CALL functionCallFrame(ExecState* exec)
> {
>     unsigned frameNumber = 1;
>@@ -1385,8 +1396,10 @@ static CodeBlock* codeBlockFromArg(ExecS
>     return nullptr;
> }
> 
>-// Usage: print("codeblock = " + $vm.codeBlockFor(functionObj))
>-// Usage: print("codeblock = " + $vm.codeBlockFor(codeBlockToken))
>+// Usage: $vm.print("codeblock = ", $vm.codeBlockFor(functionObj))
>+// Usage: $vm.print("codeblock = ", $vm.codeBlockFor(codeBlockToken))
>+// Note: you cannot toString() a codeBlock because it's an internal object and not
>+// a JS object. Hence, you cannot do string concatenation with it.
> static EncodedJSValue JSC_HOST_CALL functionCodeBlockFor(ExecState* exec)
> {
>     CodeBlock* codeBlock = codeBlockFromArg(exec);
>@@ -1398,9 +1411,9 @@ static EncodedJSValue JSC_HOST_CALL func
>     return JSValue::encode(jsUndefined());
> }
> 
>-// Usage: $vm.printSourceFor(functionObj)
>-// Usage: $vm.printSourceFor(codeBlockToken)
>-static EncodedJSValue JSC_HOST_CALL functionPrintSourceFor(ExecState* exec)
>+// Usage: $vm.dumpSourceFor(functionObj)
>+// Usage: $vm.dumpSourceFor(codeBlockToken)
>+static EncodedJSValue JSC_HOST_CALL functionDumpSourceFor(ExecState* exec)
> {
>     CodeBlock* codeBlock = codeBlockFromArg(exec);
>     if (codeBlock)
>@@ -1408,9 +1421,9 @@ static EncodedJSValue JSC_HOST_CALL func
>     return JSValue::encode(jsUndefined());
> }
> 
>-// Usage: $vm.printBytecodeFor(functionObj)
>-// Usage: $vm.printBytecode(codeBlockToken)
>-static EncodedJSValue JSC_HOST_CALL functionPrintBytecodeFor(ExecState* exec)
>+// Usage: $vm.dumpBytecodeFor(functionObj)
>+// Usage: $vm.dumpBytecodeFor(codeBlock)
>+static EncodedJSValue JSC_HOST_CALL functionDumpBytecodeFor(ExecState* exec)
> {
>     CodeBlock* codeBlock = codeBlockFromArg(exec);
>     if (codeBlock)
>@@ -1418,7 +1431,7 @@ static EncodedJSValue JSC_HOST_CALL func
>     return JSValue::encode(jsUndefined());
> }
> 
>-static EncodedJSValue doPrintln(ExecState* exec, bool addLineFeed)
>+static EncodedJSValue doPrint(ExecState* exec, bool addLineFeed)
> {
>     auto scope = DECLARE_THROW_SCOPE(exec->vm());
>     for (unsigned i = 0; i < exec->argumentCount(); ++i) {
>@@ -1440,43 +1453,43 @@ static EncodedJSValue doPrintln(ExecStat
> }
> 
> // Prints a series of comma separate strings without appending a newline.
>-// Usage: $vm.print(str1, str2, str3)
>-static EncodedJSValue JSC_HOST_CALL functionPrint(ExecState* exec)
>+// Usage: $vm.dataLog(str1, str2, str3)
>+static EncodedJSValue JSC_HOST_CALL functionDataLog(ExecState* exec)
> {
>     const bool addLineFeed = false;
>-    return doPrintln(exec, addLineFeed);
>+    return doPrint(exec, addLineFeed);
> }
> 
> // Prints a series of comma separate strings and appends a newline.
>-// Usage: $vm.println(str1, str2, str3)
>-static EncodedJSValue JSC_HOST_CALL functionPrintln(ExecState* exec)
>+// Usage: $vm.print(str1, str2, str3)
>+static EncodedJSValue JSC_HOST_CALL functionPrint(ExecState* exec)
> {
>     const bool addLineFeed = true;
>-    return doPrintln(exec, addLineFeed);
>+    return doPrint(exec, addLineFeed);
> }
> 
>-// Prints the current CallFrame.
>-// Usage: $vm.printCallFrame()
>-static EncodedJSValue JSC_HOST_CALL functionPrintCallFrame(ExecState* exec)
>+// Dumps the current CallFrame.
>+// Usage: $vm.dumpCallFrame()
>+static EncodedJSValue JSC_HOST_CALL functionDumpCallFrame(ExecState* exec)
> {
>-    // When the callers call this function, they are expecting to print their
>+    // When the callers call this function, they are expecting to dump their
>     // own frame. So skip 1 for this frame.
>-    VMInspector::printCallFrame(exec, 1);
>+    VMInspector::dumpCallFrame(exec, 1);
>     return JSValue::encode(jsUndefined());
> }
> 
>-// Prints the JS stack.
>+// Dumps the JS stack.
> // Usage: $vm.printStack()
>-static EncodedJSValue JSC_HOST_CALL functionPrintStack(ExecState* exec)
>+static EncodedJSValue JSC_HOST_CALL functionDumpStack(ExecState* exec)
> {
>-    // When the callers call this function, they are expecting to print the
>+    // When the callers call this function, they are expecting to dump the
>     // stack starting their own frame. So skip 1 for this frame.
>-    VMInspector::printStack(exec, 1);
>+    VMInspector::dumpStack(exec, 1);
>     return JSValue::encode(jsUndefined());
> }
> 
> // Gets the dataLog dump of the indexingMode of the passed value.
>-// Usage: print("indexingMode = " + $vm.indexingMode(jsValue))
>+// Usage: $vm.print("indexingMode = " + $vm.indexingMode(jsValue))
> static EncodedJSValue JSC_HOST_CALL functionIndexingMode(ExecState* exec)
> {
>     if (!exec->argument(0).isObject())
>@@ -1497,7 +1510,7 @@ static EncodedJSValue JSC_HOST_CALL func
> }
> 
> // Gets the dataLog dump of a given JS value as a string.
>-// Usage: print("value = " + $vm.value(jsValue))
>+// Usage: $vm.print("value = " + $vm.value(jsValue))
> static EncodedJSValue JSC_HOST_CALL functionValue(ExecState* exec)
> {
>     WTF::StringPrintStream stream;
>@@ -1511,7 +1524,7 @@ static EncodedJSValue JSC_HOST_CALL func
> }
> 
> // Gets the pid of the current process.
>-// Usage: print("pid = " + $vm.getpid())
>+// Usage: $vm.print("pid = " + $vm.getpid())
> static EncodedJSValue JSC_HOST_CALL functionGetPID(ExecState*)
> {
>     return JSValue::encode(jsNumber(getCurrentProcessID()));
>@@ -1968,13 +1981,13 @@ void JSDollarVM::finishCreation(VM& vm)
>     addFunction(vm, "callFrame", functionCallFrame, 1);
>     addFunction(vm, "codeBlockFor", functionCodeBlockFor, 1);
>     addFunction(vm, "codeBlockForFrame", functionCodeBlockForFrame, 1);
>-    addFunction(vm, "printSourceFor", functionPrintSourceFor, 1);
>-    addFunction(vm, "printBytecodeFor", functionPrintBytecodeFor, 1);
>+    addFunction(vm, "dumpSourceFor", functionDumpSourceFor, 1);
>+    addFunction(vm, "dumpBytecodeFor", functionDumpBytecodeFor, 1);
> 
>+    addFunction(vm, "dataLog", functionDataLog, 1);
>     addFunction(vm, "print", functionPrint, 1);
>-    addFunction(vm, "println", functionPrintln, 1);
>-    addFunction(vm, "printCallFrame", functionPrintCallFrame, 0);
>-    addFunction(vm, "printStack", functionPrintStack, 0);
>+    addFunction(vm, "dumpCallFrame", functionDumpCallFrame, 0);
>+    addFunction(vm, "dumpStack", functionDumpStack, 0);
> 
>     addFunction(vm, "indexingMode", functionIndexingMode, 1);
>     addFunction(vm, "inlineCapacity", functionInlineCapacity, 1);
>Index: Source/JavaScriptCore/tools/VMInspector.cpp
>===================================================================
>--- Source/JavaScriptCore/tools/VMInspector.cpp	(revision 233275)
>+++ Source/JavaScriptCore/tools/VMInspector.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -326,14 +326,14 @@ CodeBlock* VMInspector::codeBlockForFram
>     return functor.codeBlock;
> }
> 
>-class PrintFrameFunctor {
>+class DumpFrameFunctor {
> public:
>     enum Action {
>-        PrintOne,
>-        PrintAll
>+        DumpOne,
>+        DumpAll
>     };
> 
>-    PrintFrameFunctor(Action action, unsigned framesToSkip)
>+    DumpFrameFunctor(Action action, unsigned framesToSkip)
>         : m_action(action)
>         , m_framesToSkip(framesToSkip)
>     {
>@@ -347,7 +347,7 @@ public:
>                 out.print("[", (m_currentFrame - m_framesToSkip - 1), "] ");
>             });
>         }
>-        if (m_action == PrintOne && m_currentFrame > m_framesToSkip)
>+        if (m_action == DumpOne && m_currentFrame > m_framesToSkip)
>             return StackVisitor::Done;
>         return StackVisitor::Continue;
>     }
>@@ -358,25 +358,25 @@ private:
>     mutable unsigned m_currentFrame { 0 };
> };
> 
>-void VMInspector::printCallFrame(CallFrame* callFrame, unsigned framesToSkip)
>+void VMInspector::dumpCallFrame(CallFrame* callFrame, unsigned framesToSkip)
> {
>     if (!ensureCurrentThreadOwnsJSLock(callFrame))
>         return;
>-    PrintFrameFunctor functor(PrintFrameFunctor::PrintOne, framesToSkip);
>+    DumpFrameFunctor functor(DumpFrameFunctor::DumpOne, framesToSkip);
>     callFrame->iterate(functor);
> }
> 
>-void VMInspector::printStack(CallFrame* topCallFrame, unsigned framesToSkip)
>+void VMInspector::dumpStack(CallFrame* topCallFrame, unsigned framesToSkip)
> {
>     if (!ensureCurrentThreadOwnsJSLock(topCallFrame))
>         return;
>     if (!topCallFrame)
>         return;
>-    PrintFrameFunctor functor(PrintFrameFunctor::PrintAll, framesToSkip);
>+    DumpFrameFunctor functor(DumpFrameFunctor::DumpAll, framesToSkip);
>     topCallFrame->iterate(functor);
> }
> 
>-void VMInspector::printValue(JSValue value)
>+void VMInspector::dumpValue(JSValue value)
> {
>     dataLog(value);
> }
>Index: Source/JavaScriptCore/tools/VMInspector.h
>===================================================================
>--- Source/JavaScriptCore/tools/VMInspector.h	(revision 233275)
>+++ Source/JavaScriptCore/tools/VMInspector.h	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2017 Apple Inc. All rights reserved.
>+ * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -69,9 +69,9 @@ public:
>     JS_EXPORT_PRIVATE static bool isValidCell(Heap*, JSCell*);
>     JS_EXPORT_PRIVATE static bool isValidCodeBlock(ExecState*, CodeBlock*);
>     JS_EXPORT_PRIVATE static CodeBlock* codeBlockForFrame(CallFrame* topCallFrame, unsigned frameNumber);
>-    JS_EXPORT_PRIVATE static void printCallFrame(CallFrame*, unsigned framesToSkip = 0);
>-    JS_EXPORT_PRIVATE static void printStack(CallFrame* topCallFrame, unsigned framesToSkip = 0);
>-    JS_EXPORT_PRIVATE static void printValue(JSValue);
>+    JS_EXPORT_PRIVATE static void dumpCallFrame(CallFrame*, unsigned framesToSkip = 0);
>+    JS_EXPORT_PRIVATE static void dumpStack(CallFrame* topCallFrame, unsigned framesToSkip = 0);
>+    JS_EXPORT_PRIVATE static void dumpValue(JSValue);
> 
> private:
>     template <typename Functor> void iterate(const Functor& functor)

Actions: View | Formatted Diff | Diff

Attachments on bug 187119: 343761