Repository navigation
[Microsoft.Android.Build.Tasks] Read LLVM typemap keys - #12823
simonrozsival wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Add regression coverage for preserving an existing output file when input validation fails.
Review effort: Lite
Findings: None
What changed in this PR
Adds an LLVM typemap extractor that generates canonical Java class key files for downstream ProGuard processing.
Changes:
- Supports modern byte strings and legacy
i8LLVM formats with XA4327 validation. - Normalizes, unions, and sorts class descriptors.
- Writes UTF-8, LF-delimited output and adds NUnit/xUnit coverage.
| File | Summary |
|---|---|
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/ExtractTypeMapKeysFromLlvmIrArrayTests.cs |
Tests array descriptor normalization and extraction. |
src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/ExtractTypeMapKeysFromLlvmIrTests.cs |
Tests parsing and validation; needs coverage confirming existing output remains unchanged after invalid input. |
src/Microsoft.Android.Build.Tasks/Tasks/ExtractTypeMapKeysFromLlvmIr.cs |
Implements LLVM typemap parsing and canonical key-file generation; includes a minor task-result consistency nit. |
bf8c356 to
d64c068
Compare
b9f98b3 to
39eed16
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
d64c068 to
0a3aa21
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Extract canonical Java class keys from CoreCLR LLVM string blobs without relying on optional comments. Preserve strict diagnostics and array normalization with standalone and real-emitter test fixtures. Layer 2 of 6 splitting #12821; runtime activation follows in a later layer. Co-authored-by: Copilot App <[email protected]>
0a3aa21 to
81049de
Compare
|
/review |
|
🚀 Android PR Reviewer has started processing this issue comment |
There was a problem hiding this comment.
⚠️ Needs Changes — CI pending
Findings: 0 errors · 0 warnings · 1 suggestion
The extractor is narrowly scoped, validates malformed and unsupported LLVM input thoroughly, and its tests cover both production-emitted blobs and legacy arrays. I left one non-blocking allocation suggestion in the parsing loop.
Azure build 1607882 currently has no reported failures, but only the Android Tools test lanes and CLA have completed; the platform builds and remaining checks are still running, so the PR is not yet green.
Generated by Android PR Reviewer for #12823 · copilot · gpt56 · 85.2 AIC · ⌖ 11.1 AIC · ⊞ 26K
Comment /review to run again
| nameBytes.Add (b); | ||
| return; | ||
| } | ||
| string name = Utf8.GetString (nameBytes.ToArray ()); |
There was a problem hiding this comment.
🤖 💡 Performance — nameBytes.ToArray() allocates a new byte[] for every Java type in the typemap, which can create substantial short-lived GC pressure for large applications. Since this task targets modern .NET, consider decoding CollectionsMarshal.AsSpan (nameBytes) with the span-based Utf8.GetString() overload before clearing the list.
(Rule: Avoid unnecessary allocations)
|
Closing because this LLVM IR typemap adapter did not make .NET 11, and LLVM IR typemaps are scheduled for removal in .NET 12. This layer should be omitted from the remaining typemap stack. |
## Why Layer 1 of 4 in the remaining #12821 stack, based on main. The foundation #12822 has merged; the LLVM adapter #12823 is closed and omitted. Typemap-derived ProGuard rules need the Java class keys that survive ILLink, rather than pre-link registration metadata or stale alias arrays. ## Changes Add `ExtractTypeMapKeysFromAssemblies` in the modern `Microsoft.Android.Build.Tasks` assembly. It reads surviving assembly-level generic `TypeMapAttribute` entries, unions and sorts their keys, normalizes alias suffixes and object-array descriptors, and writes canonical UTF-8 output. Empty linked stubs are valid; absent inputs, malformed assemblies/attributes/keys, and output failures report XA4327. Source-link the existing metadata helpers and enable unsafe blocks in the modern task project. Add the pinned ILLink package and its path metadata to the standalone test project, retaining its production-task `ProjectReference`. This layer does not activate the task in MSBuild or change R8 policy. Native-object extraction, orchestration, and legacy cleanup remain in later layers. The copied extractor and fixture exactly match frozen source `87bd3f8a8b512859339e5446c1f86709a6c28c3b`; the test-project native-object metadata is intentionally deferred to layer 4. ## Validation On .NET SDK `11.0.100-preview.7.26381.103`, the following focused standalone run passed **89/89**, with no skips, using the actual modern production task DLL. This includes `RealILLinkRetainsOnlyLiveTypeMapAttributes` with pinned `Microsoft.NET.ILLink.Tasks` `11.0.0-rc.2.26461.115`. ```sh dotnet test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj -v minimal --filter 'FullyQualifiedName~ExtractTypeMapKeysFromAssembliesTests|FullyQualifiedName~ExtractTypeMapKeysFromLlvmIrArrayTests|FullyQualifiedName~TypeMapProguardTests' ``` Base: `main`. Bottom of native stack #12893.
## Summary Layer 4 of 4 in native stack #12893, depending on #12827 (`simonrozsival-typemap-msbuild-pipeline`). Activate the shared retained-typemap rule pipeline so eligible CoreCLR and explicitly opted-in NativeAOT R8 builds derive Java class roots from retained typemap keys rather than keeping every ACW. This removes the dependency on large NativeAOT DGML graphs while preserving runtime/JNI roots and user-authored Java source retention. - Import the shared MSBuild pipeline, wire R8 flags/resources and incremental inputs, and gate legacy configuration generation. CoreCLR keeps scoped JNI-facing members and optimization while disabling renaming; NativeAOT retains class-wide member preservation and remains opt-in for NDK-backed object inspection. - Remove the old DGML producer, resource, diagnostics implementation, and graph-specific test fixtures; retain the interface-collection runtime test. - Add R8, build, and scoped-member device coverage; restore the three deferred `TypeMapProguardTests` methods and their attributes (17 cases); document retention behavior and retired diagnostics. ## Validation ```sh dotnet test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj \ -v minimal \ --filter 'FullyQualifiedName~TypeMapProguardTests|FullyQualifiedName~ExtractTypeMapKeys' \ -p:_AndroidTreatWarningsAsErrors=true \ -p:_NativeAotLlvmReadObjPath="$NDK_LLVM_BIN/llvm-readobj" ``` Passed **294/294**, **0 skipped**: 53 pipeline/generator cases (including all 17 restored cases) and 241 retained-key adapter cases. Used the supplied existing Android NDK `llvm-readobj` and adjacent `llvm-objdump`/`clang` executables read/execute-only. All six changed XML targets/resources parse successfully with `xmllint --nonet --noout`, including XML comments. `git diff --check` passes. **Not run:** `Xamarin.Android.Build.Tests` R8/build cases and `MSBuildDeviceIntegration` device cases. This worktree has neither a local built Android SDK (`bin/Debug/dotnet/dotnet` or Release) nor the corresponding test assemblies. No full SDK bootstrap or new tooling installation was performed for this frozen split. ## Frozen split integrity Exactly 28 paths, +601/-1081, relative to #12827. The final tree is exactly `168821457836836b55368bb7ad53c2bfd9833cb4`: frozen source `87bd3f8a8b512859339e5446c1f86709a6c28c3b` with the base XA1037 documentation/localization/JavaSourceUtils changes preserved. `TypeMapProguardTests.cs` is byte-identical to the frozen source. No non-English localization files changed. Native stack #12893 is registered with #12824, #12825, #12827, and this PR in order. The closed LLVM adapter #12823 is not in this stack.
Why
CoreCLR LLVM typemaps already contain the Java names needed for precise ProGuard rules. Extract those names from the generated data rather than optional LLVM comments, producing the canonical key-file input consumed by the foundation tasks.
Contract
Microsoft.Android.Tasks.ExtractTypeMapKeysFromLlvmIrto read the release/debug Java-name blob globals, supporting both current LLVM byte strings and legacy multilinei8arrays.xamarinbuildtasksalias for the actual legacy LLVM emitter.Stack scope
Layer 2 of 6 splitting #12821. Depends on #12822 and targets
simonrozsival-typemap-proguard-foundation.Only the LLVM extractor and its two test fixtures are added. Runtime/MSBuild target activation and R8 policy wiring follow in later layers; no other adapters, shared diagnostics, imports, or project-reference changes are included.
Validation
dotnet test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj -v minimal --filter 'FullyQualifiedName~ExtractTypeMapKeysFromLlvmIrArrayTests|FullyQualifiedName~TypeMapProguardTests': 31 passed, using the real production task DLL on .NET SDK11.0.100-preview.7.26381.103.ExtractTypeMapKeysFromLlvmIrTestsfixture. Its legacy host dependency build stops with MSB4062 becausebin/BuildDebug/net10.0/xa-prep-tasks.dllis absent. The fixture is preserved unchanged; no full SDK bootstrap was performed.87bd3f8a8b512859339e5446c1f86709a6c28c3b; the layer starts at L1 commitb9f98b3fcd22627c8f93e0d73d35a9d41773e44f.