From f09a49b9cd67b6109094c9997c733ff981e3f637 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 1 Sep 2026 17:12:41 +0200 Subject: [PATCH 1/4] Rewrite trimmable typemap JNI metadata Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../JniRemapping/JniAssemblyRewriterTests.cs | 279 ++++++++++++++++- .../CustomAttributeStringRewriter.cs | 68 ++++- .../JniRemapping/JniAssemblyRewriter.cs | 18 +- .../JniRemapping/JniRewritePlanner.cs | 280 +++++++++++++++++- .../Utilities/MetadataExtensions.cs | 16 +- 5 files changed, 645 insertions(+), 16 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs index bbdd5f43d38..5fa72f55932 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs @@ -72,6 +72,24 @@ static string FirstAttributeStringArg (MetadataReader reader, CustomAttributeHan return args.Count > 0 ? args [0] : null; } + static IReadOnlyList AttributeStringArrayArg (MetadataReader reader, CustomAttributeHandleCollection attributes, EntityHandle ctor) + { + foreach (CustomAttributeHandle handle in attributes) { + CustomAttribute attribute = reader.GetCustomAttribute (handle); + if (attribute.Constructor != ctor) { + continue; + } + + var decoded = attribute.DecodeValue (Xamarin.Android.Tasks.DummyCustomAttributeProvider.Instance); + var result = new List (); + foreach (var element in (ImmutableArray>) decoded.FixedArguments [0].Value) { + result.Add ((string) element.Value); + } + return result; + } + return []; + } + static List> LoadedStrings (PEReader peReader, MetadataReader reader, MethodDefinitionHandle method) { var result = new List> (); @@ -94,10 +112,13 @@ static List> LoadedStrings (PEReader peReader, Metadat return result; } - static void AssertTableRowCountsMatch (MetadataReader expected, MetadataReader actual) + static void AssertTableRowCountsMatch (MetadataReader expected, MetadataReader actual, params TableIndex [] except) { for (int i = 0; i < MetadataTokens.TableCount; i++) { var table = (TableIndex) i; + if (Array.IndexOf (except, table) >= 0) { + continue; + } Assert.AreEqual (expected.GetTableRowCount (table), actual.GetTableRowCount (table), $"Row count of table '{table}' changed."); } } @@ -700,6 +721,194 @@ public void RewritesAttributesAndLoadedStrings () CollectionAssert.AreEqual (new [] { "b:()V:n_Run" }, ValuesOf (LoadedStrings (peReader, reader, run))); } + [Test] + public void RewritesTrimmableTypeMapKeysAndAliases () + { + var fixture = new JniFixtureBuilder (); + fixture.Metadata.AddCustomAttribute (EntityHandle.AssemblyDefinition, fixture.TypeMapCtor3, + fixture.AttributeBlob ("acme/orig/MyView[1]", "Acme.Proxy, Fixture", "Acme.Target, Fixture")); + + int fieldStart = fixture.NextFieldRid; + int methodStart = fixture.NextMethodRid; + TypeDefinitionHandle aliasHolder = fixture.AddType ("Acme", "AliasHolder", fieldStart, methodStart); + fixture.Metadata.AddCustomAttribute (aliasHolder, fixture.JavaPeerAliasesCtor1, + fixture.StringArrayAttributeBlob ("acme/orig/MyView[0]", "acme/orig/MyView[1]", "unmapped/Type[0]")); + + const string mappingText = "acme.orig.MyView -> a.b.C:\n"; + R8Mapping mapping = Mapping (mappingText); + JniRewriteResult result = Rewrite (fixture.Serialize (), mapping); + AssertReverseScanMatchesRewrite (result.Image, mapping, mappingText); + using var peReader = new PEReader (ImmutableArray.Create (result.Image)); + MetadataReader reader = peReader.GetMetadataReader (); + + CollectionAssert.AreEqual (new [] { "a/b/C[1]", "Acme.Proxy, Fixture", "Acme.Target, Fixture" }, + AttributeStringArgs (reader, reader.GetAssemblyDefinition ().GetCustomAttributes (), fixture.TypeMapCtor3)); + CollectionAssert.AreEqual (new [] { "a/b/C[0]", "a/b/C[1]", "unmapped/Type[0]" }, + AttributeStringArrayArg (reader, reader.GetTypeDefinition (aliasHolder).GetCustomAttributes (), fixture.JavaPeerAliasesCtor1)); + } + + [Test] + public void IdentifiesUtf8FieldRvaDataStructurally () + { + var fixture = new JniFixtureBuilder (); + FieldDefinitionHandle nameField = fixture.AddUtf8Field ("onClick"); + FieldDefinitionHandle signatureField = fixture.AddUtf8Field ("(Lacme/orig/Callback;)V"); + FieldDefinitionHandle embeddedNullField = fixture.AddUtf8Field ("onClick\0not-padding"); + + using var peReader = new PEReader (ImmutableArray.Create (fixture.Serialize ())); + MetadataReader reader = peReader.GetMetadataReader (); + FieldRvaTable table = FieldRvaTable.Read (peReader, reader); + + Assert.AreEqual (3, table.Entries.Count); + + FieldRvaEntry name = table.Get (nameField); + Assert.IsNotNull (name); + Assert.IsTrue (name.IsUtf8Datum, "A __utf8_N mapped field must be recognised structurally."); + Assert.AreEqual ("onClick", name.Utf8Value); + + FieldRvaEntry signature = table.Get (signatureField); + Assert.IsNotNull (signature); + Assert.IsTrue (signature.IsUtf8Datum); + Assert.AreEqual ("(Lacme/orig/Callback;)V", signature.Utf8Value); + + FieldRvaEntry embeddedNull = table.Get (embeddedNullField); + Assert.IsNotNull (embeddedNull); + Assert.IsFalse (embeddedNull.IsUtf8Datum, "Non-zero data after the first NUL is not rewrite padding."); + } + + [Test] + public void RewritesUtf8FieldRvaJniNamesAndSignatures () + { + var fixture = new JniFixtureBuilder (); + + FieldDefinitionHandle nameField = fixture.AddUtf8Field ("onClick"); + FieldDefinitionHandle signatureField = fixture.AddUtf8Field ("(Lacme/orig/Callback;)V"); + FieldDefinitionHandle classNameField = fixture.AddUtf8Field ("acme/orig/Callback"); + FieldDefinitionHandle longNameField = fixture.AddUtf8Field ("run"); + + int fieldStart = fixture.NextFieldRid; + int methodStart = fixture.NextMethodRid; + int ctorBody = fixture.EmitBody (encoder => { + encoder.OpCode (ILOpCode.Ldarg_0); + encoder.LoadString (fixture.String ("acme/orig/MyView")); + encoder.OpCode (ILOpCode.Pop); + encoder.OpCode (ILOpCode.Ret); + }); + fixture.AddVoidMethod (".ctor", ctorBody, + MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.SpecialName | MethodAttributes.RTSpecialName); + + int registerBody = fixture.EmitBody (encoder => { + encoder.OpCode (ILOpCode.Ldsflda); + encoder.Token (nameField); + encoder.OpCode (ILOpCode.Ldsflda); + encoder.Token (signatureField); + encoder.OpCode (ILOpCode.Pop); + encoder.OpCode (ILOpCode.Pop); + encoder.OpCode (ILOpCode.Ldsflda); + encoder.Token (longNameField); + encoder.OpCode (ILOpCode.Ldsflda); + encoder.Token (signatureField); + encoder.OpCode (ILOpCode.Pop); + encoder.OpCode (ILOpCode.Pop); + encoder.OpCode (ILOpCode.Ret); + }); + fixture.AddVoidMethod ("RegisterNatives", registerBody); + + // A JavaPeerProxy-derived type carries its JNI identity in its .ctor's only ldstr. + fixture.AddType ("Acme.Orig", "MyViewProxy", fieldStart, methodStart, + TypeAttributes.Public | TypeAttributes.Sealed | TypeAttributes.Class, fixture.JavaPeerProxyReference); + + byte [] source = fixture.Serialize (); + const string mappingText = + "acme.orig.MyView -> a.b.C:\n" + + " void onClick(acme.orig.Callback) -> a\n" + + " void run(acme.orig.Callback) -> aMuchLongerObfuscatedName\n" + + "acme.orig.Callback -> a.b.Cb:\n"; + R8Mapping mapping = Mapping (mappingText); + JniRewriteResult result = Rewrite (source, mapping); + using (var rewrittenReader = new PEReader (ImmutableArray.Create (result.Image))) { + MetadataReader rewrittenMetadata = rewrittenReader.GetMetadataReader (); + FieldRvaTable rewrittenFields = FieldRvaTable.Read (rewrittenReader, rewrittenMetadata); + Assert.IsTrue (rewrittenFields.Get (nameField)?.IsUtf8Datum, "Rewritten method-name FieldRVA data should remain structurally recognizable."); + Assert.IsTrue (rewrittenFields.Get (signatureField)?.IsUtf8Datum, "Rewritten signature FieldRVA data should remain structurally recognizable."); + } + AssertReverseScanMatchesRewrite (result.Image, mapping, mappingText); + + using var peReader = new PEReader (ImmutableArray.Create (result.Image)); + MetadataReader reader = peReader.GetMetadataReader (); + + Assert.AreEqual ("a", ReadUtf8Field (peReader, reader, nameField), "The method name is renamed using the owning proxy's JNI class."); + Assert.AreEqual ("(La/b/Cb;)V", ReadUtf8Field (peReader, reader, signatureField)); + Assert.AreEqual ("a/b/Cb", ReadUtf8Field (peReader, reader, classNameField), "An unreferenced datum that is a known class name is still renamed."); + Assert.AreEqual ("aMuchLongerObfuscatedName", ReadUtf8Field (peReader, reader, longNameField), "A longer datum is relocated into a wider __utf8_N slot."); + + // Growing a datum appends exactly one new sized type; no existing token moves. + using var sourceReader = new PEReader (ImmutableArray.Create (source)); + MetadataReader before = sourceReader.GetMetadataReader (); + Assert.AreEqual (before.GetTableRowCount (TableIndex.TypeDef) + 1, reader.GetTableRowCount (TableIndex.TypeDef)); + AssertTableRowCountsMatch (before, reader, TableIndex.TypeDef, TableIndex.ClassLayout, TableIndex.NestedClass); + } + + static string ReadUtf8Field (PEReader peReader, MetadataReader reader, FieldDefinitionHandle field) + { + FieldDefinition definition = reader.GetFieldDefinition (field); + int rva = definition.GetRelativeVirtualAddress (); + Assert.AreNotEqual (0, rva, "Field has no RVA."); + + PEMemoryBlock block = peReader.GetSectionData (rva); + var bytes = new List (); + BlobReader blob = block.GetReader (0, Math.Min (block.Length, 256)); + for (byte b = blob.ReadByte (); b != 0; b = blob.ReadByte ()) { + bytes.Add (b); + } + return System.Text.Encoding.UTF8.GetString (bytes.ToArray ()); + } + + [Test] + public void FailsWhenASharedUtf8DatumNeedsTwoDifferentNames () + { + var fixture = new JniFixtureBuilder (); + + FieldDefinitionHandle shared = fixture.AddUtf8Field ("go"); + FieldDefinitionHandle signature = fixture.AddUtf8Field ("()V"); + + AddProxy (fixture, "acme/orig/P1", shared, signature); + AddProxy (fixture, "acme/orig/P2", shared, signature); + + var exception = Assert.Throws (() => Rewrite (fixture.Serialize (), Mapping ( + "acme.orig.P1 -> a.b.P1:\n" + + " void go() -> z\n" + + "acme.orig.P2 -> a.b.P2:\n" + + " void go() -> q\n"))); + StringAssert.Contains ("shared", exception.Message.ToLowerInvariant ()); + } + + static void AddProxy (JniFixtureBuilder fixture, string jniName, FieldDefinitionHandle nameField, FieldDefinitionHandle signatureField) + { + int fieldStart = fixture.NextFieldRid; + int methodStart = fixture.NextMethodRid; + + fixture.AddVoidMethod (".ctor", fixture.EmitBody (encoder => { + encoder.OpCode (ILOpCode.Ldarg_0); + encoder.LoadString (fixture.String (jniName)); + encoder.OpCode (ILOpCode.Pop); + encoder.OpCode (ILOpCode.Ret); + }), MethodAttributes.Public | MethodAttributes.HideBySig | MethodAttributes.SpecialName | MethodAttributes.RTSpecialName); + + fixture.AddVoidMethod ("RegisterNatives", fixture.EmitBody (encoder => { + encoder.OpCode (ILOpCode.Ldsflda); + encoder.Token (nameField); + encoder.OpCode (ILOpCode.Ldsflda); + encoder.Token (signatureField); + encoder.OpCode (ILOpCode.Pop); + encoder.OpCode (ILOpCode.Pop); + encoder.OpCode (ILOpCode.Ret); + })); + + fixture.AddType ("Acme.Orig", jniName.Replace ('/', '_'), fieldStart, methodStart, + TypeAttributes.Public | TypeAttributes.Sealed | TypeAttributes.Class, fixture.JavaPeerProxyReference); + } + [Test] public void SharedLoadedStringGetsOwnerSpecificReplacements () { @@ -1080,6 +1289,74 @@ public void RewrittenAssemblyStillMatchesItsPortablePdb () } } + [Test] + public void PreservesMappedFieldDataThatIsNotAJniDatum () + { + var fixture = new JniFixtureBuilder (); + + // A plain C#-style array initializer blob: not a __utf8_N datum, so it must survive + // byte-for-byte. + var payload = new byte [] { 0xDE, 0xAD, 0xBE, 0xEF, 0x01, 0x02, 0x03, 0x04 }; + TypeDefinitionHandle enclosing = fixture.EnsurePrivateImplementationDetails (); + int fieldStart = fixture.NextFieldRid; + int methodStart = fixture.NextMethodRid; + TypeDefinitionHandle arrayType = fixture.AddType (null, "__StaticArrayInitTypeSize=8", fieldStart, methodStart, + TypeAttributes.NestedPrivate | TypeAttributes.ExplicitLayout | TypeAttributes.Sealed | TypeAttributes.AnsiClass, + fixture.ValueTypeReference); + fixture.Metadata.AddTypeLayout (arrayType, packingSize: 1, size: (uint) payload.Length); + fixture.Metadata.AddNestedType (arrayType, enclosing); + + var signature = new BlobBuilder (); + new BlobEncoder (signature).FieldSignature ().Type (arrayType, isValueType: true); + int rva = fixture.MappedFieldData.Count; + fixture.MappedFieldData.WriteBytes (payload); + FieldDefinitionHandle dataField = fixture.Metadata.AddFieldDefinition ( + FieldAttributes.Static | FieldAttributes.Assembly | FieldAttributes.HasFieldRVA, + fixture.Metadata.GetOrAddString ("ArrayData"), fixture.Metadata.GetOrAddBlob (signature)); + fixture.Metadata.AddFieldRelativeVirtualAddress (dataField, rva); + + JniRewriteResult result = Rewrite (fixture.Serialize (), Mapping ("acme.orig.Nothing -> a.b.N:\n")); + + using var peReader = new PEReader (ImmutableArray.Create (result.Image)); + MetadataReader reader = peReader.GetMetadataReader (); + int newRva = reader.GetFieldDefinition (dataField).GetRelativeVirtualAddress (); + Assert.AreNotEqual (0, newRva); + CollectionAssert.AreEqual (payload, peReader.GetSectionData (newRva).GetReader (0, payload.Length).ReadBytes (payload.Length)); + } + + [Test] + public void RejectsFieldRvaValueTypeWithoutAnExplicitClassLayoutSize () + { + // A mapped value type with no ClassLayout row (or a zero size) cannot be sized safely: + // summing its instance fields would be a guess about the CLR's actual layout, and a + // wrong guess risks truncating - or reading past the end of - the mapped data. The + // rewriter must refuse rather than take that risk. + var fixture = new JniFixtureBuilder (); + + TypeDefinitionHandle enclosing = fixture.EnsurePrivateImplementationDetails (); + int fieldStart = fixture.NextFieldRid; + int methodStart = fixture.NextMethodRid; + TypeDefinitionHandle unsizedType = fixture.AddType (null, "__UnsizedBlob", fieldStart, methodStart, + TypeAttributes.NestedPrivate | TypeAttributes.ExplicitLayout | TypeAttributes.Sealed | TypeAttributes.AnsiClass, + fixture.ValueTypeReference); + fixture.Metadata.AddNestedType (unsizedType, enclosing); + // Deliberately no fixture.Metadata.AddTypeLayout (...) call: the type has no + // ClassLayout row at all. + + var signature = new BlobBuilder (); + new BlobEncoder (signature).FieldSignature ().Type (unsizedType, isValueType: true); + int rva = fixture.MappedFieldData.Count; + fixture.MappedFieldData.WriteBytes (new byte [] { 0x01, 0x02, 0x03, 0x04 }); + FieldDefinitionHandle dataField = fixture.Metadata.AddFieldDefinition ( + FieldAttributes.Static | FieldAttributes.Assembly | FieldAttributes.HasFieldRVA, + fixture.Metadata.GetOrAddString ("UnsizedData"), fixture.Metadata.GetOrAddBlob (signature)); + fixture.Metadata.AddFieldRelativeVirtualAddress (dataField, rva); + + byte [] source = fixture.Serialize (); + var ex = Assert.Throws (() => Rewrite (source, Mapping ("acme.orig.Nothing -> a.b.N:\n"))); + StringAssert.Contains ("ClassLayout", ex.Message); + } + [Test] public void RewrittenAssemblyLoadsAndRunsInTheRuntime () { diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/CustomAttributeStringRewriter.cs b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/CustomAttributeStringRewriter.cs index bccc81e89c7..48bd8706ed3 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/CustomAttributeStringRewriter.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/CustomAttributeStringRewriter.cs @@ -11,8 +11,10 @@ namespace Xamarin.Android.Tasks.JniRemapping /// (ECMA-335 II.23.3), leaving the prolog, any other fixed arguments, and every named /// argument byte-for-byte untouched. /// - /// This only supports (and only needs to support) the attributes this task rewrites: - /// Android.Runtime.RegisterAttribute and the Java.Interop.Jni*SignatureAttribute family. + /// This only supports (and only needs to support) the attributes this task rewrites - + /// Android.Runtime.RegisterAttribute, Java.Interop.JniTypeSignatureAttribute, + /// Java.Interop.JniMethodSignatureAttribute, Java.Interop.JniConstructorSignatureAttribute, + /// System.Runtime.InteropServices.TypeMapAttribute, and Java.Interop.JavaPeerAliasesAttribute. /// static class CustomAttributeStringRewriter { @@ -36,7 +38,7 @@ static class CustomAttributeStringRewriter } using var ms = new MemoryStream (originalContent.Length); - ms.Write (originalContent, 0, 2); + ms.Write (originalContent, 0, 2); // Prolog (0x0001), verbatim. int pos = 2; bool changed = false; @@ -48,6 +50,7 @@ static class CustomAttributeStringRewriter int argStart = pos; string? value; if (originalContent [pos] == 0xFF) { + // A "null string" SerString is encoded as a single 0xFF byte (ECMA-335 II.23.3). value = null; pos += 1; } else { @@ -71,6 +74,65 @@ static class CustomAttributeStringRewriter } } + // Remaining fixed arguments and NumNamed/NamedArg tail, copied verbatim. + ms.Write (originalContent, pos, originalContent.Length - pos); + + return changed ? ms.ToArray () : null; + } + + /// + /// Rewrites the elements of the first fixed argument when it is a string[]. + /// All bytes following the array are copied verbatim. + /// + public static byte []? TryRewriteStringArray (byte [] originalContent, Func rewriteElement) + { + if (originalContent.Length < 6) { + throw new JniRewriteException ("Malformed custom attribute value blob: missing string array length."); + } + + using var ms = new MemoryStream (originalContent.Length); + ms.Write (originalContent, 0, 6); // Prolog (uint16) and array length (int32), verbatim. + int count = BitConverter.ToInt32 (originalContent, 2); + if (count < -1) { + throw new JniRewriteException ($"Malformed custom attribute value blob: invalid string array length {count}."); + } + if (count == -1) { + return null; + } + + int pos = 6; + bool changed = false; + for (int i = 0; i < count; i++) { + if (pos >= originalContent.Length) { + throw new JniRewriteException ("Malformed custom attribute value blob: ran out of bytes while reading string array."); + } + + int elementStart = pos; + if (originalContent [pos] == 0xFF) { + pos++; + ms.WriteByte (0xFF); + continue; + } + + int prefixWidth = MetadataEncoding.ReadCompressedInteger (originalContent, pos, out int strByteLength); + pos += prefixWidth + strByteLength; + if (pos > originalContent.Length) { + throw new JniRewriteException ("Malformed custom attribute value blob: string array element extends past the end of the blob."); + } + + string value = Encoding.UTF8.GetString (originalContent, elementStart + prefixWidth, strByteLength); + string? newValue = rewriteElement (value); + if (newValue != null && !string.Equals (newValue, value, StringComparison.Ordinal)) { + changed = true; + byte [] utf8 = Encoding.UTF8.GetBytes (newValue); + byte [] prefix = MetadataEncoding.EncodeCompressedInteger (utf8.Length); + ms.Write (prefix, 0, prefix.Length); + ms.Write (utf8, 0, utf8.Length); + } else { + ms.Write (originalContent, elementStart, pos - elementStart); + } + } + ms.Write (originalContent, pos, originalContent.Length - pos); return changed ? ms.ToArray () : null; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniAssemblyRewriter.cs b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniAssemblyRewriter.cs index 0253b5aed5e..2cdef5eae57 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniAssemblyRewriter.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniAssemblyRewriter.cs @@ -22,9 +22,10 @@ public JniRewriteResult (byte [] image, int replacementCount, bool strongNameSig } /// - /// Rewrites JNI names embedded in Android.Runtime.RegisterAttribute, - /// the Java.Interop.Jni*SignatureAttribute family, and generated - /// JniPeerMembers/RegisterNatives ldstr strings according to an R8 mapping. + /// Rewrites the JNI names embedded in an assembly - Android.Runtime.RegisterAttribute, + /// the Java.Interop.Jni*SignatureAttribute family, the JniPeerMembers/RegisterNatives + /// ldstr strings, and the generated null-terminated UTF-8 JNI data stored in + /// FieldRVA - according to an R8 mapping. /// /// The rewrite runs in two passes. The first scans the source into an exact plan; the second /// reconstructs the whole assembly with MetadataBuilder, cloning every table row in its @@ -42,13 +43,15 @@ public static JniRewriteResult Rewrite (byte [] sourceImage, R8Mapping mapping, } MetadataReader reader = peReader.GetMetadataReader (); - JniRewritePlan plan = new JniRewritePlanner (peReader, reader, mapping, log).CreatePlan (); + FieldRvaTable fieldRvaTable = FieldRvaTable.Read (peReader, reader); + + JniRewritePlan plan = new JniRewritePlanner (peReader, reader, mapping, fieldRvaTable, log).CreatePlan (); if (plan.ReplacementCount == 0) { return new JniRewriteResult (sourceImage, 0, strongNameSignatureCleared: false); } - FieldRvaTable fieldRvaTable = FieldRvaTable.Read (peReader, reader); AssemblyRebuildResult rebuilt = new AssemblyRebuilder (peReader, reader, plan, fieldRvaTable).Build (); + return new JniRewriteResult (rebuilt.Image, plan.ReplacementCount, rebuilt.StrongNameSignatureCleared); } @@ -64,6 +67,9 @@ public static void ScanRewrittenAssembly (byte [] sourceImage, R8Mapping mapping } public static void ScanRewrittenAssembly (PEReader peReader, MetadataReader reader, R8Mapping mapping, TaskLoggingHelper log) - => new JniRewritePlanner (peReader, reader, mapping.CreateReverseMapping (), log).CreatePlan (); + { + FieldRvaTable fieldRvaTable = FieldRvaTable.Read (peReader, reader); + new JniRewritePlanner (peReader, reader, mapping.CreateReverseMapping (), fieldRvaTable, log).CreatePlan (); + } } } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlanner.cs b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlanner.cs index 45f1b0371ea..2df3f778fc0 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlanner.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlanner.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; +using System.Reflection; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; using System.Reflection.PortableExecutable; @@ -22,23 +23,52 @@ sealed class JniRewritePlanner const string JniMethodSignatureAttributeFullName = "Java.Interop.JniMethodSignatureAttribute"; const string JniConstructorSignatureAttributeFullName = "Java.Interop.JniConstructorSignatureAttribute"; const string JniEnvironmentFullName = "Android.Runtime.JNIEnv"; + const string JavaPeerAliasesAttributeFullName = "Java.Interop.JavaPeerAliasesAttribute"; + const string TypeMapAttributeFullName = "System.Runtime.InteropServices.TypeMapAttribute`1"; + + const string JavaPeerProxyNamespace = "Java.Interop"; + const string JavaPeerProxyName = "JavaPeerProxy"; + + enum Utf8Role + { + Unknown, + MethodName, + MethodSignature, + } + + readonly struct Utf8Use + { + public Utf8Role Role { get; } + public string? OwnerJniName { get; } + public string? PairedSignature { get; } + + public Utf8Use (Utf8Role role, string? ownerJniName, string? pairedSignature) + { + Role = role; + OwnerJniName = ownerJniName; + PairedSignature = pairedSignature; + } + } readonly PEReader peReader; readonly MetadataReader reader; readonly IJniNameMapping mapping; readonly R8Mapping? forwardMapping; + readonly FieldRvaTable fieldRvaTable; readonly TaskLoggingHelper log; readonly Func renameClass; readonly Dictionary ownerJniNameCache = new (); readonly Dictionary> staticJniClassAssignments = new (StringComparer.Ordinal); readonly HashSet warnedUnsafeLookupSources = new (StringComparer.Ordinal); + readonly Dictionary> utf8Uses = new (); - public JniRewritePlanner (PEReader peReader, MetadataReader reader, IJniNameMapping mapping, TaskLoggingHelper log) + public JniRewritePlanner (PEReader peReader, MetadataReader reader, IJniNameMapping mapping, FieldRvaTable fieldRvaTable, TaskLoggingHelper log) { this.peReader = peReader; this.reader = reader; this.mapping = mapping; forwardMapping = mapping as R8Mapping; + this.fieldRvaTable = fieldRvaTable; this.log = log; renameClass = className => mapping.TryMapClass (className, out string renamed) ? renamed : null; } @@ -47,9 +77,13 @@ public JniRewritePlan CreatePlan () { IndexStaticJniClassAssignments (); var plan = new JniRewritePlan (); + + PlanAssemblyAttributes (plan); foreach (TypeDefinitionHandle typeHandle in reader.TypeDefinitions) { PlanType (plan, typeHandle); } + + PlanUtf8FieldData (plan); return plan; } @@ -58,6 +92,7 @@ void PlanType (JniRewritePlan plan, TypeDefinitionHandle typeHandle) TypeDefinition typeDef = reader.GetTypeDefinition (typeHandle); string? ownerJniName = ResolveOwnerJniName (typeHandle); + PlanJavaPeerAliasesAttributes (plan, typeDef.GetCustomAttributes ()); PlanTypeLevelAttributes (plan, typeDef, ownerJniName); foreach (MethodDefinitionHandle methodHandle in typeDef.GetMethods ()) { @@ -78,9 +113,68 @@ void PlanType (JniRewritePlan plan, TypeDefinitionHandle typeHandle) } } + void PlanAssemblyAttributes (JniRewritePlan plan) + { + foreach (CustomAttributeHandle caHandle in reader.GetAssemblyDefinition ().GetCustomAttributes ()) { + CustomAttribute ca = reader.GetCustomAttribute (caHandle); + if (reader.GetCustomAttributeFullName (ca, log) != TypeMapAttributeFullName) { + continue; + } + + // TypeMapAttribute's first argument is the JNI map key. Its following + // System.Type arguments are also SerStrings, but must remain unchanged. + PlanCustomAttributeRewrite (plan, caHandle, ca, fixedArgCount: 1, + (i, value) => value != null ? TryRewriteTypeMapKey (value) : null); + } + } + + void PlanJavaPeerAliasesAttributes (JniRewritePlan plan, CustomAttributeHandleCollection attributes) + { + foreach (CustomAttributeHandle caHandle in attributes) { + CustomAttribute ca = reader.GetCustomAttribute (caHandle); + if (reader.GetCustomAttributeFullName (ca, log) != JavaPeerAliasesAttributeFullName) { + continue; + } + + BlobReader blobReader = reader.GetBlobReader (ca.Value); + byte [] originalContent = blobReader.ReadBytes (blobReader.Length); + byte []? newContent = CustomAttributeStringRewriter.TryRewriteStringArray (originalContent, TryRewriteTypeMapKey); + if (newContent != null) { + plan.AddCustomAttributeBlob (caHandle, newContent); + } + } + } + + string? TryRewriteTypeMapKey (string value) + { + int suffixStart = value.LastIndexOf ('['); + string suffix = ""; + string jniName = value; + if (suffixStart > 0 && value [value.Length - 1] == ']' && IsDecimalIndex (value, suffixStart + 1, value.Length - 1)) { + suffix = value.Substring (suffixStart); + jniName = value.Substring (0, suffixStart); + } + + return mapping.TryMapClass (jniName, out string renamed) ? renamed + suffix : null; + } + + static bool IsDecimalIndex (string value, int start, int end) + { + if (start == end) { + return false; + } + for (int i = start; i < end; i++) { + if (value [i] < '0' || value [i] > '9') { + return false; + } + } + return true; + } + /// - /// Resolves the JNI class name that owns a type from its own Register/JniTypeSignature - /// argument or, recursively, its enclosing type. + /// Resolves the JNI class name that "owns" a type: its own Register/JniTypeSignature + /// argument, the JNI name a generated JavaPeerProxy passes to its base constructor, + /// or (recursively) its enclosing type's. /// string? ResolveOwnerJniName (TypeDefinitionHandle typeHandle) { @@ -88,10 +182,11 @@ void PlanType (JniRewritePlan plan, TypeDefinitionHandle typeHandle) return cached; } + // Guard against a pathological/cyclical nesting chain while resolving. ownerJniNameCache [typeHandle] = null; TypeDefinition typeDef = reader.GetTypeDefinition (typeHandle); - string? result = TryGetTypeLevelJniName (typeDef); + string? result = TryGetTypeLevelJniName (typeDef) ?? TryGetJavaPeerProxyJniName (typeDef); if (result == null) { TypeDefinitionHandle declaring = typeDef.GetDeclaringType (); if (!declaring.IsNil) { @@ -120,6 +215,67 @@ void PlanType (JniRewritePlan plan, TypeDefinitionHandle typeHandle) return null; } + /// + /// The trimmable typemap generator emits one JavaPeerProxy subclass per Java peer + /// whose parameterless constructor passes the peer's JNI name to the base constructor as + /// its only ldstr. That is the type's JNI identity. + /// + string? TryGetJavaPeerProxyJniName (TypeDefinition typeDef) + { + if (!IsJavaPeerProxy (typeDef.BaseType)) { + return null; + } + + foreach (MethodDefinitionHandle methodHandle in typeDef.GetMethods ()) { + MethodDefinition method = reader.GetMethodDefinition (methodHandle); + if ((method.Attributes & MethodAttributes.RTSpecialName) == 0 || reader.GetString (method.Name) != ".ctor") { + continue; + } + if (method.RelativeVirtualAddress == 0) { + continue; + } + + string? found = null; + bool ambiguous = false; + byte [] il = GetILBytes (method); + IlInstructionScanner.Walk (il, (code, _, operandOffset, _) => { + if (code != (ushort) ILOpCode.Ldstr) { + return; + } + string value = ReadUserString (il, operandOffset); + if (found != null && found != value) { + ambiguous = true; + } + found ??= value; + }); + + if (!ambiguous && found != null && found.Length > 0) { + return found; + } + } + + return null; + } + + bool IsJavaPeerProxy (EntityHandle baseType) + { + if (baseType.IsNil) { + return false; + } + + if (baseType.Kind == HandleKind.TypeReference) { + TypeReference typeRef = reader.GetTypeReference ((TypeReferenceHandle) baseType); + return reader.GetString (typeRef.Name) == JavaPeerProxyName && reader.GetString (typeRef.Namespace) == JavaPeerProxyNamespace; + } + + if (baseType.Kind == HandleKind.TypeDefinition) { + TypeDefinition typeDef = reader.GetTypeDefinition ((TypeDefinitionHandle) baseType); + return reader.GetString (typeDef.Name) == JavaPeerProxyName && reader.GetString (typeDef.Namespace) == JavaPeerProxyNamespace; + } + + return false; + } + void PlanTypeLevelAttributes (JniRewritePlan plan, TypeDefinition typeDef, string? ownerJniName) { if (ownerJniName == null || !mapping.TryMapClass (ownerJniName, out string renamedClass)) { @@ -265,6 +421,7 @@ void PlanMethodBody (JniRewritePlan plan, MethodDefinitionHandle methodHandle, s IlInstructionScanner.Walk (il, (code, instructionOffset, operandOffset, operandSize) => instructions.Add (new IlInstruction (code, instructionOffset, operandOffset, operandSize))); HashSet controlFlowEntries = GetControlFlowEntryOffsets (body, il, instructions); + FieldDefinitionHandle pendingUtf8Name = default; for (int i = 0; i < instructions.Count; i++) { IlInstruction instruction = instructions [i]; @@ -277,6 +434,38 @@ void PlanMethodBody (JniRewritePlan plan, MethodDefinitionHandle methodHandle, s } else if (TryGetJniLookupKind (il, instruction, out bool isField)) { PlanLegacyJniLookup (plan, methodHandle, il, instructions, controlFlowEntries, i, isField); } + + if (instruction.Code != (ushort) ILOpCode.Ldsflda && instruction.Code != (ushort) ILOpCode.Ldsfld) { + pendingUtf8Name = default; + continue; + } + + FieldDefinitionHandle field = TryGetUtf8Field (il, instruction.OperandOffset); + if (field.IsNil) { + pendingUtf8Name = default; + continue; + } + + // The typemap generator emits `ldsflda ; ldsflda ` pairs when + // filling in a JniNativeMethod for RegisterNatives; that adjacency is what makes + // an otherwise ambiguous bare method name resolvable against the owning class. + if (pendingUtf8Name.IsNil) { + pendingUtf8Name = field; + continue; + } + + string? signature = GetUtf8Value (field); + if (signature != null && JniDescriptorText.IsValidMethodDescriptor (signature)) { + RecordUtf8Use (pendingUtf8Name, new Utf8Use (Utf8Role.MethodName, ownerJniName, signature)); + RecordUtf8Use (field, new Utf8Use (Utf8Role.MethodSignature, ownerJniName, null)); + } else { + RecordUtf8Use (pendingUtf8Name, new Utf8Use (Utf8Role.Unknown, null, null)); + RecordUtf8Use (field, new Utf8Use (Utf8Role.Unknown, null, null)); + } + pendingUtf8Name = default; + } + if (!pendingUtf8Name.IsNil) { + RecordUtf8Use (pendingUtf8Name, new Utf8Use (Utf8Role.Unknown, null, null)); } } @@ -768,6 +957,89 @@ static bool IsBareMemberName (string value) return true; } + void RecordUtf8Use (FieldDefinitionHandle field, Utf8Use use) + { + if (!utf8Uses.TryGetValue (field, out var uses)) { + utf8Uses [field] = uses = new List (); + } + uses.Add (use); + } + + FieldDefinitionHandle TryGetUtf8Field (byte [] il, int operandOffset) + { + uint token = IlInstructionScanner.ReadUInt32 (il, operandOffset); + if ((token & 0xFF000000) != 0x04000000) { + return default; // Not a FieldDefinition token. + } + + var handle = MetadataTokens.FieldDefinitionHandle ((int) (token & 0x00FFFFFF)); + FieldRvaEntry? entry = fieldRvaTable.Get (handle); + return entry != null && entry.IsUtf8Datum ? handle : default; + } + + string? GetUtf8Value (FieldDefinitionHandle field) => fieldRvaTable.Get (field)?.Utf8Value; + + void PlanUtf8FieldData (JniRewritePlan plan) + { + foreach (FieldRvaEntry entry in fieldRvaTable.Entries) { + string? value = entry.Utf8Value; + if (value == null) { + continue; + } + + string? resolved = null; + foreach (Utf8Use use in GetUses (entry.Field)) { + string? candidate = ComputeNewUtf8Value (value, use); + if (candidate == null) { + continue; + } + if (resolved != null && resolved != candidate) { + throw new JniRewriteException ( + $"The mapped UTF-8 JNI datum '{value}' is shared by more than one Java class, but the mapping renames it to both " + + $"'{resolved}' and '{candidate}'. Splitting a shared '{FieldRvaTable.Utf8FieldNamePrefix}' field would move metadata tokens, which this rewriter does not do."); + } + resolved ??= candidate; + } + + if (resolved != null && resolved != value) { + plan.AddUtf8FieldValue (entry.Field, resolved); + } + } + } + + IEnumerable GetUses (FieldDefinitionHandle field) + { + if (utf8Uses.TryGetValue (field, out var uses)) { + return uses; + } + return new [] { new Utf8Use (Utf8Role.Unknown, null, null) }; + } + + string? ComputeNewUtf8Value (string value, Utf8Use use) + { + if (use.Role == Utf8Role.MethodName && use.OwnerJniName != null && use.PairedSignature != null) { + JniDescriptorText.MethodDescriptorToJavaTypes (use.PairedSignature, out var javaParams, out string javaReturnType); + string mappingName = R8Mapping.JniMemberNameToMappingName (value); + return mapping.TryMapMethod (use.OwnerJniName, mappingName, javaParams, javaReturnType, out string renamed) ? renamed : null; + } + + if (JniDescriptorText.IsValidMethodDescriptor (value) || JniDescriptorText.IsValidFieldDescriptor (value)) { + return JniDescriptorText.TryRewriteDescriptor (value, renameClass, out string rewritten) ? rewritten : null; + } + + if (use.Role == Utf8Role.Unknown && mapping.TryMapClass (value, out string renamedClass)) { + return renamedClass; + } + + return null; + } + + byte [] GetILBytes (MethodDefinition method) + { + MethodBodyBlock body = peReader.GetMethodBody (method.RelativeVirtualAddress); + return body.GetILBytes () ?? []; + } + string ReadUserString (byte [] il, int operandOffset) { uint token = IlInstructionScanner.ReadUInt32 (il, operandOffset); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MetadataExtensions.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MetadataExtensions.cs index 75427f3aa2a..cdea0e8e457 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MetadataExtensions.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MetadataExtensions.cs @@ -24,9 +24,21 @@ public static class MetadataExtensions var type = reader.GetTypeSpecification ((TypeSpecificationHandle)ctor.Parent); BlobReader blobReader = reader.GetBlobReader (type.Signature); SignatureTypeCode typeCode = blobReader.ReadSignatureTypeCode (); + if (typeCode != SignatureTypeCode.GenericTypeInstance) { + log.LogDebugMessage ($"Unsupported TypeSpecification signature: {typeCode}"); + return null; + } + blobReader.ReadByte (); // SignatureTypeKind.Class or SignatureTypeKind.ValueType. EntityHandle typeHandle = blobReader.ReadTypeHandle (); - TypeReference typeRef = reader.GetTypeReference ((TypeReferenceHandle)typeHandle); - return reader.GetString (typeRef.Namespace) + "." + reader.GetString (typeRef.Name); + if (typeHandle.Kind == HandleKind.TypeReference) { + TypeReference typeRef = reader.GetTypeReference ((TypeReferenceHandle)typeHandle); + return reader.GetString (typeRef.Namespace) + "." + reader.GetString (typeRef.Name); + } else if (typeHandle.Kind == HandleKind.TypeDefinition) { + TypeDefinition typeDef = reader.GetTypeDefinition ((TypeDefinitionHandle)typeHandle); + return reader.GetString (typeDef.Namespace) + "." + reader.GetString (typeDef.Name); + } + log.LogDebugMessage ($"Unsupported generic type handle kind: {typeHandle.Kind}"); + return null; } else { log.LogDebugMessage ($"Unsupported EntityHandle.Kind: {ctor.Parent.Kind}"); return null; From ec26d7084b69a651cf7ace5d428337910f783ef5 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 1 Sep 2026 21:09:27 +0200 Subject: [PATCH 2/4] Handle shared FieldRVA names conservatively Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../JniRemapping/JniAssemblyRewriterTests.cs | 76 ++++++++++++++++++- .../JniRemapping/JniRewritePlanner.cs | 22 +++--- 2 files changed, 86 insertions(+), 12 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs index 5fa72f55932..9a22476a216 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs @@ -783,7 +783,6 @@ public void RewritesUtf8FieldRvaJniNamesAndSignatures () FieldDefinitionHandle nameField = fixture.AddUtf8Field ("onClick"); FieldDefinitionHandle signatureField = fixture.AddUtf8Field ("(Lacme/orig/Callback;)V"); - FieldDefinitionHandle classNameField = fixture.AddUtf8Field ("acme/orig/Callback"); FieldDefinitionHandle longNameField = fixture.AddUtf8Field ("run"); int fieldStart = fixture.NextFieldRid; @@ -839,7 +838,6 @@ public void RewritesUtf8FieldRvaJniNamesAndSignatures () Assert.AreEqual ("a", ReadUtf8Field (peReader, reader, nameField), "The method name is renamed using the owning proxy's JNI class."); Assert.AreEqual ("(La/b/Cb;)V", ReadUtf8Field (peReader, reader, signatureField)); - Assert.AreEqual ("a/b/Cb", ReadUtf8Field (peReader, reader, classNameField), "An unreferenced datum that is a known class name is still renamed."); Assert.AreEqual ("aMuchLongerObfuscatedName", ReadUtf8Field (peReader, reader, longNameField), "A longer datum is relocated into a wider __utf8_N slot."); // Growing a datum appends exactly one new sized type; no existing token moves. @@ -883,6 +881,62 @@ public void FailsWhenASharedUtf8DatumNeedsTwoDifferentNames () StringAssert.Contains ("shared", exception.Message.ToLowerInvariant ()); } + [Test] + public void FailsWhenASharedUtf8DatumMustRemainUnmappedForOneProxy () + { + var fixture = new JniFixtureBuilder (); + + FieldDefinitionHandle shared = fixture.AddUtf8Field ("go"); + FieldDefinitionHandle signature = fixture.AddUtf8Field ("()V"); + + AddProxy (fixture, "acme/orig/P1", shared, signature); + AddProxy (fixture, "acme/orig/P2", shared, signature); + + var exception = Assert.Throws (() => Rewrite (fixture.Serialize (), Mapping ( + "acme.orig.P1 -> a.b.P1:\n" + + " void go() -> z\n" + + "acme.orig.P2 -> a.b.P2:\n"))); + StringAssert.Contains ("shared", exception.Message.ToLowerInvariant ()); + StringAssert.Contains ("original value", exception.Message); + StringAssert.Contains ("'go'", exception.Message); + StringAssert.Contains ("'z'", exception.Message); + } + + [Test] + public void FailsWhenASharedUtf8DatumHasAnUnresolvedOwner () + { + var fixture = new JniFixtureBuilder (); + + FieldDefinitionHandle shared = fixture.AddUtf8Field ("go"); + FieldDefinitionHandle signature = fixture.AddUtf8Field ("()V"); + + AddProxy (fixture, "acme/orig/P1", shared, signature); + AddRegistrationTypeWithoutJniOwner (fixture, shared, signature); + + var exception = Assert.Throws (() => Rewrite (fixture.Serialize (), Mapping ( + "acme.orig.P1 -> a.b.P1:\n" + + " void go() -> z\n"))); + StringAssert.Contains ("shared", exception.Message.ToLowerInvariant ()); + StringAssert.Contains ("original value", exception.Message); + } + + [Test] + public void PreservesUnreferencedUtf8DatumThatMatchesAMappedClass () + { + var fixture = new JniFixtureBuilder (); + FieldDefinitionHandle field = fixture.AddUtf8Field ("acme/orig/Callback"); + byte [] image = fixture.Serialize (); + R8Mapping mapping = Mapping ("acme.orig.Callback -> a.b.Cb:\n"); + + JniRewriteResult result = Rewrite (image, mapping); + + Assert.AreSame (image, result.Image); + Assert.AreEqual (0, result.ReplacementCount); + CollectionAssert.IsEmpty (mapping.AccessedEntries); + using var peReader = new PEReader (ImmutableArray.Create (result.Image)); + Assert.AreEqual ("acme/orig/Callback", ReadUtf8Field (peReader, peReader.GetMetadataReader (), field)); + } + static void AddProxy (JniFixtureBuilder fixture, string jniName, FieldDefinitionHandle nameField, FieldDefinitionHandle signatureField) { int fieldStart = fixture.NextFieldRid; @@ -909,6 +963,24 @@ static void AddProxy (JniFixtureBuilder fixture, string jniName, FieldDefinition TypeAttributes.Public | TypeAttributes.Sealed | TypeAttributes.Class, fixture.JavaPeerProxyReference); } + static void AddRegistrationTypeWithoutJniOwner (JniFixtureBuilder fixture, FieldDefinitionHandle nameField, FieldDefinitionHandle signatureField) + { + int fieldStart = fixture.NextFieldRid; + int methodStart = fixture.NextMethodRid; + + fixture.AddVoidMethod ("RegisterNatives", fixture.EmitBody (encoder => { + encoder.OpCode (ILOpCode.Ldsflda); + encoder.Token (nameField); + encoder.OpCode (ILOpCode.Ldsflda); + encoder.Token (signatureField); + encoder.OpCode (ILOpCode.Pop); + encoder.OpCode (ILOpCode.Pop); + encoder.OpCode (ILOpCode.Ret); + })); + + fixture.AddType ("Acme.Orig", "UnknownOwner", fieldStart, methodStart); + } + [Test] public void SharedLoadedStringGetsOwnerSpecificReplacements () { diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlanner.cs b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlanner.cs index 2df3f778fc0..dc8c7999663 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlanner.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlanner.cs @@ -995,8 +995,9 @@ void PlanUtf8FieldData (JniRewritePlan plan) } if (resolved != null && resolved != candidate) { throw new JniRewriteException ( - $"The mapped UTF-8 JNI datum '{value}' is shared by more than one Java class, but the mapping renames it to both " + - $"'{resolved}' and '{candidate}'. Splitting a shared '{FieldRvaTable.Utf8FieldNamePrefix}' field would move metadata tokens, which this rewriter does not do."); + $"The UTF-8 JNI datum '{value}' is shared by uses that require incompatible values '{resolved}' and '{candidate}'. " + + $"At least one use may require the original value because its owning Java class or member mapping could not be resolved. " + + $"Splitting a shared '{FieldRvaTable.Utf8FieldNamePrefix}' field would move metadata tokens, which this rewriter does not do."); } resolved ??= candidate; } @@ -1017,20 +1018,21 @@ IEnumerable GetUses (FieldDefinitionHandle field) string? ComputeNewUtf8Value (string value, Utf8Use use) { - if (use.Role == Utf8Role.MethodName && use.OwnerJniName != null && use.PairedSignature != null) { - JniDescriptorText.MethodDescriptorToJavaTypes (use.PairedSignature, out var javaParams, out string javaReturnType); - string mappingName = R8Mapping.JniMemberNameToMappingName (value); - return mapping.TryMapMethod (use.OwnerJniName, mappingName, javaParams, javaReturnType, out string renamed) ? renamed : null; + if (use.Role == Utf8Role.MethodName) { + if (use.OwnerJniName != null && use.PairedSignature != null) { + JniDescriptorText.MethodDescriptorToJavaTypes (use.PairedSignature, out var javaParams, out string javaReturnType); + string mappingName = R8Mapping.JniMemberNameToMappingName (value); + if (mapping.TryMapMethod (use.OwnerJniName, mappingName, javaParams, javaReturnType, out string renamed)) { + return renamed; + } + } + return value; } if (JniDescriptorText.IsValidMethodDescriptor (value) || JniDescriptorText.IsValidFieldDescriptor (value)) { return JniDescriptorText.TryRewriteDescriptor (value, renameClass, out string rewritten) ? rewritten : null; } - if (use.Role == Utf8Role.Unknown && mapping.TryMapClass (value, out string renamedClass)) { - return renamedClass; - } - return null; } From 351afd53b9d62d5ea8f83d52b31ead5c8eb5269f Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Wed, 2 Sep 2026 00:22:41 +0200 Subject: [PATCH 3/4] Avoid sharing owner-specific JNI method names Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Generator/PEAssemblyBuilder.cs | 72 ++++++++--- .../Generator/TypeMapAssemblyEmitter.cs | 30 ++++- .../JniRemapping/JniAssemblyRewriterTests.cs | 80 ++++++++++++ .../TypeMapAssemblyGeneratorTests.cs | 117 ++++++++++++++---- 4 files changed, 254 insertions(+), 45 deletions(-) diff --git a/src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/PEAssemblyBuilder.cs b/src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/PEAssemblyBuilder.cs index 6434c515e27..2a0ba4da1c5 100644 --- a/src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/PEAssemblyBuilder.cs +++ b/src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/PEAssemblyBuilder.cs @@ -42,9 +42,10 @@ sealed class PEAssemblyBuilder // Avoids creating duplicate __utf8_N types when multiple fields share the same size. readonly Dictionary _sizedTypeCache = new (); - // Deduplication cache for UTF-8 string RVA fields. Strings like "()V" that repeat across - // many proxy types are stored once and shared via the same FieldDefinitionHandle. - readonly Dictionary _utf8FieldCache = new (StringComparer.Ordinal); + // JNI signatures are owner-independent and can safely share one RVA field. JNI method names + // are owner-specific after R8 rewriting, so each registration receives its own field. + readonly Dictionary _sharedUtf8FieldCache = new (StringComparer.Ordinal); + readonly Dictionary> _uniqueUtf8FieldCache = new (StringComparer.Ordinal); TypeDefinitionHandle _privateImplDetailsType; int _utf8FieldCounter; @@ -273,31 +274,57 @@ TypeReferenceHandle MakeTypeRefForManagedName (EntityHandle scope, string manage } /// - /// Emits deduplicated RVA fields containing the supplied null-terminated UTF-8 strings. + /// Emits RVA fields containing the supplied null-terminated UTF-8 strings. + /// are deduplicated, while every occurrence in + /// receives a separate field. /// Fields are grouped by size so each group is emitted contiguously on its sized helper /// type before any consuming types are emitted. /// - public void PrepareUtf8Fields (IEnumerable values) + public void PrepareUtf8Fields (IEnumerable sharedValues, IEnumerable uniqueValues) { - var valuesBySize = new SortedDictionary> (); - foreach (string value in values) { + var sharedValuesBySize = new SortedDictionary> (); + foreach (string value in sharedValues) { int size = System.Text.Encoding.UTF8.GetByteCount (value) + 1; - if (!valuesBySize.TryGetValue (size, out var valuesForSize)) { + if (!sharedValuesBySize.TryGetValue (size, out var valuesForSize)) { valuesForSize = new SortedSet (StringComparer.Ordinal); - valuesBySize.Add (size, valuesForSize); + sharedValuesBySize.Add (size, valuesForSize); } valuesForSize.Add (value); } - foreach (var group in valuesBySize) { - var sizedType = GetOrCreateSizedType (group.Key); - foreach (string value in group.Value) { - AddUtf8Field (value, sizedType); + var uniqueValuesBySize = new SortedDictionary> (); + foreach (string value in uniqueValues) { + int size = System.Text.Encoding.UTF8.GetByteCount (value) + 1; + if (!uniqueValuesBySize.TryGetValue (size, out var valuesForSize)) { + valuesForSize = new SortedDictionary (StringComparer.Ordinal); + uniqueValuesBySize.Add (size, valuesForSize); + } + valuesForSize.TryGetValue (value, out int count); + valuesForSize [value] = count + 1; + } + + var sizes = new SortedSet (sharedValuesBySize.Keys); + sizes.UnionWith (uniqueValuesBySize.Keys); + foreach (int size in sizes) { + var sizedType = GetOrCreateSizedType (size); + if (sharedValuesBySize.TryGetValue (size, out var sharedForSize)) { + foreach (string value in sharedForSize) { + _sharedUtf8FieldCache.Add (value, AddUtf8Field (value, sizedType)); + } + } + if (uniqueValuesBySize.TryGetValue (size, out var uniqueForSize)) { + foreach (var pair in uniqueForSize) { + var fields = new Queue (pair.Value); + for (int i = 0; i < pair.Value; i++) { + fields.Enqueue (AddUtf8Field (pair.Key, sizedType)); + } + _uniqueUtf8FieldCache.Add (pair.Key, fields); + } } } } - void AddUtf8Field (string value, TypeDefinitionHandle sizedType) + FieldDefinitionHandle AddUtf8Field (string value, TypeDefinitionHandle sizedType) { // Encode to null-terminated UTF-8 (all JNI names/signatures are ASCII). _sigBlob.Clear (); @@ -313,8 +340,7 @@ void AddUtf8Field (string value, TypeDefinitionHandle sizedType) Metadata.GetOrAddBlob (_sigBlob)); Metadata.AddFieldRelativeVirtualAddress (fieldHandle, rva); - - _utf8FieldCache [value] = fieldHandle; + return fieldHandle; } /// @@ -322,13 +348,25 @@ void AddUtf8Field (string value, TypeDefinitionHandle sizedType) /// public FieldDefinitionHandle GetUtf8Field (string value) { - if (_utf8FieldCache.TryGetValue (value, out var existing)) { + if (_sharedUtf8FieldCache.TryGetValue (value, out var existing)) { return existing; } throw new InvalidOperationException ($"UTF-8 field '{value}' was not prepared before type emission."); } + /// + /// Returns and consumes one previously prepared unique UTF-8 RVA field. + /// + public FieldDefinitionHandle GetUniqueUtf8Field (string value) + { + if (_uniqueUtf8FieldCache.TryGetValue (value, out var fields) && fields.Count > 0) { + return fields.Dequeue (); + } + + throw new InvalidOperationException ($"Unique UTF-8 field '{value}' was not prepared before type emission."); + } + void EnsurePrivateImplDetailsType () { if (!_privateImplDetailsType.IsNil) { diff --git a/src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs b/src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs index 903415d2a84..2a2a3ba2ff2 100644 --- a/src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs +++ b/src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Reflection; using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; @@ -196,7 +197,10 @@ void EmitCore (TypeMapAssemblyData model, bool useSharedTypemapUniverse) } EmitMemberReferences (); - _pe.PrepareUtf8Fields (EnumerateNativeRegistrationStrings (model.ProxyTypes)); + var validRegistrations = EnumerateValidNativeRegistrations (model.ProxyTypes); + _pe.PrepareUtf8Fields ( + validRegistrations.Select (registration => registration.JniSignature), + validRegistrations.Select (registration => registration.JniMethodName)); // Track wrapper targets → handles for RegisterNatives. var wrapperHandles = new Dictionary (); @@ -220,17 +224,30 @@ void EmitCore (TypeMapAssemblyData model, bool useSharedTypemapUniverse) _pe.EmitIgnoresAccessChecksToAttribute (model.IgnoresAccessChecksTo); } - static IEnumerable EnumerateNativeRegistrationStrings (IReadOnlyList proxies) + static List EnumerateValidNativeRegistrations (IReadOnlyList proxies) { + var wrapperTargets = new HashSet (); + foreach (var proxy in proxies) { + foreach (var method in proxy.UcoMethods) { + wrapperTargets.Add (UcoWrapperTargetData.From (proxy, method.WrapperName)); + } + foreach (var constructor in proxy.UcoConstructors) { + wrapperTargets.Add (UcoWrapperTargetData.From (proxy, constructor.WrapperName)); + } + } + + var registrations = new List (); foreach (var proxy in proxies) { if (!proxy.IsAcw) { continue; } foreach (var registration in proxy.NativeRegistrations) { - yield return registration.JniMethodName; - yield return registration.JniSignature; + if (wrapperTargets.Contains (registration.WrapperTarget)) { + registrations.Add (registration); + } } } + return registrations; } static List OrderProxiesForWrapperTargets (IReadOnlyList proxies) @@ -1656,11 +1673,12 @@ void EmitRegisterNatives (JavaPeerProxyData proxy, return; } - // Get the prepared, deduplicated RVA fields for each unique name/signature string. + // Method names are unique per registration because R8 member mappings are owner-specific. + // Signatures remain safely deduplicated because descriptor class mappings are owner-independent. var nameFields = new FieldDefinitionHandle [validRegs.Count]; var sigFields = new FieldDefinitionHandle [validRegs.Count]; for (int i = 0; i < validRegs.Count; i++) { - nameFields [i] = _pe.GetUtf8Field (validRegs [i].Reg.JniMethodName); + nameFields [i] = _pe.GetUniqueUtf8Field (validRegs [i].Reg.JniMethodName); sigFields [i] = _pe.GetUtf8Field (validRegs [i].Reg.JniSignature); } diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs index 9a22476a216..53c43739348 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs @@ -7,6 +7,7 @@ using System.Reflection.Metadata; using System.Reflection.Metadata.Ecma335; using System.Reflection.PortableExecutable; +using Microsoft.Android.Sdk.TrimmableTypeMap; using Microsoft.Build.Framework; using Microsoft.Build.Utilities; using NUnit.Framework; @@ -881,6 +882,85 @@ public void FailsWhenASharedUtf8DatumNeedsTwoDifferentNames () StringAssert.Contains ("shared", exception.Message.ToLowerInvariant ()); } + [Test] + public void RewritesGeneratedTypeMapWithOwnerSpecificMethodNames () + { + byte [] source = GenerateTypeMapWithSharedMethodName (); + var warnings = new List (); + + JniRewriteResult result = Rewrite (source, Mapping ( + "test.First -> a.b.First:\n" + + " void n_Run() -> a\n" + + "test.Second -> a.b.Second:\n" + + " void n_Run() -> b\n"), warnings); + + CollectionAssert.AreEquivalent (new [] { "a", "b", "()V" }, ReadUtf8Values (result.Image)); + CollectionAssert.DoesNotContain (warnings.Select (warning => warning.Code).ToArray (), "XA4326"); + } + + [Test] + public void RewritesGeneratedTypeMapWithMappedAndUnmappedMethodNames () + { + byte [] source = GenerateTypeMapWithSharedMethodName (); + var warnings = new List (); + + JniRewriteResult result = Rewrite (source, Mapping ( + "test.First -> a.b.First:\n" + + " void n_Run() -> a\n" + + "test.Second -> test.Second:\n"), warnings); + + CollectionAssert.AreEquivalent (new [] { "a", "n_Run", "()V" }, ReadUtf8Values (result.Image)); + CollectionAssert.DoesNotContain (warnings.Select (warning => warning.Code).ToArray (), "XA4326"); + } + + static byte [] GenerateTypeMapWithSharedMethodName () + { + var peers = new [] { + CreatePeer ("test/First", "Test.First"), + CreatePeer ("test/Second", "Test.Second"), + }; + using var stream = new MemoryStream (); + new TypeMapAssemblyGenerator (new Version (11, 0, 0, 0)).Generate (peers, stream, "OwnerSpecificNames"); + return stream.ToArray (); + + static JavaPeerInfo CreatePeer (string javaName, string managedName) + { + int separator = managedName.LastIndexOf ('.'); + return new JavaPeerInfo { + JavaName = javaName, + CompatJniName = javaName, + ManagedTypeName = managedName, + ManagedTypeNamespace = managedName.Substring (0, separator), + ManagedTypeShortName = managedName.Substring (separator + 1), + AssemblyName = "TestAsm", + DoNotGenerateAcw = false, + ActivationCtor = new ActivationCtorInfo { + DeclaringTypeName = managedName, + DeclaringAssemblyName = "TestAsm", + Style = ActivationCtorStyle.XamarinAndroid, + }, + MarshalMethods = [ + new MarshalMethodInfo { + JniName = "run", + NativeCallbackName = "n_Run", + JniSignature = "()V", + ManagedMethodName = "Run", + }, + ], + }; + } + } + + static string [] ReadUtf8Values (byte [] image) + { + using var peReader = new PEReader (ImmutableArray.Create (image)); + MetadataReader reader = peReader.GetMetadataReader (); + return FieldRvaTable.Read (peReader, reader).Entries + .Select (entry => entry.Utf8Value) + .OfType () + .ToArray (); + } + [Test] public void FailsWhenASharedUtf8DatumMustRemainUnmappedForOneProxy () { diff --git a/tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs b/tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs index c0eee1a9bfd..4eb6099f677 100644 --- a/tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs +++ b/tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs @@ -1264,6 +1264,11 @@ static List ReadCallTokens (byte [] ilBytes) return ReadInlineMethodTokens (ilBytes, 0x28); } + static List ReadLoadStaticFieldAddressTokens (byte [] ilBytes) + { + return ReadInlineMethodTokens (ilBytes, 0x7F); + } + static List ReadInlineMethodTokens (byte [] ilBytes, byte opcode) { var tokens = new List (); @@ -1544,19 +1549,17 @@ public void Generate_ExportProxy_StructuredGenericArgumentThrows () } [Fact] - public void Generate_MultipleAcwProxies_DeduplicatesUtf8Strings () + public void Generate_MultipleAcwProxies_DeduplicatesSignaturesButNotMethodNames () { var peers = ScanFixtures (); - // Get all ACW peers — they likely share signatures like "()V" var acwPeers = peers.Where (p => !p.DoNotGenerateAcw && p.MarshalMethods.Count > 0).ToList (); Assert.True (acwPeers.Count >= 2, "Need at least 2 ACW peers to test deduplication"); + var model = ModelBuilder.Build (acwPeers, "DedupTest.dll", "DedupTest"); using var stream = GenerateAssembly (acwPeers, "DedupTest"); using var pe = new PEReader (stream); var reader = pe.GetMetadataReader (); - // Count fields with HasFieldRVA — these are our UTF-8 RVA fields. - // With deduplication, common strings like "()V" should appear only once. var rvaFields = reader.FieldDefinitions .Select (h => reader.GetFieldDefinition (h)) .Where (f => (f.Attributes & FieldAttributes.HasFieldRVA) != 0) @@ -1567,24 +1570,94 @@ public void Generate_MultipleAcwProxies_DeduplicatesUtf8Strings () Assert.StartsWith ("__utf8_", reader.GetString (declaringType.Name)); }); - // Collect all JNI method names and signatures from the ACW peers - var allStrings = acwPeers - .SelectMany (p => p.MarshalMethods) - .SelectMany (m => new [] { m.JniName, m.JniSignature }) - .ToList (); - var uniqueStrings = allStrings.Distinct ().Count (); - - // With dedup, RVA field count should equal unique string count, not total string count. - // Also include constructor registrations (nctor_*), so use <= for a safe assertion. - Assert.True (rvaFields.Count <= uniqueStrings + acwPeers.Count * 2, - $"Expected at most {uniqueStrings + acwPeers.Count * 2} RVA fields (unique strings + ctor names/sigs), " + - $"but found {rvaFields.Count}. Deduplication may not be working."); - - // The key assertion: fewer RVA fields than total strings means dedup is working - if (allStrings.Count > uniqueStrings) { - Assert.True (rvaFields.Count < allStrings.Count, - $"Expected fewer RVA fields ({rvaFields.Count}) than total strings ({allStrings.Count}) due to deduplication"); - } + var registrations = model.ProxyTypes.SelectMany (proxy => proxy.NativeRegistrations).ToList (); + int expectedFieldCount = registrations.Count + + registrations.Select (registration => registration.JniSignature).Distinct (StringComparer.Ordinal).Count (); + Assert.Equal (expectedFieldCount, rvaFields.Count); + } + + [Fact] + public void Generate_SharedMethodNameUsesDistinctFieldsWhileSignatureRemainsShared () + { + var first = MakeAcwPeer ("test/First", "Test.First", "TestAsm") with { + JavaConstructors = [], + MarshalMethods = [ + new MarshalMethodInfo { + JniName = "run", + NativeCallbackName = "n_Run", + JniSignature = "()V", + ManagedMethodName = "Run", + }, + ], + }; + var second = MakeAcwPeer ("test/Second", "Test.Second", "TestAsm") with { + JavaConstructors = [], + MarshalMethods = [ + new MarshalMethodInfo { + JniName = "run", + NativeCallbackName = "n_Run", + JniSignature = "()V", + ManagedMethodName = "Run", + }, + ], + }; + + using var stream = GenerateAssembly ([first, second], "OwnerSpecificNames"); + using var pe = new PEReader (stream); + var reader = pe.GetMetadataReader (); + + var firstFields = ReadRegisterNativesFieldTokens (pe, reader, "Test_First_Proxy"); + var secondFields = ReadRegisterNativesFieldTokens (pe, reader, "Test_Second_Proxy"); + + Assert.Equal (2, firstFields.Count); + Assert.Equal (2, secondFields.Count); + Assert.NotEqual (firstFields [0], secondFields [0]); + Assert.Equal (firstFields [1], secondFields [1]); + } + + [Fact] + public void Generate_RegistrationWithoutWrapperDoesNotConsumeUtf8Field () + { + var peer = MakeAcwPeer ("test/Valid", "Test.Valid", "TestAsm") with { + JavaConstructors = [], + MarshalMethods = [ + new MarshalMethodInfo { + JniName = "run", + NativeCallbackName = "n_Run", + JniSignature = "()V", + ManagedMethodName = "Run", + }, + ], + }; + var model = ModelBuilder.Build ([peer], "MissingWrapper.dll", "MissingWrapper"); + model.ProxyTypes.Single ().NativeRegistrations.Add (new NativeRegistrationData { + JniMethodName = "n_Missing", + JniSignature = "(I)V", + WrapperMethodName = "missing_uco", + WrapperTarget = new UcoWrapperTargetData { + TypeNamespace = "_TypeMap.Proxies", + TypeName = "Missing_Proxy", + MethodName = "missing_uco", + }, + }); + + using var stream = new MemoryStream (); + new TypeMapAssemblyEmitter (new Version (11, 0, 0, 0)).Emit (model, stream); + stream.Position = 0; + using var pe = new PEReader (stream); + var reader = pe.GetMetadataReader (); + + Assert.Equal (2, reader.GetTableRowCount (TableIndex.FieldRva)); + Assert.Equal (2, ReadRegisterNativesFieldTokens (pe, reader, "Test_Valid_Proxy").Count); + } + + static List ReadRegisterNativesFieldTokens (PEReader pe, MetadataReader reader, string proxyTypeName) + { + var proxy = FindProxyType (reader, proxyTypeName); + var method = reader.GetMethodDefinition (FindMethodDefinition (reader, proxy, "RegisterNatives")); + var ilBytes = pe.GetMethodBody (method.RelativeVirtualAddress).GetILBytes (); + Assert.NotNull (ilBytes); + return ReadLoadStaticFieldAddressTokens (ilBytes); } [Fact] From 079338e009fbaa3ceaba51b69275e1c47948c5f6 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 3 Sep 2026 21:33:58 +0200 Subject: [PATCH 4/4] Address JNI metadata rewrite review Validate string-array custom attribute prologs and decode their element count explicitly as little-endian. Rename the test IL token reader to reflect that it handles both method and field tokens. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../JniRemapping/JniAssemblyRewriterTests.cs | 9 ++++++++ .../CustomAttributeStringRewriter.cs | 23 +++++++++++++------ .../TypeMapAssemblyGeneratorTests.cs | 10 ++++---- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs index 53c43739348..21c93763bdc 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs @@ -173,6 +173,15 @@ public void RejectsInvalidCustomAttributeProlog () StringAssert.Contains ("expected 0x0001 prolog", exception.Message); } + [Test] + public void RejectsInvalidStringArrayCustomAttributeProlog () + { + var exception = Assert.Throws (() => + CustomAttributeStringRewriter.TryRewriteStringArray (new byte [] { 0x00, 0x00, 0x01, 0x00, 0x00, 0x00 }, _ => null)); + + StringAssert.Contains ("expected 0x0001 prolog", exception.Message); + } + [Test] public void DoesNotRewriteUnrelatedBareMemberAndDescriptorStrings () { diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/CustomAttributeStringRewriter.cs b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/CustomAttributeStringRewriter.cs index 48bd8706ed3..586197afd78 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/CustomAttributeStringRewriter.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/CustomAttributeStringRewriter.cs @@ -30,12 +30,7 @@ static class CustomAttributeStringRewriter /// public static byte []? TryRewrite (byte [] originalContent, int fixedArgCount, Func rewriteArg) { - if (originalContent.Length < 2) { - throw new JniRewriteException ("Malformed custom attribute value blob: missing 2-byte prolog."); - } - if (originalContent [0] != 0x01 || originalContent [1] != 0x00) { - throw new JniRewriteException ("Malformed custom attribute value blob: expected 0x0001 prolog."); - } + ValidateProlog (originalContent); using var ms = new MemoryStream (originalContent.Length); ms.Write (originalContent, 0, 2); // Prolog (0x0001), verbatim. @@ -86,13 +81,17 @@ static class CustomAttributeStringRewriter /// public static byte []? TryRewriteStringArray (byte [] originalContent, Func rewriteElement) { + ValidateProlog (originalContent); if (originalContent.Length < 6) { throw new JniRewriteException ("Malformed custom attribute value blob: missing string array length."); } using var ms = new MemoryStream (originalContent.Length); ms.Write (originalContent, 0, 6); // Prolog (uint16) and array length (int32), verbatim. - int count = BitConverter.ToInt32 (originalContent, 2); + int count = originalContent [2] | + (originalContent [3] << 8) | + (originalContent [4] << 16) | + (originalContent [5] << 24); if (count < -1) { throw new JniRewriteException ($"Malformed custom attribute value blob: invalid string array length {count}."); } @@ -136,5 +135,15 @@ static class CustomAttributeStringRewriter ms.Write (originalContent, pos, originalContent.Length - pos); return changed ? ms.ToArray () : null; } + + static void ValidateProlog (byte [] originalContent) + { + if (originalContent.Length < 2) { + throw new JniRewriteException ("Malformed custom attribute value blob: missing 2-byte prolog."); + } + if (originalContent [0] != 0x01 || originalContent [1] != 0x00) { + throw new JniRewriteException ("Malformed custom attribute value blob: expected 0x0001 prolog."); + } + } } } diff --git a/tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs b/tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs index 4eb6099f677..b46b18d7f92 100644 --- a/tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs +++ b/tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs @@ -1256,20 +1256,20 @@ static List FindMethodDefinitions (MetadataReader reader static List ReadLdftnTokens (byte [] ilBytes) { - return ReadInlineMethodTokens (ilBytes, 0xFE, 0x06); + return ReadInlineMetadataTokens (ilBytes, 0xFE, 0x06); } static List ReadCallTokens (byte [] ilBytes) { - return ReadInlineMethodTokens (ilBytes, 0x28); + return ReadInlineMetadataTokens (ilBytes, 0x28); } static List ReadLoadStaticFieldAddressTokens (byte [] ilBytes) { - return ReadInlineMethodTokens (ilBytes, 0x7F); + return ReadInlineMetadataTokens (ilBytes, 0x7F); } - static List ReadInlineMethodTokens (byte [] ilBytes, byte opcode) + static List ReadInlineMetadataTokens (byte [] ilBytes, byte opcode) { var tokens = new List (); for (int i = 0; i < ilBytes.Length - 4; i++) { @@ -1285,7 +1285,7 @@ static List ReadInlineMethodTokens (byte [] ilBytes, byte opcode) return tokens; } - static List ReadInlineMethodTokens (byte [] ilBytes, byte opcodePrefix, byte opcode) + static List ReadInlineMetadataTokens (byte [] ilBytes, byte opcodePrefix, byte opcode) { var tokens = new List (); for (int i = 0; i < ilBytes.Length - 5; i++) {