From 1c2d2591d4a73415d71d468264dd8e5676862255 Mon Sep 17 00:00:00 2001 From: Kyle Farnung Date: Tue, 18 Jul 2017 16:18:05 -0700 Subject: [PATCH] Report script loads from eval when debugging * Added code to report the load of scripts on eval in order to support stepping into the eval code. * Added protections to prevent re-entrancy when reparsing scripts * Updated debugger test baselines --- lib/Jsrt/JsrtDebugManager.cpp | 55 +++---- lib/Jsrt/JsrtDebugUtils.cpp | 15 +- lib/Jsrt/JsrtDebugUtils.h | 3 +- lib/Runtime/Debug/DebugContext.cpp | 27 +++- lib/Runtime/Debug/DebugContext.h | 3 + .../blockScopeEvalTest.js.dbg.baseline | 9 ++ ...kScopeSlotArraySiblingTest.js.dbg.baseline | 75 ++++++++++ .../breakpoints.js.dbg.baseline | 140 ++++++++++++++++++ .../DebuggerCommon/bug_222633.js.dbg.baseline | 7 + .../DebuggerCommon/bug_256729.js.dbg.baseline | 15 ++ .../globalFuncVars.js.dbg.baseline | 90 +++++++++++ test/DebuggerCommon/rlexe.xml | 4 +- 12 files changed, 407 insertions(+), 36 deletions(-) create mode 100644 test/DebuggerCommon/blockScopeEvalTest.js.dbg.baseline create mode 100644 test/DebuggerCommon/bug_222633.js.dbg.baseline diff --git a/lib/Jsrt/JsrtDebugManager.cpp b/lib/Jsrt/JsrtDebugManager.cpp index ec2ea114439..6c8b9263a56 100644 --- a/lib/Jsrt/JsrtDebugManager.cpp +++ b/lib/Jsrt/JsrtDebugManager.cpp @@ -154,6 +154,21 @@ HRESULT JsrtDebugManager::DbgRegisterFunction(Js::ScriptContext* scriptContext, { utf8SourceInfo->SetDebugDocument(debugDocument); } + + // Raising events during the middle of a source reparse allows the host to reenter the + // script context and cause memory race conditions. Suppressing these events during a + // reparse prevents the issue. Since the host was already expected to call JsDiagGetScripts + // once the attach is completed to get the list of parsed scripts, there is no change in + // behavior. + if (this->debugEventCallback != nullptr && + !scriptContext->GetDebugContext()->GetIsReparsingSource()) + { + JsrtDebugEventObject debugEventObject(scriptContext); + Js::DynamicObject* eventDataObject = debugEventObject.GetEventDataObject(); + JsrtDebugUtils::AddSourceMetadataToObject(eventDataObject, utf8SourceInfo); + + this->CallDebugEventCallback(JsDiagDebugEventSourceCompile, eventDataObject, scriptContext, false /*isBreak*/); + } } return S_OK; @@ -170,16 +185,8 @@ void JsrtDebugManager::ReportScriptCompile_TTD(Js::FunctionBody* body, Js::Utf8S Js::ScriptContext* scriptContext = utf8SourceInfo->GetScriptContext(); JsrtDebugEventObject debugEventObject(scriptContext); - Js::DynamicObject* eventDataObject = debugEventObject.GetEventDataObject(); - JsrtDebugUtils::AddFileNameOrScriptTypeToObject(eventDataObject, utf8SourceInfo); - JsrtDebugUtils::AddLineCountToObject(eventDataObject, utf8SourceInfo); - JsrtDebugUtils::AddPropertyToObject(eventDataObject, JsrtDebugPropertyId::sourceLength, utf8SourceInfo->GetCchLength(), utf8SourceInfo->GetScriptContext()); - - JsDiagDebugEvent jsDiagDebugEvent = JsDiagDebugEventCompileError; - - JsrtDebugDocumentManager* debugDocumentManager = this->GetDebugDocumentManager(); Assert(debugDocumentManager != nullptr); @@ -188,15 +195,13 @@ void JsrtDebugManager::ReportScriptCompile_TTD(Js::FunctionBody* body, Js::Utf8S if(debugDocument != nullptr) { utf8SourceInfo->SetDebugDocument(debugDocument); - - // Only add scriptId if everything is ok as scriptId is used for other operations - JsrtDebugUtils::AddScriptIdToObject(eventDataObject, utf8SourceInfo); } - jsDiagDebugEvent = JsDiagDebugEventSourceCompile; + + JsrtDebugUtils::AddSourceMetadataToObject(eventDataObject, utf8SourceInfo); if(notify) { - this->CallDebugEventCallback(jsDiagDebugEvent, eventDataObject, scriptContext, false /*isBreak*/); + this->CallDebugEventCallback(JsDiagDebugEventSourceCompile, eventDataObject, scriptContext, false /*isBreak*/); } } #endif @@ -208,13 +213,8 @@ void JsrtDebugManager::ReportScriptCompile(Js::JavascriptFunction* scriptFunctio Js::ScriptContext* scriptContext = utf8SourceInfo->GetScriptContext(); JsrtDebugEventObject debugEventObject(scriptContext); - Js::DynamicObject* eventDataObject = debugEventObject.GetEventDataObject(); - JsrtDebugUtils::AddFileNameOrScriptTypeToObject(eventDataObject, utf8SourceInfo); - JsrtDebugUtils::AddLineCountToObject(eventDataObject, utf8SourceInfo); - JsrtDebugUtils::AddPropertyToObject(eventDataObject, JsrtDebugPropertyId::sourceLength, utf8SourceInfo->GetCchLength(), utf8SourceInfo->GetScriptContext()); - JsDiagDebugEvent jsDiagDebugEvent = JsDiagDebugEventCompileError; if (scriptFunction == nullptr) @@ -235,13 +235,13 @@ void JsrtDebugManager::ReportScriptCompile(Js::JavascriptFunction* scriptFunctio if (debugDocument != nullptr) { utf8SourceInfo->SetDebugDocument(debugDocument); - - // Only add scriptId if everything is ok as scriptId is used for other operations - JsrtDebugUtils::AddScriptIdToObject(eventDataObject, utf8SourceInfo); } + jsDiagDebugEvent = JsDiagDebugEventSourceCompile; } + JsrtDebugUtils::AddSourceMetadataToObject(eventDataObject, utf8SourceInfo); + this->CallDebugEventCallback(jsDiagDebugEvent, eventDataObject, scriptContext, false /*isBreak*/); } } @@ -460,11 +460,7 @@ void JsrtDebugManager::CallDebugEventCallbackForBreak(JsDiagDebugEvent debugEven Js::DynamicObject* JsrtDebugManager::GetScript(Js::Utf8SourceInfo* utf8SourceInfo) { Js::DynamicObject* scriptObject = utf8SourceInfo->GetScriptContext()->GetLibrary()->CreateObject(); - - JsrtDebugUtils::AddScriptIdToObject(scriptObject, utf8SourceInfo); - JsrtDebugUtils::AddFileNameOrScriptTypeToObject(scriptObject, utf8SourceInfo); - JsrtDebugUtils::AddLineCountToObject(scriptObject, utf8SourceInfo); - JsrtDebugUtils::AddPropertyToObject(scriptObject, JsrtDebugPropertyId::sourceLength, utf8SourceInfo->GetCchLength(), utf8SourceInfo->GetScriptContext()); + JsrtDebugUtils::AddSourceMetadataToObject(scriptObject, utf8SourceInfo); return scriptObject; } @@ -542,11 +538,8 @@ Js::DynamicObject* JsrtDebugManager::GetSource(Js::ScriptContext* scriptContext, { sourceObject = (Js::DynamicObject*)Js::CrossSite::MarshalVar(utf8SourceInfo->GetScriptContext(), scriptContext->GetLibrary()->CreateObject()); - JsrtDebugUtils::AddScriptIdToObject(sourceObject, utf8SourceInfo); - JsrtDebugUtils::AddFileNameOrScriptTypeToObject(sourceObject, utf8SourceInfo); - JsrtDebugUtils::AddLineCountToObject(sourceObject, utf8SourceInfo); - JsrtDebugUtils::AddPropertyToObject(sourceObject, JsrtDebugPropertyId::sourceLength, utf8SourceInfo->GetCchLength(), utf8SourceInfo->GetScriptContext()); - JsrtDebugUtils::AddSouceToObject(sourceObject, utf8SourceInfo); + JsrtDebugUtils::AddSourceMetadataToObject(sourceObject, utf8SourceInfo); + JsrtDebugUtils::AddSourceToObject(sourceObject, utf8SourceInfo); } return sourceObject; diff --git a/lib/Jsrt/JsrtDebugUtils.cpp b/lib/Jsrt/JsrtDebugUtils.cpp index d83c478459d..f354172c3fb 100644 --- a/lib/Jsrt/JsrtDebugUtils.cpp +++ b/lib/Jsrt/JsrtDebugUtils.cpp @@ -89,7 +89,7 @@ void JsrtDebugUtils::AddLineCountToObject(Js::DynamicObject * object, Js::Utf8So JsrtDebugUtils::AddPropertyToObject(object, JsrtDebugPropertyId::lineCount, (uint32)utf8SourceInfo->GetLineCount(), utf8SourceInfo->GetScriptContext()); } -void JsrtDebugUtils::AddSouceToObject(Js::DynamicObject * object, Js::Utf8SourceInfo * utf8SourceInfo) +void JsrtDebugUtils::AddSourceToObject(Js::DynamicObject * object, Js::Utf8SourceInfo * utf8SourceInfo) { int32 cchLength = utf8SourceInfo->GetCchLength(); AutoArrayPtr sourceContent(HeapNewNoThrowArray(char16, cchLength + 1), cchLength + 1); @@ -107,6 +107,19 @@ void JsrtDebugUtils::AddSouceToObject(Js::DynamicObject * object, Js::Utf8Source } } +void JsrtDebugUtils::AddSourceMetadataToObject(Js::DynamicObject * object, Js::Utf8SourceInfo * utf8SourceInfo) +{ + JsrtDebugUtils::AddFileNameOrScriptTypeToObject(object, utf8SourceInfo); + JsrtDebugUtils::AddLineCountToObject(object, utf8SourceInfo); + JsrtDebugUtils::AddPropertyToObject(object, JsrtDebugPropertyId::sourceLength, utf8SourceInfo->GetCchLength(), utf8SourceInfo->GetScriptContext()); + + if (utf8SourceInfo->HasDebugDocument()) + { + // Only add the script ID in cases where a debug document exists + JsrtDebugUtils::AddScriptIdToObject(object, utf8SourceInfo); + } +} + void JsrtDebugUtils::AddVarPropertyToObject(Js::DynamicObject * object, const char16 * propertyName, Js::Var value, Js::ScriptContext * scriptContext) { const Js::PropertyRecord* propertyRecord; diff --git a/lib/Jsrt/JsrtDebugUtils.h b/lib/Jsrt/JsrtDebugUtils.h index fb2be102258..dec25f50ff3 100644 --- a/lib/Jsrt/JsrtDebugUtils.h +++ b/lib/Jsrt/JsrtDebugUtils.h @@ -24,7 +24,8 @@ class JsrtDebugUtils static void AddLineColumnToObject(Js::DynamicObject* object, Js::FunctionBody* functionBody, int byteCodeOffset); static void AddSourceLengthAndTextToObject(Js::DynamicObject* object, Js::FunctionBody* functionBody, int byteCodeOffset); static void AddLineCountToObject(Js::DynamicObject* object, Js::Utf8SourceInfo* utf8SourceInfo); - static void AddSouceToObject(Js::DynamicObject* object, Js::Utf8SourceInfo* utf8SourceInfo); + static void AddSourceToObject(Js::DynamicObject* object, Js::Utf8SourceInfo* utf8SourceInfo); + static void AddSourceMetadataToObject(Js::DynamicObject* object, Js::Utf8SourceInfo* utf8SourceInfo); static void AddVarPropertyToObject(Js::DynamicObject* object, const char16* propertyName, Js::Var value, Js::ScriptContext* scriptContext); static void AddPropertyToObject(Js::DynamicObject* object, JsrtDebugPropertyId propertyId, double value, Js::ScriptContext* scriptContext); diff --git a/lib/Runtime/Debug/DebugContext.cpp b/lib/Runtime/Debug/DebugContext.cpp index 5883a79d8f1..5593d934482 100644 --- a/lib/Runtime/Debug/DebugContext.cpp +++ b/lib/Runtime/Debug/DebugContext.cpp @@ -10,7 +10,8 @@ namespace Js scriptContext(scriptContext), hostDebugContext(nullptr), diagProbesContainer(nullptr), - debuggerMode(DebuggerMode::NotDebugging) + debuggerMode(DebuggerMode::NotDebugging), + isReparsingSource(false) { Assert(scriptContext != nullptr); } @@ -141,6 +142,30 @@ namespace Js HRESULT DebugContext::RundownSourcesAndReparse(bool shouldPerformSourceRundown, bool shouldReparseFunctions) { + struct AutoRestoreIsReparsingSource + { + AutoRestoreIsReparsingSource(DebugContext* debugContext, bool shouldReparseFunctions) + : debugContext(debugContext) + , shouldReparseFunctions(shouldReparseFunctions) + { + if (this->shouldReparseFunctions) + { + this->debugContext->isReparsingSource = true; + } + } + ~AutoRestoreIsReparsingSource() + { + if (this->shouldReparseFunctions) + { + this->debugContext->isReparsingSource = false; + } + } + + private: + DebugContext* debugContext; + bool shouldReparseFunctions; + } autoRestoreIsReparsingSource(this, shouldReparseFunctions); + OUTPUT_TRACE(Js::DebuggerPhase, _u("DebugContext::RundownSourcesAndReparse scriptContext 0x%p, shouldPerformSourceRundown %d, shouldReparseFunctions %d\n"), this->scriptContext, shouldPerformSourceRundown, shouldReparseFunctions); diff --git a/lib/Runtime/Debug/DebugContext.h b/lib/Runtime/Debug/DebugContext.h index 5756b02d0a7..2cb8f06a44d 100644 --- a/lib/Runtime/Debug/DebugContext.h +++ b/lib/Runtime/Debug/DebugContext.h @@ -60,11 +60,14 @@ namespace Js HostDebugContext * GetHostDebugContext() const { return hostDebugContext; } + bool GetIsReparsingSource() const { return this->isReparsingSource; } + private: ScriptContext * scriptContext; HostDebugContext* hostDebugContext; DebuggerMode debuggerMode; ProbeContainer* diagProbesContainer; + bool isReparsingSource; // Private Functions void WalkAndAddUtf8SourceInfo(Js::Utf8SourceInfo* sourceInfo, JsUtil::List *utf8SourceInfoList); diff --git a/test/DebuggerCommon/blockScopeEvalTest.js.dbg.baseline b/test/DebuggerCommon/blockScopeEvalTest.js.dbg.baseline new file mode 100644 index 00000000000..f44d5140d0d --- /dev/null +++ b/test/DebuggerCommon/blockScopeEvalTest.js.dbg.baseline @@ -0,0 +1,9 @@ +[ + { + "this": "Object {...}", + "locals": { + "b": "string [Uninitialized block variable]" + }, + "scopes0": "undefined undefined" + } +] \ No newline at end of file diff --git a/test/DebuggerCommon/blockScopeSlotArraySiblingTest.js.dbg.baseline b/test/DebuggerCommon/blockScopeSlotArraySiblingTest.js.dbg.baseline index 9dc73418742..a6d4068b9f3 100644 --- a/test/DebuggerCommon/blockScopeSlotArraySiblingTest.js.dbg.baseline +++ b/test/DebuggerCommon/blockScopeSlotArraySiblingTest.js.dbg.baseline @@ -1,6 +1,81 @@ [ { "evaluate": { + "level_0_identifier_0=='level0'": "boolean true" + } + }, + { + "evaluate": { + "level_0_identifier_1=='level0'": "boolean true" + } + }, + { + "evaluate": { + "level_0_identifier_2=='level0'": "boolean true" + } + }, + { + "evaluate": { + "level_1_identifier_0=='level1'": "boolean true" + } + }, + { + "evaluate": { + "arguments[0]=='level1'": "boolean true" + } + }, + { + "evaluate": { + "level_1_identifier_1[0]=='level1'": "boolean true" + } + }, + { + "evaluate": { + "level_1_identifier_2=='level1'": "boolean true" + } + }, + { + "evaluate": { + "level_1_identifier_3=='level1'": "boolean true" + } + }, + { + "evaluate": { + "level_1_identifier_4=='level1'": "boolean true" + } + }, + { + "evaluate": { + "level_1_identifier_0=='level1level1'": "boolean true" + } + }, + { + "evaluate": { + "arguments[0]=='level1level1'": "boolean true" + } + }, + { + "evaluate": { + "level_1_identifier_1[0]=='level1level1'": "boolean true" + } + }, + { + "evaluate": { + "level_1_identifier_2=='level1level1'": "boolean true" + } + }, + { + "evaluate": { + "level_1_identifier_3=='level1'": "boolean true" + } + }, + { + "evaluate": { + "level_1_identifier_4=='level1level1'": "boolean true" + } + }, + { + "evaluate": { "level_0_identifier_0=='level0level1'": "boolean true" } }, diff --git a/test/DebuggerCommon/breakpoints.js.dbg.baseline b/test/DebuggerCommon/breakpoints.js.dbg.baseline index 282804e0cf8..722310798e9 100644 --- a/test/DebuggerCommon/breakpoints.js.dbg.baseline +++ b/test/DebuggerCommon/breakpoints.js.dbg.baseline @@ -3,6 +3,76 @@ "this": { "f": "function " }, + "scopes0": { + "arguments": "Object {...}", + "x": "Object {...}", + "test": "undefined undefined" + }, + "globals": { + "f": "function " + } + }, + { + "callStack": [ + { + "line": 0, + "column": 0, + "sourceText": "x.a++", + "function": "eval code" + }, + { + "line": 8, + "column": 1, + "sourceText": "eval(\"x.a++;/**bp:locals(1);stack()**/ x.b++;\")", + "function": "f" + }, + { + "line": 15, + "column": 0, + "sourceText": "f()", + "function": "Global code" + } + ] + }, + { + "this": { + "f": "function " + }, + "scopes0": { + "arguments": "Object {...}", + "x": "Object {...}", + "test": "undefined undefined" + }, + "globals": { + "f": "function " + } + }, + { + "callStack": [ + { + "line": 0, + "column": 0, + "sourceText": "x.c = \"test\\\"string\"", + "function": "eval code" + }, + { + "line": 10, + "column": 1, + "sourceText": "eval(\"x.c = \\\"test\\\\\\\"string\\\";/**bp:locals(1);stack()**/ x.b++;\")", + "function": "f" + }, + { + "line": 15, + "column": 0, + "sourceText": "f()", + "function": "Global code" + } + ] + }, + { + "this": { + "f": "function " + }, "locals": { "arguments": { "#__proto__": "Object {...}", @@ -42,6 +112,76 @@ "this": { "f": "function " }, + "scopes0": { + "arguments": "Object {...}", + "x": "Object {...}", + "test": "undefined undefined" + }, + "globals": { + "f": "function " + } + }, + { + "callStack": [ + { + "line": 0, + "column": 0, + "sourceText": "x.a++", + "function": "eval code" + }, + { + "line": 8, + "column": 1, + "sourceText": "eval(\"x.a++;/**bp:locals(1);stack()**/ x.b++;\")", + "function": "f" + }, + { + "line": 16, + "column": 0, + "sourceText": "f()", + "function": "Global code" + } + ] + }, + { + "this": { + "f": "function " + }, + "scopes0": { + "arguments": "Object {...}", + "x": "Object {...}", + "test": "undefined undefined" + }, + "globals": { + "f": "function " + } + }, + { + "callStack": [ + { + "line": 0, + "column": 0, + "sourceText": "x.c = \"test\\\"string\"", + "function": "eval code" + }, + { + "line": 10, + "column": 1, + "sourceText": "eval(\"x.c = \\\"test\\\\\\\"string\\\";/**bp:locals(1);stack()**/ x.b++;\")", + "function": "f" + }, + { + "line": 16, + "column": 0, + "sourceText": "f()", + "function": "Global code" + } + ] + }, + { + "this": { + "f": "function " + }, "locals": { "arguments": { "#__proto__": "Object {...}", diff --git a/test/DebuggerCommon/bug_222633.js.dbg.baseline b/test/DebuggerCommon/bug_222633.js.dbg.baseline new file mode 100644 index 00000000000..242ac2d19b3 --- /dev/null +++ b/test/DebuggerCommon/bug_222633.js.dbg.baseline @@ -0,0 +1,7 @@ +[ + { + "evaluate": { + "level_3_identifier_0=='level3level3'": "Error " + } + } +] \ No newline at end of file diff --git a/test/DebuggerCommon/bug_256729.js.dbg.baseline b/test/DebuggerCommon/bug_256729.js.dbg.baseline index 8516abb92a5..5d7fda3d040 100644 --- a/test/DebuggerCommon/bug_256729.js.dbg.baseline +++ b/test/DebuggerCommon/bug_256729.js.dbg.baseline @@ -3,5 +3,20 @@ "evaluate": { "level_1_identifier_0": "Error " } + }, + { + "evaluate": { + "level_1_identifier_1": "string level1" + } + }, + { + "evaluate": { + "level_3_identifier_1": "string level3" + } + }, + { + "evaluate": { + "level_1_identifier_0": "string level1" + } } ] \ No newline at end of file diff --git a/test/DebuggerCommon/globalFuncVars.js.dbg.baseline b/test/DebuggerCommon/globalFuncVars.js.dbg.baseline index c61d315a422..64f4f1ed2a9 100644 --- a/test/DebuggerCommon/globalFuncVars.js.dbg.baseline +++ b/test/DebuggerCommon/globalFuncVars.js.dbg.baseline @@ -6,6 +6,96 @@ "obj": "undefined undefined", "c": "undefined undefined" }, + "scopes0": { + "arguments": "Object {...}", + "mm": "Array [22,33]", + "f1": "function function f1() {}", + "a": "number 10", + "b": "Object {...}", + "c1": "undefined undefined" + }, + "globals": { + "foo": "function ", + "bar": "function ", + "obj": "undefined undefined", + "c": "undefined undefined" + } + }, + { + "evaluate": { + "mm": "Array [22,33]" + } + }, + { + "callStack": [ + { + "line": 3, + "column": 1, + "sourceText": "ex1", + "function": "eval code" + }, + { + "line": 10, + "column": 4, + "sourceText": "eval(' try { \\n abc.def = 10;\\n } catch(ex1) { \\n ex1; /**bp:stack();locals(1);evaluate(\"ex1\");evaluate(\"c1\")**/ } \\nc;')", + "function": "foo" + }, + { + "line": 13, + "column": 0, + "sourceText": "foo()", + "function": "Global code" + } + ] + }, + { + "this": { + "foo": "function ", + "bar": "function ", + "obj": "undefined undefined", + "c": "undefined undefined" + }, + "locals": { + "ex1": { + "#__proto__": "Error ", + "message": "string ", + "description": "string ", + "number": "number -2146823279", + "stack": "string " + } + }, + "scopes0": { + "arguments": "Object {...}", + "mm": "Array [22,33]", + "f1": "function function f1() {}", + "a": "number 10", + "b": "Object {...}", + "c1": "Array [1]" + }, + "globals": { + "foo": "function ", + "bar": "function ", + "obj": "undefined undefined", + "c": "undefined undefined" + } + }, + { + "evaluate": { + "ex1": "Error " + } + }, + { + "evaluate": { + "c1": "Array [1]" + } + }, + { + "this": { + "foo": "function ", + "bar": "function ", + "obj": "undefined undefined", + "c": "undefined undefined" + }, "locals": { "foo": { "#__proto__": "function ", diff --git a/test/DebuggerCommon/rlexe.xml b/test/DebuggerCommon/rlexe.xml index 6508d2cd6df..e200b040dbf 100644 --- a/test/DebuggerCommon/rlexe.xml +++ b/test/DebuggerCommon/rlexe.xml @@ -572,7 +572,7 @@ blockScopeEvalTest.js - -InspectMaxStringLength:33 -debuglaunch -dbgbaseline:emptyJson.dbg.baseline + -InspectMaxStringLength:33 -debuglaunch -dbgbaseline:blockScopeEvalTest.js.dbg.baseline @@ -911,7 +911,7 @@ bug_222633.js - -debuglaunch -dbgbaseline:emptyJson.dbg.baseline + -debuglaunch -dbgbaseline:bug_222633.js.dbg.baseline exclude_dynapogo