Skip to content

Commit 8c13540

Browse files
Handle shared FieldRVA names conservatively
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 3353eea commit 8c13540

2 files changed

Lines changed: 86 additions & 12 deletions

File tree

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

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -699,7 +699,6 @@ public void RewritesUtf8FieldRvaJniNamesAndSignatures ()
699699

700700
FieldDefinitionHandle nameField = fixture.AddUtf8Field ("onClick");
701701
FieldDefinitionHandle signatureField = fixture.AddUtf8Field ("(Lacme/orig/Callback;)V");
702-
FieldDefinitionHandle classNameField = fixture.AddUtf8Field ("acme/orig/Callback");
703702
FieldDefinitionHandle longNameField = fixture.AddUtf8Field ("run");
704703

705704
int fieldStart = fixture.NextFieldRid;
@@ -755,7 +754,6 @@ public void RewritesUtf8FieldRvaJniNamesAndSignatures ()
755754

756755
Assert.AreEqual ("a", ReadUtf8Field (peReader, reader, nameField), "The method name is renamed using the owning proxy's JNI class.");
757756
Assert.AreEqual ("(La/b/Cb;)V", ReadUtf8Field (peReader, reader, signatureField));
758-
Assert.AreEqual ("a/b/Cb", ReadUtf8Field (peReader, reader, classNameField), "An unreferenced datum that is a known class name is still renamed.");
759757
Assert.AreEqual ("aMuchLongerObfuscatedName", ReadUtf8Field (peReader, reader, longNameField), "A longer datum is relocated into a wider __utf8_N slot.");
760758

761759
// Growing a datum appends exactly one new sized type; no existing token moves.
@@ -799,6 +797,62 @@ public void FailsWhenASharedUtf8DatumNeedsTwoDifferentNames ()
799797
StringAssert.Contains ("shared", exception.Message.ToLowerInvariant ());
800798
}
801799

800+
[Test]
801+
public void FailsWhenASharedUtf8DatumMustRemainUnmappedForOneProxy ()
802+
{
803+
var fixture = new JniFixtureBuilder ();
804+
805+
FieldDefinitionHandle shared = fixture.AddUtf8Field ("go");
806+
FieldDefinitionHandle signature = fixture.AddUtf8Field ("()V");
807+
808+
AddProxy (fixture, "acme/orig/P1", shared, signature);
809+
AddProxy (fixture, "acme/orig/P2", shared, signature);
810+
811+
var exception = Assert.Throws<JniRewriteException> (() => Rewrite (fixture.Serialize (), Mapping (
812+
"acme.orig.P1 -> a.b.P1:\n" +
813+
" void go() -> z\n" +
814+
"acme.orig.P2 -> a.b.P2:\n")));
815+
StringAssert.Contains ("shared", exception.Message.ToLowerInvariant ());
816+
StringAssert.Contains ("original value", exception.Message);
817+
StringAssert.Contains ("'go'", exception.Message);
818+
StringAssert.Contains ("'z'", exception.Message);
819+
}
820+
821+
[Test]
822+
public void FailsWhenASharedUtf8DatumHasAnUnresolvedOwner ()
823+
{
824+
var fixture = new JniFixtureBuilder ();
825+
826+
FieldDefinitionHandle shared = fixture.AddUtf8Field ("go");
827+
FieldDefinitionHandle signature = fixture.AddUtf8Field ("()V");
828+
829+
AddProxy (fixture, "acme/orig/P1", shared, signature);
830+
AddRegistrationTypeWithoutJniOwner (fixture, shared, signature);
831+
832+
var exception = Assert.Throws<JniRewriteException> (() => Rewrite (fixture.Serialize (), Mapping (
833+
"acme.orig.P1 -> a.b.P1:\n" +
834+
" void go() -> z\n")));
835+
StringAssert.Contains ("shared", exception.Message.ToLowerInvariant ());
836+
StringAssert.Contains ("original value", exception.Message);
837+
}
838+
839+
[Test]
840+
public void PreservesUnreferencedUtf8DatumThatMatchesAMappedClass ()
841+
{
842+
var fixture = new JniFixtureBuilder ();
843+
FieldDefinitionHandle field = fixture.AddUtf8Field ("acme/orig/Callback");
844+
byte [] image = fixture.Serialize ();
845+
R8Mapping mapping = Mapping ("acme.orig.Callback -> a.b.Cb:\n");
846+
847+
JniRewriteResult result = Rewrite (image, mapping);
848+
849+
Assert.AreSame (image, result.Image);
850+
Assert.AreEqual (0, result.ReplacementCount);
851+
CollectionAssert.IsEmpty (mapping.AccessedEntries);
852+
using var peReader = new PEReader (ImmutableArray.Create (result.Image));
853+
Assert.AreEqual ("acme/orig/Callback", ReadUtf8Field (peReader, peReader.GetMetadataReader (), field));
854+
}
855+
802856
static void AddProxy (JniFixtureBuilder fixture, string jniName, FieldDefinitionHandle nameField, FieldDefinitionHandle signatureField)
803857
{
804858
int fieldStart = fixture.NextFieldRid;
@@ -825,6 +879,24 @@ static void AddProxy (JniFixtureBuilder fixture, string jniName, FieldDefinition
825879
TypeAttributes.Public | TypeAttributes.Sealed | TypeAttributes.Class, fixture.JavaPeerProxyReference);
826880
}
827881

882+
static void AddRegistrationTypeWithoutJniOwner (JniFixtureBuilder fixture, FieldDefinitionHandle nameField, FieldDefinitionHandle signatureField)
883+
{
884+
int fieldStart = fixture.NextFieldRid;
885+
int methodStart = fixture.NextMethodRid;
886+
887+
fixture.AddVoidMethod ("RegisterNatives", fixture.EmitBody (encoder => {
888+
encoder.OpCode (ILOpCode.Ldsflda);
889+
encoder.Token (nameField);
890+
encoder.OpCode (ILOpCode.Ldsflda);
891+
encoder.Token (signatureField);
892+
encoder.OpCode (ILOpCode.Pop);
893+
encoder.OpCode (ILOpCode.Pop);
894+
encoder.OpCode (ILOpCode.Ret);
895+
}));
896+
897+
fixture.AddType ("Acme.Orig", "UnknownOwner", fieldStart, methodStart);
898+
}
899+
828900
[Test]
829901
public void SharedLoadedStringGetsOwnerSpecificReplacements ()
830902
{

src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlanner.cs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -980,8 +980,9 @@ void PlanUtf8FieldData (JniRewritePlan plan)
980980
}
981981
if (resolved != null && resolved != candidate) {
982982
throw new JniRewriteException (
983-
$"The mapped UTF-8 JNI datum '{value}' is shared by more than one Java class, but the mapping renames it to both " +
984-
$"'{resolved}' and '{candidate}'. Splitting a shared '{FieldRvaTable.Utf8FieldNamePrefix}' field would move metadata tokens, which this rewriter does not do.");
983+
$"The UTF-8 JNI datum '{value}' is shared by uses that require incompatible values '{resolved}' and '{candidate}'. " +
984+
$"At least one use may require the original value because its owning Java class or member mapping could not be resolved. " +
985+
$"Splitting a shared '{FieldRvaTable.Utf8FieldNamePrefix}' field would move metadata tokens, which this rewriter does not do.");
985986
}
986987
resolved ??= candidate;
987988
}
@@ -1002,20 +1003,21 @@ IEnumerable<Utf8Use> GetUses (FieldDefinitionHandle field)
10021003

10031004
string? ComputeNewUtf8Value (string value, Utf8Use use)
10041005
{
1005-
if (use.Role == Utf8Role.MethodName && use.OwnerJniName != null && use.PairedSignature != null) {
1006-
JniDescriptorText.MethodDescriptorToJavaTypes (use.PairedSignature, out var javaParams, out string javaReturnType);
1007-
string mappingName = R8Mapping.JniMemberNameToMappingName (value);
1008-
return mapping.TryMapMethod (use.OwnerJniName, mappingName, javaParams, javaReturnType, out string renamed) ? renamed : null;
1006+
if (use.Role == Utf8Role.MethodName) {
1007+
if (use.OwnerJniName != null && use.PairedSignature != null) {
1008+
JniDescriptorText.MethodDescriptorToJavaTypes (use.PairedSignature, out var javaParams, out string javaReturnType);
1009+
string mappingName = R8Mapping.JniMemberNameToMappingName (value);
1010+
if (mapping.TryMapMethod (use.OwnerJniName, mappingName, javaParams, javaReturnType, out string renamed)) {
1011+
return renamed;
1012+
}
1013+
}
1014+
return value;
10091015
}
10101016

10111017
if (JniDescriptorText.IsValidMethodDescriptor (value) || JniDescriptorText.IsValidFieldDescriptor (value)) {
10121018
return JniDescriptorText.TryRewriteDescriptor (value, renameClass, out string rewritten) ? rewritten : null;
10131019
}
10141020

1015-
if (use.Role == Utf8Role.Unknown && mapping.TryMapClass (value, out string renamedClass)) {
1016-
return renamedClass;
1017-
}
1018-
10191021
return null;
10201022
}
10211023

0 commit comments

Comments
 (0)