Repository navigation
[Microsoft.Android.Build.Tasks] Move R8 remapping task to modern assembly - #12844
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A concrete correctness gap in MetadataRawColumns.GetExportedTypeDefinitionId() (missing row-number validation) should be fixed before merging to avoid confusing failures on malformed metadata.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
This PR relocates the R8 mapping + JNI rewrite subsystem (and related tests) into Microsoft.Android.Build.Tasks, wiring the SDK targets to load GenerateR8JniRemapping from the modern task assembly while keeping behavior consistent with the parent PR.
Changes:
- Switched
Microsoft.Android.Sdk.R8JniRemapping.targetsto loadMicrosoft.Android.Tasks.GenerateR8JniRemappingfromMicrosoft.Android.Build.Tasks.dllusing the existing Full/CoreUsingTaskpattern. - Added/ported JNI remapping + assembly-rewrite utilities (including NativeAOT object scanning via ELF) into
src/Microsoft.Android.Build.Tasks/Utilities/JniRemappingand updated/added focused unit tests. - Updated build/test projects and packaging to reference/copy the modern task assembly dependencies (including
ELFSharp.dll).
| File | Description |
|---|---|
| tests/MSBuildDeviceIntegration/MSBuildDeviceIntegration.csproj | Links R8Mapping.cs for net10-compatible device integration build usage. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests.csproj | Adds a project reference to Microsoft.Android.Build.Tasks for integration coverage. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.R8JniRemapping.targets | Loads GenerateR8JniRemapping from the modern task assembly and updates task assembly path normalization. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/R8Mapping.cs | Minor lookup/nullable adjustments in mapping logic. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/NativeResourceSectionCopier.cs | Adds PE Win32 resource-section relocation helper for rebuilt assemblies. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/NativeAotJniRetention.cs | Adds NativeAOT ELF-based retention scanning to select required mapping entries. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/MetadataRawColumns.cs | Adds raw metadata-table column readers for columns not exposed by MetadataReader. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/MetadataEncoding.cs | Adds ECMA-335 compressed integer encode/decode helpers for blob rewriting. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/LdstrRewriter.cs | Adds ldstr classification/rewriting for JNI-bearing literal patterns. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlanner.cs | Adds “plan” pass to discover exact metadata/IL changes needed for rewrite. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/JniRewritePlan.cs | Adds storage for planned CA/US/FieldRVA replacements keyed by use site. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/JniRewriteException.cs | Adds dedicated exception type for rewrite failures/malformed inputs. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/JniDescriptorText.cs | Adds JNI descriptor parsing/rewriting helpers (source ↔ token conversions). |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/JniAssemblyRewriter.cs | Adds top-level rewrite/scan entrypoints for rebuilding managed assemblies. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/IlOpcodeTable.cs | Adds minimal IL operand-size table used by the IL scanner. |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/FieldRvaTable.cs | Adds FieldRVA reader/decoder for mapped field data (incl. UTF-8 JNI data). |
| src/Microsoft.Android.Build.Tasks/Utilities/JniRemapping/CustomAttributeStringRewriter.cs | Adds targeted fixed-string rewriting for custom attribute value blobs. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/Utilities/JniRemapping/R8MappingTests.cs | Adds unit tests validating mapping parsing/lookup behavior. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/Utilities/JniRemapping/NativeResourceSectionCopierTests.cs | Adds unit tests validating resource-section relocation and corruption handling. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/Utilities/JniRemapping/LdstrRewriterTests.cs | Adds unit tests for ldstr rewriting across supported literal forms. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/Utilities/JniRemapping/JniFixtureBuilder.cs | Adds PE fixture builder used by end-to-end rewrite tests. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/Utilities/JniRemapping/JniDescriptorTextTests.cs | Adds unit tests for descriptor parsing/validation/conversion helpers. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/Utilities/JniRemapping/AssemblyRebuilderTests.cs | Adds end-to-end assembly rebuild/edge-case tests (resources, FieldRVA, SN). |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/Tasks/RewriteJniNamesForR8Tests.cs | Updates tests to reference Microsoft.Android.Tasks task namespace. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/Tasks/GenerateR8JniRemappingTests.cs | Updates tests for task move and keeps aliased coverage for legacy tasks. |
| src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/JniAssemblyRewriterTypeMapTests.cs | Updates tests to use non-aliased Xamarin.Android.Tasks.JniRemapping types. |
| src/Microsoft.Android.Build.Tasks/Tasks/RewriteJniNamesForR8.cs | Moves task to Microsoft.Android.Tasks namespace and uses invariant formatting for errors. |
| src/Microsoft.Android.Build.Tasks/Tasks/GenerateR8JniRemapping.cs | Moves task to Microsoft.Android.Tasks namespace and normalizes string.Format culture usage. |
| src/Microsoft.Android.Build.Tasks/Microsoft.Android.Build.Tasks.csproj | Adds ELFSharp dependency, enables unsafe blocks, and links shared localized resources. |
| build-tools/installers/create-installers.targets | Ensures ELFSharp.dll is included in installer MSBuild payload. |
12ec1e8 to
4eca0b9
Compare
4eca0b9 to
9d74331
Compare
## 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`
9d74331 to
3a222f8
Compare
3a222f8 to
1fe751e
Compare
1fe751e to
b9cc984
Compare
b9cc984 to
299d5af
Compare
299d5af to
71f7a58
Compare
Depends on #12846. This is the consolidated runtime foundation for the R8 runtime-remapping stack. It supersedes the earlier split runtime implementation in #12796 and #12817 without abandoning that workstream. It adds the generic JNI remapping support shared by Java.Interop, Mono.Android, MonoVM, CoreCLR, and NativeAOT, including forward/reverse type remapping, descriptor-aware method and constructor remapping, field remapping, Java hiding/fallback behavior, and focused tests. The generated application object contains read-only remapping tables and a single `jni_remapping_data` descriptor. Native startup passes that descriptor to managed initialization; type, reverse-type, method, and field lookup algorithms remain in `JniRemappingLookup.cs`. No C++ remapping lookup implementation or lookup P/Invokes are introduced. This PR intentionally contains no R8 build orchestration and no managed assembly rewriting. Producer-side R8 mapping ingestion and generated-table wiring are in the next stack layer, #12848; public mode and build orchestration follow in #12692, with the modern task-assembly boundary in #12844. ## APK size and disabled-remapping footprint The measured Simple/CoreCLR baselines were refreshed from the complete test attachments in [CI build 1618183](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1618183): | Configuration | Previous APK bytes | Current APK bytes | Change | |---|---:|---:|---:| | Without R8 | 6,526,395 | 6,530,491 | +4,096 (0.063%) | | With R8 | 6,526,395 | 6,526,395 | 0 | The test failure was the **per-file** threshold on `libxamarin-app.so`, not a large APK regression: that library grew from 11,744 to 12,368 bytes (+624, 5.05%). Isolated re-links using the archived CI object files attribute this exactly: - **400 bytes** for the expanded empty remapping ABI: the 48-byte descriptor, reverse-type and field placeholders, and their ELF symbol/hash/string/relocation bookkeeping. - **224 bytes** for the two runtime configuration entries that explicitly disable remapping. Removing only these entries produces 12,144 bytes; replacing only the remapping object with the previous two-table layout produces 11,968 bytes. Removing both reproduces the previous 11,744-byte library. The `.text` section remains **36 bytes** in all four variants; this is fixed data/configuration overhead, not added remapping executable code. For this no-remapping application, all four table counts are zero and both remapping switches are configured `false`. Inspection of the actual linked **`Mono.Android.Runtime.dll`** confirms that `JniRemappingLookup` is absent. The earlier approximately 100 KiB cost from retaining the managed remapping implementation has not returned. The full baseline comparison also crosses the **.NET 11 RC2 to .NET 12 alpha update inherited from `main` in #12939**, so its other changes must not be attributed wholesale to this PR. The APK's CoreCLR, JIT, globalization, and System.Native binaries are byte-identical to the new runtime pack; the cached previous runtime pack matches the old reference sizes. For example, `libcoreclr.so` shrank 103,792 uncompressed bytes, while `libclrjit.so` grew 15,896 bytes. The assembly store grew 23,880 bytes and `libmonodroid.so` shrank 2,896 bytes. Summed across all entries, **uncompressed contents actually shrank 65,868 bytes**. APK size measures the **compressed and signed archive**, not the sum of those uncompressed sizes. The current APK uses DEFLATE for its native libraries, an 8 KiB signing block, and 4 KiB signing alignment. The nearest retained pre-upgrade comparison, [CI build 1617831](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1617831), shows the larger assembly store (+23,216 compressed bytes) and JIT (+6,970) almost offset by the smaller CoreCLR (-28,694) and other entries: compressed payload grows just **1,034 bytes**, while ZIP/signing/alignment overhead grows **3,062 bytes**, producing the observed **4 KiB APK step**. `libxamarin-app.so` itself adds only **107 compressed bytes** in that comparison. **Comparison limitation:** the exact reference APK was generated locally before the runtime update and was not retained. Build 1617831 has slightly different dex/store/monodroid entries, although its total APK size matches the reference. The compressed/padding breakdown is therefore an explicitly identified historical CI comparison, not an exact reconstruction of the local reference or a same-toolchain `main`-versus-PR A/B test. The isolated 400/224-byte native attribution and removal of the managed lookup are independently confirmed.
33ee7b2 to
a9c0f83
Compare
be2039e to
b4711c1
Compare
Context: #12535 This is the product policy and build-orchestration layer for opt-in R8 runtime JNI remapping. It is stacked on: - #12846 — removes the abandoned managed-assembly rewriting implementation - #12847 — adds generic runtime JNI remapping support - #12848 — parses the final R8 mapping and generates native runtime lookup tables A final dependent PR, #12844, moves the .NET-only R8 mapping task into `Microsoft.Android.Build.Tasks`. This PR does not rewrite managed assemblies. Managed bindings retain their original JNI names; the lower layers generate native tables that translate lookups to the names and descriptors emitted by R8. ## Opt-in For a trimmed CoreCLR or NativeAOT application: ```xml <PropertyGroup Condition="'$(Configuration)' == 'Release'"> <AndroidLinkTool>r8</AndroidLinkTool> <AndroidTypeMapImplementation>trimmable</AndroidTypeMapImplementation> <PublishTrimmed>true</PublishTrimmed> <AndroidR8ObfuscationMode>runtime-remapping</AndroidR8ObfuscationMode> </PropertyGroup> ``` `runtime-remapping` is the sole opt-in; there is no separate enable property. It does not apply to library projects. The existing `private-members` and `disabled` behavior remains unchanged. Unknown mode values report XA1050, while incompatible runtime-remapping configurations report XA4329. ## Build orchestration - Runs R8 once, after ILLink or all per-RID NativeAOT ILC compilations. - Converts that final mapping into runtime remapping tables before native linking. - Defers NativeAOT linking until the shared R8 pass completes, then relinks each RID without rerunning ILC. - Applies the runtime-remapping-specific bootstrap, native-callback, manifest, resource, and JNI safety keep rules. - Preserves existing MAM remapping precedence and asset handling. - Tracks mapping inputs, generated XML, native table sources, task assemblies, native-link inputs, configuration changes, missing-output recovery, and opt-out for incremental builds. - Preserves generated ProGuard rule timestamps when their contents are unchanged so managed-only rebuilds do not rerun R8 unnecessarily. ## Coverage in this layer Host tests cover mode defaults, configuration validation, keep-rule policy, NativeAOT ProGuard configuration, packaging metadata, and incremental behavior. Build/device integration tests cover renamed types and members, constructors, overloads, inherited lookups, peer activation, single-pass ordering, multi-RID builds, missing-output recovery, and opt-out for CoreCLR and NativeAOT. The generic lookup semantics and mapping/table-generation tests live in #12847 and #12848 respectively. ## Experimental limitations NativeAOT literal matching is conservative and can retain extra entries. Arbitrarily computed JNI names may require explicit remaps or keep rules. Conservative public/nested-class, interface, bootstrap, and native-callback keeps limit obfuscation. Existing Intune/R8 conflict handling is not full remapping-chain composition, and ambiguous reverse mappings for merged classes are omitted. This remains an experimental opt-in, not a production-readiness claim. ---- - [x] Useful description of why the change is necessary. - [x] Links to related issues and dependent layers. - [x] Unit and integration coverage. Fixes: #12535
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
Co-authored-by: Copilot App <[email protected]>
31f99c7 to
2254727
Compare
e56e073 to
beecab4
Compare
|
@dalexsoto review |
dalexsoto
left a comment
There was a problem hiding this comment.
The complete 19-file correctness and integration review leaves one incremental-build blocker. The modern/legacy task split, NET task registration, and separated test layering are accepted; no obsolete rewrite subsystem needs to be restored.
Keep the legacy task DLL in the mixed targets' inputs (Microsoft.Android.Sdk.R8JniRemapping.targets:12). _AndroidR8JniTaskAssembly now names only the modern DLL, and the four targets' Inputs at lines 27, 45, 83 and 103 consequently lose the legacy DLL timestamp. Those targets still execute legacy MergeRemapXml, GenerateJniRemappingNativeCode, and CompileNativeAssembly.
For a source-built local workload, build an app, rebuild only the legacy task DLL under the same SDK version, then rebuild the unchanged app. With the modern DLL, mapping, managed/ILC inputs and cache unchanged, these targets stay up-to-date despite an implementation change. Merged XML, LLVM sources and remapping objects can retain the previous implementation's output until Clean or another input changes. The property cache tracks version/settings/assets hash, not task-DLL identity or timestamp, and preserves its timestamp with WriteOnlyWhenDifferent. Import tracking does not include task DLLs; skipped bodies do not execute Touch, and downstream linking sees unchanged objects. A version-changing upgrade invalidates the cache, but does not cover this same-version update path.
Add the normalized legacy task-assembly path alongside the modern DLL in all four targets' inputs. That preserves the intended assembly boundary and restores the pre-relocation dependency behavior. This is a deterministic source-level dependency proof, not an executed reproduction or CI-failure claim. The separate completeness pass and final independent search found no additional actionable blocker.
Include both task assembly timestamps in all four mixed R8 JNI-remapping targets so same-version legacy task updates invalidate generated output. Cover modern and legacy DLL changes, absolute and relative paths, and unchanged-build skipping without referencing the legacy task project. Co-authored-by: Copilot App <[email protected]>
|
@dalexsoto Addressed in db967ca. All four R8 JNI-remapping targets now include the normalized legacy task DLL alongside the modern DLL in their Added 16 executed MSBuild incremental regressions covering the four targets, both DLLs, and relative/absolute paths. The tests retain the shipped input expressions and path-normalization target, substituting the task body to isolate the incremental decision. They verify initial execution, an unchanged-build skip, regeneration after changing only one DLL timestamp, and another unchanged-build skip. Before the fix, all eight legacy-DLL cases failed while the eight modern-DLL cases passed; after the fix, all 16 pass. Validation on macOS: dotnet test src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/Microsoft.Android.Build.Tasks.Tests.csproj -v minimal -p:_AndroidTreatWarningsAsErrors=true -p:_NativeAotLlvmReadObjPath=/Users/simon/android-toolchain/ndk/toolchains/llvm/prebuilt/darwin-x86_64/bin/llvm-readobj --filter "FullyQualifiedName~TypeMapProguardTargetsTests|FullyQualifiedName~GenerateR8JniRemappingTests|FullyQualifiedName~R8MappingTests|FullyQualifiedName~JniDescriptorTextTests|FullyQualifiedName~JniRemappingAssemblyScannerTests" --logger "trx;LogFileName=alex-r8-after-ndk.trx" --results-directory bin/TestDebug/ReviewValidationResult: 259 passed, 1 skipped, 0 failed, including all 16 new regressions. The skipped test requires explicit |
dalexsoto
left a comment
There was a problem hiding this comment.
All four mixed R8-remapping targets now track the normalized modern and legacy task DLLs, fixing the stale-output invalidation issue while preserving task registration and the assembly boundary. The current whole-change and end-to-end review leaves no blocking issue.

Summary
This PR is stacked on and depends on #12692. That parent is built on the focused lower chain #12846, #12847, and #12848.
This final layer is only a task-assembly boundary change for the runtime-R8 remapping producer. It moves
GenerateR8JniRemappingand the .NET-only helpers it directly requires intoMicrosoft.Android.Build.Tasks:JniRemappingAssemblyScannerR8MappingJniDescriptorTextNativeAotJniRetentionThe generic runtime table producer remains in
Xamarin.Android.Build.Tasks:GenerateJniRemappingNativeCodeandJniRemappingNativeCodeGeneratorare unchanged and continue to serve both MAM and R8 remapping.The relocation also:
GenerateR8JniRemappingfrom the modern assembly using the existing Full/CoreUsingTaskpatternELFSharp.dllbeside the modern task assembly undertools/netMSBuildDeviceIntegrationcompatible by linkingR8Mapping.csinstead of referencing the net11 task projectMergeRemapXmlandGenerateJniRemappingNativeCodeThis PR does not restore or relocate the removed managed assembly rewriter, rewrite planners/rebuilders, rewrite-only tests, or
XA4325/XA4326behavior.Validation
Microsoft.Android.Build.TasksbuildXamarin.Android.Build.TasksbuildMSBuildDeviceIntegrationproject buildtools/net/ELFSharp.dll