From 75662e16a4b5cafa6dbf869868965d9197a5a852 Mon Sep 17 00:00:00 2001 From: Alexis Christoforides Date: Tue, 17 Mar 2020 15:43:53 -0400 Subject: [PATCH 1/7] Port CoreCLR's TypeNameBuilder to C#, and use it in Mono too Mono's System.Reflection.Emit creates type names that fail to be normalized or shaped in all ways that CoreCLR does. Port CoreCLR's mixed-mode thread-unsafe implementation to thread-safe C#, and start using it in Mono for names in TypeBuilder. Fixes issues with e.g. null-delimited type names being passed to different Reflection.Emit builders. Contributes to https://github.com/dotnet/runtime/issues/2389. --- .../System.Private.CoreLib.csproj | 1 - .../src/System/Reflection/Emit/AQNBuilder.cs | 158 ------- src/coreclr/src/vm/ecalllist.h | 19 - src/coreclr/src/vm/typestring.cpp | 132 ------ src/coreclr/src/vm/typestring.h | 16 - .../System.Private.CoreLib.Shared.projitems | 1 + .../System/Reflection/Emit/TypeNameBuilder.cs | 429 ++++++++++++++++++ .../Reflection/Emit/TypeBuilder.Mono.cs | 10 +- 8 files changed, 435 insertions(+), 331 deletions(-) delete mode 100644 src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/AQNBuilder.cs create mode 100644 src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs diff --git a/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj index 2d168eebe3ac4..f1bf18586b6f4 100644 --- a/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/src/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -173,7 +173,6 @@ - diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/AQNBuilder.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/AQNBuilder.cs deleted file mode 100644 index 61d203745cddd..0000000000000 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/AQNBuilder.cs +++ /dev/null @@ -1,158 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System.Collections.Generic; -using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; - -namespace System.Reflection.Emit -{ - // TypeNameBuilder is NOT thread safe NOR reliable - internal class TypeNameBuilder - { - internal enum Format - { - ToString, - FullName, - AssemblyQualifiedName, - } - - #region QCalls - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern IntPtr CreateTypeNameBuilder(); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void ReleaseTypeNameBuilder(IntPtr pAQN); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void OpenGenericArguments(IntPtr tnb); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void CloseGenericArguments(IntPtr tnb); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void OpenGenericArgument(IntPtr tnb); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void CloseGenericArgument(IntPtr tnb); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void AddName(IntPtr tnb, string name); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void AddPointer(IntPtr tnb); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void AddByRef(IntPtr tnb); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void AddSzArray(IntPtr tnb); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void AddArray(IntPtr tnb, int rank); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void AddAssemblySpec(IntPtr tnb, string assemblySpec); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void ToString(IntPtr tnb, StringHandleOnStack retString); - [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)] - private static extern void Clear(IntPtr tnb); - #endregion - - #region Static Members - - // TypeNameBuilder is NOT thread safe NOR reliable - internal static string? ToString(Type type, Format format) - { - if (format == Format.FullName || format == Format.AssemblyQualifiedName) - { - if (!type.IsGenericTypeDefinition && type.ContainsGenericParameters) - return null; - } - - TypeNameBuilder tnb = new TypeNameBuilder(CreateTypeNameBuilder()); - tnb.Clear(); - tnb.ConstructAssemblyQualifiedNameWorker(type, format); - string? toString = tnb.ToString(); - tnb.Dispose(); - return toString; - } - #endregion - - #region Private Data Members - private IntPtr m_typeNameBuilder; - #endregion - - #region Constructor - private TypeNameBuilder(IntPtr typeNameBuilder) { m_typeNameBuilder = typeNameBuilder; } - internal void Dispose() { ReleaseTypeNameBuilder(m_typeNameBuilder); } - #endregion - - #region private Members - private void AddElementType(Type elementType) - { - if (elementType.HasElementType) - AddElementType(elementType.GetElementType()!); - - if (elementType.IsPointer) - AddPointer(); - else if (elementType.IsByRef) - AddByRef(); - else if (elementType.IsSZArray) - AddSzArray(); - else if (elementType.IsArray) - AddArray(elementType.GetArrayRank()); - } - - private void ConstructAssemblyQualifiedNameWorker(Type type, Format format) - { - Type rootType = type; - - while (rootType.HasElementType) - rootType = rootType.GetElementType()!; - - // Append namespace + nesting + name - List nestings = new List(); - for (Type? t = rootType; t != null; t = t.IsGenericParameter ? null : t.DeclaringType) - nestings.Add(t); - - for (int i = nestings.Count - 1; i >= 0; i--) - { - Type enclosingType = nestings[i]; - string name = enclosingType.Name; - - if (i == nestings.Count - 1 && enclosingType.Namespace != null && enclosingType.Namespace.Length != 0) - name = enclosingType.Namespace + "." + name; - - AddName(name); - } - - // Append generic arguments - if (rootType.IsGenericType && (!rootType.IsGenericTypeDefinition || format == Format.ToString)) - { - Type[] genericArguments = rootType.GetGenericArguments(); - - OpenGenericArguments(); - for (int i = 0; i < genericArguments.Length; i++) - { - Format genericArgumentsFormat = format == Format.FullName ? Format.AssemblyQualifiedName : format; - - OpenGenericArgument(); - ConstructAssemblyQualifiedNameWorker(genericArguments[i], genericArgumentsFormat); - CloseGenericArgument(); - } - CloseGenericArguments(); - } - - // Append pointer, byRef and array qualifiers - AddElementType(type); - - if (format == Format.AssemblyQualifiedName) - AddAssemblySpec(type.Module.Assembly.FullName!); - } - - private void OpenGenericArguments() { OpenGenericArguments(m_typeNameBuilder); } - private void CloseGenericArguments() { CloseGenericArguments(m_typeNameBuilder); } - private void OpenGenericArgument() { OpenGenericArgument(m_typeNameBuilder); } - private void CloseGenericArgument() { CloseGenericArgument(m_typeNameBuilder); } - private void AddName(string name) { AddName(m_typeNameBuilder, name); } - private void AddPointer() { AddPointer(m_typeNameBuilder); } - private void AddByRef() { AddByRef(m_typeNameBuilder); } - private void AddSzArray() { AddSzArray(m_typeNameBuilder); } - private void AddArray(int rank) { AddArray(m_typeNameBuilder, rank); } - private void AddAssemblySpec(string assemblySpec) { AddAssemblySpec(m_typeNameBuilder, assemblySpec); } - public override string? ToString() { string? ret = null; ToString(m_typeNameBuilder, new StringHandleOnStack(ref ret)); return ret; } - private void Clear() { Clear(m_typeNameBuilder); } - #endregion - } -} diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h index 7717ac319ade2..78ca688279ca9 100644 --- a/src/coreclr/src/vm/ecalllist.h +++ b/src/coreclr/src/vm/ecalllist.h @@ -434,24 +434,6 @@ FCFuncStart(gMdUtf8String) QCFuncElement("HashCaseInsensitive", MdUtf8String::HashCaseInsensitive) FCFuncEnd() -FCFuncStart(gTypeNameBuilder) - QCFuncElement("CreateTypeNameBuilder", TypeNameBuilder::_CreateTypeNameBuilder) - QCFuncElement("ReleaseTypeNameBuilder", TypeNameBuilder::_ReleaseTypeNameBuilder) - QCFuncElement("OpenGenericArguments", TypeNameBuilder::_OpenGenericArguments) - QCFuncElement("CloseGenericArguments", TypeNameBuilder::_CloseGenericArguments) - QCFuncElement("OpenGenericArgument", TypeNameBuilder::_OpenGenericArgument) - QCFuncElement("CloseGenericArgument", TypeNameBuilder::_CloseGenericArgument) - QCFuncElement("AddName", TypeNameBuilder::_AddName) - QCFuncElement("AddPointer", TypeNameBuilder::_AddPointer) - QCFuncElement("AddByRef", TypeNameBuilder::_AddByRef) - QCFuncElement("AddSzArray", TypeNameBuilder::_AddSzArray) - QCFuncElement("AddArray", TypeNameBuilder::_AddArray) - QCFuncElement("AddAssemblySpec", TypeNameBuilder::_AddAssemblySpec) - QCFuncElement("ToString", TypeNameBuilder::_ToString) - QCFuncElement("Clear", TypeNameBuilder::_Clear) -FCFuncEnd() - - FCFuncStart(gSafeTypeNameParserHandle) QCFuncElement("_ReleaseTypeNameParser", TypeName::QReleaseTypeNameParser) FCFuncEnd() @@ -1315,7 +1297,6 @@ FCClassElement("TimerQueue", "System.Threading", gTimerFuncs) FCClassElement("Type", "System", gSystem_Type) FCClassElement("TypeBuilder", "System.Reflection.Emit", gCOMClassWriter) FCClassElement("TypeLoadException", "System", gTypeLoadExceptionFuncs) -FCClassElement("TypeNameBuilder", "System.Reflection.Emit", gTypeNameBuilder) FCClassElement("TypeNameParser", "System", gTypeNameParser) FCClassElement("TypedReference", "System", gTypedReferenceFuncs) #ifdef FEATURE_COMINTEROP diff --git a/src/coreclr/src/vm/typestring.cpp b/src/coreclr/src/vm/typestring.cpp index ff4f4a7d49107..07d60e028471a 100644 --- a/src/coreclr/src/vm/typestring.cpp +++ b/src/coreclr/src/vm/typestring.cpp @@ -28,138 +28,6 @@ #include "ex.h" #include "typedesc.h" -#if !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE) -TypeNameBuilder * QCALLTYPE TypeNameBuilder::_CreateTypeNameBuilder() -{ - QCALL_CONTRACT; - - TypeNameBuilder * retVal = NULL; - BEGIN_QCALL; - retVal = new TypeNameBuilder(); - END_QCALL; - - return retVal; -} - -void QCALLTYPE TypeNameBuilder::_ReleaseTypeNameBuilder(TypeNameBuilder * pTnb) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - delete pTnb; - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_ToString(TypeNameBuilder * pTnb, QCall::StringHandleOnStack retString) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - retString.Set(*pTnb->GetString()); - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_AddName(TypeNameBuilder * pTnb, LPCWSTR wszName) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - pTnb->AddName(wszName); - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_AddAssemblySpec(TypeNameBuilder * pTnb, LPCWSTR wszAssemblySpec) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - pTnb->AddAssemblySpec(wszAssemblySpec); - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_OpenGenericArguments(TypeNameBuilder * pTnb) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - pTnb->OpenGenericArguments(); - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_CloseGenericArguments(TypeNameBuilder * pTnb) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - pTnb->CloseGenericArguments(); - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_OpenGenericArgument(TypeNameBuilder * pTnb) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - pTnb->OpenGenericArgument(); - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_CloseGenericArgument(TypeNameBuilder * pTnb) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - pTnb->CloseGenericArgument(); - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_AddPointer(TypeNameBuilder * pTnb) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - pTnb->AddPointer(); - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_AddByRef(TypeNameBuilder * pTnb) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - pTnb->AddByRef(); - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_AddSzArray(TypeNameBuilder * pTnb) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - pTnb->AddSzArray(); - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_AddArray(TypeNameBuilder * pTnb, DWORD dwRank) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - pTnb->AddArray(dwRank); - END_QCALL; -} - -void QCALLTYPE TypeNameBuilder::_Clear(TypeNameBuilder * pTnb) -{ - QCALL_CONTRACT; - - BEGIN_QCALL; - pTnb->Clear(); - END_QCALL; -} - -#endif - // // TypeNameBuilder // diff --git a/src/coreclr/src/vm/typestring.h b/src/coreclr/src/vm/typestring.h index 7e5bdf3f3fc9c..c259e43709d7d 100644 --- a/src/coreclr/src/vm/typestring.h +++ b/src/coreclr/src/vm/typestring.h @@ -32,22 +32,6 @@ class TypeString; class TypeNameBuilder { -public: - static void QCALLTYPE _ReleaseTypeNameBuilder(TypeNameBuilder * pTnb); - static TypeNameBuilder * QCALLTYPE _CreateTypeNameBuilder(); - static void QCALLTYPE _OpenGenericArguments(TypeNameBuilder * pTnb); - static void QCALLTYPE _CloseGenericArguments(TypeNameBuilder *pTnb); - static void QCALLTYPE _OpenGenericArgument(TypeNameBuilder * pTnb); - static void QCALLTYPE _CloseGenericArgument(TypeNameBuilder * pTnb); - static void QCALLTYPE _AddName(TypeNameBuilder * pTnb, LPCWSTR wszName); - static void QCALLTYPE _AddPointer(TypeNameBuilder * pTnb); - static void QCALLTYPE _AddByRef(TypeNameBuilder * pTnb); - static void QCALLTYPE _AddSzArray(TypeNameBuilder * pTnb); - static void QCALLTYPE _AddArray(TypeNameBuilder * pTnb, DWORD dwRank); - static void QCALLTYPE _AddAssemblySpec(TypeNameBuilder * pTnb, LPCWSTR wszAssemblySpec); - static void QCALLTYPE _ToString(TypeNameBuilder * pTnb, QCall::StringHandleOnStack retString); - static void QCALLTYPE _Clear(TypeNameBuilder * pTnb); - private: friend class TypeString; friend SString* TypeName::ToString(SString*, BOOL, BOOL, BOOL); diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index b5a654a61e63b..165ff7c45ff34 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -478,6 +478,7 @@ + diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs new file mode 100644 index 0000000000000..e59603ce1c2dd --- /dev/null +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs @@ -0,0 +1,429 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. +// +// This TypeNameBuilder is ported from CoreCLR's original. +// It replaces the C++ bits of the implementation with a faithful C# port. + +using System.Collections.Generic; +using System.Collections; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Text; +using System.Diagnostics; + +namespace System.Reflection.Emit +{ + internal class TypeNameBuilder + { + private enum ParseState + { + START = 0x0001, + NAME = 0x0004, + GENARGS = 0x0008, + PTRARR = 0x0010, + BYREF = 0x0020, + ASSEMSPEC = 0x0080, + } + + private ParseState parseState; + private StringBuilder str = new StringBuilder(); + private int instNesting; + private bool firstInstArg; + private bool nestedName; + private bool hasAssemblySpec; + private bool useAngleBracketsForGenerics; + private List stack = new List(); + private int stackIdx; + + public TypeNameBuilder() + { + parseState = ParseState.START; + } + + public void OpenGenericArguments() + { + CheckParseState(ParseState.NAME); + + parseState = ParseState.START; + instNesting++; + firstInstArg = true; + + if (useAngleBracketsForGenerics) + Append('<'); + else + Append('['); + } + + public void CloseGenericArguments() + { + CheckParseState(ParseState.START); + + Debug.Assert(instNesting != 0); + + parseState = ParseState.GENARGS; + + instNesting--; + + if (firstInstArg) + { + str.Remove(str.Length - 1, 1); + } + else + { + if (useAngleBracketsForGenerics) + Append('>'); + else + Append(']'); + } + } + + public void OpenGenericArgument() + { + CheckParseState(ParseState.START); + + Debug.Assert(instNesting != 0); + + parseState = ParseState.START; + nestedName = false; + + if (!firstInstArg) + Append(','); + + firstInstArg = false; + + if (useAngleBracketsForGenerics) + Append('<'); + else + Append('['); + + PushOpenGenericArgument(); + } + + public void CloseGenericArgument() + { + CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR | ParseState.BYREF | ParseState.ASSEMSPEC); + + Debug.Assert(instNesting != 0); + + parseState = ParseState.START; + + if (hasAssemblySpec) + { + if (useAngleBracketsForGenerics) + Append('>'); + else + Append(']'); + } + + PopOpenGenericArgument(); + } + + public void AddName(string name) + { + Debug.Assert(name != null); + + CheckParseState(ParseState.START | ParseState.NAME); + + parseState = ParseState.NAME; + + if (nestedName) + Append('+'); + + nestedName = true; + + EscapeName(name!); + } + + public void AddPointer() + { + CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR); + + parseState = ParseState.PTRARR; + + Append('*'); + } + + public void AddByRef() + { + CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR); + + parseState = ParseState.BYREF; + + Append('&'); + } + + public void AddSzArray() + { + CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR); + + parseState = ParseState.PTRARR; + + Append("[]"); + } + + public void AddArray(int rank) + { + CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR); + + parseState = ParseState.PTRARR; + + if (rank <= 0) + throw new ArgumentOutOfRangeException(); + + if (rank == 1) + { + Append("[*]"); + } + else if (rank > 64) + { + // Only taken in an error path, runtime will not load arrays of more than 32 dimensions + Append($"[{rank}]"); + } + else + { + Append('['); + for (int i = 1; i < rank; i++) + Append(','); + Append(']'); + } + } + + public void AddAssemblySpec(string assemblySpec) + { + CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR | ParseState.BYREF); + + parseState = ParseState.ASSEMSPEC; + + if (assemblySpec != null && !assemblySpec.Equals("")) + { + Append(", "); + + if (instNesting > 0) + { + EscapeEmbeddedAssemblyName(assemblySpec); + } + else + { + EscapeAssemblyName(assemblySpec); + } + + hasAssemblySpec = true; + } + } + + public override string ToString() + { + CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR | ParseState.BYREF | ParseState.ASSEMSPEC); + + Debug.Assert(instNesting == 0); + + return str.ToString(); + } + + private static bool ContainsReservedChar(string name) + { + foreach (char c in name) + { + if (c == '\0') + break; + if (IsTypeNameReservedChar(c)) + return true; + } + return false; + } + + private static bool IsTypeNameReservedChar(char ch) + { + switch (ch) + { + case ',': + case '[': + case ']': + case '&': + case '*': + case '+': + case '\\': + return true; + + default: + return false; + } + } + + private void EscapeName(string name) + { + if (TypeNameBuilder.ContainsReservedChar(name)) + { + foreach (char c in name) + { + if (c == '\0') + break; + if (TypeNameBuilder.IsTypeNameReservedChar(c)) + str.Append('\\'); + str.Append(c); + } + } + else + Append(name); + } + + private void EscapeAssemblyName(string name) + { + Append(name); + } + + private void EscapeEmbeddedAssemblyName(string name) + { + bool containsReservedChar = false; + + foreach (char c in name) + { + if (c == ']') + { + containsReservedChar = true; + break; + } + } + + if (containsReservedChar) + { + foreach (char c in name) + { + if (c == ']') + Append('\\'); + + Append(c); + } + } + else + { + Append(name); + } + } + + private void CheckParseState(ParseState validState) + { + Debug.Assert((parseState & validState) != 0); + } + + private void PushOpenGenericArgument() + { + stack.Add(str.Length); + stackIdx++; + } + + private void PopOpenGenericArgument() + { + int index = stack[--stackIdx]; + stack.RemoveAt(stackIdx); + + if (!hasAssemblySpec) + str.Remove(index - 1, 1); + + hasAssemblySpec = false; + } + + private void SetUseAngleBracketsForGenerics(bool value) + { + useAngleBracketsForGenerics = value; + } + + private void Append(string pStr) + { + foreach (char c in pStr) + { + if (c == '\0') + break; + str.Append(c); + } + } + + private void Append(char c) + { + str.Append(c); + } + + internal enum Format + { + ToString, + FullName, + AssemblyQualifiedName, + } + + internal static string? ToString(Type type, Format format) + { + if (format == Format.FullName || format == Format.AssemblyQualifiedName) + { + if (!type.IsGenericTypeDefinition && type.ContainsGenericParameters) + return null; + } + + TypeNameBuilder tnb = new TypeNameBuilder(); + ConstructAssemblyQualifiedNameWorker(tnb, type, format); + return tnb.ToString(); + } + + private static void AddElementType(TypeNameBuilder tnb, Type elementType) + { + if (elementType.HasElementType) + AddElementType(tnb, elementType.GetElementType()!); + + if (elementType.IsPointer) + tnb.AddPointer(); + else if (elementType.IsByRef) + tnb.AddByRef(); + else if (elementType.IsSZArray) + tnb.AddSzArray(); + else if (elementType.IsArray) + tnb.AddArray(elementType.GetArrayRank()); + } + + private static void ConstructAssemblyQualifiedNameWorker(TypeNameBuilder tnb, Type type, Format format) + { + Type rootType = type; + + while (rootType.HasElementType) + rootType = rootType.GetElementType()!; + + // Append namespace + nesting + name + List nestings = new List(); + for (Type? t = rootType; t != null; t = t.IsGenericParameter ? null : t.DeclaringType) + nestings.Add(t); + + for (int i = nestings.Count - 1; i >= 0; i--) + { + Type enclosingType = nestings[i]; + string name = enclosingType.Name; + + if (i == nestings.Count - 1 && enclosingType.Namespace != null && enclosingType.Namespace.Length != 0) + name = enclosingType.Namespace + "." + name; + + tnb.AddName(name); + } + + // Append generic arguments + if (rootType.IsGenericType && (!rootType.IsGenericTypeDefinition || format == Format.ToString)) + { + Type[] genericArguments = rootType.GetGenericArguments(); + + tnb.OpenGenericArguments(); + for (int i = 0; i < genericArguments.Length; i++) + { + Format genericArgumentsFormat = format == Format.FullName ? Format.AssemblyQualifiedName : format; + + tnb.OpenGenericArgument(); + ConstructAssemblyQualifiedNameWorker(tnb, genericArguments[i], genericArgumentsFormat); + tnb.CloseGenericArgument(); + } + tnb.CloseGenericArguments(); + } + + // Append pointer, byRef and array qualifiers + AddElementType(tnb, type); + + if (format == Format.AssemblyQualifiedName) + tnb.AddAssemblySpec(type.Module.Assembly.FullName!); + } + } +} diff --git a/src/mono/netcore/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.Mono.cs b/src/mono/netcore/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.Mono.cs index 872c3c703f61a..d50c2cd9026b1 100644 --- a/src/mono/netcore/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.Mono.cs +++ b/src/mono/netcore/System.Private.CoreLib/src/System/Reflection/Emit/TypeBuilder.Mono.cs @@ -144,7 +144,7 @@ public override string AssemblyQualifiedName { get { - return fullname.DisplayName + ", " + Assembly.FullName; + return TypeNameBuilder.ToString(this, TypeNameBuilder.Format.AssemblyQualifiedName); } } @@ -212,7 +212,7 @@ public override string FullName { get { - return fullname.DisplayName; + return TypeNameBuilder.ToString(this, TypeNameBuilder.Format.FullName); } } @@ -826,19 +826,19 @@ TypeInfo CreateTypeInfo() if (parent != null) { if (parent.IsSealed) - throw new TypeLoadException("Could not load type '" + FullName + "' from assembly '" + Assembly + "' because the parent type is sealed."); + throw new TypeLoadException("Could not load type '" + fullname.DisplayName + "' from assembly '" + Assembly + "' because the parent type is sealed."); if (parent.IsGenericTypeDefinition) throw new BadImageFormatException(); } if (parent == typeof(Enum) && methods != null) - throw new TypeLoadException("Could not load type '" + FullName + "' from assembly '" + Assembly + "' because it is an enum with methods."); + throw new TypeLoadException("Could not load type '" + fullname.DisplayName + "' from assembly '" + Assembly + "' because it is an enum with methods."); if (interfaces != null) { foreach (Type iface in interfaces) { if (iface.IsNestedPrivate && iface.Assembly != Assembly) - throw new TypeLoadException("Could not load type '" + FullName + "' from assembly '" + Assembly + "' because it is implements the inaccessible interface '" + iface.FullName + "'."); + throw new TypeLoadException("Could not load type '" + fullname.DisplayName + "' from assembly '" + Assembly + "' because it is implements the inaccessible interface '" + iface.FullName + "'."); if (iface.IsGenericTypeDefinition) throw new BadImageFormatException(); if (!iface.IsInterface) From 06fc308b3385a6a5df49473bdc3598266126916f Mon Sep 17 00:00:00 2001 From: Alexis Christoforides Date: Mon, 30 Mar 2020 17:04:30 -0400 Subject: [PATCH 2/7] Update src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs Co-Authored-By: Jan Kotas --- .../src/System/Reflection/Emit/TypeNameBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs index e59603ce1c2dd..a963a02e99759 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs @@ -253,7 +253,7 @@ private static bool IsTypeNameReservedChar(char ch) private void EscapeName(string name) { - if (TypeNameBuilder.ContainsReservedChar(name)) + if (ContainsReservedChar(name)) { foreach (char c in name) { From 26e8ae12f494d0c336b04abee47b571a7905e644 Mon Sep 17 00:00:00 2001 From: Alexis Christoforides Date: Mon, 30 Mar 2020 17:04:40 -0400 Subject: [PATCH 3/7] Update src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs Co-Authored-By: Jan Kotas --- .../src/System/Reflection/Emit/TypeNameBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs index a963a02e99759..f9554e772de16 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs @@ -259,7 +259,7 @@ private void EscapeName(string name) { if (c == '\0') break; - if (TypeNameBuilder.IsTypeNameReservedChar(c)) + if (IsTypeNameReservedChar(c)) str.Append('\\'); str.Append(c); } From d7d5962fa5ebba6dbf8974b49b297c93b7f5428b Mon Sep 17 00:00:00 2001 From: Alexis Christoforides Date: Tue, 31 Mar 2020 09:48:41 -0400 Subject: [PATCH 4/7] Review feedback --- .../System/Reflection/Emit/TypeNameBuilder.cs | 197 ++++++------------ 1 file changed, 69 insertions(+), 128 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs index f9554e772de16..2a797bae8dfcd 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs @@ -16,40 +16,21 @@ namespace System.Reflection.Emit { internal class TypeNameBuilder { - private enum ParseState - { - START = 0x0001, - NAME = 0x0004, - GENARGS = 0x0008, - PTRARR = 0x0010, - BYREF = 0x0020, - ASSEMSPEC = 0x0080, - } - - private ParseState parseState; - private StringBuilder str = new StringBuilder(); - private int instNesting; - private bool firstInstArg; - private bool nestedName; - private bool hasAssemblySpec; - private bool useAngleBracketsForGenerics; - private List stack = new List(); - private int stackIdx; - - public TypeNameBuilder() - { - parseState = ParseState.START; - } + private StringBuilder _str = new StringBuilder(); + private int _instNesting; + private bool _firstInstArg; + private bool _nestedName; + private bool _hasAssemblySpec; + private bool _useAngleBracketsForGenerics; + private List _stack = new List(); + private int _stackIdx; public void OpenGenericArguments() { - CheckParseState(ParseState.NAME); - - parseState = ParseState.START; - instNesting++; - firstInstArg = true; + _instNesting++; + _firstInstArg = true; - if (useAngleBracketsForGenerics) + if (_useAngleBracketsForGenerics) Append('<'); else Append('['); @@ -57,21 +38,17 @@ public void OpenGenericArguments() public void CloseGenericArguments() { - CheckParseState(ParseState.START); - - Debug.Assert(instNesting != 0); + Debug.Assert(_instNesting != 0); - parseState = ParseState.GENARGS; + _instNesting--; - instNesting--; - - if (firstInstArg) + if (_firstInstArg) { - str.Remove(str.Length - 1, 1); + _str.Remove(_str.Length - 1, 1); } else { - if (useAngleBracketsForGenerics) + if (_useAngleBracketsForGenerics) Append('>'); else Append(']'); @@ -80,19 +57,16 @@ public void CloseGenericArguments() public void OpenGenericArgument() { - CheckParseState(ParseState.START); - - Debug.Assert(instNesting != 0); + Debug.Assert(_instNesting != 0); - parseState = ParseState.START; - nestedName = false; + _nestedName = false; - if (!firstInstArg) + if (!_firstInstArg) Append(','); - firstInstArg = false; + _firstInstArg = false; - if (useAngleBracketsForGenerics) + if (_useAngleBracketsForGenerics) Append('<'); else Append('['); @@ -102,15 +76,11 @@ public void OpenGenericArgument() public void CloseGenericArgument() { - CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR | ParseState.BYREF | ParseState.ASSEMSPEC); - - Debug.Assert(instNesting != 0); - - parseState = ParseState.START; + Debug.Assert(_instNesting != 0); - if (hasAssemblySpec) + if (_hasAssemblySpec) { - if (useAngleBracketsForGenerics) + if (_useAngleBracketsForGenerics) Append('>'); else Append(']'); @@ -123,51 +93,31 @@ public void AddName(string name) { Debug.Assert(name != null); - CheckParseState(ParseState.START | ParseState.NAME); - - parseState = ParseState.NAME; - - if (nestedName) + if (_nestedName) Append('+'); - nestedName = true; + _nestedName = true; EscapeName(name!); } public void AddPointer() { - CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR); - - parseState = ParseState.PTRARR; - Append('*'); } public void AddByRef() { - CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR); - - parseState = ParseState.BYREF; - Append('&'); } public void AddSzArray() { - CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR); - - parseState = ParseState.PTRARR; - Append("[]"); } public void AddArray(int rank) { - CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR); - - parseState = ParseState.PTRARR; - if (rank <= 0) throw new ArgumentOutOfRangeException(); @@ -178,7 +128,7 @@ public void AddArray(int rank) else if (rank > 64) { // Only taken in an error path, runtime will not load arrays of more than 32 dimensions - Append($"[{rank}]"); + _str.Append('[').Append(rank).Append(']'); } else { @@ -191,15 +141,11 @@ public void AddArray(int rank) public void AddAssemblySpec(string assemblySpec) { - CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR | ParseState.BYREF); - - parseState = ParseState.ASSEMSPEC; - if (assemblySpec != null && !assemblySpec.Equals("")) { Append(", "); - if (instNesting > 0) + if (_instNesting > 0) { EscapeEmbeddedAssemblyName(assemblySpec); } @@ -208,17 +154,15 @@ public void AddAssemblySpec(string assemblySpec) EscapeAssemblyName(assemblySpec); } - hasAssemblySpec = true; + _hasAssemblySpec = true; } } public override string ToString() { - CheckParseState(ParseState.NAME | ParseState.GENARGS | ParseState.PTRARR | ParseState.BYREF | ParseState.ASSEMSPEC); - - Debug.Assert(instNesting == 0); + Debug.Assert(_instNesting == 0); - return str.ToString(); + return _str.ToString(); } private static bool ContainsReservedChar(string name) @@ -260,8 +204,8 @@ private void EscapeName(string name) if (c == '\0') break; if (IsTypeNameReservedChar(c)) - str.Append('\\'); - str.Append(c); + _str.Append('\\'); + _str.Append(c); } } else @@ -302,31 +246,26 @@ private void EscapeEmbeddedAssemblyName(string name) } } - private void CheckParseState(ParseState validState) - { - Debug.Assert((parseState & validState) != 0); - } - private void PushOpenGenericArgument() { - stack.Add(str.Length); - stackIdx++; + _stack.Add(_str.Length); + _stackIdx++; } private void PopOpenGenericArgument() { - int index = stack[--stackIdx]; - stack.RemoveAt(stackIdx); + int index = _stack[--_stackIdx]; + _stack.RemoveAt(_stackIdx); - if (!hasAssemblySpec) - str.Remove(index - 1, 1); + if (!_hasAssemblySpec) + _str.Remove(index - 1, 1); - hasAssemblySpec = false; + _hasAssemblySpec = false; } private void SetUseAngleBracketsForGenerics(bool value) { - useAngleBracketsForGenerics = value; + _useAngleBracketsForGenerics = value; } private void Append(string pStr) @@ -335,13 +274,13 @@ private void Append(string pStr) { if (c == '\0') break; - str.Append(c); + _str.Append(c); } } private void Append(char c) { - str.Append(c); + _str.Append(c); } internal enum Format @@ -359,27 +298,29 @@ internal enum Format return null; } - TypeNameBuilder tnb = new TypeNameBuilder(); - ConstructAssemblyQualifiedNameWorker(tnb, type, format); + var tnb = new TypeNameBuilder(); + tnb.AddAssemblyQualifiedName(type, format); return tnb.ToString(); } - private static void AddElementType(TypeNameBuilder tnb, Type elementType) + private void AddElementType(Type type) { - if (elementType.HasElementType) - AddElementType(tnb, elementType.GetElementType()!); - - if (elementType.IsPointer) - tnb.AddPointer(); - else if (elementType.IsByRef) - tnb.AddByRef(); - else if (elementType.IsSZArray) - tnb.AddSzArray(); - else if (elementType.IsArray) - tnb.AddArray(elementType.GetArrayRank()); + if (!type.HasElementType) + return; + + AddElementType(type.GetElementType()!); + + if (type.IsPointer) + AddPointer(); + else if (type.IsByRef) + AddByRef(); + else if (type.IsSZArray) + AddSzArray(); + else if (type.IsArray) + AddArray(type.GetArrayRank()); } - private static void ConstructAssemblyQualifiedNameWorker(TypeNameBuilder tnb, Type type, Format format) + private void AddAssemblyQualifiedName(Type type, Format format) { Type rootType = type; @@ -387,7 +328,7 @@ private static void ConstructAssemblyQualifiedNameWorker(TypeNameBuilder tnb, Ty rootType = rootType.GetElementType()!; // Append namespace + nesting + name - List nestings = new List(); + var nestings = new List(); for (Type? t = rootType; t != null; t = t.IsGenericParameter ? null : t.DeclaringType) nestings.Add(t); @@ -399,7 +340,7 @@ private static void ConstructAssemblyQualifiedNameWorker(TypeNameBuilder tnb, Ty if (i == nestings.Count - 1 && enclosingType.Namespace != null && enclosingType.Namespace.Length != 0) name = enclosingType.Namespace + "." + name; - tnb.AddName(name); + AddName(name); } // Append generic arguments @@ -407,23 +348,23 @@ private static void ConstructAssemblyQualifiedNameWorker(TypeNameBuilder tnb, Ty { Type[] genericArguments = rootType.GetGenericArguments(); - tnb.OpenGenericArguments(); + OpenGenericArguments(); for (int i = 0; i < genericArguments.Length; i++) { Format genericArgumentsFormat = format == Format.FullName ? Format.AssemblyQualifiedName : format; - tnb.OpenGenericArgument(); - ConstructAssemblyQualifiedNameWorker(tnb, genericArguments[i], genericArgumentsFormat); - tnb.CloseGenericArgument(); + OpenGenericArgument(); + AddAssemblyQualifiedName(genericArguments[i], genericArgumentsFormat); + CloseGenericArgument(); } - tnb.CloseGenericArguments(); + CloseGenericArguments(); } // Append pointer, byRef and array qualifiers - AddElementType(tnb, type); + AddElementType(type); if (format == Format.AssemblyQualifiedName) - tnb.AddAssemblySpec(type.Module.Assembly.FullName!); + AddAssemblySpec(type.Module.Assembly.FullName!); } } } From ce0cc8c4bdb3d2ad31363d38811b4e3d19ecf762 Mon Sep 17 00:00:00 2001 From: Alexis Christoforides Date: Wed, 1 Apr 2020 14:55:40 -0400 Subject: [PATCH 5/7] Update src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs Co-Authored-By: Jan Kotas --- .../src/System/Reflection/Emit/TypeNameBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs index 2a797bae8dfcd..f025ee31e4bc8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs @@ -98,7 +98,7 @@ public void AddName(string name) _nestedName = true; - EscapeName(name!); + EscapeName(name); } public void AddPointer() From b4cf76a72b4c77cafdbaf85e1630c548f0622dfe Mon Sep 17 00:00:00 2001 From: Alexis Christoforides Date: Wed, 1 Apr 2020 20:26:23 -0400 Subject: [PATCH 6/7] Update src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs Co-Authored-By: Marek Safar --- .../src/System/Reflection/Emit/TypeNameBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs index f025ee31e4bc8..79b9905a6a575 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs @@ -219,7 +219,7 @@ private void EscapeAssemblyName(string name) private void EscapeEmbeddedAssemblyName(string name) { - bool containsReservedChar = false; + bool constainsReservedChar = name.Contains(']'); foreach (char c in name) { From 6abeeb466b0f41fdd1141f290f6e0fb841157d6f Mon Sep 17 00:00:00 2001 From: Alexis Christoforides Date: Wed, 1 Apr 2020 23:07:24 -0400 Subject: [PATCH 7/7] Review feedback --- .../System/Reflection/Emit/TypeNameBuilder.cs | 55 ++++++------------- 1 file changed, 16 insertions(+), 39 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs index 79b9905a6a575..5f9bb76cb38bf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Reflection/Emit/TypeNameBuilder.cs @@ -25,7 +25,11 @@ internal class TypeNameBuilder private List _stack = new List(); private int _stackIdx; - public void OpenGenericArguments() + private TypeNameBuilder() + { + } + + private void OpenGenericArguments() { _instNesting++; _firstInstArg = true; @@ -36,7 +40,7 @@ public void OpenGenericArguments() Append('['); } - public void CloseGenericArguments() + private void CloseGenericArguments() { Debug.Assert(_instNesting != 0); @@ -55,7 +59,7 @@ public void CloseGenericArguments() } } - public void OpenGenericArgument() + private void OpenGenericArgument() { Debug.Assert(_instNesting != 0); @@ -74,7 +78,7 @@ public void OpenGenericArgument() PushOpenGenericArgument(); } - public void CloseGenericArgument() + private void CloseGenericArgument() { Debug.Assert(_instNesting != 0); @@ -89,7 +93,7 @@ public void CloseGenericArgument() PopOpenGenericArgument(); } - public void AddName(string name) + private void AddName(string name) { Debug.Assert(name != null); @@ -101,25 +105,9 @@ public void AddName(string name) EscapeName(name); } - public void AddPointer() - { - Append('*'); - } - - public void AddByRef() - { - Append('&'); - } - - public void AddSzArray() - { - Append("[]"); - } - - public void AddArray(int rank) + private void AddArray(int rank) { - if (rank <= 0) - throw new ArgumentOutOfRangeException(); + Debug.Assert(rank > 0); if (rank == 1) { @@ -139,7 +127,7 @@ public void AddArray(int rank) } } - public void AddAssemblySpec(string assemblySpec) + private void AddAssemblySpec(string assemblySpec) { if (assemblySpec != null && !assemblySpec.Equals("")) { @@ -219,18 +207,7 @@ private void EscapeAssemblyName(string name) private void EscapeEmbeddedAssemblyName(string name) { - bool constainsReservedChar = name.Contains(']'); - - foreach (char c in name) - { - if (c == ']') - { - containsReservedChar = true; - break; - } - } - - if (containsReservedChar) + if (name.Contains(']')) { foreach (char c in name) { @@ -311,11 +288,11 @@ private void AddElementType(Type type) AddElementType(type.GetElementType()!); if (type.IsPointer) - AddPointer(); + Append('*'); else if (type.IsByRef) - AddByRef(); + Append('&'); else if (type.IsSZArray) - AddSzArray(); + Append("[]"); else if (type.IsArray) AddArray(type.GetArrayRank()); }