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

Crash when accessing static hidden methods from zero touch nodes #11238

Merged
merged 14 commits into from
Nov 12, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ internal IEnumerable<CompletionData> GetCompletionsOnType(string code, string st
}
}

return members?.Select(x => CompletionData.ConvertMirrorToCompletionData(x));
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
return members?.Distinct(new StaticMirrorNameComparer()).Select(x => CompletionData.ConvertMirrorToCompletionData(x));
aparajit-pratap marked this conversation as resolved.
Show resolved Hide resolved
}

/// <summary>
Expand Down Expand Up @@ -183,6 +183,7 @@ internal IEnumerable<CompletionData> GetFunctionSignatures(string code, string f
candidates = type.GetOverloadsOnInstance(functionName);
}
}

pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
return candidates.Select(x => CompletionData.ConvertMirrorToCompletionData(x));
}

Expand Down
36 changes: 20 additions & 16 deletions src/Engine/ProtoCore/Lang/CallSite.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1118,23 +1118,26 @@ private FunctionEndPoint SelectFEPFromMultiple(StackFrame stackFrame, RuntimeCor
Validity.Assert(svThisPtr.IsPointer,
"this pointer wasn't a pointer. {89635B06-AD53-4170-ADA5-065EB2AE5858}");

int typeID = svThisPtr.metaData.type;

//Test for exact match
List<FunctionEndPoint> exactFeps = new List<FunctionEndPoint>();

foreach (FunctionEndPoint fep in feps)
if (fep.ClassOwnerIndex == typeID)
exactFeps.Add(fep);
// We have 2 possible cases here:
// 1. Static method call (i.e no this pointer)
// Here we will cover the specific case of static method hiding.
// We do not need to check actually if the method has the "IsHideBySig" (https://docs.microsoft.com/en-us/dotnet/api/system.reflection.methodbase.ishidebysig)
// because static methods can only be hidden.
// In this case we simply select the function that belongs to the calling class.
// The assumption here is that all function end points in "feps" have already been checked that they have the same signature.
// 2. Method call from an instance of a class.
// Try to match with methods of that class.
int typeID = svThisPtr.Pointer == Constants.kInvalidIndex ? stackFrame.ClassScope : svThisPtr.metaData.type;
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved

if (exactFeps.Count == 1)
//Test for exact match
IEnumerable<FunctionEndPoint> exactFeps = feps.Where(x => x.ClassOwnerIndex == typeID);
if (exactFeps.Count() == 1)
{
return exactFeps[0];
return exactFeps.First();
}



//Walk the class tree structure to find the method

while (runtimeCore.DSExecutable.classTable.ClassNodes[typeID].Base != Constants.kInvalidIndex)
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure this part of the matching algorithm makes sense for global functions...
TypeId here (svThisPtr.metaData.type) seems to always be initialized to 0 and ClassNodes[0] seems to always holdsthe Null primitive type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. I think typeID in the case of global functions will be 0 and its base id will be -1 (invalid index) in that case.

{
typeID = runtimeCore.DSExecutable.classTable.ClassNodes[typeID].Base;
Expand All @@ -1149,15 +1152,15 @@ private FunctionEndPoint SelectFEPFromMultiple(StackFrame stackFrame, RuntimeCor

foreach (FunctionEndPoint fep in feps)
{
int noArbitraries = 0;
int numArbitraryRanks = 0;

for (int i = 0; i < argumentsList.Count; i++)
{
if (fep.FormalParams[i].rank == Constants.kArbitraryRank)
noArbitraries++;

numberOfArbitraryRanks.Add(noArbitraries);
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
numArbitraryRanks++;
}

numberOfArbitraryRanks.Add(numArbitraryRanks);
}

int smallest = Int32.MaxValue;
Expand Down Expand Up @@ -1461,6 +1464,7 @@ private StackValue DispatchNew(
candidatesFeps.Add(fep);
}
}

funcGroup = new FunctionGroup(candidatesFeps);

#endregion
Expand Down
69 changes: 56 additions & 13 deletions src/Engine/ProtoCore/Reflection/Mirror.cs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,23 @@ public override string ToString()
}
}

//
/// <summary>
/// Comparer class that defines methods to support the comparison of StaticMirror objects for equality.
/// </summary>
internal class StaticMirrorNameComparer : IEqualityComparer<StaticMirror>
{
public bool Equals(StaticMirror x, StaticMirror y)
{
return x.Name == y.Name;
}

public int GetHashCode(StaticMirror obj)
{
return obj.Name.GetHashCode();
}
}

/// <summary>
/// A ClassMirror object reflects upon the type of a single designscript variable
/// The information here is populated during the code generation phase
Expand Down Expand Up @@ -337,23 +354,30 @@ internal ClassMirror(StackValue svData, ProtoCore.Core core)
/// <summary>
/// Returns the constructors and static methods and properties
/// belonging to the type and its base types
/// Excludes hidden methods and properties from base types.
/// </summary>
/// <returns></returns>
public IEnumerable<StaticMirror> GetMembers()
{
// TODO: Factor out reflection functionality for both LibraryServices and Mirrors
List<StaticMirror> members = new List<StaticMirror>();
members.AddRange(GetConstructors().GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(GetFunctions().Where(m => m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(GetProperties().Where(m => m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));

// Filter out hidden functions/properties:
// Create a set of unique function, property and constructor descriptions.
// In this case we use ToString() to get the uniques description of the members (func signature, property names, constructor names).
var derivedClassMembers = new HashSet<string>();
members.ForEach(x => derivedClassMembers.Add(x.ToString()));

IEnumerable<ClassMirror> baseClasses = this.GetClassHierarchy();
foreach (var baseClass in baseClasses)
{
members.AddRange(baseClass.GetFunctions().Where(m => m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(baseClass.GetProperties().Where(m => m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(baseClass.GetFunctions().Where(m => m.IsStatic && !derivedClassMembers.Contains(m.ToString())).GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(baseClass.GetProperties().Where(m => m.IsStatic && !derivedClassMembers.Contains(m.ToString())).GroupBy(x => x.Name).Select(y => y.First()));
}

members.AddRange(this.GetConstructors().GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(this.GetFunctions().Where(m => m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(this.GetProperties().Where(m => m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));
return members;
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down Expand Up @@ -468,20 +492,28 @@ public IEnumerable<MethodMirror> GetOverloads(string methodName)
/// <summary>
/// Given a method name, return the matching list of
/// constructors or static methods on this type and its base types
/// Excludes hidden methods from base types.
/// </summary>
/// <param name="methodName"></param>
/// <returns></returns>
public IEnumerable<MethodMirror> GetOverloadsOnType(string methodName)
{
List<MethodMirror> members = new List<MethodMirror>();
members.AddRange(this.GetConstructors().Where(x => x.MethodName == methodName));
members.AddRange(this.GetFunctions().Where(x => x.IsStatic && x.MethodName == methodName));

// Filter out hidden functions/properties:
// Create a set of unique function and constructor descriptions.
// In this case we use ToString() to get the uniques description of the members (func signature, constructor names).
var derivedClassMembers = new HashSet<string>();
members.ForEach(x => derivedClassMembers.Add(x.ToString()));
Copy link
Contributor

@aparajit-pratap aparajit-pratap Nov 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you going to make the same simplification in this commit over here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetOverloadsOnType must show duplicate names, so I cannot apply the simplification.
The existing code will filter duplicate signatures .


IEnumerable<ClassMirror> baseClasses = this.GetClassHierarchy();
foreach (var baseClass in baseClasses)
{
members.AddRange(baseClass.GetFunctions().Where(x => x.IsStatic && x.MethodName == methodName));
members.AddRange(baseClass.GetFunctions().Where(x => x.IsStatic && x.MethodName == methodName && !derivedClassMembers.Contains(x.ToString())));
}

members.AddRange(this.GetConstructors().Where(x => x.MethodName == methodName));
members.AddRange(this.GetFunctions().Where(x => x.IsStatic && x.MethodName == methodName));

return members;
}

Expand All @@ -504,20 +536,31 @@ public IEnumerable<MethodMirror> GetOverloadsOnInstance(string methodName)
return members;
}

/// <summary>
/// Returns the instance methods and properties
/// belonging to the type and its base types
/// Excludes hidden methods and properties from base types.
/// </summary>
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
public IEnumerable<StaticMirror> GetInstanceMembers()
{
// TODO: Factor out reflection functionality for both LibraryServices and Mirrors
List<StaticMirror> members = new List<StaticMirror>();
members.AddRange(this.GetFunctions().Where(m => !m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(this.GetProperties().Where(m => !m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));

// Filter out hidden functions/properties:
// Create a set of unique function and property descriptions.
// In this case we use ToString() to get the uniques description of the members (func signature, property names).
var derivedClassMembers = new HashSet<string>();
members.ForEach(x => derivedClassMembers.Add(x.ToString()));

IEnumerable<ClassMirror> baseClasses = this.GetClassHierarchy();
foreach (var baseClass in baseClasses)
{
members.AddRange(baseClass.GetFunctions().Where(m => !m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(baseClass.GetProperties().Where(m => !m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(baseClass.GetFunctions().Where(m => !m.IsStatic && !derivedClassMembers.Contains(m.ToString())).GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(baseClass.GetProperties().Where(m => !m.IsStatic && !derivedClassMembers.Contains(m.ToString())).GroupBy(x => x.Name).Select(y => y.First()));
}

members.AddRange(this.GetFunctions().Where(m => !m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));
members.AddRange(this.GetProperties().Where(m => !m.IsStatic).GroupBy(x => x.Name).Select(y => y.First()));
return members;
}

Expand Down