Skip to content

Commit 80bca70

Browse files
Avoid sharing owner-specific JNI method names
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent c3a63cf commit 80bca70

4 files changed

Lines changed: 254 additions & 45 deletions

File tree

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

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,10 @@ sealed class PEAssemblyBuilder
4242
// Avoids creating duplicate __utf8_N types when multiple fields share the same size.
4343
readonly Dictionary<int, TypeDefinitionHandle> _sizedTypeCache = new ();
4444

45-
// Deduplication cache for UTF-8 string RVA fields. Strings like "()V" that repeat across
46-
// many proxy types are stored once and shared via the same FieldDefinitionHandle.
47-
readonly Dictionary<string, FieldDefinitionHandle> _utf8FieldCache = new (StringComparer.Ordinal);
45+
// JNI signatures are owner-independent and can safely share one RVA field. JNI method names
46+
// are owner-specific after R8 rewriting, so each registration receives its own field.
47+
readonly Dictionary<string, FieldDefinitionHandle> _sharedUtf8FieldCache = new (StringComparer.Ordinal);
48+
readonly Dictionary<string, Queue<FieldDefinitionHandle>> _uniqueUtf8FieldCache = new (StringComparer.Ordinal);
4849
TypeDefinitionHandle _privateImplDetailsType;
4950
int _utf8FieldCounter;
5051

@@ -273,31 +274,57 @@ TypeReferenceHandle MakeTypeRefForManagedName (EntityHandle scope, string manage
273274
}
274275

275276
/// <summary>
276-
/// Emits deduplicated RVA fields containing the supplied null-terminated UTF-8 strings.
277+
/// Emits RVA fields containing the supplied null-terminated UTF-8 strings.
278+
/// <paramref name="sharedValues"/> are deduplicated, while every occurrence in
279+
/// <paramref name="uniqueValues"/> receives a separate field.
277280
/// Fields are grouped by size so each group is emitted contiguously on its sized helper
278281
/// type before any consuming types are emitted.
279282
/// </summary>
280-
public void PrepareUtf8Fields (IEnumerable<string> values)
283+
public void PrepareUtf8Fields (IEnumerable<string> sharedValues, IEnumerable<string> uniqueValues)
281284
{
282-
var valuesBySize = new SortedDictionary<int, SortedSet<string>> ();
283-
foreach (string value in values) {
285+
var sharedValuesBySize = new SortedDictionary<int, SortedSet<string>> ();
286+
foreach (string value in sharedValues) {
284287
int size = System.Text.Encoding.UTF8.GetByteCount (value) + 1;
285-
if (!valuesBySize.TryGetValue (size, out var valuesForSize)) {
288+
if (!sharedValuesBySize.TryGetValue (size, out var valuesForSize)) {
286289
valuesForSize = new SortedSet<string> (StringComparer.Ordinal);
287-
valuesBySize.Add (size, valuesForSize);
290+
sharedValuesBySize.Add (size, valuesForSize);
288291
}
289292
valuesForSize.Add (value);
290293
}
291294

292-
foreach (var group in valuesBySize) {
293-
var sizedType = GetOrCreateSizedType (group.Key);
294-
foreach (string value in group.Value) {
295-
AddUtf8Field (value, sizedType);
295+
var uniqueValuesBySize = new SortedDictionary<int, SortedDictionary<string, int>> ();
296+
foreach (string value in uniqueValues) {
297+
int size = System.Text.Encoding.UTF8.GetByteCount (value) + 1;
298+
if (!uniqueValuesBySize.TryGetValue (size, out var valuesForSize)) {
299+
valuesForSize = new SortedDictionary<string, int> (StringComparer.Ordinal);
300+
uniqueValuesBySize.Add (size, valuesForSize);
301+
}
302+
valuesForSize.TryGetValue (value, out int count);
303+
valuesForSize [value] = count + 1;
304+
}
305+
306+
var sizes = new SortedSet<int> (sharedValuesBySize.Keys);
307+
sizes.UnionWith (uniqueValuesBySize.Keys);
308+
foreach (int size in sizes) {
309+
var sizedType = GetOrCreateSizedType (size);
310+
if (sharedValuesBySize.TryGetValue (size, out var sharedForSize)) {
311+
foreach (string value in sharedForSize) {
312+
_sharedUtf8FieldCache.Add (value, AddUtf8Field (value, sizedType));
313+
}
314+
}
315+
if (uniqueValuesBySize.TryGetValue (size, out var uniqueForSize)) {
316+
foreach (var pair in uniqueForSize) {
317+
var fields = new Queue<FieldDefinitionHandle> (pair.Value);
318+
for (int i = 0; i < pair.Value; i++) {
319+
fields.Enqueue (AddUtf8Field (pair.Key, sizedType));
320+
}
321+
_uniqueUtf8FieldCache.Add (pair.Key, fields);
322+
}
296323
}
297324
}
298325
}
299326

300-
void AddUtf8Field (string value, TypeDefinitionHandle sizedType)
327+
FieldDefinitionHandle AddUtf8Field (string value, TypeDefinitionHandle sizedType)
301328
{
302329
// Encode to null-terminated UTF-8 (all JNI names/signatures are ASCII).
303330
_sigBlob.Clear ();
@@ -313,22 +340,33 @@ void AddUtf8Field (string value, TypeDefinitionHandle sizedType)
313340
Metadata.GetOrAddBlob (_sigBlob));
314341

315342
Metadata.AddFieldRelativeVirtualAddress (fieldHandle, rva);
316-
317-
_utf8FieldCache [value] = fieldHandle;
343+
return fieldHandle;
318344
}
319345

320346
/// <summary>
321347
/// Returns a previously prepared UTF-8 RVA field.
322348
/// </summary>
323349
public FieldDefinitionHandle GetUtf8Field (string value)
324350
{
325-
if (_utf8FieldCache.TryGetValue (value, out var existing)) {
351+
if (_sharedUtf8FieldCache.TryGetValue (value, out var existing)) {
326352
return existing;
327353
}
328354

329355
throw new InvalidOperationException ($"UTF-8 field '{value}' was not prepared before type emission.");
330356
}
331357

358+
/// <summary>
359+
/// Returns and consumes one previously prepared unique UTF-8 RVA field.
360+
/// </summary>
361+
public FieldDefinitionHandle GetUniqueUtf8Field (string value)
362+
{
363+
if (_uniqueUtf8FieldCache.TryGetValue (value, out var fields) && fields.Count > 0) {
364+
return fields.Dequeue ();
365+
}
366+
367+
throw new InvalidOperationException ($"Unique UTF-8 field '{value}' was not prepared before type emission.");
368+
}
369+
332370
void EnsurePrivateImplDetailsType ()
333371
{
334372
if (!_privateImplDetailsType.IsNil) {

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

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.IO;
4+
using System.Linq;
45
using System.Reflection;
56
using System.Reflection.Metadata;
67
using System.Reflection.Metadata.Ecma335;
@@ -195,7 +196,10 @@ void EmitCore (TypeMapAssemblyData model, bool useSharedTypemapUniverse)
195196
}
196197
EmitMemberReferences ();
197198

198-
_pe.PrepareUtf8Fields (EnumerateNativeRegistrationStrings (model.ProxyTypes));
199+
var validRegistrations = EnumerateValidNativeRegistrations (model.ProxyTypes);
200+
_pe.PrepareUtf8Fields (
201+
validRegistrations.Select (registration => registration.JniSignature),
202+
validRegistrations.Select (registration => registration.JniMethodName));
199203

200204
// Track wrapper targets → handles for RegisterNatives.
201205
var wrapperHandles = new Dictionary<UcoWrapperTargetData, MethodDefinitionHandle> ();
@@ -219,17 +223,30 @@ void EmitCore (TypeMapAssemblyData model, bool useSharedTypemapUniverse)
219223
_pe.EmitIgnoresAccessChecksToAttribute (model.IgnoresAccessChecksTo);
220224
}
221225

222-
static IEnumerable<string> EnumerateNativeRegistrationStrings (IReadOnlyList<JavaPeerProxyData> proxies)
226+
static List<NativeRegistrationData> EnumerateValidNativeRegistrations (IReadOnlyList<JavaPeerProxyData> proxies)
223227
{
228+
var wrapperTargets = new HashSet<UcoWrapperTargetData> ();
229+
foreach (var proxy in proxies) {
230+
foreach (var method in proxy.UcoMethods) {
231+
wrapperTargets.Add (UcoWrapperTargetData.From (proxy, method.WrapperName));
232+
}
233+
foreach (var constructor in proxy.UcoConstructors) {
234+
wrapperTargets.Add (UcoWrapperTargetData.From (proxy, constructor.WrapperName));
235+
}
236+
}
237+
238+
var registrations = new List<NativeRegistrationData> ();
224239
foreach (var proxy in proxies) {
225240
if (!proxy.IsAcw) {
226241
continue;
227242
}
228243
foreach (var registration in proxy.NativeRegistrations) {
229-
yield return registration.JniMethodName;
230-
yield return registration.JniSignature;
244+
if (wrapperTargets.Contains (registration.WrapperTarget)) {
245+
registrations.Add (registration);
246+
}
231247
}
232248
}
249+
return registrations;
233250
}
234251

235252
static List<JavaPeerProxyData> OrderProxiesForWrapperTargets (IReadOnlyList<JavaPeerProxyData> proxies)
@@ -1631,11 +1648,12 @@ void EmitRegisterNatives (JavaPeerProxyData proxy,
16311648
return;
16321649
}
16331650

1634-
// Get the prepared, deduplicated RVA fields for each unique name/signature string.
1651+
// Method names are unique per registration because R8 member mappings are owner-specific.
1652+
// Signatures remain safely deduplicated because descriptor class mappings are owner-independent.
16351653
var nameFields = new FieldDefinitionHandle [validRegs.Count];
16361654
var sigFields = new FieldDefinitionHandle [validRegs.Count];
16371655
for (int i = 0; i < validRegs.Count; i++) {
1638-
nameFields [i] = _pe.GetUtf8Field (validRegs [i].Reg.JniMethodName);
1656+
nameFields [i] = _pe.GetUniqueUtf8Field (validRegs [i].Reg.JniMethodName);
16391657
sigFields [i] = _pe.GetUtf8Field (validRegs [i].Reg.JniSignature);
16401658
}
16411659

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Reflection.Metadata;
88
using System.Reflection.Metadata.Ecma335;
99
using System.Reflection.PortableExecutable;
10+
using Microsoft.Android.Sdk.TrimmableTypeMap;
1011
using Microsoft.Build.Framework;
1112
using Microsoft.Build.Utilities;
1213
using NUnit.Framework;
@@ -797,6 +798,85 @@ public void FailsWhenASharedUtf8DatumNeedsTwoDifferentNames ()
797798
StringAssert.Contains ("shared", exception.Message.ToLowerInvariant ());
798799
}
799800

801+
[Test]
802+
public void RewritesGeneratedTypeMapWithOwnerSpecificMethodNames ()
803+
{
804+
byte [] source = GenerateTypeMapWithSharedMethodName ();
805+
var warnings = new List<BuildWarningEventArgs> ();
806+
807+
JniRewriteResult result = Rewrite (source, Mapping (
808+
"test.First -> a.b.First:\n" +
809+
" void n_Run() -> a\n" +
810+
"test.Second -> a.b.Second:\n" +
811+
" void n_Run() -> b\n"), warnings);
812+
813+
CollectionAssert.AreEquivalent (new [] { "a", "b", "()V" }, ReadUtf8Values (result.Image));
814+
CollectionAssert.DoesNotContain (warnings.Select (warning => warning.Code).ToArray (), "XA4326");
815+
}
816+
817+
[Test]
818+
public void RewritesGeneratedTypeMapWithMappedAndUnmappedMethodNames ()
819+
{
820+
byte [] source = GenerateTypeMapWithSharedMethodName ();
821+
var warnings = new List<BuildWarningEventArgs> ();
822+
823+
JniRewriteResult result = Rewrite (source, Mapping (
824+
"test.First -> a.b.First:\n" +
825+
" void n_Run() -> a\n" +
826+
"test.Second -> test.Second:\n"), warnings);
827+
828+
CollectionAssert.AreEquivalent (new [] { "a", "n_Run", "()V" }, ReadUtf8Values (result.Image));
829+
CollectionAssert.DoesNotContain (warnings.Select (warning => warning.Code).ToArray (), "XA4326");
830+
}
831+
832+
static byte [] GenerateTypeMapWithSharedMethodName ()
833+
{
834+
var peers = new [] {
835+
CreatePeer ("test/First", "Test.First"),
836+
CreatePeer ("test/Second", "Test.Second"),
837+
};
838+
using var stream = new MemoryStream ();
839+
new TypeMapAssemblyGenerator (new Version (11, 0, 0, 0)).Generate (peers, stream, "OwnerSpecificNames");
840+
return stream.ToArray ();
841+
842+
static JavaPeerInfo CreatePeer (string javaName, string managedName)
843+
{
844+
int separator = managedName.LastIndexOf ('.');
845+
return new JavaPeerInfo {
846+
JavaName = javaName,
847+
CompatJniName = javaName,
848+
ManagedTypeName = managedName,
849+
ManagedTypeNamespace = managedName.Substring (0, separator),
850+
ManagedTypeShortName = managedName.Substring (separator + 1),
851+
AssemblyName = "TestAsm",
852+
DoNotGenerateAcw = false,
853+
ActivationCtor = new ActivationCtorInfo {
854+
DeclaringTypeName = managedName,
855+
DeclaringAssemblyName = "TestAsm",
856+
Style = ActivationCtorStyle.XamarinAndroid,
857+
},
858+
MarshalMethods = [
859+
new MarshalMethodInfo {
860+
JniName = "run",
861+
NativeCallbackName = "n_Run",
862+
JniSignature = "()V",
863+
ManagedMethodName = "Run",
864+
},
865+
],
866+
};
867+
}
868+
}
869+
870+
static string [] ReadUtf8Values (byte [] image)
871+
{
872+
using var peReader = new PEReader (ImmutableArray.Create (image));
873+
MetadataReader reader = peReader.GetMetadataReader ();
874+
return FieldRvaTable.Read (peReader, reader).Entries
875+
.Select (entry => entry.Utf8Value)
876+
.OfType<string> ()
877+
.ToArray ();
878+
}
879+
800880
[Test]
801881
public void FailsWhenASharedUtf8DatumMustRemainUnmappedForOneProxy ()
802882
{

0 commit comments

Comments
 (0)