Repository navigation
[r8-obfuscation] Enable R8 JNI name obfuscation for NativeAOT - #12634
simonrozsival wants to merge 4 commits into
Conversation
f51fbae to
f88d2f4
Compare
a7bff4c to
a8fff0b
Compare
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
1cee050 to
65740e9
Compare
4c591ed to
80dbc97
Compare
2f3c599 to
5e41251
Compare
80dbc97 to
2099dc9
Compare
5e41251 to
cbc7457
Compare
2099dc9 to
07eaa87
Compare
cbc7457 to
61d0a23
Compare
07eaa87 to
7c2485f
Compare
cbb6df2 to
f547135
Compare
61d0a23 to
ed2522d
Compare
f547135 to
ec9422a
Compare
ec9422a to
c544a34
Compare
60b031c to
15c4b79
Compare
c544a34 to
1912f90
Compare
|
Why is this rewriting necessary? Can't we tell R8 not to rename things we need? There must be an escape hatch for reflection. What we're doing is reflection. Bugs in rewritten assemblies pretty much always land on runtime team's plate to troubleshoot. dotnet/maui#37782 (comment) was the most recent one and that one was extra annoying because people who knew about the rewriting (they edited the rewriter files in the past) were on the thread and did not say there was a rewrite. I'm not a big fan of these approaches. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
NativeAOT correctly removes an unreferenced managed peer from the final R8 reachability set. Keep the test peer live through MainActivity and assert its exact reachability provenance before checking the final mapping. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The generated ACW keep file is intentionally excluded from final R8, so NativeAOT correctly removed the unreferenced library peer. Reference it from the app and assert its reachability provenance before validating the public mapping. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
15c4b79 to
6a05a22
Compare
1912f90 to
4226ed9
Compare
|
@MichalStrehovsky we are looking into ways to support Google's new requirements on app optimizations (https://android-developers.googleblog.com/2026/08/app-quality-memory-optimization-secure-onboarding.html). We're currently using a |
|
@simonrozsival Michal's question was whether you can meet the requirement by generating fine-grained |
|
I'm trying this out as an alternative, but we will have to submit an app to Google Play to test the full end-to-end: |
|
I see. We'll explore other other options which won't require assembly rewriting. We can revisit the idea of changing the Java type name and signature constants directly in the string constants only if we can't maintain reasonable performance. Thanks for the feedback. |
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
⚠️ Needs Changes
Found 1 error: the new NativeAOT configuration task unconditionally rewrites outputs, breaking timestamp preservation and causing avoidable R8/downstream packaging work on incremental builds. The two-pass NativeAOT/R8 wiring and accompanying coverage otherwise align with the stated design.
CI is currently red: 42/44 checks passed, with dotnet-android (Package Tests macOS > Tests > APKs 2) and the aggregate dotnet-android check failing. Azure build details could not be retrieved from this environment, so the package-test failure remains unclassified.
Generated by Android PR Reviewer for #12634 · gpt56 · 506.1 AIC · ⌖ 9.11 AIC · ⊞ 25.7K
Comment /review to run again
| } else { | ||
| GenerateProguardConfiguration.WriteMappedRules (writer, reachableR8Entries); | ||
| } | ||
| File.WriteAllText (OutputFile, writer.ToString ()); |
There was a problem hiding this comment.
🤖 ❌ MSBuild tasks — This replaces the content-preserving Files.CopyIfStringChanged() with an unconditional write. When this target reruns but produces unchanged rules, it touches _ProguardProjectConfiguration; line 151 similarly touches the reachability manifest. Both are _CompileToDalvik inputs, so this unnecessarily reruns R8 and downstream packaging. Please preserve timestamps for both outputs and cover a same-content producer rerun.
Rule: Use Files.CopyIfStringChanged() (Postmortem #53)
## Summary Remove the experimental build-time managed-assembly rewriting approach for R8 so any future assembly-rewriting design can start from a clean foundation. This cleanup is related to #12535 and the original prototype in #12575. ## What this PR undoes This removes the managed rewriting implementation developed across the R8 obfuscation stack: - #12629 added the PE metadata rebuild substrate. - #12630 added managed JNI metadata rewriting from R8 mappings. - #12631 added rewriting for generated trimmable type-map assemblies. - #12632 and #12634 integrated and tested that rewriting approach for CoreCLR and NativeAOT. Concretely, this PR removes the rewrite task, rewrite-only metadata/IL utilities, dedicated tests and fixtures, and the XA4325/XA4326 resources and documentation. The intent is to abandon this implementation rather than preserve an unused rewriting stack that a future design would need to work around. ## What remains in place - The generic `R8Mapping` parser introduced in #12628 remains, along with its tests and the `MSBuildDeviceIntegration` consumer. It is independently useful for reading R8 mapping files. - The ordinary R8 configuration and `private-members` obfuscation/optimization policy from #12668 remain unchanged. This PR does **not** disable R8 or remove private-member obfuscation. - Existing D8/R8 packaging, ProGuard rule handling, and non-rewriting build behavior remain unchanged. - Runtime remapping remains active as the stacked follow-up work in #12847, #12848, #12692, and #12844. Those PRs implement the alternative opt-in strategy without managed assembly rewriting and are not part of this cleanup diff. We may revisit managed assembly rewriting in .NET 12 based on customer feedback and performance data, but with a fresh design rather than this implementation. ## Validation - Built `Xamarin.Android.Build.Tasks` - Ran `Microsoft.Android.Build.Tasks.Tests` - Ran `Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests` - Built `MSBuildDeviceIntegration` - Ran focused `R8MappingTests`
Related to #12535
Depends on #12632
Layer 6 of 6 and the top of the replacement stack for draft PR #12575. Extends the shared two-pass R8 JNI name-obfuscation pipeline to NativeAOT, including incremental pre-ILC assembly rewriting, conservative JNI class/member reachability, seed-compatible final keep rules, and prebuilt runtime JNI startup keeps.
Testing:
Xamarin.Android.Build.Tasks.csprojbuilds successfully.Xamarin.Android.Build.Tests.csprojbuilds successfully.TypeMapAssemblyGeneratorTests: 121/121 passed, including owner-specific JNI method-name FieldRVA coverage.NETSDK1147: To build this project, the following workloads must be installed: android; no unrelated workload was installed.