Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report script loads from eval when debugging #3391

Merged
merged 1 commit into from
Jul 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 24 additions & 31 deletions lib/Jsrt/JsrtDebugManager.cpp
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
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
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
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
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
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
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