Skip to content

Commit

Permalink
[MERGE #3391 @kfarnung] Report script loads from eval when debugging
Browse files Browse the repository at this point in the history 
Merge pull request #3391 from kfarnung:evaldebug

* 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
  • Loading branch information
@kfarnung
kfarnung committed Jul 25, 2017
2 parents 804093e + 1c2d259 commit 1b75f1b
Show file tree
Hide file tree
Showing 12 changed files with 407 additions and 36 deletions.
55 changes: 24 additions & 31 deletions lib/Jsrt/JsrtDebugManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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*/);
}
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
15 changes: 14 additions & 1 deletion lib/Jsrt/JsrtDebugUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<char16> sourceContent(HeapNewNoThrowArray(char16, cchLength + 1), cchLength + 1);
Expand All @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion lib/Jsrt/JsrtDebugUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
27 changes: 26 additions & 1 deletion lib/Runtime/Debug/DebugContext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ namespace Js
scriptContext(scriptContext),
hostDebugContext(nullptr),
diagProbesContainer(nullptr),
debuggerMode(DebuggerMode::NotDebugging)
debuggerMode(DebuggerMode::NotDebugging),
isReparsingSource(false)
{
Assert(scriptContext != nullptr);
}
Expand Down Expand Up @@ -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);

Expand Down
3 changes: 3 additions & 0 deletions lib/Runtime/Debug/DebugContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Js::Utf8SourceInfo *, Recycler, false, Js::CopyRemovePolicy, RecyclerPointerComparer> *utf8SourceInfoList);
Expand Down
9 changes: 9 additions & 0 deletions test/DebuggerCommon/blockScopeEvalTest.js.dbg.baseline
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[
{
"this": "Object {...}",
"locals": {
"b": "string [Uninitialized block variable]"
},
"scopes0": "undefined undefined"
}
]
75 changes: 75 additions & 0 deletions test/DebuggerCommon/blockScopeSlotArraySiblingTest.js.dbg.baseline
View file
Original file line number Diff line number Diff line change
@@ -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"
}
},
Expand Down
Loading

0 comments on commit 1b75f1b

Please sign in to comment.