[r8-obfuscation] Rewrite trimmable typemap JNI metadata - #12631
[r8-obfuscation] Rewrite trimmable typemap JNI metadata#12631simonrozsival wants to merge 4 commits into
Conversation
6f38f84 to
df5c3ee
Compare
df5c3ee to
7aa7696
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
3373599 to
4cb3e0a
Compare
4cb3e0a to
97d02a5
Compare
97d02a5 to
d05381e
Compare
d05381e to
80bca70
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
80bca70 to
6d05845
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
6d05845 to
e2031e4
Compare
e2031e4 to
3eeeaaa
Compare
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new custom-attribute string-array rewriter reads the array length using host endianness and doesn’t validate the required attribute prolog, which can misparse blobs and should be fixed before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/CustomAttributeStringRewriter.cs — ❌ The custom attribute string-array rewriter doesn’t validate the 0x0001 prolog and reads the array… |
|
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs — 💡 ReadInlineMethodTokens is now used for ldsflda (field) tokens as well as method tokens, so… |
What changed in this PR
This PR extends the R8 JNI name remapping pipeline to cover trimmable typemap metadata that is stored outside simple ldstr strings, so managed JNI metadata stays consistent with R8 mappings when obfuscation is enabled.
Changes:
- Add support for rewriting FieldRVA-backed null-terminated UTF-8 JNI data (method names and descriptors) using R8 mappings, with conflict detection when shared data would require incompatible rewrites.
- Extend custom-attribute rewriting to cover
TypeMapAttribute<T>keys andJavaPeerAliasesAttributestring arrays. - Update the trimmable typemap generator to deduplicate signatures but not method names, since R8 member renames are owner-specific.
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs | Updates generator tests to reflect signature-only deduplication and adds field-token assertions. |
| src/Xamarin.Android.Build.Tasks/Utilities/MetadataExtensions.cs | Improves GetCustomAttributeFullName handling for generic TypeSpecification parents (TypeRef/TypeDef). |
| src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlanner.cs | Plans rewrites for FieldRVA UTF-8 data, TypeMapAttribute keys, and JavaPeerAliasesAttribute arrays; adds JavaPeerProxy owner resolution via .ctor IL. |
| src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniAssemblyRewriter.cs | Plumbs FieldRvaTable into rewrite planning and reverse-scan validation. |
| src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/CustomAttributeStringRewriter.cs | Documents expanded scope and adds a helper to rewrite string-array fixed arguments. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniAssemblyRewriterTests.cs | Adds coverage for typemap key/alias rewriting and FieldRVA UTF-8 rewrite behavior and failure modes. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/TypeMapAssemblyEmitter.cs | Filters registrations to those with wrappers and prepares UTF-8 fields as (shared signatures, unique method names). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/PEAssemblyBuilder.cs | Splits UTF-8 FieldRVA preparation into shared vs unique caches and exposes GetUniqueUtf8Field. |
There was a problem hiding this comment.
Reviewed the JNI metadata rewrite, owner-specific FieldRVA allocation, reverse-scan behavior, alias rewriting, wrapper reuse, and the accompanying regression coverage. I found no actionable issues in the changed lines.
CI build #1580573 is still in progress; the currently visible checks show no failures, so this is not yet a green-build determination.
Generated by Android PR Reviewer for #12631 · gpt56 · 322.2 AIC · ⌖ 12.9 AIC · ⊞ 25.7K
Comment /review to run again
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
3eeeaaa to
351afd5
Compare
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>


Related to #12535
Depends on #12630
Layer 4 of 6 in the replacement stack for PR #12575. Build-pipeline activation follows in layers 5 and 6.