Skip to content

Refactor missing reference errors to allow not throwing for all cases #50437

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

Merged
davidwrighton merged 4 commits into from
Mar 31, 2021
Merged

Refactor missing reference errors to allow not throwing for all cases #50437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f0d165
Refactor missing reference errors to allow not throwing for all cases
davidwrighton Mar 30, 2021
79a6b28
Fix IL verifier
davidwrighton Mar 30, 2021
c2575ad
Fix missed case
davidwrighton Mar 30, 2021
ad1655d
Address code review feedback
davidwrighton Mar 31, 2021
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
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/Compiler/TypeExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static DefType GetClosestDefType(this TypeDesc type)
{
if (!type.IsArrayTypeWithoutGenericInterfaces())
{
MetadataType arrayShadowType = type.Context.SystemModule.GetType("System", "Array`1", throwIfNotFound: false);
MetadataType arrayShadowType = type.Context.SystemModule.GetType("System", "Array`1", NotFoundBehavior.ReturnNull);
if (arrayShadowType != null)
{
return arrayShadowType.MakeInstantiatedType(((ArrayType)type).ElementType);
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/tools/Common/TypeSystem/Common/ExceptionStringID.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,6 @@ public enum ExceptionStringID

// BadImageFormatException
BadImageFormatGeneric,
BadImageFormatSpecific,
}
}
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/TypeSystem/Common/MetadataTypeSystemContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public virtual void SetSystemModule(ModuleDesc systemModule)
{
// Require System.Object to be present as a minimal sanity check.
// The set of required well-known types is not strictly defined since different .NET profiles implement different subsets.
MetadataType type = systemModule.GetType("System", s_wellKnownTypeNames[typeIndex], typeIndex == (int)WellKnownType.Object);
MetadataType type = systemModule.GetType("System", s_wellKnownTypeNames[typeIndex], typeIndex == (int)WellKnownType.Object ? NotFoundBehavior.Throw : NotFoundBehavior.ReturnNull);
if (type != null)
{
type.SetWellKnownType((WellKnownType)(typeIndex + 1));
Expand Down
8 changes: 7 additions & 1 deletion src/coreclr/tools/Common/TypeSystem/Common/ModuleDesc.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;

namespace Internal.TypeSystem
Expand Down Expand Up @@ -28,8 +29,13 @@ public ModuleDesc(TypeSystemContext context, IAssemblyDesc assembly)

/// <summary>
/// Gets a type in this module with the specified name.
/// If notFoundBehavior == NotFoundBehavior.ReturnResolutionFailure
/// then ModuleDesc.GetTypeResolutionFailure will be set to the failure, and the function will return null
/// </summary>
public abstract MetadataType GetType(string nameSpace, string name, bool throwIfNotFound = true);
public abstract MetadataType GetType(string nameSpace, string name, NotFoundBehavior notFoundBehavior = NotFoundBehavior.Throw);

[ThreadStatic]
public static ResolutionFailure GetTypeResolutionFailure;

/// <summary>
/// Gets the global &lt;Module&gt; type.
Expand Down
12 changes: 12 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Common/NotFoundBehavior.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Internal.TypeSystem
{
public enum NotFoundBehavior
{
Throw,
ReturnNull,
ReturnResolutionFailure
}
}
3 changes: 3 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Common/Properties/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,4 +180,7 @@
<data name="BadImageFormatGeneric" xml:space="preserve">
<value>The format of a DLL or executable being loaded is invalid</value>
</data>
<data name="BadImageFormatSpecific" xml:space="preserve">
<value>The format of a DLL or executable being loaded is invalid with {0}</value>
</data>
</root>
110 changes: 110 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Common/ResolutionFailure.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Internal.TypeSystem
{
public sealed class ResolutionFailure
{
private enum FailureType

@MichalStrehovsky MichalStrehovsky Mar 31, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wondering if it would be easier to just have a single field in this that is the exception instance.

sealed class ResolutionFailure
{
    private readonly Exception _exception;

    public ResolutionFailure(Exception ex) => _exception = ex;

    public void Throw() => throw _exception;
}

We would need to update ThrowHelper so that we have CreateXXX methods along with ThrowXXX methods on it.

Then the usage is "new ResolutionFailure(ThrowHelper.CreateFileNotFoundException(...));`

@davidwrighton davidwrighton Mar 31, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I considered that, but unfortunately, as these ResolutionFailure objects are persistent, it can result in multiple throws with the exact same exception in them. While this does work, it results in stack traces in the exception being somewhat nondeterministic, which I very much disapprove of.

{
TypeLoadException1,
TypeLoadException2,
TypeLoadException3,
MissingMethodException1,
MissingFieldException1,
MissingAssemblyException1,
}

private ResolutionFailure() { }

private FailureType _failureType;
private string _namespace;
private string _name;
private string _moduleName;
private ModuleDesc _module;
private TypeDesc _owningType;
private MethodSignature _methodSignature;


public static ResolutionFailure GetTypeLoadResolutionFailure(string nestedTypeName, ModuleDesc module)
{
ResolutionFailure failure = new ResolutionFailure();
failure._failureType = FailureType.TypeLoadException1;
failure._name = nestedTypeName;
failure._module = module;
return failure;
}

public static ResolutionFailure GetTypeLoadResolutionFailure(string @namespace, string name, ModuleDesc module)
{
ResolutionFailure failure = new ResolutionFailure();
failure._failureType = FailureType.TypeLoadException2;
failure._namespace = @namespace;
failure._name = name;
failure._module = module;
return failure;
}

public static ResolutionFailure GetTypeLoadResolutionFailure(string @namespace, string name, string moduleName)
{
ResolutionFailure failure = new ResolutionFailure();
failure._failureType = FailureType.TypeLoadException3;
failure._namespace = @namespace;
failure._name = name;
failure._moduleName = moduleName;
return failure;
}

public static ResolutionFailure GetMissingMethodFailure(TypeDesc owningType, string methodName, MethodSignature signature)
{
ResolutionFailure failure = new ResolutionFailure();
failure._failureType = FailureType.MissingMethodException1;
failure._methodSignature = signature;
failure._name = methodName;
failure._owningType = owningType;
return failure;
}

public static ResolutionFailure GetMissingFieldFailure(TypeDesc owningType, string fieldName)
{
ResolutionFailure failure = new ResolutionFailure();
failure._failureType = FailureType.MissingMethodException1;
failure._name = fieldName;
failure._owningType = owningType;
return failure;
}

public static ResolutionFailure GetAssemblyResolutionFailure(string simpleName)
{
ResolutionFailure failure = new ResolutionFailure();
failure._failureType = FailureType.MissingAssemblyException1;
failure._name = simpleName;
return failure;
}

public void Throw()
{
switch(_failureType)
{
case FailureType.TypeLoadException1:
ThrowHelper.ThrowTypeLoadException(_name, _module);
break;
case FailureType.TypeLoadException2:
ThrowHelper.ThrowTypeLoadException(_namespace, _name, _module);
break;
case FailureType.TypeLoadException3:
ThrowHelper.ThrowTypeLoadException(_namespace, _name, _moduleName);
break;
case FailureType.MissingMethodException1:
ThrowHelper.ThrowMissingMethodException(_owningType, _name, _methodSignature);
break;
case FailureType.MissingFieldException1:
ThrowHelper.ThrowMissingFieldException(_owningType, _name);
break;
case FailureType.MissingAssemblyException1:
ThrowHelper.ThrowFileNotFoundException(ExceptionStringID.FileLoadErrorGeneric, _name);
break;
}
}
}
}
5 changes: 5 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Common/ThrowHelper.Common.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ public static void ThrowTypeLoadException(string @namespace, string name, Module
ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, Format.Type(@namespace, name), Format.Module(module));
}

public static void ThrowTypeLoadException(string @namespace, string name, string moduleName)
{
ThrowTypeLoadException(ExceptionStringID.ClassLoadGeneral, Format.Type(@namespace, name), moduleName);
}

[System.Diagnostics.DebuggerHidden]
public static void ThrowTypeLoadException(TypeDesc type)
{
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Common/ThrowHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ public static void ThrowBadImageFormatException()
throw new TypeSystemException.BadImageFormatException();
}

[System.Diagnostics.DebuggerHidden]
public static void ThrowBadImageFormatException(string message)
{
throw new TypeSystemException.BadImageFormatException(message);
}

private static partial class Format
{
public static string OwningModule(TypeDesc type)
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/tools/Common/TypeSystem/Common/TypeSystemException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,12 @@ internal BadImageFormatException()
: base(ExceptionStringID.BadImageFormatGeneric)
{
}

internal BadImageFormatException(string reason)
: base(ExceptionStringID.BadImageFormatSpecific, reason)
{

}
}
}
}
2 changes: 1 addition & 1 deletion src/coreclr/tools/Common/TypeSystem/Common/Utilities/CustomAttributeTypeNameParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ private static MetadataType GetType(this ModuleDesc module, string fullName, boo
namespaceName = fullName.Substring(0, split);
typeName = fullName.Substring(split + 1);
}
return module.GetType(namespaceName, typeName, throwIfNotFound);
return module.GetType(namespaceName, typeName, throwIfNotFound ? NotFoundBehavior.Throw : NotFoundBehavior.ReturnNull);

@trylek trylek Mar 30, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would it make sense to propagate the new enum up the call graph instead of decoding the bool "in the middle"?

@davidwrighton davidwrighton Mar 30, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Plausibly, but this change is already quite large enough, and very few components other than the internals of the typesystem actually can do anything useful with ResolutionFailure objects. So I don't think we need to perturb the api of the typesystem more than I already have.

}

private static AssemblyName FindAssemblyIfNamePresent(string name)
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/Common/TypeSystem/Ecma/EcmaField.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ private TypeDesc InitializeFieldType()
var metadataReader = MetadataReader;
BlobReader signatureReader = metadataReader.GetBlobReader(metadataReader.GetFieldDefinition(_handle).Signature);

EcmaSignatureParser parser = new EcmaSignatureParser(Module, signatureReader);
EcmaSignatureParser parser = new EcmaSignatureParser(Module, signatureReader, NotFoundBehavior.Throw);
var fieldType = parser.ParseFieldSignature();
return (_fieldType = fieldType);
}
Expand Down Expand Up @@ -264,7 +264,7 @@ public override MarshalAsDescriptor GetMarshalAsDescriptor()
if ((definition.Attributes & FieldAttributes.HasFieldMarshal) != 0)
{
BlobReader marshalAsReader = reader.GetBlobReader(definition.GetMarshallingDescriptor());
EcmaSignatureParser parser = new EcmaSignatureParser(_type.EcmaModule, marshalAsReader);
EcmaSignatureParser parser = new EcmaSignatureParser(_type.EcmaModule, marshalAsReader, NotFoundBehavior.Throw);
return parser.ParseMarshalAsDescriptor();
}

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/tools/Common/TypeSystem/Ecma/EcmaMethod.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ private MethodSignature InitializeSignature()
var metadataReader = MetadataReader;
BlobReader signatureReader = metadataReader.GetBlobReader(metadataReader.GetMethodDefinition(_handle).Signature);

EcmaSignatureParser parser = new EcmaSignatureParser(Module, signatureReader);
EcmaSignatureParser parser = new EcmaSignatureParser(Module, signatureReader, NotFoundBehavior.Throw);
var signature = parser.ParseMethodSignature();
return (_signature = signature);
}
Expand Down Expand Up @@ -573,7 +573,7 @@ private MarshalAsDescriptor GetMarshalAsDescriptor(Parameter parameter)
{
MetadataReader metadataReader = MetadataReader;
BlobReader marshalAsReader = metadataReader.GetBlobReader(parameter.GetMarshallingDescriptor());
EcmaSignatureParser parser = new EcmaSignatureParser(Module, marshalAsReader);
EcmaSignatureParser parser = new EcmaSignatureParser(Module, marshalAsReader, NotFoundBehavior.Throw);
MarshalAsDescriptor marshalAs = parser.ParseMarshalAsDescriptor();
Debug.Assert(marshalAs != null);
return marshalAs;
Expand Down
Loading