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.

Sign up for GitHub

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

Jump to bottom

[wasm][debugger] Detect new keyword in derived members on GetProperties #73230

Merged
merged 5 commits into from
Aug 9, 2022
Merged

[wasm][debugger] Detect new keyword in derived members on GetProperties #73230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5d073ac
Fix of VHO for most test cases.
ilonatommy Aug 1, 2022
e333812
Cleanup.
ilonatommy Aug 2, 2022
041ec49
New testcases with auto-props.
ilonatommy Aug 2, 2022
397ab8b
Adjusted ProtectionLvlTest to new changes.
ilonatommy Aug 2, 2022
7455c5e
Merge branch 'main' into get-properties-vho-69788
ilonatommy Aug 4, 2022
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
90 changes: 64 additions & 26 deletions src/mono/wasm/debugger/BrowserDebugProxy/MemberObjectsExplorer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,17 @@ private static string GetNamePrefixForValues(string memberName, string typeName,
return $"{memberName} ({justClassName})";
}

private static async Task<JObject> ReadFieldValue(MonoSDBHelper sdbHelper, MonoBinaryReader reader, FieldTypeClass field, int objectId, TypeInfoWithDebugInformation typeInfo, int fieldValueType, bool isOwn, GetObjectCommandOptions getObjectOptions, CancellationToken token)
private static async Task<JObject> ReadFieldValue(
MonoSDBHelper sdbHelper,
MonoBinaryReader reader,
FieldTypeClass field,
int objectId,
TypeInfoWithDebugInformation typeInfo,
int fieldValueType,
bool isOwn,
int parentTypeId,
GetObjectCommandOptions getObjectOptions,
CancellationToken token)
{
var fieldValue = await sdbHelper.ValueCreator.ReadAsVariableValue(
reader,
Expand All @@ -62,7 +72,10 @@ private static async Task<JObject> ReadFieldValue(MonoSDBHelper sdbHelper, MonoB
_ => "internal"
};
if (field.IsBackingField)
{
fieldValue["__isBackingField"] = true;
fieldValue["__parentTypeId"] = parentTypeId;
}
if (field.Attributes.HasFlag(FieldAttributes.Static))
fieldValue["__isStatic"] = true;

Expand Down Expand Up @@ -189,6 +202,7 @@ public static async Task<JArray> ExpandFieldValues(
MonoSDBHelper sdbHelper,
DotnetObjectId id,
int containerTypeId,
int parentTypeId,
IReadOnlyList<FieldTypeClass> fields,
GetObjectCommandOptions getCommandOptions,
bool isOwn,
Expand Down Expand Up @@ -220,7 +234,7 @@ public static async Task<JArray> ExpandFieldValues(
int valtype = retDebuggerCmdReader.ReadByte();
retDebuggerCmdReader.BaseStream.Position = initialPos;

JObject fieldValue = await ReadFieldValue(sdbHelper, retDebuggerCmdReader, field, id.Value, typeInfo, valtype, isOwn, getCommandOptions, token);
JObject fieldValue = await ReadFieldValue(sdbHelper, retDebuggerCmdReader, field, id.Value, typeInfo, valtype, isOwn, parentTypeId, getCommandOptions, token);
numFieldsRead++;

if (!Enum.TryParse(fieldValue["__state"].Value<string>(), out DebuggerBrowsableState fieldState)
Expand Down Expand Up @@ -302,7 +316,8 @@ public static async Task<Dictionary<string, JObject>> ExpandPropertyValues(
bool isOwn,
CancellationToken token,
Dictionary<string, JObject> allMembers,
bool includeStatic)
bool includeStatic,
int parentTypeId = -1)
{
using var retDebuggerCmdReader = await sdbHelper.GetTypePropertiesReader(typeId, token);
if (retDebuggerCmdReader == null)
Expand Down Expand Up @@ -330,14 +345,16 @@ public static async Task<Dictionary<string, JObject>> ExpandPropertyValues(
MethodInfoWithDebugInformation getterInfo = await sdbHelper.GetMethodInfo(getMethodId, token);
MethodAttributes getterAttrs = getterInfo.Info.Attributes;
MethodAttributes getterMemberAccessAttrs = getterAttrs & MethodAttributes.MemberAccessMask;
MethodAttributes vtableLayout = getterAttrs & MethodAttributes.VtableLayoutMask;
bool isNewSlot = (vtableLayout & MethodAttributes.NewSlot) == MethodAttributes.NewSlot;

typePropertiesBrowsableInfo.TryGetValue(propName, out DebuggerBrowsableState? state);

// handle parents' members:
if (!allMembers.TryGetValue(propName, out JObject existingMember))
{
// new member
await AddProperty(getMethodId, state, propName, getterMemberAccessAttrs, isStatic);
await AddProperty(getMethodId, parentTypeId, state, propName, getterMemberAccessAttrs, isStatic, isNewSlot: isNewSlot);
continue;
}

Expand All @@ -358,24 +375,29 @@ public static async Task<Dictionary<string, JObject>> ExpandPropertyValues(
}

var overriddenOrHiddenPropName = $"{propName} ({parentSuffix})";
MethodAttributes vtableLayout = getterAttrs & MethodAttributes.VtableLayoutMask;
bool wasOverriddenByDerivedType = (vtableLayout & MethodAttributes.NewSlot) == MethodAttributes.NewSlot;
if (wasOverriddenByDerivedType)
if (isNewSlot)
{
/*
* property was overridden by a derived type member. We want to show
* only the overridden members. So, remove the backing field
* for this auto-property that was added, with the type name suffix
*
* Two cases:
* 1. auto-prop in base, overridden by auto-prop in derived
* 2. auto-prop in base, overridden by prop in derived
*
* And in both cases we want to remove the backing field for the auto-prop for
* *this* base type
*/
allMembers.Remove(overriddenOrHiddenPropName);
continue;
// this has `new` keyword if it is newSlot but direct child was not a newSlot:
var child = allMembers.FirstOrDefault(
kvp => (kvp.Key == propName || kvp.Key.StartsWith($"{propName} (")) && kvp.Value["__parentTypeId"]?.Value<int>() == typeId).Value;
bool wasOverriddenByDerivedType = child != null && child["__isNewSlot"]?.Value<bool>() != true;
if (wasOverriddenByDerivedType)
{
/*
* property was overridden by a derived type member. We want to show
* only the overridden members. So, remove the backing field
* for this auto-property that was added, with the type name suffix
*
* Two cases:
* 1. auto-prop in base, overridden by auto-prop in derived
* 2. auto-prop in base, overridden by prop in derived
*
* And in both cases we want to remove the backing field for the auto-prop for
* *this* base type
*/
allMembers.Remove(overriddenOrHiddenPropName);
continue;
}
}

/*
Expand All @@ -388,7 +410,7 @@ public static async Task<Dictionary<string, JObject>> ExpandPropertyValues(
{
// hiding with a non-auto property, so nothing to adjust
// add the new property
await AddProperty(getMethodId, state, overriddenOrHiddenPropName, getterMemberAccessAttrs, isStatic);
await AddProperty(getMethodId, parentTypeId, state, overriddenOrHiddenPropName, getterMemberAccessAttrs, isStatic, isNewSlot: isNewSlot);
continue;
}

Expand Down Expand Up @@ -420,7 +442,14 @@ async Task UpdateBackingFieldWithPropertyAttributes(JObject backingField, string
allMembers[evalue["name"].Value<string>()] = evalue;
}

async Task AddProperty(int getMethodId, DebuggerBrowsableState? state, string propNameWithSufix, MethodAttributes getterAttrs, bool isPropertyStatic)
async Task AddProperty(
int getMethodId,
int parentTypeId,
DebuggerBrowsableState? state,
string propNameWithSufix,
MethodAttributes getterAttrs,
bool isPropertyStatic,
bool isNewSlot)
{
string returnTypeName = await sdbHelper.GetReturnType(getMethodId, token);
JObject propRet = null;
Expand Down Expand Up @@ -448,6 +477,11 @@ async Task AddProperty(int getMethodId, DebuggerBrowsableState? state, string pr
_ => "internal"
};
propRet["__state"] = state?.ToString();
if (parentTypeId != -1)
{
propRet["__parentTypeId"] = parentTypeId;
propRet["__isNewSlot"] = isNewSlot;
}

string namePrefix = GetNamePrefixForValues(propNameWithSufix, typeName, isOwn, state);
var expandedMembers = await GetExpandedMemberValues(
Expand Down Expand Up @@ -529,9 +563,11 @@ public static async Task<GetMembersResult> GetObjectMemberValues(
ArraySegment<byte> getPropertiesParamBuffer = commandParamsObjWriter.GetParameterBuffer();

var allMembers = new Dictionary<string, JObject>();
for (int i = 0; i < typeIdsIncludingParents.Count; i++)
int typeIdsCnt = typeIdsIncludingParents.Count;
for (int i = 0; i < typeIdsCnt; i++)
{
int typeId = typeIdsIncludingParents[i];
int parentTypeId = i + 1 < typeIdsCnt ? typeIdsIncludingParents[i + 1] : -1;
string typeName = await sdbHelper.GetTypeName(typeId, token);
// 0th id is for the object itself, and then its ancestors
bool isOwn = i == 0;
Expand All @@ -541,7 +577,7 @@ public static async Task<GetMembersResult> GetObjectMemberValues(
if (thisTypeFields.Count > 0)
{
var allFields = await ExpandFieldValues(
sdbHelper, id, typeId, thisTypeFields, getCommandType, isOwn, includeStatic, token);
sdbHelper, id, typeId, parentTypeId, thisTypeFields, getCommandType, isOwn, includeStatic, token);

if (getCommandType.HasFlag(GetObjectCommandOptions.AccessorPropertiesOnly))
{
Expand All @@ -566,7 +602,8 @@ public static async Task<GetMembersResult> GetObjectMemberValues(
isOwn,
token,
allMembers,
includeStatic);
includeStatic,
parentTypeId);

// ownProperties
// Note: ownProperties should mean that we return members of the klass itself,
Expand All @@ -577,6 +614,7 @@ public static async Task<GetMembersResult> GetObjectMemberValues(
/*if (accessorPropertiesOnly)
break;*/
}

return GetMembersResult.FromValues(allMembers.Values, sortByAccessLevel);

static void AddOnlyNewFieldValuesByNameTo(JArray namedValues, IDictionary<string, JObject> valuesDict, string typeName, bool isOwn)
Expand Down
17 changes: 13 additions & 4 deletions src/mono/wasm/debugger/DebuggerTestSuite/GetPropertiesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ public GetPropertiesTests(ITestOutputHelper testOutput) : base(testOutput)
public static TheoryData<string, bool?, bool?, string[], Dictionary<string, (JObject, bool)>, bool> ClassGetPropertiesTestData(bool is_async)
{
// FIXME: invoking getter on the hidden(base) properties - is that supported??
// FIXME: add case - v -> o -> n, v -> n -> o
var data = new TheoryData<string, bool?, bool?, string[], Dictionary<string, (JObject, bool)>, bool>();

var type_name = "DerivedClass2";
Expand All @@ -34,6 +33,9 @@ public GetPropertiesTests(ITestOutputHelper testOutput) : base(testOutput)
{"BaseBase_PropertyForVHO", (TGetter("BaseBase_PropertyForVHO", TString("Derived#BaseBase_PropertyForVHO")), true)},
{"BaseBase_PropertyForVOH", (TGetter("BaseBase_PropertyForVOH", TString("Derived#BaseBase_PropertyForVOH")), true)},
// {"BaseBase_PropertyForVOO", (TGetter("BaseBase_PropertyForVOO", TString("Derived#BaseBase_PropertyForVOO")), true)}, //FixMe: Issue #69788
{"BaseBase_AutoPropertyForVHO", (TString("Derived#BaseBase_AutoPropertyForVHO"), true)},
{"BaseBase_AutoPropertyForVOH", (TString("Derived#BaseBase_AutoPropertyForVOH"), true)},
// {"BaseBase_AutoPropertyForVOO", (TGetter("BaseBase_AutoPropertyForVOO", TString("Derived#BaseBase_AutoPropertyForVOO")), true)}, //FixMe: Issue #69788

// protected / internal:
{"BaseBase_AutoPropertyForHidingWithProperty", (TGetter("BaseBase_AutoPropertyForHidingWithProperty", TString("Derived#BaseBase_AutoPropertyForHidingWithProperty")), true)},
Expand All @@ -53,6 +55,7 @@ public GetPropertiesTests(ITestOutputHelper testOutput) : base(testOutput)
{"FirstName", (TGetter("FirstName", TString("BaseClass#FirstName")), false)},
{"LastName", (TGetter("LastName", TString("BaseClass#LastName")), false)},
{"BaseBase_PropertyForVOH (BaseClass2)", (TGetter("BaseBase_PropertyForVOH (BaseClass2)", TString("Base#BaseBase_PropertyForVOH")), false)},
{"BaseBase_AutoPropertyForVOH (BaseClass2)", (TString("Base#BaseBase_AutoPropertyForVOH"), false)},

// protected / internal:
{"BaseBase_PropertyForHidingWithField (BaseClass2)", (TNumber(110), false)},
Expand All @@ -75,7 +78,8 @@ public GetPropertiesTests(ITestOutputHelper testOutput) : base(testOutput)
{"BaseBase_FieldForHidingWithAutoProperty (BaseBaseClass2)", (TString("BaseBase#BaseBase_FieldForHidingWithAutoProperty"), false)},
{"BaseBase_PropertyForHidingWithAutoProperty (BaseBaseClass2)", (TGetter("BaseBase_PropertyForHidingWithAutoProperty (BaseBaseClass2)", TString("BaseBase#BaseBase_PropertyForHidingWithAutoProperty")), false)},
{"BaseBase_AutoPropertyForHidingWithAutoProperty (BaseBaseClass2)", (TString("BaseBase#BaseBase_AutoPropertyForHidingWithAutoProperty"), false)},
// {"BaseBase_PropertyForVHO (BaseBaseClass2)", (TGetter("BaseBase_PropertyForVHO (BaseBaseClass2)", TString("BaseBase#BaseBase_PropertyForVHO")), false)}, // FixMe: Issue #69788
{"BaseBase_PropertyForVHO (BaseBaseClass2)", (TGetter("BaseBase_PropertyForVHO (BaseBaseClass2)", TString("BaseBase#BaseBase_PropertyForVHO")), false)},
{"BaseBase_AutoPropertyForVHO (BaseBaseClass2)", (TString("BaseBase#BaseBase_AutoPropertyForVHO"), false)},
};

// default, all properties
Expand Down Expand Up @@ -117,7 +121,7 @@ public GetPropertiesTests(ITestOutputHelper testOutput) : base(testOutput)
"BaseBase_PropertyForHidingWithProperty (BaseBaseClass2)",
"BaseBase_PropertyForHidingWithAutoProperty (BaseBaseClass2)",
"Base_VirtualPropertyNotOverriddenOrHidden",
// "BaseBase_PropertyForVHO (BaseBaseClass2)" // FixMe: Issue #69788
"BaseBase_PropertyForVHO (BaseBaseClass2)"
};

var only_own_accessors = new[]
Expand Down Expand Up @@ -457,6 +461,9 @@ public static TheoryData<Dictionary<string, JObject>, Dictionary<string, JObject
{"BaseBase_PropertyForVHO", TGetter("BaseBase_PropertyForVHO", TString("Derived#BaseBase_PropertyForVHO"))},
{"BaseBase_PropertyForVOH", TGetter("BaseBase_PropertyForVOH", TString("Derived#BaseBase_PropertyForVOH"))},
// {"BaseBase_PropertyForVOO", TGetter("BaseBase_PropertyForVOO", TString("Derived#BaseBase_PropertyForVOO"))}, //FixMe: Issue #69788
{"BaseBase_AutoPropertyForVHO", TString("Derived#BaseBase_AutoPropertyForVHO")},
{"BaseBase_AutoPropertyForVOH", TString("Derived#BaseBase_AutoPropertyForVOH")},
// {"BaseBase_AutoPropertyForVOO", TString("Derived#BaseBase_AutoPropertyForVOO")}, //FixMe: Issue #69788

// inherited from Base:
{"BaseBase_AutoPropertyForHidingWithField", TNumber(115)},
Expand All @@ -466,6 +473,7 @@ public static TheoryData<Dictionary<string, JObject>, Dictionary<string, JObject
{"LastName", TGetter("LastName", TString("BaseClass#LastName"))},
{"Base_VirtualPropertyNotOverriddenOrHidden", TGetter("Base_VirtualPropertyNotOverriddenOrHidden", TDateTime(new DateTime(2124, 5, 7, 1, 9, 2)))},
{"BaseBase_PropertyForVOH (BaseClass2)", TGetter("BaseBase_PropertyForVOH (BaseClass2)", TString("Base#BaseBase_PropertyForVOH"))},
{"BaseBase_AutoPropertyForVOH (BaseClass2)", TString("Base#BaseBase_AutoPropertyForVOH")},

// inherited from BaseBase:
{"BaseBase_FieldForHidingWithField (BaseBaseClass2)", TNumber(5)},
Expand All @@ -477,7 +485,8 @@ public static TheoryData<Dictionary<string, JObject>, Dictionary<string, JObject
{"BaseBase_FieldForHidingWithAutoProperty (BaseBaseClass2)", TString("BaseBase#BaseBase_FieldForHidingWithAutoProperty")},
{"BaseBase_PropertyForHidingWithAutoProperty (BaseBaseClass2)", TGetter("BaseBase_PropertyForHidingWithAutoProperty (BaseBaseClass2)", TString("BaseBase#BaseBase_PropertyForHidingWithAutoProperty"))},
{"BaseBase_AutoPropertyForHidingWithAutoProperty (BaseBaseClass2)", TString("BaseBase#BaseBase_AutoPropertyForHidingWithAutoProperty")},
// {"BaseBase_PropertyForVHO (BaseBaseClass2)", TGetter("BaseBase_PropertyForVHO (BaseBaseClass2)", TString("BaseBase#BaseBase_PropertyForVHO"))}, // FixMe: Issue #69788
{"BaseBase_PropertyForVHO (BaseBaseClass2)", TGetter("BaseBase_PropertyForVHO (BaseBaseClass2)", TString("BaseBase#BaseBase_PropertyForVHO"))},
{"BaseBase_AutoPropertyForVHO (BaseBaseClass2)", TString("BaseBase#BaseBase_AutoPropertyForVHO")},
};

var internal_protected_props = new Dictionary<string, JObject>(){
Expand Down
Loading