Skip to content

Commit 5b32352

Browse files
Disable speculative array TypeMap emission (ILLink crash)
Reverts the runtime swap and factory-method removal from the previous commit. Keeps the new `ITypeMapWithAliasing.TryGetType` raw lookup and `TrimmableTypeMap.TryGetArrayType` helper as scaffolding for the eventual ILLink-fixed approach. The speculative `[L<jni>;` / `[[L<jni>;` / `[[[L<jni>;` TypeMap entries crash ILLink with: System.NotSupportedException: TypeDefinition cannot be resolved from 'Mono.Cecil.ArrayType' type at Mono.Linker.LinkContext.Resolve(TypeReference typeReference) at Mono.Linker.TypeMapHandler.RecordTypeMapEntry(...) (3-arg form) at Mono.Linker.TypeMapHandler.MarkTypeMapAttribute(...) (2-arg form) at Mono.Linker.TypeMapHandler.ProcessExternalTypeMapGroupSeen(...) Both 2-arg and 3-arg TypeMap forms are affected — `MarkTypeMapAttribute` calls `LinkContext.Resolve` on the `TargetType` slot (constructor arg index 1) for any TypeMap, and Cecil's `ArrayType` is not a `TypeDefinition`. There is no shape of `TypeMapAttribute` today that accepts a closed array `Type`. `EmitArrayEntries` is now a documented no-op. `JavaPeerContainerFactory<T>.CreateArray` and `CreateHigherRankArray` are restored. `JNIEnv.ArrayCreateInstance` falls back to the legacy per-T factory under trimmable. Trimmable + CoreCLR lane after revert: 917 total, 0 errors, 3 failures (pre-existing `TryGetJniNameForManagedType_*`, out of scope — same baseline as #11225). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4b0b089 commit 5b32352

4 files changed

Lines changed: 73 additions & 145 deletions

File tree

src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ModelBuilder.cs

Lines changed: 13 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -215,52 +215,24 @@ static void EmitPeers (TypeMapAssemblyData model, string jniName,
215215
const int MaxArrayRank = 3;
216216

217217
/// <summary>
218-
/// Emits speculative <c>[L&lt;jni&gt;;</c> / <c>[[L&lt;jni&gt;;</c> / <c>[[[L&lt;jni&gt;;</c> TypeMap entries
219-
/// for a single peer. Each entry maps a JNI array name to the corresponding closed
220-
/// managed array type (e.g. <c>typeof(SomePeer[])</c>) and is emitted as a 3-arg
221-
/// (conditional) attribute so the trimmer can drop entries whose array target type is
222-
/// not live in the shipped app.
218+
/// (Disabled — see remarks.) Would emit speculative <c>[L&lt;jni&gt;;</c> /
219+
/// <c>[[L&lt;jni&gt;;</c> / <c>[[[L&lt;jni&gt;;</c> TypeMap entries pointing at closed
220+
/// managed array types so <c>JNIEnv.ArrayCreateInstance</c> could resolve arrays
221+
/// via <see cref="Array.CreateInstanceFromArrayType"/> instead of through
222+
/// <c>JavaPeerContainerFactory&lt;T&gt;</c>.
223223
/// </summary>
224224
/// <remarks>
225-
/// Skips:
226-
/// <list type="bullet">
227-
/// <item>Open-generic peers (<c>JavaPeerInfo.IsGenericDefinition</c>) — <c>typeof(T&lt;&gt;[])</c> is invalid.</item>
228-
/// <item>JNI keyword keys (<c>Z</c>, <c>B</c>, …) — primitives are handled by
229-
/// <c>JniRuntime.JniTypeManager.GetPrimitiveArrayTypesForSimpleReference</c>; emitting
230-
/// array entries here would collide with that built-in path.</item>
231-
/// <item>Alias groups — multiple peers sharing a JNI name would produce duplicate
232-
/// array-key entries. Array-typemap support for aliases would need its own
233-
/// indexed-alias scheme; deferred until a real-world need is identified.</item>
234-
/// </list>
225+
/// Currently a no-op. Speculative emission is blocked by an ILLink limitation:
226+
/// <c>Mono.Linker.TypeMapHandler</c> calls <c>LinkContext.Resolve</c> on the
227+
/// <c>TargetType</c> slot of every <c>TypeMapAttribute</c>, which throws
228+
/// <c>NotSupportedException ("TypeDefinition cannot be resolved from
229+
/// 'Mono.Cecil.ArrayType' type")</c> for closed array types. This affects both
230+
/// 2-arg and 3-arg forms — there is no <c>TypeMapAttribute</c> shape that
231+
/// accepts an array <see cref="Type"/> today.
235232
/// </remarks>
236233
static void EmitArrayEntries (TypeMapAssemblyData model, string jniName, List<JavaPeerInfo> peersForName)
237234
{
238-
// Primitive single-letter JNI keywords are handled by the legacy primitive path.
239-
// Skip them so we don't shadow the built-in [Z, [B, etc. array entries.
240-
if (jniName.Length == 1 && IsJniPrimitiveKeyword (jniName [0])) {
241-
return;
242-
}
243-
244-
// Alias groups would produce duplicate JNI array keys (one per peer). Defer
245-
// alias-aware array emission until we have a concrete use case.
246-
if (peersForName.Count != 1) {
247-
return;
248-
}
249-
250-
var peer = peersForName [0];
251-
if (peer.IsGenericDefinition) {
252-
return;
253-
}
254-
255-
for (int rank = 1; rank <= MaxArrayRank; rank++) {
256-
string arrayJniName = string.Concat (new string ('[', rank), "L", jniName, ";");
257-
string arrayTargetRef = AssemblyQualify (peer.ManagedTypeName + Brackets (rank), peer.AssemblyName);
258-
model.Entries.Add (new TypeMapAttributeData {
259-
JniName = arrayJniName,
260-
ProxyTypeReference = arrayTargetRef,
261-
TargetTypeReference = arrayTargetRef,
262-
});
263-
}
235+
// Intentionally empty. See remarks above.
264236
}
265237

266238
static string Brackets (int rank)

src/Mono.Android/Android.Runtime/JNIEnv.cs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,16 @@ public static partial class JNIEnv {
2626

2727
static Array ArrayCreateInstance (Type elementType, int length)
2828
{
29+
// TODO: when the ILLink limitation around array types in TypeMap
30+
// attributes is resolved, switch this to use
31+
// TrimmableTypeMap.TryGetArrayType + Array.CreateInstanceFromArrayType
32+
// (the helpers are already in place) and remove
33+
// JavaPeerContainerFactory<T>.CreateArray. Today the speculative array
34+
// TypeMap emission crashes ILLink, so we keep using the per-T factory.
2935
if (RuntimeFeature.TrimmableTypeMap) {
30-
if (TrimmableTypeMap.Instance.TryGetArrayType (elementType, out var arrayType)) {
31-
return Array.CreateInstanceFromArrayType (arrayType, length);
32-
}
33-
throw new NotSupportedException (
34-
$"No TrimmableTypeMap array entry for element type '{elementType}'. " +
35-
$"Add an [assembly: TypeMap] entry for the closed array type or report an issue.");
36+
var factory = TrimmableTypeMap.Instance?.GetContainerFactory (elementType);
37+
if (factory is not null)
38+
return factory.CreateArray (length, 1);
3639
}
3740

3841
#pragma warning disable IL3050 // Array.CreateInstance is not AOT-safe, but this is the legacy fallback path

src/Mono.Android/Java.Interop/JavaPeerContainerFactory.cs

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,25 @@
1010
namespace Java.Interop
1111
{
1212
/// <summary>
13-
/// AOT-safe factory for creating typed containers (lists, collections, dictionaries)
14-
/// without using <c>MakeGenericType()</c>.
13+
/// AOT-safe factory for creating typed containers (arrays, lists, collections, dictionaries)
14+
/// without using <c>MakeGenericType()</c> or <c>Array.CreateInstance()</c>.
1515
/// </summary>
1616
/// <remarks>
17-
/// Array creation has been moved out of this factory: <c>JNIEnv.ArrayCreateInstance</c>
18-
/// now resolves closed array types via the trimmable typemap and
19-
/// <see cref="Array.CreateInstanceFromArrayType"/> instead of going through a per-T
20-
/// factory instantiation.
17+
/// Array creation is still routed through this factory because the planned
18+
/// <c>JNIEnv.ArrayCreateInstance</c> + <c>Array.CreateInstanceFromArrayType</c>
19+
/// path requires speculative <c>[L&lt;jni&gt;;</c> <c>TypeMap</c> attribute
20+
/// entries that ILLink's <c>TypeMapHandler</c> currently cannot process
21+
/// (<c>NotSupportedException</c> on <c>Mono.Cecil.ArrayType</c>).
22+
/// <see cref="Microsoft.Android.Runtime.TrimmableTypeMap.TryGetArrayType"/> is
23+
/// in place for the future swap once the ILLink limitation is resolved.
2124
/// </remarks>
2225
public abstract class JavaPeerContainerFactory
2326
{
27+
/// <summary>
28+
/// Creates a typed jagged array. Rank 1 = T[], rank 2 = T[][], etc.
29+
/// </summary>
30+
internal abstract Array CreateArray (int length, int rank);
31+
2432
/// <summary>
2533
/// Creates a typed <c>JavaList&lt;T&gt;</c> from a JNI handle.
2634
/// </summary>
@@ -72,6 +80,35 @@ public sealed class JavaPeerContainerFactory<
7280

7381
JavaPeerContainerFactory () { }
7482

83+
// TODO (https://github.com/dotnet/android/issues/10794): This might cause unnecessary code bloat for NativeAOT. Revisit
84+
// how we use this API and instead use a differnet approach that uses AOT-safe `Array.CreateInstanceFromArrayType`
85+
// with statically provided array types based on a statically known array type.
86+
// (See PR #11238 and the ILLink limitation it documents — the planned typemap-based
87+
// approach is already wired in via `TrimmableTypeMap.TryGetArrayType` but cannot
88+
// be activated until ILLink learns to handle `Mono.Cecil.ArrayType` in
89+
// `TypeMapAttribute`.)
90+
internal override Array CreateArray (int length, int rank) => rank switch {
91+
1 => new T [length],
92+
2 => new T [length][],
93+
3 => new T [length][][],
94+
_ when rank >= 0 => CreateHigherRankArray (length, rank),
95+
_ => throw new ArgumentOutOfRangeException (nameof (rank), rank, "Rank must be non-negative."),
96+
};
97+
98+
static Array CreateHigherRankArray (int length, int rank)
99+
{
100+
if (!RuntimeFeature.IsDynamicCodeSupported) {
101+
throw new NotSupportedException ($"Cannot create array of rank {rank} because dynamic code is not supported.");
102+
}
103+
104+
var arrayType = typeof (T);
105+
for (int i = 0; i < rank; i++) {
106+
arrayType = arrayType.MakeArrayType ();
107+
}
108+
109+
return Array.CreateInstanceFromArrayType (arrayType, length);
110+
}
111+
75112
internal override IList CreateList (IntPtr handle, JniHandleOwnership transfer)
76113
=> new Android.Runtime.JavaList<T> (handle, transfer);
77114

tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapModelBuilderTests.cs

Lines changed: 8 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -778,20 +778,16 @@ public class PeBlobValidation
778778
[Fact]
779779
public void FullPipeline_Mixed2ArgAnd3Arg_BothSurviveRoundTrip ()
780780
{
781-
// With ForceUnconditionalEntries, both peer entries are emitted as 2-arg unconditional.
782-
// In addition, ModelBuilder emits speculative 3-arg conditional array entries (ranks 1-3)
783-
// per non-aliased peer, which also round-trip through the PE blob.
781+
// With ForceUnconditionalEntries, both are emitted as 2-arg unconditional
784782
var objectPeer = FindFixtureByJavaName ("java/lang/Object");
785783
var activityPeer = FindFixtureByJavaName ("android/app/Activity");
786784

787785
var model = BuildModel (new [] { objectPeer, activityPeer }, "MixedBlob");
788-
Assert.Equal (2, NonArrayEntries (model).Count);
789-
// 2 peers × 3 array ranks = 6 speculative array entries; total 2 + 6 = 8.
790-
Assert.Equal (8, model.Entries.Count);
786+
Assert.Equal (2, model.Entries.Count);
791787

792788
EmitAndVerify (model, "MixedBlob", (pe, reader) => {
793789
var attrs = ReadAllTypeMapAttributeBlobs (reader);
794-
Assert.Equal (8, attrs.Count);
790+
Assert.Equal (2, attrs.Count);
795791

796792
var objectEntry = attrs.FirstOrDefault (a => a.jniName == "java/lang/Object");
797793
Assert.NotNull (objectEntry.jniName);
@@ -800,11 +796,6 @@ public void FullPipeline_Mixed2ArgAnd3Arg_BothSurviveRoundTrip ()
800796
var activityEntry = attrs.FirstOrDefault (a => a.jniName == "android/app/Activity");
801797
Assert.NotNull (activityEntry.jniName);
802798
Assert.Null (activityEntry.targetRef); // unconditional due to ForceUnconditionalEntries
803-
804-
// Spot-check that array entries also survive the round trip.
805-
Assert.Contains (attrs, a => a.jniName == "[Landroid/app/Activity;");
806-
Assert.Contains (attrs, a => a.jniName == "[[Landroid/app/Activity;");
807-
Assert.Contains (attrs, a => a.jniName == "[[[Landroid/app/Activity;");
808799
});
809800
}
810801

@@ -857,92 +848,17 @@ public void FullPipeline_McwBinding_Emits2ArgAttribute_WithWorkaround ()
857848

858849
public class ArrayEntries
859850
{
860-
[Fact]
861-
public void Build_SinglePeer_EmitsRanks1Through3 ()
862-
{
863-
var peer = MakeMcwPeer ("foo/Bar", "Foo.Bar", "App");
864-
var model = BuildModel (new [] { peer });
865-
866-
Assert.Equal ("[Lfoo/Bar;", model.Entries.Single (e => e.JniName == "[Lfoo/Bar;").JniName);
867-
Assert.Equal ("[[Lfoo/Bar;", model.Entries.Single (e => e.JniName == "[[Lfoo/Bar;").JniName);
868-
Assert.Equal ("[[[Lfoo/Bar;", model.Entries.Single (e => e.JniName == "[[[Lfoo/Bar;").JniName);
869-
}
851+
// EmitArrayEntries is currently a no-op (see ModelBuilder.EmitArrayEntries
852+
// remarks — blocked by an ILLink TypeMapHandler limitation around
853+
// Mono.Cecil.ArrayType). These tests pin that behavior so we know if it
854+
// changes back.
870855

871856
[Fact]
872-
public void Build_ArrayEntries_PointAtClosedManagedArrayTypes ()
857+
public void Build_DoesNotEmitArrayEntries_PendingILLinkFix ()
873858
{
874859
var peer = MakeMcwPeer ("foo/Bar", "Foo.Bar", "App");
875860
var model = BuildModel (new [] { peer });
876861

877-
var rank1 = model.Entries.Single (e => e.JniName == "[Lfoo/Bar;");
878-
Assert.Equal ("Foo.Bar[], App", rank1.ProxyTypeReference);
879-
Assert.Equal ("Foo.Bar[], App", rank1.TargetTypeReference);
880-
881-
var rank2 = model.Entries.Single (e => e.JniName == "[[Lfoo/Bar;");
882-
Assert.Equal ("Foo.Bar[][], App", rank2.ProxyTypeReference);
883-
Assert.Equal ("Foo.Bar[][], App", rank2.TargetTypeReference);
884-
885-
var rank3 = model.Entries.Single (e => e.JniName == "[[[Lfoo/Bar;");
886-
Assert.Equal ("Foo.Bar[][][], App", rank3.ProxyTypeReference);
887-
Assert.Equal ("Foo.Bar[][][], App", rank3.TargetTypeReference);
888-
}
889-
890-
[Fact]
891-
public void Build_ArrayEntries_AreConditional ()
892-
{
893-
// Array entries must be conditional (3-arg) so the trimmer can drop
894-
// entries whose target array type is not live in the shipped app.
895-
var peer = MakeMcwPeer ("foo/Bar", "Foo.Bar", "App");
896-
var model = BuildModel (new [] { peer });
897-
898-
foreach (var arrayEntry in model.Entries.Where (e => e.JniName.StartsWith ("[", StringComparison.Ordinal))) {
899-
Assert.False (arrayEntry.IsUnconditional, $"Array entry {arrayEntry.JniName} must be conditional (3-arg)");
900-
Assert.NotNull (arrayEntry.TargetTypeReference);
901-
}
902-
}
903-
904-
[Fact]
905-
public void Build_OpenGenericPeer_DoesNotEmitArrayEntries ()
906-
{
907-
// typeof(JavaList<>[]) is invalid, so open generics must be skipped.
908-
var openGeneric = MakeMcwPeer ("java/util/ArrayList", "Android.Runtime.JavaList`1", "Mono.Android")
909-
with { IsGenericDefinition = true };
910-
var model = BuildModel (new [] { openGeneric });
911-
912-
Assert.DoesNotContain (model.Entries, e => e.JniName.StartsWith ("[", StringComparison.Ordinal));
913-
}
914-
915-
[Fact]
916-
public void Build_AliasGroup_DoesNotEmitArrayEntries ()
917-
{
918-
// Alias groups (multiple peers sharing one JNI name) would produce duplicate
919-
// JNI array keys; they're skipped pending an alias-aware design.
920-
var peers = new List<JavaPeerInfo> {
921-
MakeMcwPeer ("test/Dup", "Test.First", "App"),
922-
MakeMcwPeer ("test/Dup", "Test.Second", "App"),
923-
};
924-
var model = BuildModel (peers);
925-
926-
Assert.DoesNotContain (model.Entries, e => e.JniName.StartsWith ("[", StringComparison.Ordinal));
927-
}
928-
929-
[Theory]
930-
[InlineData ("Z")]
931-
[InlineData ("B")]
932-
[InlineData ("C")]
933-
[InlineData ("S")]
934-
[InlineData ("I")]
935-
[InlineData ("J")]
936-
[InlineData ("F")]
937-
[InlineData ("D")]
938-
public void Build_PrimitiveJniKeyword_DoesNotEmitArrayEntries (string jniKeyword)
939-
{
940-
// Primitive JNI keyword keys (Z, B, C, S, I, J, F, D) are handled by the legacy
941-
// JniRuntime.JniTypeManager.GetPrimitiveArrayTypesForSimpleReference path. Emitting
942-
// array entries here would shadow that built-in handling.
943-
var peer = MakeMcwPeer (jniKeyword, "FakePrimitive.Wrapper", "App");
944-
var model = BuildModel (new [] { peer });
945-
946862
Assert.DoesNotContain (model.Entries, e => e.JniName.StartsWith ("[", StringComparison.Ordinal));
947863
}
948864
}

0 commit comments

Comments
 (0)