Skip to content

Commit ae67ab6

Browse files
jtschusterjtschuster
committed
Fix crash in message formatting with try/catch and fix NAOT failure.
- Wrap message formatting in try/catch and give warning if format fails - Guard MakeGenericMethodSite dependency creation with call to CheckConstraints
1 parent 342936c commit ae67ab6

File tree

4 files changed

+90
-5
lines changed

4 files changed

+90
-5
lines changed

‎src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/HandleCallAction.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
using DependencyList = ILCompiler.DependencyAnalysisFramework.DependencyNodeCore<ILCompiler.DependencyAnalysis.NodeFactory>.DependencyList;
1717
using MultiValue = ILLink.Shared.DataFlow.ValueSet<ILLink.Shared.DataFlow.SingleValue>;
1818
using WellKnownType = ILLink.Shared.TypeSystemProxy.WellKnownType;
19+
using System;
1920

2021
#nullable enable
2122

@@ -697,7 +698,8 @@ private sealed class MakeGenericMethodSite : INodeWithRuntimeDeterminedDependenc
697698
public IEnumerable<DependencyNodeCore<NodeFactory>.DependencyListEntry> InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation)
698699
{
699700
var list = new DependencyList();
700-
RootingHelpers.TryGetDependenciesForReflectedMethod(ref list, factory, _method.InstantiateSignature(typeInstantiation, methodInstantiation), "MakeGenericMethod");
701+
if (_method.InstantiateSignature(typeInstantiation, methodInstantiation).CheckConstraints(new InstantiationContext(typeInstantiation, methodInstantiation)))
702+
RootingHelpers.TryGetDependenciesForReflectedMethod(ref list, factory, _method.InstantiateSignature(typeInstantiation, methodInstantiation), "MakeGenericMethod");
701703
return list;
702704
}
703705
}
@@ -711,7 +713,8 @@ private sealed class MakeGenericTypeSite : INodeWithRuntimeDeterminedDependencie
711713
public IEnumerable<DependencyNodeCore<NodeFactory>.DependencyListEntry> InstantiateDependencies(NodeFactory factory, Instantiation typeInstantiation, Instantiation methodInstantiation)
712714
{
713715
var list = new DependencyList();
714-
RootingHelpers.TryGetDependenciesForReflectedType(ref list, factory, _type.InstantiateSignature(typeInstantiation, methodInstantiation), "MakeGenericType");
716+
if (_type.InstantiateSignature(typeInstantiation, methodInstantiation).CheckConstraints(new InstantiationContext(typeInstantiation, methodInstantiation)))
717+
RootingHelpers.TryGetDependenciesForReflectedType(ref list, factory, _type.InstantiateSignature(typeInstantiation, methodInstantiation), "MakeGenericType");
715718
return list;
716719
}
717720
}

‎src/tools/illink/src/ILLink.Shared/DiagnosticString.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ public DiagnosticString (string diagnosticResourceStringName)
2828
}
2929

3030
public string GetMessage (params string[] args) =>
31-
string.Format (_messageFormat, args);
31+
MessageFormat.TryFormat (_messageFormat, args);
3232

3333
public string GetMessageFormat () => _messageFormat;
3434

3535
public string GetTitle (params string[] args) =>
36-
string.Format (_titleFormat, args);
36+
MessageFormat.TryFormat (_titleFormat, args);
3737

3838
public string GetTitleFormat () => _titleFormat;
3939
}

‎src/tools/illink/src/ILLink.Shared/MessageFormat.cs‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
// This is needed due to NativeAOT which doesn't enable nullable globally yet
55
#nullable enable
66

7+
using System;
8+
79
namespace ILLink.Shared
810
{
911
internal static class MessageFormat
@@ -33,7 +35,24 @@ public static string FormatRequiresAttributeMismatch (bool memberHasAttribute, b
3335
(true, false) => SharedStrings.DerivedRequiresMismatchMessage
3436
};
3537

36-
return string.Format (format, args);
38+
return TryFormat (format, args);
39+
}
40+
41+
public static string TryFormat (string format, params object[] args)
42+
{
43+
string formattedString;
44+
try
45+
{
46+
formattedString = string.Format(format, args);
47+
}
48+
catch (FormatException)
49+
{
50+
#pragma warning disable RS1035 // Do not use APIs banned for analyzers - We just need a newline
51+
formattedString = "Internal error formatting diagnostic. Please report the issue at https://aka.ms/report-illink" + Environment.NewLine
52+
+ $"'{format}', " + $"[{string.Join(", ", args)}]";
53+
#pragma warning restore RS1035 // Do not use APIs banned for analyzers
54+
}
55+
return formattedString;
3756
}
3857
}
3958
}

‎src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/NullableAnnotations.cs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ public static void Main ()
6565
GetUnderlyingTypeOnNonNullableKnownType.Test ();
6666
MakeGenericTypeWithUnknownValue (new object[2] { 1, 2 });
6767
MakeGenericTypeWithKnowAndUnknownArray ();
68+
RequiresOnNullableMakeGenericType.Test();
6869
}
6970

7071
[Kept]
@@ -97,6 +98,68 @@ static void RequireAllFromMadeGenericNullableOfTypeWithMethodWithRuc ()
9798
typeof (Nullable<>).MakeGenericType (typeof (TestStructWithRucMethod)).RequiresAll ();
9899
}
99100

101+
public class RequiresOnNullableMakeGenericType
102+
{
103+
[Kept]
104+
static Type UnannotatedField;
105+
[Kept]
106+
[KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))]
107+
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
108+
static Type FieldWithMethods;
109+
[Kept]
110+
[UnexpectedWarning("IL2090", Tool.TrimmerAnalyzerAndNativeAot, "https://github.com/dotnet/runtime/issues/93800")]
111+
static void Field()
112+
{
113+
typeof (Nullable<>).MakeGenericType (UnannotatedField).GetMethods ();
114+
typeof (Nullable<>).MakeGenericType (FieldWithMethods).GetMethods ();
115+
}
116+
117+
[Kept]
118+
[UnexpectedWarning("IL2090", Tool.TrimmerAnalyzerAndNativeAot, "https://github.com/dotnet/runtime/issues/93800")]
119+
static void Parameter(
120+
Type unannotated,
121+
[KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))]
122+
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] Type annotated)
123+
{
124+
typeof (Nullable<>).MakeGenericType (unannotated).GetMethods ();
125+
typeof (Nullable<>).MakeGenericType (annotated).GetMethods ();
126+
}
127+
128+
[Kept]
129+
[ExpectedWarning("IL2090", "TUnannotated")]
130+
static void TypeParameter<
131+
TUnannotated,
132+
[KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))]
133+
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)] TAnnotated>()
134+
{
135+
typeof (Nullable<>).MakeGenericType (typeof(TUnannotated)).GetMethods ();
136+
typeof (Nullable<>).MakeGenericType (typeof(TAnnotated)).GetMethods ();
137+
}
138+
139+
[Kept]
140+
[UnexpectedWarning("IL2090", Tool.TrimmerAnalyzerAndNativeAot, "https://github.com/dotnet/runtime/issues/93800")]
141+
static void ReturnValue()
142+
{
143+
typeof (Nullable<>).MakeGenericType (GetUnannotated()).GetMethods ();
144+
typeof (Nullable<>).MakeGenericType (GetAnnotated()).GetMethods ();
145+
}
146+
[Kept]
147+
static Type GetUnannotated() => null;
148+
[Kept]
149+
[return: KeptAttributeAttribute(typeof(DynamicallyAccessedMembersAttribute))]
150+
[return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods)]
151+
static Type GetAnnotated() => null;
152+
153+
[Kept]
154+
public static void Test()
155+
{
156+
Field();
157+
Parameter(null, null);
158+
TypeParameter<object, object>();
159+
ReturnValue();
160+
}
161+
}
162+
100163
[Kept]
101164
static void TestRequireRucMethodThroughNullable ()
102165
{

0 commit comments

Comments
 (0)