Skip to content

Commit

Permalink
Fix type parsing issues in ILLink and ILC (#104060)
Browse files Browse the repository at this point in the history
- Fixes #98955

   We will now produce a warning when a non-assembly-qualified type
   flows into a string location annotated with
   DynamicallyAccessedMembers, and we don't try to look up or mark the
   type (since we don't know which assemblies will be searched at
   runtime by the Type.GetType call).

- Fixes #103906

   The ILLink intrinsic handling for `Type.GetType` will now look in
   corelib for generic arguments, matching native AOT.

This replaces the existing warning IL2105. This uses a new warning
instead of repurposing IL2105, because we already documented IL2105
and older versions of ILLink will produce it. Best to avoid any
confusion about them.
  • Loading branch information
sbomer authored and pull[bot] committed Oct 7, 2024
1 parent 04df3b8 commit ed3a399
Show file tree
Hide file tree
Showing 30 changed files with 308 additions and 136 deletions.
16 changes: 16 additions & 0 deletions docs/tools/illink/error-codes.md
Original file line number Diff line number Diff line change
Expand Up @@ -1870,6 +1870,22 @@ void TestMethod()
}
```

#### `IL2122`: Type 'typeName' is not assembly qualified. Type name strings used for dynamically accessing a type should be assembly qualified.

- The type name string passed to a location with `DynamicallyAccessedMembers` requirements was not assembly-qualified, so the trimmer cannot guarantee that the type is preserved. Consider using an assembly-qualified name instead.

```C#
// warning IL2122: Type 'MyClass' is not assembly qualified. Type name strings used for dynamically accessing a type should be assembly qualified.
GetTypeWrapper("MyClass");

class MyClass { }

// May be defined in another assembly, so at runtime Type.GetType will look in that assembly for "MyClass".
void GetTypeWrapper([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] string typeName)
{
var type = Type.GetType(typeName);
}
```

## Single-File Warning Codes

Expand Down
2 changes: 1 addition & 1 deletion eng/testing/tests.mobile.targets
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
<PublishTrimmed>true</PublishTrimmed>
<!-- Suppress trimming warnings as these are tests -->
<SuppressTrimAnalysisWarnings>true</SuppressTrimAnalysisWarnings>
<NoWarn>$(NoWarn);IL2103;IL2105;IL2025;IL2111</NoWarn>
<NoWarn>$(NoWarn);IL2103;IL2025;IL2111;IL2122</NoWarn>

<!-- Reduce library test app size by trimming framework library features -->
<DebuggerSupport Condition="'$(DebuggerSupport)' == '' and '$(Configuration)' != 'Debug'">false</DebuggerSupport>
Expand Down
2 changes: 1 addition & 1 deletion eng/testing/tests.singlefile.targets
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
<IlcSdkPath>$(CoreCLRAotSdkDir)</IlcSdkPath>
<IlcFrameworkPath>$(NetCoreAppCurrentTestHostSharedFrameworkPath)</IlcFrameworkPath>
<IlcFrameworkNativePath>$(NetCoreAppCurrentTestHostSharedFrameworkPath)</IlcFrameworkNativePath>
<NoWarn>$(NoWarn);IL1005;IL2105;IL3000;IL3001;IL3002;IL3003;IL3050;IL3051;IL3052;IL3053</NoWarn>
<NoWarn>$(NoWarn);IL1005;IL2122;IL3000;IL3001;IL3002;IL3003;IL3050;IL3051;IL3052;IL3053</NoWarn>
<TrimMode>partial</TrimMode>
<SuppressTrimAnalysisWarnings>true</SuppressTrimAnalysisWarnings>
<TrimmerSingleWarn>false</TrimmerSingleWarn>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,18 @@ public static TypeDesc GetTypeByCustomAttributeTypeName(this ModuleDesc module,
}

public static TypeDesc GetTypeByCustomAttributeTypeNameForDataFlow(string name, ModuleDesc callingModule,
TypeSystemContext context, List<ModuleDesc> referencedModules, out bool typeWasNotFoundInAssemblyNorBaseLibrary)
TypeSystemContext context, List<ModuleDesc> referencedModules, bool needsAssemblyName, out bool failedBecauseNotFullyQualified)
{
typeWasNotFoundInAssemblyNorBaseLibrary = false;

failedBecauseNotFullyQualified = false;
if (!TypeName.TryParse(name.AsSpan(), out TypeName parsed, s_typeNameParseOptions))
return null;

if (needsAssemblyName && !IsFullyQualified(parsed))
{
failedBecauseNotFullyQualified = true;
return null;
}

TypeNameResolver resolver = new()
{
_context = context,
Expand All @@ -52,8 +57,33 @@ public static TypeDesc GetTypeByCustomAttributeTypeNameForDataFlow(string name,

TypeDesc type = resolver.Resolve(parsed);

typeWasNotFoundInAssemblyNorBaseLibrary = resolver._typeWasNotFoundInAssemblyNorBaseLibrary;
return type;

static bool IsFullyQualified(TypeName typeName)
{
if (typeName.AssemblyName is null)
{
return false;
}

if (typeName.IsArray || typeName.IsPointer || typeName.IsByRef)
{
return IsFullyQualified(typeName.GetElementType());
}

if (typeName.IsConstructedGenericType)
{
foreach (var typeArgument in typeName.GetGenericArguments())
{
if (!IsFullyQualified(typeArgument))
{
return false;
}
}
}

return true;
}
}

private struct TypeNameResolver
Expand All @@ -64,7 +94,6 @@ private struct TypeNameResolver
internal Func<ModuleDesc, string, MetadataType> _canonResolver;

internal List<ModuleDesc> _referencedModules;
internal bool _typeWasNotFoundInAssemblyNorBaseLibrary;

internal TypeDesc Resolve(TypeName typeName)
{
Expand Down Expand Up @@ -136,8 +165,6 @@ private TypeDesc GetSimpleType(TypeName typeName)
return type;
}
}

_typeWasNotFoundInAssemblyNorBaseLibrary = true;
}

if (_throwIfNotFound)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,13 @@ internal bool TryResolveTypeNameAndMark(string typeName, in DiagnosticContext di

List<ModuleDesc> referencedModules = new();
TypeDesc foundType = CustomAttributeTypeNameParser.GetTypeByCustomAttributeTypeNameForDataFlow(typeName, callingModule, diagnosticContext.Origin.MemberDefinition!.Context,
referencedModules, out bool typeWasNotFoundInAssemblyNorBaseLibrary);
referencedModules, needsAssemblyName, out bool failedBecauseNotFullyQualified);
if (foundType == null)
{
if (needsAssemblyName && typeWasNotFoundInAssemblyNorBaseLibrary)
diagnosticContext.AddDiagnostic(DiagnosticId.TypeWasNotFoundInAssemblyNorBaseLibrary, typeName);

if (failedBecauseNotFullyQualified)
{
diagnosticContext.AddDiagnostic(DiagnosticId.TypeNameIsNotAssemblyQualified, typeName);
}
type = default;
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Mono.Cecil;
using Mono.Cecil.Cil;
using Mono.Linker.Tests.Cases.Expectations.Assertions;
using Mono.Linker.Tests.Cases.Expectations.Helpers;
using Mono.Linker.Tests.Extensions;
using Xunit;
using MetadataType = Internal.TypeSystem.MetadataType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class Program
/// </summary>
static int Main(string[] args)
{
TypeDescriptionProviderAttribute attr = new TypeDescriptionProviderAttribute("Program+MyTypeDescriptionProvider");
TypeDescriptionProviderAttribute attr = new TypeDescriptionProviderAttribute("Program+MyTypeDescriptionProvider, project");
if (!RunTest(attr))
{
return -1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public DebuggerProxy(MyClass instance)
}
}

[DebuggerTypeProxy("Program+MyClassWithProxyStringProxy")]
[DebuggerTypeProxy("Program+MyClassWithProxyStringProxy, project")]
public class MyClassWithProxyString
{
public string Name { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ public DebuggerVisualizerObjectSource(MyClass instance)
}
}

[DebuggerVisualizer("Program+MyClassWithVisualizerStringVisualizer")]
[DebuggerVisualizer("Program+MyClassWithVisualizerStringVisualizer2", "Program+MyClassWithVisualizerStringVisualizerObjectSource")]
[DebuggerVisualizer("Program+MyClassWithVisualizerStringVisualizer, project")]
[DebuggerVisualizer("Program+MyClassWithVisualizerStringVisualizer2, project", "Program+MyClassWithVisualizerStringVisualizerObjectSource, project")]
public class MyClassWithVisualizerString
{
public string Name { get; set; }
Expand Down
2 changes: 1 addition & 1 deletion src/mono/msbuild/apple/build/AppleBuild.LocalBuild.targets
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
<PropertyGroup Condition="'$(EnableAggressiveTrimming)' == 'true'">
<!-- Suppress trimming warnings as these are tests -->
<SuppressTrimAnalysisWarnings>true</SuppressTrimAnalysisWarnings>
<NoWarn>$(NoWarn);IL2103;IL2105;IL2025;IL2111</NoWarn>
<NoWarn>$(NoWarn);IL2103;IL2025;IL2111;IL2122</NoWarn>
</PropertyGroup>

<!-- This .targets file is also imported by the runtime Trimming tests, and we want to be able to manually configure trimming in them so this
Expand Down
5 changes: 3 additions & 2 deletions src/tools/illink/src/ILLink.Shared/DiagnosticId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public enum DiagnosticId
_unused_RearrangedXmlWarning2 = 2021,
XmlCouldNotFindMatchingConstructorForCustomAttribute = 2022,
XmlMoreThanOneReturnElementForMethod = 2023,
XmlMoreThanOneValyForParameterOfMethod = 2024,
XmlMoreThanOneValueForParameterOfMethod = 2024,
XmlDuplicatePreserveMember = 2025,
RequiresUnreferencedCode = 2026,
AttributeShouldOnlyBeUsedOnceOnMember = 2027,
Expand Down Expand Up @@ -169,7 +169,7 @@ public enum DiagnosticId
InvalidIsTrimmableValue = 2102,
PropertyAccessorParameterInLinqExpressionsCannotBeStaticallyDetermined = 2103,
AssemblyProducedTrimWarnings = 2104,
TypeWasNotFoundInAssemblyNorBaseLibrary = 2105,
_unused_TypeWasNotFoundInAssemblyNorBaseLibrary = 2105,
DynamicallyAccessedMembersOnMethodReturnValueCanOnlyApplyToTypesOrStrings = 2106,
MethodsAreAssociatedWithStateMachine = 2107,
InvalidScopeInUnconditionalSuppressMessage = 2108,
Expand All @@ -186,6 +186,7 @@ public enum DiagnosticId
DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMember = 2119,
DynamicallyAccessedMembersOnTypeReferencesCompilerGeneratedMemberOnBase = 2120,
RedundantSuppression = 2121,
TypeNameIsNotAssemblyQualified = 2122,

// Single-file diagnostic ids.
AvoidAssemblyLocationInSingleFile = 3000,
Expand Down
10 changes: 8 additions & 2 deletions src/tools/illink/src/ILLink.Shared/SharedStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -525,10 +525,10 @@
<data name="XmlMoreThanOneReturnElementForMethodMessage" xml:space="preserve">
<value>There is more than one 'return' child element specified for method '{0}'.</value>
</data>
<data name="XmlMoreThanOneValyForParameterOfMethodTitle" xml:space="preserve">
<data name="XmlMoreThanOneValueForParameterOfMethodTitle" xml:space="preserve">
<value>Method has more than one parameter for the XML element 'parameter'. There can only be one value specified for each parameter.</value>
</data>
<data name="XmlMoreThanOneValyForParameterOfMethodMessage" xml:space="preserve">
<data name="XmlMoreThanOneValueForParameterOfMethodMessage" xml:space="preserve">
<value>More than one value specified for parameter '{0}' of method '{1}'.</value>
</data>
<data name="XmlDuplicatePreserveMemberTitle" xml:space="preserve">
Expand Down Expand Up @@ -1209,4 +1209,10 @@
<data name="InvalidFeatureGuardTitle" xml:space="preserve">
<value>Invalid FeatureGuardAttribute.</value>
</data>
<data name="TypeNameIsNotAssemblyQualifiedMessage" xml:space="preserve">
<value>Type '{0}' is not assembly qualified. Type name strings used for dynamically accessing a type should be assembly qualified.</value>
</data>
<data name="TypeNameIsNotAssemblyQualifiedTitle" xml:space="preserve">
<value>Type name is not assembly qualified.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void Invoke (in MultiValue value, ValueWithDynamicallyAccessedMembers tar
} else if (uniqueValue is SystemTypeValue systemTypeValue) {
MarkTypeForDynamicallyAccessedMembers (systemTypeValue.RepresentedType, targetValue.DynamicallyAccessedMemberTypes);
} else if (uniqueValue is KnownStringValue knownStringValue) {
if (!TryResolveTypeNameAndMark (knownStringValue.Contents, true, out TypeProxy foundType)) {
if (!TryResolveTypeNameAndMark (knownStringValue.Contents, needsAssemblyName: true, out TypeProxy foundType)) {
// Intentionally ignore - it's not wrong for code to call Type.GetType on non-existing name, the code might expect null/exception back.
} else {
MarkTypeForDynamicallyAccessedMembers (foundType, targetValue.DynamicallyAccessedMemberTypes);
Expand Down
12 changes: 12 additions & 0 deletions src/tools/illink/src/linker/CompatibilitySuppressions.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1509,6 +1509,12 @@
<Left>ref/net9.0/illink.dll</Left>
<Right>lib/net9.0/illink.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Mono.Linker.LinkContext.get_SystemModuleName</Target>
<Left>ref/net9.0/illink.dll</Left>
<Right>lib/net9.0/illink.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Mono.Linker.LinkContext.set_KeepComInterfaces(System.Boolean)</Target>
Expand All @@ -1521,6 +1527,12 @@
<Left>ref/net9.0/illink.dll</Left>
<Right>lib/net9.0/illink.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:Mono.Linker.LinkContext.set_SystemModuleName(System.String)</Target>
<Left>ref/net9.0/illink.dll</Left>
<Right>lib/net9.0/illink.dll</Right>
</Suppression>
<Suppression>
<DiagnosticId>CP0008</DiagnosticId>
<Target>T:Mono.Linker.LinkContext</Target>
Expand Down
34 changes: 19 additions & 15 deletions src/tools/illink/src/linker/Linker.Steps/LinkAttributesParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void Parse (AttributeInfo xmlInfo)
continue;
}

CustomAttribute? customAttribute = CreateCustomAttribute (attributeNav, attributeType);
CustomAttribute? customAttribute = CreateCustomAttribute (attributeNav, attributeType, provider);
if (customAttribute != null) {
_context.LogMessage ($"Assigning external custom attribute '{FormatCustomAttribute (customAttribute)}' instance to '{provider}'.");
customAttributesBuilder.Add (customAttribute);
Expand Down Expand Up @@ -153,9 +153,9 @@ static string FormatCustomAttribute (CustomAttribute ca)
return _context.MarkedKnownMembers.RemoveAttributeInstancesAttributeDefinition = td;
}

CustomAttribute? CreateCustomAttribute (XPathNavigator nav, TypeDefinition attributeType)
CustomAttribute? CreateCustomAttribute (XPathNavigator nav, TypeDefinition attributeType, ICustomAttributeProvider provider)
{
CustomAttributeArgument[] arguments = ReadCustomAttributeArguments (nav, attributeType);
CustomAttributeArgument[] arguments = ReadCustomAttributeArguments (nav, provider);

MethodDefinition? constructor = FindBestMatchingConstructor (attributeType, arguments);
if (constructor == null) {
Expand Down Expand Up @@ -223,22 +223,22 @@ void ReadCustomAttributeProperties (XPathNavigator nav, TypeDefinition attribute
}
}

CustomAttributeArgument[] ReadCustomAttributeArguments (XPathNavigator nav, TypeDefinition attributeType)
CustomAttributeArgument[] ReadCustomAttributeArguments (XPathNavigator nav, ICustomAttributeProvider provider)
{
ArrayBuilder<CustomAttributeArgument> args = default;

foreach (XPathNavigator argumentNav in nav.SelectChildren ("argument", string.Empty)) {
CustomAttributeArgument? caa = ReadCustomAttributeArgument (argumentNav, attributeType);
CustomAttributeArgument? caa = ReadCustomAttributeArgument (argumentNav, provider);
if (caa is not null)
args.Add (caa.Value);
}

return args.ToArray () ?? Array.Empty<CustomAttributeArgument> ();
}

CustomAttributeArgument? ReadCustomAttributeArgument (XPathNavigator nav, IMemberDefinition memberWithAttribute)
CustomAttributeArgument? ReadCustomAttributeArgument (XPathNavigator nav, ICustomAttributeProvider provider)
{
TypeReference? typeref = ResolveArgumentType (nav, memberWithAttribute);
TypeReference? typeref = ResolveArgumentType (nav, provider);
if (typeref is null)
return null;

Expand Down Expand Up @@ -295,8 +295,8 @@ CustomAttributeArgument[] ReadCustomAttributeArguments (XPathNavigator nav, Type
if (!typeref.IsTypeOf (WellKnownType.System_Type))
goto default;

var diagnosticContext = new DiagnosticContext (new MessageOrigin (memberWithAttribute), diagnosticsEnabled: true, _context);
if (!_context.TypeNameResolver.TryResolveTypeName (svalue, diagnosticContext, out TypeReference? type, out _)) {
var diagnosticContext = new DiagnosticContext (new MessageOrigin (provider), diagnosticsEnabled: true, _context);
if (!_context.TypeNameResolver.TryResolveTypeName (svalue, diagnosticContext, out TypeReference? type, out _, needsAssemblyName: false)) {
_context.LogError (GetMessageOriginForPosition (nav), DiagnosticId.CouldNotResolveCustomAttributeTypeValue, svalue);
return null;
}
Expand All @@ -308,7 +308,7 @@ CustomAttributeArgument[] ReadCustomAttributeArguments (XPathNavigator nav, Type
var arrayArgumentIterator = nav.SelectChildren ("argument", string.Empty);
ArrayBuilder<CustomAttributeArgument> elements = default;
foreach (XPathNavigator elementNav in arrayArgumentIterator) {
if (ReadCustomAttributeArgument (elementNav, memberWithAttribute) is CustomAttributeArgument arg) {
if (ReadCustomAttributeArgument (elementNav, provider) is CustomAttributeArgument arg) {
// To match Cecil, elements of a list that are subclasses of the list type must be boxed in the base type
// e.g. object[] { 73 } translates to Cecil.CAA { Type: object[] : Value: CAA{ Type: object, Value: CAA{ Type: int, Value: 73} } }
if (arg.Type == elementType) {
Expand Down Expand Up @@ -338,14 +338,14 @@ CustomAttributeArgument[] ReadCustomAttributeArguments (XPathNavigator nav, Type
return null;
}

TypeReference? ResolveArgumentType (XPathNavigator nav, IMemberDefinition memberWithAttribute)
TypeReference? ResolveArgumentType (XPathNavigator nav, ICustomAttributeProvider provider)
{
string typeName = GetAttribute (nav, "type");
if (string.IsNullOrEmpty (typeName))
typeName = "System.String";

var diagnosticContext = new DiagnosticContext (new MessageOrigin (memberWithAttribute), diagnosticsEnabled: true, _context);
if (!_context.TypeNameResolver.TryResolveTypeName (typeName, diagnosticContext, out TypeReference? typeref, out _)) {
var diagnosticContext = new DiagnosticContext (new MessageOrigin (provider), diagnosticsEnabled: true, _context);
if (!_context.TypeNameResolver.TryResolveTypeName (typeName, diagnosticContext, out TypeReference? typeref, out _, needsAssemblyName: false)) {
_context.LogError (GetMessageOriginForPosition (nav), DiagnosticId.TypeUsedWithAttributeValueCouldNotBeFound, typeName, nav.Value);
return null;
}
Expand Down Expand Up @@ -501,7 +501,7 @@ void ProcessParameters (MethodDefinition method, XPathNavigator nav)
foreach (ParameterDefinition parameter in method.Parameters) {
if (paramName == parameter.Name) {
if (parameter.HasCustomAttributes || _attributeInfo.CustomAttributes.ContainsKey (parameter))
LogWarning (parameterNav, DiagnosticId.XmlMoreThanOneValyForParameterOfMethod, paramName, method.GetDisplayName ());
LogWarning (parameterNav, DiagnosticId.XmlMoreThanOneValueForParameterOfMethod, paramName, method.GetDisplayName ());
_attributeInfo.AddCustomAttributes (parameter, attributes, origins);
break;
}
Expand All @@ -513,11 +513,15 @@ void ProcessParameters (MethodDefinition method, XPathNavigator nav)

void ProcessReturnParameters (MethodDefinition method, XPathNavigator nav)
{
Debug.Assert (_attributeInfo != null);
bool firstAppearance = true;
foreach (XPathNavigator returnNav in nav.SelectChildren ("return", string.Empty)) {
if (firstAppearance) {
firstAppearance = false;
PopulateAttributeInfo (method.MethodReturnType, returnNav);
var (attributes, origins) = ProcessAttributes (returnNav, method);
if (attributes != null && origins != null) {
_attributeInfo.AddCustomAttributes (method.MethodReturnType, attributes, origins);
}
} else {
LogWarning (returnNav, DiagnosticId.XmlMoreThanOneReturnElementForMethod, method.GetDisplayName ());
}
Expand Down
4 changes: 0 additions & 4 deletions src/tools/illink/src/linker/Linker.Steps/MarkStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@
using Mono.Collections.Generic;
using Mono.Linker.Dataflow;

using AssemblyNameInfo = System.Reflection.Metadata.AssemblyNameInfo;
using TypeName = System.Reflection.Metadata.TypeName;
using TypeNameParseOptions = System.Reflection.Metadata.TypeNameParseOptions;

namespace Mono.Linker.Steps
{

Expand Down
Loading

0 comments on commit ed3a399

Please sign in to comment.