From e0d2900425b3ddcf6a7366ea4f76fce47f398b97 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 1 Apr 2026 16:27:29 -0700 Subject: [PATCH 1/2] [build] Move RewriteMarshalMethods to the inner (per-RID) build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the RewriteMarshalMethods MSBuild task from the outer build (inside _GenerateJavaStubs) to a new _RewriteMarshalMethods target in the inner build, running after _PostTrimmingPipeline but before CreateReadyToRunImages and IlcCompile. This enables marshal methods to work with CoreCLR R2R builds, where Cecil cannot write mixed-mode assemblies produced by R2R. The task is now fully self-contained: it derives the target architecture from RuntimeIdentifier, creates its own XAAssemblyResolver, scans and classifies marshal methods via MarshalMethodsCollection.FromAssemblies, and rewrites assemblies in a single pass — no longer depending on in-process NativeCodeGenState from BuildEngine4. The outer build's GenerateJavaStubs re-classifies the already-rewritten assemblies, detecting _mm_wrapper methods and producing ConvertedMarshalMethodEntry objects. MarshalMethodCecilAdapter now merges ConvertedMarshalMethods into the object model so downstream native code generation (GenerateNativeMarshalMethodSources) produces correct callback tables. ConvertedMarshalMethodEntry sets NativeCallbackWrapper so the base NativeCallback property returns the wrapper method's metadata. --- ...crosoft.Android.Sdk.TypeMap.LlvmIr.targets | 27 ++- .../Tasks/RewriteMarshalMethods.cs | 162 ++++++++++-------- .../Utilities/MarshalMethodCecilAdapter.cs | 18 ++ .../Utilities/MarshalMethodsClassifier.cs | 7 + 4 files changed, 134 insertions(+), 80 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets index aa145ccb839..76f74839088 100644 --- a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets +++ b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets @@ -78,13 +78,6 @@ EnableNativeRuntimeLinking="$(_AndroidEnableNativeRuntimeLinking)"> - - - + + + + <_RewriteMarshalMethodsAssembly Include="@(ResolvedFileToPublish)" Condition=" '%(Extension)' == '.dll' " /> + + + + diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/RewriteMarshalMethods.cs b/src/Xamarin.Android.Build.Tasks/Tasks/RewriteMarshalMethods.cs index e8af07e4737..3d35b7483fb 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/RewriteMarshalMethods.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/RewriteMarshalMethods.cs @@ -1,6 +1,7 @@ #nullable enable -using System.Collections.Concurrent; +using System.Collections.Generic; using System.Linq; +using Java.Interop.Tools.Cecil; using Microsoft.Android.Build.Tasks; using Microsoft.Build.Framework; using Xamarin.Android.Tools; @@ -13,15 +14,22 @@ namespace Xamarin.Android.Tasks; /// attributes, significantly improving startup performance and reducing runtime overhead for Android applications. /// /// -/// This task operates on the marshal method classifications produced by earlier pipeline stages and: -/// -/// 1. Retrieves marshal method classifications from the build pipeline state -/// 2. Parses environment files to determine exception transition behavior -/// 3. Rewrites assemblies to replace dynamic registration with static marshal methods -/// 4. Optionally builds managed lookup tables for runtime marshal method resolution -/// 5. Reports statistics on marshal method generation and any fallback to dynamic registration -/// -/// The rewriting process creates native callback wrappers for methods that have non-blittable +/// This task runs in the inner (per-RID) build after ILLink and _PostTrimmingPipeline but before +/// CreateReadyToRunImages or IlcCompile. It is fully self-contained: it creates its own +/// assembly resolver, scans assemblies for marshal method candidates, classifies them, and rewrites them +/// in a single pass. +/// +/// The rewriting process: +/// +/// 1. Derives the target architecture from the +/// 2. Creates an assembly resolver with ReadWrite+InMemory mode for Cecil +/// 3. Scans assemblies to discover marshal method candidates via +/// 4. Parses environment files to determine exception transition behavior +/// 5. Rewrites assemblies to replace dynamic registration with static marshal methods +/// 6. Optionally builds managed lookup tables for runtime marshal method resolution +/// 7. Reports statistics on marshal method generation and any fallback to dynamic registration +/// +/// The rewriting creates native callback wrappers for methods that have non-blittable /// parameters or return types, ensuring compatibility with the [UnmanagedCallersOnly] attribute /// while maintaining proper marshaling semantics. /// @@ -32,6 +40,13 @@ public class RewriteMarshalMethods : AndroidTask /// public override string TaskPrefix => "RMM"; + /// + /// Gets or sets the resolved assemblies to scan and rewrite. + /// These are the post-ILLink assemblies from ResolvedFileToPublish in the inner build. + /// + [Required] + public ITaskItem [] Assemblies { get; set; } = []; + /// /// Gets or sets whether to enable managed marshal methods lookup tables. /// When enabled, generates runtime lookup structures that allow dynamic resolution @@ -47,42 +62,53 @@ public class RewriteMarshalMethods : AndroidTask public ITaskItem [] Environments { get; set; } = []; /// - /// Gets or sets the intermediate output directory path. Required for retrieving - /// build state objects that contain marshal method classifications. + /// Gets or sets the runtime identifier for this inner build (e.g., "android-arm64"). + /// Used to derive the target architecture and ABI for assembly scanning and rewriting. /// [Required] - public string IntermediateOutputDirectory { get; set; } = ""; + public string RuntimeIdentifier { get; set; } = ""; /// /// Executes the marshal method rewriting task. This is the main entry point that - /// coordinates the entire assembly rewriting process across all target architectures. + /// coordinates the entire assembly rewriting process for the current target architecture. /// /// /// true if the task completed successfully; false if errors occurred during processing. /// /// /// The execution flow is: - /// - /// 1. Retrieve native code generation state from previous pipeline stages - /// 2. Parse environment files for configuration (e.g., broken exception transitions) - /// 3. For each target architecture: - /// - Rewrite assemblies to use marshal methods - /// - Add special case methods (e.g., TypeManager methods) - /// - Optionally build managed lookup tables - /// 4. Report statistics on marshal method generation - /// 5. Log warnings for methods that must fall back to dynamic registration - /// + /// + /// 1. Derive the target architecture and ABI from the RuntimeIdentifier + /// 2. Build a dictionary of assemblies and set Abi metadata on each item + /// 3. Create an assembly resolver with ReadWrite+InMemory mode + /// 4. Scan assemblies and classify marshal methods via MarshalMethodsCollection.FromAssemblies + /// 5. Parse environment files for configuration (e.g., broken exception transitions) + /// 6. Rewrite assemblies to use marshal methods + /// 7. Add special case methods (e.g., TypeManager methods) + /// 8. Optionally build managed lookup tables + /// 9. Report statistics on marshal method generation + /// 10. Log warnings for methods that must fall back to dynamic registration + /// /// The task handles the ordering dependency between special case methods and managed /// lookup tables - special cases must be added first so they appear in the lookup tables. /// public override bool RunTask () { - // Retrieve the stored NativeCodeGenState from the build pipeline - // This contains marshal method classifications from earlier stages - var nativeCodeGenStates = BuildEngine4.GetRegisteredTaskObjectAssemblyLocal> ( - MonoAndroidHelper.GetProjectBuildSpecificTaskObjectKey (GenerateJavaStubs.NativeCodeGenStateRegisterTaskKey, WorkingDirectory, IntermediateOutputDirectory), - RegisteredTaskObjectLifetime.Build - ); + AndroidTargetArch arch = MonoAndroidHelper.RidToArch (RuntimeIdentifier); + string abi = MonoAndroidHelper.RidToAbi (RuntimeIdentifier); + + // Build the assemblies dictionary and set Abi metadata required by XAJavaTypeScanner + var assemblies = new Dictionary (Assemblies.Length); + foreach (var asm in Assemblies) { + asm.SetMetadata ("Abi", abi); + assemblies [asm.ItemSpec] = asm; + } + + // Create a resolver with ReadWrite+InMemory mode so Cecil can modify assemblies in place + using XAAssemblyResolver resolver = MonoAndroidHelper.MakeResolver (Log, useMarshalMethods: true, arch, assemblies); + + // Scan and classify marshal methods + MarshalMethodsCollection classifier = MarshalMethodsCollection.FromAssemblies (arch, [.. assemblies.Values], resolver, Log); // Parse environment files to determine configuration settings // We need to parse the environment files supplied by the user to see if they want to use broken exception transitions. This information is needed @@ -91,42 +117,33 @@ public override bool RunTask () var environmentParser = new EnvironmentFilesParser (); bool brokenExceptionTransitionsEnabled = environmentParser.AreBrokenExceptionTransitionsEnabled (Environments); - // Process each target architecture - foreach (var kvp in nativeCodeGenStates) { - NativeCodeGenState state = kvp.Value; - - if (state.Classifier is null) { - Log.LogError ("state.Classifier cannot be null if marshal methods are enabled"); - return false; - } - - // Handle the ordering dependency between special case methods and managed lookup tables - if (!EnableManagedMarshalMethodsLookup) { - // Standard path: rewrite first, then add special cases - RewriteMethods (state, brokenExceptionTransitionsEnabled); - state.Classifier.AddSpecialCaseMethods (); - } else { - // Managed lookup path: add special cases first so they appear in lookup tables - // We need to run `AddSpecialCaseMethods` before `RewriteMarshalMethods` so that we can see the special case - // methods (such as TypeManager.n_Activate_mm) when generating the managed lookup tables. - state.Classifier.AddSpecialCaseMethods (); - state.ManagedMarshalMethodsLookupInfo = new ManagedMarshalMethodsLookupInfo (Log); - RewriteMethods (state, brokenExceptionTransitionsEnabled); - } - - // Report statistics on marshal method generation - Log.LogDebugMessage ($"[{state.TargetArch}] Number of generated marshal methods: {state.Classifier.MarshalMethods.Count}"); - if (state.Classifier.DynamicallyRegisteredMarshalMethods.Count > 0) { - Log.LogWarning ($"[{state.TargetArch}] Number of methods in the project that will be registered dynamically: {state.Classifier.DynamicallyRegisteredMarshalMethods.Count}"); - } - - // Count and report methods that need blittable workaround wrappers - var wrappedCount = state.Classifier.MarshalMethods.Sum (m => m.Value.Count (m2 => m2.NeedsBlittableWorkaround)); - - if (wrappedCount > 0) { - // TODO: change to LogWarning once the generator can output code which requires no non-blittable wrappers - Log.LogDebugMessage ($"[{state.TargetArch}] Number of methods in the project that need marshal method wrappers: {wrappedCount}"); - } + // Handle the ordering dependency between special case methods and managed lookup tables + ManagedMarshalMethodsLookupInfo? managedLookupInfo = null; + if (!EnableManagedMarshalMethodsLookup) { + // Standard path: rewrite first, then add special cases + RewriteAssemblies (arch, classifier, resolver, managedLookupInfo, brokenExceptionTransitionsEnabled); + classifier.AddSpecialCaseMethods (); + } else { + // Managed lookup path: add special cases first so they appear in lookup tables + // We need to run `AddSpecialCaseMethods` before `RewriteMarshalMethods` so that we can see the special case + // methods (such as TypeManager.n_Activate_mm) when generating the managed lookup tables. + classifier.AddSpecialCaseMethods (); + managedLookupInfo = new ManagedMarshalMethodsLookupInfo (Log); + RewriteAssemblies (arch, classifier, resolver, managedLookupInfo, brokenExceptionTransitionsEnabled); + } + + // Report statistics on marshal method generation + Log.LogDebugMessage ($"[{arch}] Number of generated marshal methods: {classifier.MarshalMethods.Count}"); + if (classifier.DynamicallyRegisteredMarshalMethods.Count > 0) { + Log.LogWarning ($"[{arch}] Number of methods in the project that will be registered dynamically: {classifier.DynamicallyRegisteredMarshalMethods.Count}"); + } + + // Count and report methods that need blittable workaround wrappers + var wrappedCount = classifier.MarshalMethods.Sum (m => m.Value.Count (m2 => m2.NeedsBlittableWorkaround)); + + if (wrappedCount > 0) { + // TODO: change to LogWarning once the generator can output code which requires no non-blittable wrappers + Log.LogDebugMessage ($"[{arch}] Number of methods in the project that need marshal method wrappers: {wrappedCount}"); } return !Log.HasLoggedErrors; @@ -137,7 +154,10 @@ public override bool RunTask () /// Creates and executes the that handles /// the low-level assembly modification operations. /// - /// The native code generation state containing marshal method classifications and resolver. + /// The target Android architecture. + /// The marshal methods classifier containing method classifications. + /// The assembly resolver used to load and resolve assemblies. + /// Optional managed marshal methods lookup info for building lookup tables. /// /// Whether to generate code compatible with broken exception transitions. /// This affects how wrapper methods handle exceptions during JNI calls. @@ -150,13 +170,9 @@ public override bool RunTask () /// - Modifying assembly references and imports /// - Building managed lookup table entries /// - void RewriteMethods (NativeCodeGenState state, bool brokenExceptionTransitionsEnabled) + void RewriteAssemblies (AndroidTargetArch arch, MarshalMethodsCollection classifier, XAAssemblyResolver resolver, ManagedMarshalMethodsLookupInfo? managedLookupInfo, bool brokenExceptionTransitionsEnabled) { - if (state.Classifier == null) { - return; - } - - var rewriter = new MarshalMethodsAssemblyRewriter (Log, state.TargetArch, state.Classifier, state.Resolver, state.ManagedMarshalMethodsLookupInfo); + var rewriter = new MarshalMethodsAssemblyRewriter (Log, arch, classifier, resolver, managedLookupInfo); rewriter.Rewrite (brokenExceptionTransitionsEnabled); } } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodCecilAdapter.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodCecilAdapter.cs index 7c6119f5347..0a94faa874b 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodCecilAdapter.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodCecilAdapter.cs @@ -68,6 +68,24 @@ static NativeCodeGenStateObject CreateNativeCodeGenState (AndroidTargetArch arch obj.MarshalMethods.Add (group.Key, methods); } + // Also include methods that were already rewritten in the inner build. + // After the inner build rewrites assemblies, the outer build's GenerateJavaStubs + // re-classifies them and puts already-rewritten methods (with _mm_wrapper + + // [UnmanagedCallersOnly]) into ConvertedMarshalMethods instead of MarshalMethods. + // We must include these so that GenerateNativeMarshalMethodSources produces + // correct native callback tables. + foreach (var group in state.Classifier.ConvertedMarshalMethods) { + if (!obj.MarshalMethods.TryGetValue (group.Key, out var methods)) { + methods = new List (group.Value.Count); + obj.MarshalMethods.Add (group.Key, methods); + } + + foreach (var method in group.Value) { + var entry = CreateEntry (method, state.ManagedMarshalMethodsLookupInfo); + methods.Add (entry); + } + } + return obj; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs index a1929fbea35..df3889db265 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs @@ -240,6 +240,13 @@ public ConvertedMarshalMethodEntry (TypeDefinition declaringType, MethodDefiniti : base (declaringType, nativeCallback, connector, registeredMethod, implementedMethod, callbackField, jniTypeName, jniName, jniSignature, needsBlittableWorkaround) { ConvertedNativeCallback = convertedNativeCallback ?? throw new ArgumentNullException (nameof (convertedNativeCallback)); + + // Set NativeCallbackWrapper so that the base NativeCallback property returns the + // converted wrapper method (e.g. n_MyMethod_mm_wrapper) instead of the original + // native callback. Downstream consumers (MarshalMethodCecilAdapter, native code + // generators) rely on NativeCallback returning the wrapper's MetadataToken and + // FullName to produce correct native callback tables. + NativeCallbackWrapper = convertedNativeCallback; } } From 0ec9a28ba77fc982add22de6d8076b4cf7104437 Mon Sep 17 00:00:00 2001 From: Sven Boemer Date: Wed, 1 Apr 2026 16:57:30 -0700 Subject: [PATCH 2/2] [build] Fix ConvertedMarshalMethodEntry NativeCallback returning wrapper instead of original Remove NativeCallbackWrapper assignment from ConvertedMarshalMethodEntry so NativeCallback preserves its design contract of returning the original method. Instead, MarshalMethodCecilAdapter.CreateEntry() now explicitly checks for ConvertedMarshalMethodEntry and uses ConvertedNativeCallback for native callback tables, which need the wrapper's MetadataToken and [UnmanagedCallersOnly] attribute. --- .../Utilities/MarshalMethodCecilAdapter.cs | 12 ++++++++++-- .../Utilities/MarshalMethodsClassifier.cs | 7 ------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodCecilAdapter.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodCecilAdapter.cs index 0a94faa874b..ce2249b0945 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodCecilAdapter.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodCecilAdapter.cs @@ -91,6 +91,14 @@ static NativeCodeGenStateObject CreateNativeCodeGenState (AndroidTargetArch arch static MarshalMethodEntryObject CreateEntry (MarshalMethodEntry entry, ManagedMarshalMethodsLookupInfo? info) { + // For converted entries, use the wrapper method (ConvertedNativeCallback) for native + // callback tables — it has the correct MetadataToken and [UnmanagedCallersOnly] attribute. + // For regular entries, NativeCallback already returns the wrapper after the rewriter sets + // NativeCallbackWrapper. + MethodDefinition nativeCallbackMethod = entry is ConvertedMarshalMethodEntry converted + ? converted.ConvertedNativeCallback + : entry.NativeCallback; + var obj = new MarshalMethodEntryObject ( declaringType: CreateDeclaringType (entry.DeclaringType), implementedMethod: CreateMethod (entry.ImplementedMethod), @@ -98,12 +106,12 @@ static MarshalMethodEntryObject CreateEntry (MarshalMethodEntry entry, ManagedMa jniTypeName: entry.JniTypeName, jniMethodName: entry.JniMethodName, jniMethodSignature: entry.JniMethodSignature, - nativeCallback: CreateMethod (entry.NativeCallback), + nativeCallback: CreateMethod (nativeCallbackMethod), registeredMethod: CreateMethodBase (entry.RegisteredMethod) ); if (info is not null) { - (uint assemblyIndex, uint classIndex, uint methodIndex) = info.GetIndex (entry.NativeCallback); + (uint assemblyIndex, uint classIndex, uint methodIndex) = info.GetIndex (nativeCallbackMethod); obj.NativeCallback.AssemblyIndex = assemblyIndex; obj.NativeCallback.ClassIndex = classIndex; diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs index df3889db265..a1929fbea35 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsClassifier.cs @@ -240,13 +240,6 @@ public ConvertedMarshalMethodEntry (TypeDefinition declaringType, MethodDefiniti : base (declaringType, nativeCallback, connector, registeredMethod, implementedMethod, callbackField, jniTypeName, jniName, jniSignature, needsBlittableWorkaround) { ConvertedNativeCallback = convertedNativeCallback ?? throw new ArgumentNullException (nameof (convertedNativeCallback)); - - // Set NativeCallbackWrapper so that the base NativeCallback property returns the - // converted wrapper method (e.g. n_MyMethod_mm_wrapper) instead of the original - // native callback. Downstream consumers (MarshalMethodCecilAdapter, native code - // generators) rely on NativeCallback returning the wrapper's MetadataToken and - // FullName to produce correct native callback tables. - NativeCallbackWrapper = convertedNativeCallback; } }