From 4e32f8f598455855b688d0f8843fdb04192124ab Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 22 Apr 2025 18:53:26 +0200 Subject: [PATCH 01/24] Changes from https://github.com/dotnet/android/pull/10065 --- .../Utilities/TypeMapCecilAdapter.cs | 10 ++- .../Utilities/TypeMapGenerator.cs | 2 +- ...eMappingDebugNativeAssemblyGeneratorCLR.cs | 73 ++++--------------- .../Xamarin.Android.Common.targets | 36 ++++++++- src/native/clr/host/typemap.cc | 28 +++++++ 5 files changed, 83 insertions(+), 66 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapCecilAdapter.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapCecilAdapter.cs index e48923c4771..ab8288ff33c 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapCecilAdapter.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapCecilAdapter.cs @@ -32,8 +32,10 @@ public static (TypeMapDebugDataSets dataSets, bool foundJniNativeRegistration) G var managedToJava = new List (); var foundJniNativeRegistration = false; - foreach (var td in types) { - foundJniNativeRegistration = JniAddNativeMethodRegistrationAttributeFound (foundJniNativeRegistration, td); + var javaDuplicates = new Dictionary> (StringComparer.Ordinal); + var uniqueAssemblies = needUniqueAssemblies ? new Dictionary (StringComparer.OrdinalIgnoreCase) : null; + foreach (TypeDefinition td in state.AllJavaTypes) { + UpdateApplicationConfig (state, td); TypeMapDebugEntry entry = GetDebugEntry (td, cache); HandleDebugDuplicates (javaDuplicates, entry, td, cache); @@ -60,11 +62,11 @@ public static (TypeMapDebugDataSets dataSets, bool foundJniNativeRegistration) G SyncDebugDuplicates (javaDuplicates); - return (new TypeMapDebugDataSets { + return new TypeMapDebugDataSets { JavaToManaged = javaToManaged, ManagedToJava = managedToJava, UniqueAssemblies = uniqueAssemblies != null ? new List (uniqueAssemblies.Values) : null - }, foundJniNativeRegistration); + }; } public static ReleaseGenerationState GetReleaseGenerationState (NativeCodeGenState state) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs index 7a22e76f2a0..3183f49154b 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs @@ -155,7 +155,7 @@ public void Generate (bool debugBuild, bool skipJniAddNativeMethodRegistrationAt void GenerateDebugNativeAssembly (string outputDirectory) { - TypeMapDebugDataSets dataSets = state.GetDebugNativeEntries (needUniqueAssemblies: runtime == AndroidRuntime.CoreCLR); + TypeMapDebugDataSets dataSets = TypeMapCecilAdapter.GetDebugNativeEntries (state, needUniqueAssemblies: runtime == AndroidRuntime.CoreCLR); var data = new ModuleDebugData { EntryCount = (uint)dataSets.JavaToManaged.Count, diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs index 7990cd12e51..b93189f3b8c 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs @@ -26,14 +26,15 @@ sealed class TypeMapContextDataProvider : NativeAssemblerStructContextDataProvid public override ulong GetBufferSize (object data, string fieldName) { var map_module = EnsureType (data); - return fieldName switch { - "java_to_managed" => map_module.entry_count, - "managed_to_java" => map_module.entry_count, - _ => 0 - }; + if (String.Compare ("java_to_managed", fieldName, StringComparison.Ordinal) == 0 || + String.Compare ("managed_to_java", fieldName, StringComparison.Ordinal) == 0) { + return map_module.entry_count; + } + + return 0; } - public override string? GetPointedToSymbolName (object data, string fieldName) + public override string GetPointedToSymbolName (object data, string fieldName) { var map_module = EnsureType (data); @@ -108,38 +109,11 @@ public override string GetComment (object data, string fieldName) [NativeAssemblerStructContextDataProvider (typeof (TypeMapEntryContextDataProvider))] sealed class TypeMapEntry { - [NativeAssembler (Ignore = true)] - public string From = String.Empty; - - [NativeAssembler (Ignore = true)] - public string To = String.Empty; - [NativeAssembler (UsesDataProvider = true)] - public uint from; - - [NativeAssembler (NumberFormat = LlvmIrVariableNumberFormat.Hexadecimal)] - public ulong from_hash; + public string from; [NativeAssembler (UsesDataProvider = true)] - public uint to; - }; - - // Order of fields and their type must correspond *exactly* to that in - // src/native/clr/include/xamarin-app.hh TypeMapManagedTypeInfo structure - [NativeAssemblerStructContextDataProvider (typeof (TypeMapManagedTypeInfoContextDataProvider))] - sealed class TypeMapManagedTypeInfo - { - [NativeAssembler (Ignore = true)] - public string AssemblyName = String.Empty; - - [NativeAssembler (Ignore = true)] - public string ManagedTypeName = String.Empty; - - [NativeAssembler (UsesDataProvider = true)] - public uint assembly_name_index; - - [NativeAssembler (UsesDataProvider = true, NumberFormat = LlvmIrVariableNumberFormat.Hexadecimal)] - public uint managed_type_token_id; + public string to; }; // Order of fields and their type must correspond *exactly* to that in @@ -169,7 +143,7 @@ sealed class TypeMap sealed class TypeMapAssembly { [NativeAssembler (Ignore = true)] - public string Name = String.Empty; + public string Name; [NativeAssembler (Ignore = true)] public Guid MVID; @@ -183,15 +157,13 @@ sealed class TypeMapAssembly } readonly TypeMapGenerator.ModuleDebugData data; - StructureInfo? typeMapEntryStructureInfo; - StructureInfo? typeMapStructureInfo; - StructureInfo? typeMapAssemblyStructureInfo; - StructureInfo? typeMapManagedTypeInfoStructureInfo; + StructureInfo typeMapEntryStructureInfo; + StructureInfo typeMapStructureInfo; + StructureInfo typeMapAssemblyStructureInfo; List> javaToManagedMap; List> managedToJavaMap; List> uniqueAssemblies; - List> managedTypeInfos; - StructureInstance? type_map; + StructureInstance type_map; public TypeMappingDebugNativeAssemblyGeneratorCLR (TaskLoggingHelper log, TypeMapGenerator.ModuleDebugData data) : base (log) @@ -212,10 +184,6 @@ protected override void Construct (LlvmIrModule module) { module.DefaultStringGroup = "tmd"; - if (data.UniqueAssemblies == null) { - throw new InvalidOperationException ("Internal error: unique assemblies collection must be present"); - } - MapStructures (module); var managedTypeNames = new LlvmIrStringBlob (); @@ -288,18 +256,7 @@ protected override void Construct (LlvmIrModule module) }; uniqueAssemblies.Add (new StructureInstance (typeMapAssemblyStructureInfo, entry)); } - - uniqueAssemblies.Sort ((StructureInstance a, StructureInstance b) => { - if (a.Instance == null) { - return b.Instance == null ? 0 : -1; - } - - if (b.Instance == null) { - return 1; - } - - return a.Instance.mvid_hash.CompareTo (b.Instance.mvid_hash); - }); + uniqueAssemblies.Sort ((StructureInstance a, StructureInstance b) => a.Instance.mvid_hash.CompareTo (b.Instance.mvid_hash)); var managedTypeInfos = new List> (); // Java-to-managed maps don't use hashes since many mappings have multiple instances diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index 6421833e60e..def9301bd9a 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -1880,9 +1880,39 @@ because xbuild doesn't support framework reference assemblies. + ResolvedAssemblies="@(_ResolvedAssemblies)" + ResolvedUserAssemblies="@(_ResolvedUserAssemblies)" + SatelliteAssemblies="@(_AndroidResolvedSatellitePaths)" + IntermediateOutputDirectory="$(IntermediateOutputPath)" + NativeLibraries="@(AndroidNativeLibrary);@(EmbeddedNativeLibrary);@(FrameworkNativeLibrary)" + MonoComponents="@(_MonoComponent)" + MainAssembly="$(TargetPath)" + OutputDirectory="$(_AndroidIntermediateJavaSourceDirectory)mono" + EnvironmentOutputDirectory="$(IntermediateOutputPath)android" + TargetFrameworkVersion="$(TargetFrameworkVersion)" + Manifest="$(IntermediateOutputPath)android\AndroidManifest.xml" + Environments="@(_EnvironmentFiles)" + AndroidAotMode="$(AndroidAotMode)" + AndroidAotEnableLazyLoad="$(AndroidAotEnableLazyLoad)" + EnableLLVM="$(EnableLLVM)" + HttpClientHandlerType="$(AndroidHttpClientHandlerType)" + TlsProvider="$(AndroidTlsProvider)" + Debug="$(AndroidIncludeDebugSymbols)" + AndroidSequencePointsMode="$(_SequencePointsMode)" + EnableSGenConcurrent="$(AndroidEnableSGenConcurrent)" + SupportedAbis="@(_BuildTargetAbis)" + AndroidPackageName="$(_AndroidPackage)" + EnablePreloadAssembliesDefault="$(_AndroidEnablePreloadAssembliesDefault)" + PackageNamingPolicy="$(AndroidPackageNamingPolicy)" + BoundExceptionType="$(AndroidBoundExceptionType)" + RuntimeConfigBinFilePath="$(_BinaryRuntimeConfigPath)" + UseAssemblyStore="$(_AndroidUseAssemblyStore)" + EnableMarshalMethods="$(_AndroidUseMarshalMethods)" + EnableManagedMarshalMethodsLookup="$(_AndroidUseManagedMarshalMethodsLookup)" + CustomBundleConfigFile="$(AndroidBundleConfigurationFile)" + TargetsCLR="$(_AndroidUseCLR)" + ProjectRuntimeConfigFilePath="$(ProjectRuntimeConfigFilePath)" + > bool +{ + log_debug (LOG_ASSEMBLY, "typemap_java_to_managed: looking up type '{}'", optional_string (java_type_name)); + if (FastTiming::enabled ()) [[unlikely]] { + internal_timing.start_event (TimingEventKind::JavaToManaged); + } + + if (java_type_name == nullptr) [[unlikely]] { + log_warn (LOG_ASSEMBLY, "typemap: type name not specified in typemap_java_to_managed"); + return false; + } + + bool ret; +#if defined(RELEASE) + ret = typemap_java_to_managed_release (java_type_name, assembly_name, managed_type_token_id); +#else + ret = typemap_java_to_managed_debug (java_type_name, assembly_name, managed_type_token_id); +#endif + + if (FastTiming::enabled ()) [[unlikely]] { + internal_timing.end_event (); + } + + return ret; +} From 39931bc260ac933eb83b7cd70094169d6423e880 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 24 Apr 2025 22:27:23 +0200 Subject: [PATCH 02/24] WIP --- .../Android.Runtime/AndroidRuntime.cs | 2 + src/Mono.Android/Java.Interop/TypeManager.cs | 3 + .../LlvmIrGenerator/LlvmIrGenerator.cs | 57 ++----------------- .../LlvmIrGenerator/LlvmIrStringBlob.cs | 19 +------ ...appingReleaseNativeAssemblyGeneratorCLR.cs | 16 +++--- src/native/clr/host/internal-pinvokes.cc | 3 +- 6 files changed, 23 insertions(+), 77 deletions(-) diff --git a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs index 3f6d8f86cf2..ec0f0891c86 100644 --- a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs +++ b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs @@ -265,9 +265,11 @@ public AndroidTypeManager (bool jniAddNativeMethodRegistrationAttributePresent) protected override IEnumerable GetTypesForSimpleReference (string jniSimpleReference) { + RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"#1 GetTypesForSimpleReference (\"{jniSimpleReference}\")"); foreach (var ti in base.GetTypesForSimpleReference (jniSimpleReference)) yield return ti; + RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"#2 GetTypesForSimpleReference (\"{jniSimpleReference}\")"); var t = Java.Interop.TypeManager.GetJavaToManagedType (jniSimpleReference); if (t != null) yield return t; diff --git a/src/Mono.Android/Java.Interop/TypeManager.cs b/src/Mono.Android/Java.Interop/TypeManager.cs index adc07f0edb1..e526c52bd39 100644 --- a/src/Mono.Android/Java.Interop/TypeManager.cs +++ b/src/Mono.Android/Java.Interop/TypeManager.cs @@ -131,6 +131,7 @@ static Type[] GetParameterTypes (string? signature) [UnconditionalSuppressMessage ("Trimming", "IL2057", Justification = "Type.GetType() can never statically know the string value from parameter 'typename_ptr'.")] static void n_Activate (IntPtr jnienv, IntPtr jclass, IntPtr typename_ptr, IntPtr signature_ptr, IntPtr jobject, IntPtr parameters_ptr) { + RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"TypeManager.n_Activate"); if (!global::Java.Interop.JniEnvironment.BeginMarshalMethod (jnienv, out var __envp, out var __r)) return; @@ -253,6 +254,7 @@ static Type monovm_typemap_java_to_managed (string java_type_name) internal static Type? GetJavaToManagedType (string class_name) { + RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"TypeManager.GetJavaToManagedType (\"{class_name}\")"); Type? type = JNIEnvInit.RuntimeType switch { DotNetRuntimeType.MonoVM => monovm_typemap_java_to_managed (class_name), DotNetRuntimeType.CoreCLR => clr_typemap_java_to_managed (class_name), @@ -281,6 +283,7 @@ static Type monovm_typemap_java_to_managed (string java_type_name) [UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = "TypeManager.CreateProxy() does not statically know the value of the 'type' local variable.")] internal static IJavaPeerable? CreateInstance (IntPtr handle, JniHandleOwnership transfer, Type? targetType) { + RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"TypeManager.CreateInstance ()"); Type? type = null; IntPtr class_ptr = JNIEnv.GetObjectClass (handle); string? class_name = GetClassName (class_ptr); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs index 28f8a57809e..785a3aea39f 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs @@ -817,65 +817,16 @@ public void WriteValue (GeneratorWriteContext context, Type type, object? value, void WriteStringBlobArray (GeneratorWriteContext context, LlvmIrStringBlob blob) { - const uint stride = 16; - Type elementType = typeof(byte); - - LlvmIrVariableNumberFormat oldNumberFormat = context.NumberFormat; - context.NumberFormat = LlvmIrVariableNumberFormat.Hexadecimal; - WriteArrayValueStart (context); foreach (LlvmIrStringBlob.StringInfo si in blob.GetSegments ()) { if (si.Offset > 0) { - context.Output.Write (','); - context.Output.WriteLine (); - context.Output.WriteLine (); - } - - context.Output.Write (context.CurrentIndent); - WriteCommentLine (context, $" '{si.Value}' @ {si.Offset}"); - WriteBytes (si.Bytes); - } - context.Output.WriteLine (); - WriteArrayValueEnd (context); - context.NumberFormat = oldNumberFormat; - - void WriteBytes (byte[] bytes) - { - ulong counter = 0; - bool first = true; - foreach (byte b in bytes) { - if (!first) { - WriteCommaWithStride (counter); - } else { - context.Output.Write (context.CurrentIndent); - first = false; - } - - counter++; - WriteByteTypeAndValue (b); - } - - WriteCommaWithStride (counter); - WriteByteTypeAndValue (0); // Terminating NUL is counted for each string, but not included in its bytes - } - - void WriteCommaWithStride (ulong counter) - { - context.Output.Write (','); - if (stride == 1 || counter % stride == 0) { context.Output.WriteLine (); - context.Output.Write (context.CurrentIndent); - } else { - context.Output.Write (' '); } - } - - void WriteByteTypeAndValue (byte v) - { - WriteType (context, elementType, v, out _); - context.Output.Write (' '); - WriteValue (context, elementType, v); + WriteCommentLine (context, $" {si.Value}"); + // TODO: write bytes, 16 hex numbers per line + context.Output.WriteLine (", u0x00,"); // Terminating NUL is counted for each string, but not included in its bytes } + throw new NotImplementedException (); } void WriteStructureValue (GeneratorWriteContext context, StructureInstance? instance) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs index 410efcdcb4d..aa6a26eea1a 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs @@ -36,37 +36,24 @@ public record struct StringInfo (int Offset, int Length, byte[] Bytes, string Va int offset; if (segments.Count > 0) { StringInfo lastSegment = segments[segments.Count - 1]; - offset = lastSegment.Offset + lastSegment.Length + 1; // Include trailing NUL here + offset = lastSegment.Offset + lastSegment.Length; } else { offset = 0; } info = new StringInfo ( Offset: offset, - Length: bytes.Length, + Length: bytes.Length + 1, // MUST include the terminating NUL ("virtual") Bytes: bytes, Value: s ); segments.Add (info); cache.Add (s, info); - size += info.Length + 1; // Account for the trailing NUL + size += info.Length; return (info.Offset, info.Length); } - public int GetIndexOf (string s) - { - if (String.IsNullOrEmpty (s)) { - return -1; - } - - if (!cache.TryGetValue (s, out StringInfo info)) { - return -1; - } - - return info.Offset; - } - public IEnumerable GetSegments () { foreach (StringInfo si in segments) { diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGeneratorCLR.cs index 03662f4d459..8dbd9eb38c0 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGeneratorCLR.cs @@ -219,9 +219,8 @@ sealed class ConstructionState public Dictionary JavaTypesByName; public List> JavaMap; public List AllModulesData; + public List AssemblyNames; public LlvmIrStringBlob AssemblyNamesBlob; - public LlvmIrStringBlob JavaTypeNamesBlob; - public LlvmIrStringBlob ManagedTypeNamesBlob; } readonly NativeTypeMappingData mappingData; @@ -298,10 +297,12 @@ protected override void Construct (LlvmIrModule module) } module.AddGlobalVariable ("java_to_managed_map", cs.JavaMap, LlvmIrVariableOptions.GlobalConstant, " Java to managed map"); - module.AddGlobalVariable ("java_type_names", cs.JavaTypeNamesBlob, LlvmIrVariableOptions.GlobalConstant, " Java type names"); - module.AddGlobalVariable ("java_type_names_size", (ulong)cs.JavaTypeNamesBlob.Size, LlvmIrVariableOptions.GlobalConstant, " Java type names blob size"); - module.AddGlobalVariable ("managed_type_names", cs.ManagedTypeNamesBlob, LlvmIrVariableOptions.GlobalConstant, " Managed type names"); - module.AddGlobalVariable ("managed_assembly_names", cs.AssemblyNamesBlob, LlvmIrVariableOptions.GlobalConstant, " Managed assembly names"); + module.AddGlobalVariable ("java_type_names", cs.JavaNames, LlvmIrVariableOptions.GlobalConstant, " Java type names"); + module.AddGlobalVariable ("managed_type_names", cs.ManagedTypeNames, LlvmIrVariableOptions.GlobalConstant, " Managed type names"); + module.AddGlobalVariable ("managed_assembly_names", cs.AssemblyNames, LlvmIrVariableOptions.GlobalConstant, " Managed assembly names"); + + // WIP + module.AddGlobalVariable ("managed_assembly_names_blob", cs.AssemblyNamesBlob, LlvmIrVariableOptions.GlobalConstant, " Managed assembly names"); } void SortEntriesAndUpdateJavaIndexes (LlvmIrVariable variable, LlvmIrModuleTarget target, object? callerState) @@ -432,6 +433,7 @@ void InitMapModules (ConstructionState cs) var seenAssemblyNames = new Dictionary (StringComparer.OrdinalIgnoreCase); cs.MapModules = new List> (); + cs.AssemblyNames = new List (); cs.AssemblyNamesBlob = new (); foreach (TypeMapGenerator.ModuleReleaseData data in mappingData.Modules) { string mapName = $"module{moduleCounter++}_managed_to_java"; @@ -443,7 +445,7 @@ void InitMapModules (ConstructionState cs) duplicateMapName = $"{mapName}_duplicates"; } - (int assemblyNameIndex, int assemblyNameLength) = cs.AssemblyNamesBlob.Add (data.AssemblyName); + (int stringOffset, int stringLength) = cs.AssemblyNamesBlob.Add (data.AssemblyName); var map_module = new TypeMapModule { MVID = data.Mvid, MapSymbolName = mapName, diff --git a/src/native/clr/host/internal-pinvokes.cc b/src/native/clr/host/internal-pinvokes.cc index 2bcca553fd6..75dd45d3e59 100644 --- a/src/native/clr/host/internal-pinvokes.cc +++ b/src/native/clr/host/internal-pinvokes.cc @@ -33,7 +33,8 @@ const char* clr_typemap_managed_to_java (const char *typeName, const uint8_t *mv bool clr_typemap_java_to_managed (const char *java_type_name, char const** assembly_name, uint32_t *managed_type_token_id) noexcept { - return TypeMapper::java_to_managed (java_type_name, assembly_name, managed_type_token_id); + log_debug (LOG_ASSEMBLY, __PRETTY_FUNCTION__); + return TypeMapper::typemap_java_to_managed (java_type_name, assembly_name, managed_type_token_id); } void monodroid_log (LogLevel level, LogCategories category, const char *message) noexcept From 12e6277a8ff598210f266e1baa3863a8c14b868e Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Fri, 25 Apr 2025 12:20:51 +0200 Subject: [PATCH 03/24] Fix after rebase --- .../Xamarin.Android.Common.targets | 36 ++----------------- 1 file changed, 3 insertions(+), 33 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index def9301bd9a..6421833e60e 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -1880,39 +1880,9 @@ because xbuild doesn't support framework reference assemblies. + MainAssembly="$(TargetPath)" + OutputDirectory="$(_AndroidIntermediateJavaSourceDirectory)mono" + ResolvedUserAssemblies="@(_ResolvedUserAssemblies)"> Date: Fri, 25 Apr 2025 17:52:11 +0200 Subject: [PATCH 04/24] Use string blobs to decrease number of relocations --- .../LlvmIrGenerator/LlvmIrGenerator.cs | 57 +++++++++++++++++-- .../LlvmIrGenerator/LlvmIrStringBlob.cs | 6 +- ...eMappingDebugNativeAssemblyGeneratorCLR.cs | 57 +------------------ ...appingReleaseNativeAssemblyGeneratorCLR.cs | 16 +++--- src/native/clr/host/typemap.cc | 8 ++- src/native/clr/include/xamarin-app.hh | 2 - .../xamarin-app-stub/application_dso_stub.cc | 2 - 7 files changed, 70 insertions(+), 78 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs index 785a3aea39f..28f8a57809e 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs @@ -817,16 +817,65 @@ public void WriteValue (GeneratorWriteContext context, Type type, object? value, void WriteStringBlobArray (GeneratorWriteContext context, LlvmIrStringBlob blob) { + const uint stride = 16; + Type elementType = typeof(byte); + + LlvmIrVariableNumberFormat oldNumberFormat = context.NumberFormat; + context.NumberFormat = LlvmIrVariableNumberFormat.Hexadecimal; + WriteArrayValueStart (context); foreach (LlvmIrStringBlob.StringInfo si in blob.GetSegments ()) { if (si.Offset > 0) { + context.Output.Write (','); + context.Output.WriteLine (); + context.Output.WriteLine (); + } + + context.Output.Write (context.CurrentIndent); + WriteCommentLine (context, $" '{si.Value}' @ {si.Offset}"); + WriteBytes (si.Bytes); + } + context.Output.WriteLine (); + WriteArrayValueEnd (context); + context.NumberFormat = oldNumberFormat; + + void WriteBytes (byte[] bytes) + { + ulong counter = 0; + bool first = true; + foreach (byte b in bytes) { + if (!first) { + WriteCommaWithStride (counter); + } else { + context.Output.Write (context.CurrentIndent); + first = false; + } + + counter++; + WriteByteTypeAndValue (b); + } + + WriteCommaWithStride (counter); + WriteByteTypeAndValue (0); // Terminating NUL is counted for each string, but not included in its bytes + } + + void WriteCommaWithStride (ulong counter) + { + context.Output.Write (','); + if (stride == 1 || counter % stride == 0) { context.Output.WriteLine (); + context.Output.Write (context.CurrentIndent); + } else { + context.Output.Write (' '); } + } + + void WriteByteTypeAndValue (byte v) + { + WriteType (context, elementType, v, out _); - WriteCommentLine (context, $" {si.Value}"); - // TODO: write bytes, 16 hex numbers per line - context.Output.WriteLine (", u0x00,"); // Terminating NUL is counted for each string, but not included in its bytes + context.Output.Write (' '); + WriteValue (context, elementType, v); } - throw new NotImplementedException (); } void WriteStructureValue (GeneratorWriteContext context, StructureInstance? instance) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs index aa6a26eea1a..0f74ecc8d1c 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs @@ -36,20 +36,20 @@ public record struct StringInfo (int Offset, int Length, byte[] Bytes, string Va int offset; if (segments.Count > 0) { StringInfo lastSegment = segments[segments.Count - 1]; - offset = lastSegment.Offset + lastSegment.Length; + offset = lastSegment.Offset + lastSegment.Length + 1; // Include trailing NUL here } else { offset = 0; } info = new StringInfo ( Offset: offset, - Length: bytes.Length + 1, // MUST include the terminating NUL ("virtual") + Length: bytes.Length, Bytes: bytes, Value: s ); segments.Add (info); cache.Add (s, info); - size += info.Length; + size += info.Length + 1; // Account for the trailing NUL return (info.Offset, info.Length); } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs index b93189f3b8c..e5f3bf48f6a 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs @@ -16,10 +16,6 @@ class TypeMappingDebugNativeAssemblyGeneratorCLR : LlvmIrComposer const string TypeMapSymbol = "type_map"; const string UniqueAssembliesSymbol = "type_map_unique_assemblies"; const string AssemblyNamesBlobSymbol = "type_map_assembly_names"; - const string ManagedTypeNamesBlobSymbol = "type_map_managed_type_names"; - const string JavaTypeNamesBlobSymbol = "type_map_java_type_names"; - const string TypeMapUsesHashesSymbol = "typemap_use_hashes"; - const string TypeMapManagedTypeInfoSymbol = "type_map_managed_type_info"; sealed class TypeMapContextDataProvider : NativeAssemblerStructContextDataProvider { @@ -191,61 +187,9 @@ protected override void Construct (LlvmIrModule module) // CoreCLR supports only 64-bit targets, so we can make things simpler by hashing all the things here instead of // in a callback during code generation - - // Probability of xxHash clashes on managed type names is very low, it might be hard to find such type names that - // would create collision, so in order to be able to test the string-based managed-to-java typemaps, we check whether - // the `CI_TYPEMAP_DEBUG_USE_STRINGS` environment variable is present and not empty. If it's not in the environment - // or its value is an empty string, we default to using hashes for the managed-to-java type maps. - bool typemap_uses_hashes = String.IsNullOrEmpty (Environment.GetEnvironmentVariable ("CI_TYPEMAP_DEBUG_USE_STRINGS")); - var usedHashes = new Dictionary (); - foreach (TypeMapGenerator.TypeMapDebugEntry entry in data.ManagedToJavaMap) { - (int managedTypeNameOffset, int _) = managedTypeNames.Add (entry.ManagedName); - (int javaTypeNameOffset, int _) = javaTypeNames.Add (entry.JavaName); - var m2j = new TypeMapEntry { - From = entry.ManagedName, - To = entry.JavaName, - - from = (uint)managedTypeNameOffset, - from_hash = typemap_uses_hashes ? MonoAndroidHelper.GetXxHash (entry.ManagedName, is64Bit: true) : 0, - to = (uint)javaTypeNameOffset, - }; - managedToJavaMap.Add (new StructureInstance (typeMapEntryStructureInfo, m2j)); - - if (!typemap_uses_hashes) { - continue; - } - - if (usedHashes.ContainsKey (m2j.from_hash)) { - typemap_uses_hashes = false; - // It could be a warning, but it's not really actionable - users might not be able to rename the clashing types - Log.LogMessage ($"Detected xxHash conflict between managed type names '{entry.ManagedName}' and '{usedHashes[m2j.from_hash]}' when mapping to Java type '{entry.JavaName}'."); - } else { - usedHashes[m2j.from_hash] = entry.ManagedName; - } - } - // Input is sorted on name, we need to re-sort it on hashes, if used - if (typemap_uses_hashes) { - managedToJavaMap.Sort ((StructureInstance a, StructureInstance b) => { - if (a.Instance == null) { - return b.Instance == null ? 0 : -1; - } - - if (b.Instance == null) { - return 1; - } - - return a.Instance.from_hash.CompareTo (b.Instance.from_hash); - }); - } - - if (!typemap_uses_hashes) { - Log.LogMessage ("Managed-to-java typemaps will use string-based matching."); - } - var assemblyNamesBlob = new LlvmIrStringBlob (); foreach (TypeMapGenerator.TypeMapDebugAssembly asm in data.UniqueAssemblies) { (int assemblyNameOffset, int assemblyNameLength) = assemblyNamesBlob.Add (asm.Name); - var entry = new TypeMapAssembly { Name = asm.Name, MVID = asm.MVID, @@ -296,6 +240,7 @@ protected override void Construct (LlvmIrModule module) entry_count = data.EntryCount, unique_assemblies_count = (ulong)data.UniqueAssemblies.Count, + assembly_names_blob_size = (ulong)assemblyNamesBlob.Size, }; type_map = new StructureInstance (typeMapStructureInfo, map); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGeneratorCLR.cs index 8dbd9eb38c0..03662f4d459 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGeneratorCLR.cs @@ -219,8 +219,9 @@ sealed class ConstructionState public Dictionary JavaTypesByName; public List> JavaMap; public List AllModulesData; - public List AssemblyNames; public LlvmIrStringBlob AssemblyNamesBlob; + public LlvmIrStringBlob JavaTypeNamesBlob; + public LlvmIrStringBlob ManagedTypeNamesBlob; } readonly NativeTypeMappingData mappingData; @@ -297,12 +298,10 @@ protected override void Construct (LlvmIrModule module) } module.AddGlobalVariable ("java_to_managed_map", cs.JavaMap, LlvmIrVariableOptions.GlobalConstant, " Java to managed map"); - module.AddGlobalVariable ("java_type_names", cs.JavaNames, LlvmIrVariableOptions.GlobalConstant, " Java type names"); - module.AddGlobalVariable ("managed_type_names", cs.ManagedTypeNames, LlvmIrVariableOptions.GlobalConstant, " Managed type names"); - module.AddGlobalVariable ("managed_assembly_names", cs.AssemblyNames, LlvmIrVariableOptions.GlobalConstant, " Managed assembly names"); - - // WIP - module.AddGlobalVariable ("managed_assembly_names_blob", cs.AssemblyNamesBlob, LlvmIrVariableOptions.GlobalConstant, " Managed assembly names"); + module.AddGlobalVariable ("java_type_names", cs.JavaTypeNamesBlob, LlvmIrVariableOptions.GlobalConstant, " Java type names"); + module.AddGlobalVariable ("java_type_names_size", (ulong)cs.JavaTypeNamesBlob.Size, LlvmIrVariableOptions.GlobalConstant, " Java type names blob size"); + module.AddGlobalVariable ("managed_type_names", cs.ManagedTypeNamesBlob, LlvmIrVariableOptions.GlobalConstant, " Managed type names"); + module.AddGlobalVariable ("managed_assembly_names", cs.AssemblyNamesBlob, LlvmIrVariableOptions.GlobalConstant, " Managed assembly names"); } void SortEntriesAndUpdateJavaIndexes (LlvmIrVariable variable, LlvmIrModuleTarget target, object? callerState) @@ -433,7 +432,6 @@ void InitMapModules (ConstructionState cs) var seenAssemblyNames = new Dictionary (StringComparer.OrdinalIgnoreCase); cs.MapModules = new List> (); - cs.AssemblyNames = new List (); cs.AssemblyNamesBlob = new (); foreach (TypeMapGenerator.ModuleReleaseData data in mappingData.Modules) { string mapName = $"module{moduleCounter++}_managed_to_java"; @@ -445,7 +443,7 @@ void InitMapModules (ConstructionState cs) duplicateMapName = $"{mapName}_duplicates"; } - (int stringOffset, int stringLength) = cs.AssemblyNamesBlob.Add (data.AssemblyName); + (int assemblyNameIndex, int assemblyNameLength) = cs.AssemblyNamesBlob.Add (data.AssemblyName); var map_module = new TypeMapModule { MVID = data.Mvid, MapSymbolName = mapName, diff --git a/src/native/clr/host/typemap.cc b/src/native/clr/host/typemap.cc index 8320950fd5a..7ea9cef40c7 100644 --- a/src/native/clr/host/typemap.cc +++ b/src/native/clr/host/typemap.cc @@ -156,8 +156,12 @@ auto TypeMapper::managed_to_java_debug (const char *typeName, const uint8_t *mvi TypeMapAssembly const& assm = type_map_unique_assemblies[idx]; full_type_name.append (", "sv); - // We explicitly trust the build process here, with regards to validity of offsets - full_type_name.append (&type_map_assembly_names[assm.name_offset], assm.name_length); + if (assm.name_offset < type_map.assembly_names_blob_size) [[likely]] { + full_type_name.append (&type_map_assembly_names[assm.name_offset], assm.name_length); + log_debug (LOG_ASSEMBLY, "Fixed-up type name: '{}'", full_type_name.get ()); + } else { + log_warn (LOG_ASSEMBLY, "Invalid assembly name offset {}", assm.name_offset); + } } else { log_warn (LOG_ASSEMBLY, "typemap: unable to look up assembly name for type '{}', trying without it.", typeName); } diff --git a/src/native/clr/include/xamarin-app.hh b/src/native/clr/include/xamarin-app.hh index 8caa09323d4..aa825a97b4b 100644 --- a/src/native/clr/include/xamarin-app.hh +++ b/src/native/clr/include/xamarin-app.hh @@ -340,8 +340,6 @@ extern "C" { [[gnu::visibility("default")]] extern const TypeMapManagedTypeInfo type_map_managed_type_info[]; [[gnu::visibility("default")]] extern const TypeMapAssembly type_map_unique_assemblies[]; [[gnu::visibility("default")]] extern const char type_map_assembly_names[]; - [[gnu::visibility("default")]] extern const char type_map_managed_type_names[]; - [[gnu::visibility("default")]] extern const char type_map_java_type_names[]; #else [[gnu::visibility("default")]] extern const uint32_t managed_to_java_map_module_count; [[gnu::visibility("default")]] extern const uint32_t java_type_count; diff --git a/src/native/clr/xamarin-app-stub/application_dso_stub.cc b/src/native/clr/xamarin-app-stub/application_dso_stub.cc index 7919083ef1f..293d013ab9a 100644 --- a/src/native/clr/xamarin-app-stub/application_dso_stub.cc +++ b/src/native/clr/xamarin-app-stub/application_dso_stub.cc @@ -24,8 +24,6 @@ const bool typemap_use_hashes = true; const TypeMapManagedTypeInfo type_map_managed_type_info[] = {}; const TypeMapAssembly type_map_unique_assemblies[] = {}; const char type_map_assembly_names[] = {}; -const char type_map_managed_type_names[] = {}; -const char type_map_java_type_names[] = {}; #else const uint32_t managed_to_java_map_module_count = 0; const uint32_t java_type_count = 0; From 251063714692bb199047df99a95e9b92f6234c8d Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Fri, 25 Apr 2025 18:42:53 +0200 Subject: [PATCH 05/24] Optimize debug maps a bit --- ...eMappingDebugNativeAssemblyGeneratorCLR.cs | 56 ++++++++++++- src/native/clr/host/typemap.cc | 82 ++++++++----------- src/native/clr/include/host/typemap.hh | 8 +- src/native/clr/include/xamarin-app.hh | 12 +-- .../xamarin-app-stub/application_dso_stub.cc | 2 + 5 files changed, 92 insertions(+), 68 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs index e5f3bf48f6a..dd4c14e99e9 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs @@ -16,6 +16,8 @@ class TypeMappingDebugNativeAssemblyGeneratorCLR : LlvmIrComposer const string TypeMapSymbol = "type_map"; const string UniqueAssembliesSymbol = "type_map_unique_assemblies"; const string AssemblyNamesBlobSymbol = "type_map_assembly_names"; + const string ManagedTypeNamesBlobSymbol = "type_map_managed_type_names"; + const string JavaTypeNamesBlobSymbol = "type_map_java_type_names"; sealed class TypeMapContextDataProvider : NativeAssemblerStructContextDataProvider { @@ -53,11 +55,11 @@ public override string GetComment (object data, string fieldName) var entry = EnsureType (data); if (String.Compare ("from", fieldName, StringComparison.Ordinal) == 0) { - return $" from: '{entry.From}'"; + return $"from: {entry.From}"; } if (String.Compare ("to", fieldName, StringComparison.Ordinal) == 0) { - return $" to: '{entry.To}'"; + return $"to: {entry.To}"; } return String.Empty; @@ -105,11 +107,23 @@ public override string GetComment (object data, string fieldName) [NativeAssemblerStructContextDataProvider (typeof (TypeMapEntryContextDataProvider))] sealed class TypeMapEntry { + [NativeAssembler (Ignore = true)] + public string From; + + [NativeAssembler (Ignore = true)] + public string To; + [NativeAssembler (UsesDataProvider = true)] - public string from; + public uint from; + + [NativeAssembler (NumberFormat = LlvmIrVariableNumberFormat.Hexadecimal)] + public ulong from_hash; [NativeAssembler (UsesDataProvider = true)] - public string to; + public uint to; + + [NativeAssembler (NumberFormat = LlvmIrVariableNumberFormat.Hexadecimal)] + public ulong to_hash; }; // Order of fields and their type must correspond *exactly* to that in @@ -187,6 +201,40 @@ protected override void Construct (LlvmIrModule module) // CoreCLR supports only 64-bit targets, so we can make things simpler by hashing all the things here instead of // in a callback during code generation + foreach (TypeMapGenerator.TypeMapDebugEntry entry in data.ManagedToJavaMap) { + (int managedTypeNameOffset, int _) = managedTypeNames.Add (entry.ManagedName); + (int javaTypeNameOffset, int _) = javaTypeNames.Add (entry.JavaName); + var m2j = new TypeMapEntry { + From = entry.ManagedName, + To = entry.JavaName, + + from = (uint)managedTypeNameOffset, + from_hash = MonoAndroidHelper.GetXxHash (entry.ManagedName, is64Bit: true), + to = (uint)javaTypeNameOffset, + to_hash = MonoAndroidHelper.GetXxHash (entry.JavaName, is64Bit: true), + }; + managedToJavaMap.Add (new StructureInstance (typeMapEntryStructureInfo, m2j)); + } + managedToJavaMap.Sort ((StructureInstance a, StructureInstance b) => a.Instance.from_hash.CompareTo (b.Instance.from_hash)); + + foreach (TypeMapGenerator.TypeMapDebugEntry entry in data.JavaToManagedMap) { + TypeMapGenerator.TypeMapDebugEntry managedEntry = entry.DuplicateForJavaToManaged != null ? entry.DuplicateForJavaToManaged : entry; + (int managedTypeNameOffset, int _) = managedTypeNames.Add (entry.ManagedName); + (int javaTypeNameOffset, int _) = javaTypeNames.Add (entry.JavaName); + + var j2m = new TypeMapEntry { + From = entry.JavaName, + To = managedEntry.SkipInJavaToManaged ? String.Empty : entry.ManagedName, + + from = (uint)javaTypeNameOffset, + from_hash = MonoAndroidHelper.GetXxHash (entry.JavaName, is64Bit: true), + to = managedEntry.SkipInJavaToManaged ? uint.MaxValue : (uint)managedTypeNameOffset, + to_hash = managedEntry.SkipInJavaToManaged ? 0 : MonoAndroidHelper.GetXxHash (entry.ManagedName, is64Bit: true), + }; + javaToManagedMap.Add (new StructureInstance (typeMapEntryStructureInfo, j2m)); + } + javaToManagedMap.Sort ((StructureInstance a, StructureInstance b) => a.Instance.from_hash.CompareTo (b.Instance.from_hash)); + var assemblyNamesBlob = new LlvmIrStringBlob (); foreach (TypeMapGenerator.TypeMapDebugAssembly asm in data.UniqueAssemblies) { (int assemblyNameOffset, int assemblyNameLength) = assemblyNamesBlob.Add (asm.Name); diff --git a/src/native/clr/host/typemap.cc b/src/native/clr/host/typemap.cc index 7ea9cef40c7..3e4326963da 100644 --- a/src/native/clr/host/typemap.cc +++ b/src/native/clr/host/typemap.cc @@ -64,30 +64,44 @@ namespace { } #if defined(DEBUG) -[[gnu::always_inline, gnu::flatten]] -auto TypeMapper::find_index_by_name (const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) noexcept -> ssize_t +[[gnu::always_inline]] +auto TypeMapper::typemap_type_to_type_debug (const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) noexcept -> const char* { - log_debug (LOG_ASSEMBLY, "typemap: map {} -> {} uses strings", from_name, to_name); - - auto equal = [](TypeMapEntry const& entry, const char *key, const char (&name_map)[]) -> bool { - if (entry.from == std::numeric_limits::max ()) [[unlikely]] { + log_debug (LOG_ASSEMBLY, "Looking up {} type '{}'", from_name, optional_string (typeName)); + auto equal = [](TypeMapEntry const& entry, hash_t key) -> bool { + if (entry.from == std::numeric_limits::max ()) { return 1; } - const char *type_name = &name_map[entry.from]; - return strcmp (type_name, key) == 0; + return entry.from_hash == key; }; - auto less_than = [](TypeMapEntry const& entry, const char *key, const char (&name_map)[]) -> bool { - if (entry.from == std::numeric_limits::max ()) [[unlikely]] { + auto less_than = [](TypeMapEntry const& entry, hash_t key) -> bool { + if (entry.from == std::numeric_limits::max ()) { return 1; } - const char *type_name = &name_map[entry.from]; - return strcmp (type_name, key) < 0; + return entry.from_hash < key; }; - return Search::binary_search (name_map, typeName, map, type_map.entry_count); + hash_t type_name_hash = xxhash::hash (typeName, strlen (typeName)); + ssize_t idx = Search::binary_search (type_name_hash, map, type_map.entry_count); + if (idx >= 0) [[likely]] { + TypeMapEntry const& entry = map[idx]; + const char *mapped_name = &name_map[entry.to]; + + log_debug ( + LOG_ASSEMBLY, + "{} type '{}' maps to {} type '{}'", + from_name, + optional_string (typeName), + to_name, + optional_string (mapped_name) + ); + return mapped_name; + } + + return nullptr; } [[gnu::always_inline, gnu::flatten]] @@ -166,12 +180,7 @@ auto TypeMapper::managed_to_java_debug (const char *typeName, const uint8_t *mvi log_warn (LOG_ASSEMBLY, "typemap: unable to look up assembly name for type '{}', trying without it.", typeName); } - // If hashes are used for matching, the type names array is not used. If, however, string-based matching is in - // effect, the managed type name is looked up and then... - idx = find_index_by_hash (full_type_name.get (), type_map.managed_to_java, type_map_managed_type_names, MANAGED, JAVA); - - // ...either method gives us index into the Java type names array - return index_to_name (idx, full_type_name.get (), type_map.managed_to_java, type_map_java_type_names, MANAGED, JAVA); + return typemap_type_to_type_debug (full_type_name.get (), type_map.managed_to_java, type_map_java_type_names, MANAGED, JAVA); } #endif // def DEBUG @@ -344,37 +353,10 @@ auto TypeMapper::managed_to_java (const char *typeName, const uint8_t *mvid) noe [[gnu::flatten]] auto TypeMapper::java_to_managed_debug (const char *java_type_name, char const** assembly_name, uint32_t *managed_type_token_id) noexcept -> bool { - if (assembly_name == nullptr || managed_type_token_id == nullptr) [[unlikely]] { - log_warn (LOG_ASSEMBLY, "Managed land called java-to-managed mapping function with invalid pointers"); - return false; - } - - // We need to find entry matching the Java type name, which will then... - ssize_t idx = find_index_by_name (java_type_name, type_map.java_to_managed, type_map_java_type_names, JAVA, MANAGED); - - // ..provide us with the managed type name index - const char *name = index_to_name (idx, java_type_name, type_map.java_to_managed, type_map_managed_type_names, JAVA, MANAGED); - if (name == nullptr) { - *assembly_name = nullptr; - *managed_type_token_id = 0; - return false; - } - - // We explicitly trust the build process here, with regards to the size of the arrays - TypeMapManagedTypeInfo const& type_info = type_map_managed_type_info[idx]; - *assembly_name = &type_map_assembly_names[type_info.assembly_name_index]; - *managed_type_token_id = type_info.managed_type_token_id; - - log_debug ( - LOG_ASSEMBLY, - "Mapped Java type '{}' to managed type '{}' in assembly '{}' and with token '{:x}'", - optional_string (java_type_name), - name, - *assembly_name, - *managed_type_token_id - ); - - return true; + // FIXME: this is currently VERY broken + *assembly_name = nullptr; + *managed_type_token_id = 0; + return typemap_type_to_type_debug (java_type_name, type_map.java_to_managed, type_map_managed_type_names, JAVA, MANAGED); } #else // def DEBUG diff --git a/src/native/clr/include/host/typemap.hh b/src/native/clr/include/host/typemap.hh index eea9564cc98..0c4347c88e7 100644 --- a/src/native/clr/include/host/typemap.hh +++ b/src/native/clr/include/host/typemap.hh @@ -27,11 +27,9 @@ namespace xamarin::android { static auto find_java_to_managed_entry (hash_t name_hash) noexcept -> const TypeMapJava*; #else - static auto index_to_name (ssize_t index, const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) -> const char*; - static auto find_index_by_hash (const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) noexcept -> ssize_t; - static auto find_index_by_name (const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) noexcept -> ssize_t; - static auto managed_to_java_debug (const char *typeName, const uint8_t *mvid) noexcept -> const char*; - static auto java_to_managed_debug (const char *java_type_name, char const** assembly_name, uint32_t *managed_type_token_id) noexcept -> bool; + static auto typemap_type_to_type_debug (const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) noexcept -> const char*; + static auto typemap_managed_to_java_debug (const char *typeName, const uint8_t *mvid) noexcept -> const char*; + static auto typemap_java_to_managed_debug (const char *java_type_name, char const** assembly_name, uint32_t *managed_type_token_id) noexcept -> bool; #endif }; } diff --git a/src/native/clr/include/xamarin-app.hh b/src/native/clr/include/xamarin-app.hh index aa825a97b4b..a7452ed450b 100644 --- a/src/native/clr/include/xamarin-app.hh +++ b/src/native/clr/include/xamarin-app.hh @@ -56,8 +56,6 @@ struct TypeMapIndexHeader uint32_t module_file_name_width; }; -// MUST match src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs -// // If any of the members is set to maximum uint32_t value it means the entry is ignored (treated // as equivalent to `nullptr` if the member was a pointer). The reasoning is that no string could // begin at this offset (well, an empty string could, but we don't have those here) @@ -66,13 +64,7 @@ struct TypeMapEntry const uint32_t from; const xamarin::android::hash_t from_hash; const uint32_t to; -}; - -// MUST match src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs -struct TypeMapManagedTypeInfo -{ - const uint32_t assembly_name_index; - const uint32_t managed_type_token_id; + const xamarin::android::hash_t to_hash; }; // MUST match src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs @@ -340,6 +332,8 @@ extern "C" { [[gnu::visibility("default")]] extern const TypeMapManagedTypeInfo type_map_managed_type_info[]; [[gnu::visibility("default")]] extern const TypeMapAssembly type_map_unique_assemblies[]; [[gnu::visibility("default")]] extern const char type_map_assembly_names[]; + [[gnu::visibility("default")]] extern const char type_map_managed_type_names[]; + [[gnu::visibility("default")]] extern const char type_map_java_type_names[]; #else [[gnu::visibility("default")]] extern const uint32_t managed_to_java_map_module_count; [[gnu::visibility("default")]] extern const uint32_t java_type_count; diff --git a/src/native/clr/xamarin-app-stub/application_dso_stub.cc b/src/native/clr/xamarin-app-stub/application_dso_stub.cc index 293d013ab9a..7919083ef1f 100644 --- a/src/native/clr/xamarin-app-stub/application_dso_stub.cc +++ b/src/native/clr/xamarin-app-stub/application_dso_stub.cc @@ -24,6 +24,8 @@ const bool typemap_use_hashes = true; const TypeMapManagedTypeInfo type_map_managed_type_info[] = {}; const TypeMapAssembly type_map_unique_assemblies[] = {}; const char type_map_assembly_names[] = {}; +const char type_map_managed_type_names[] = {}; +const char type_map_java_type_names[] = {}; #else const uint32_t managed_to_java_map_module_count = 0; const uint32_t java_type_count = 0; From ed0dbbc976f22d1f27dcf80b7795dc125df08d22 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 28 Apr 2025 14:25:24 +0200 Subject: [PATCH 06/24] Implement fallback matching when hash clashes are detected --- ...eMappingDebugNativeAssemblyGeneratorCLR.cs | 46 ++++++++++---- src/native/clr/host/internal-pinvokes.cc | 2 +- src/native/clr/host/typemap.cc | 60 +++++++++---------- src/native/clr/include/host/typemap.hh | 8 ++- src/native/clr/include/xamarin-app.hh | 1 - .../xamarin-app-stub/application_dso_stub.cc | 1 - 6 files changed, 69 insertions(+), 49 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs index dd4c14e99e9..bf4ceee5467 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs @@ -18,6 +18,7 @@ class TypeMappingDebugNativeAssemblyGeneratorCLR : LlvmIrComposer const string AssemblyNamesBlobSymbol = "type_map_assembly_names"; const string ManagedTypeNamesBlobSymbol = "type_map_managed_type_names"; const string JavaTypeNamesBlobSymbol = "type_map_java_type_names"; + const string TypeMapUsesHashesSymbol = "typemap_use_hashes"; sealed class TypeMapContextDataProvider : NativeAssemblerStructContextDataProvider { @@ -121,9 +122,6 @@ sealed class TypeMapEntry [NativeAssembler (UsesDataProvider = true)] public uint to; - - [NativeAssembler (NumberFormat = LlvmIrVariableNumberFormat.Hexadecimal)] - public ulong to_hash; }; // Order of fields and their type must correspond *exactly* to that in @@ -201,6 +199,13 @@ protected override void Construct (LlvmIrModule module) // CoreCLR supports only 64-bit targets, so we can make things simpler by hashing all the things here instead of // in a callback during code generation + + // Probability of xxHash clashes on managed type names is very low, it might be hard to find such type names that + // would create collision, so in order to be able to test the string-based managed-to-java typemaps, we check whether + // the `CI_TYPEMAP_DEBUG_USE_STRINGS` environment variable is present and not empty. If it's not in the environment + // or its value is an empty string, we default to using hashes for the managed-to-java type maps. + bool typemap_uses_hashes = String.IsNullOrEmpty (Environment.GetEnvironmentVariable ("CI_TYPEMAP_DEBUG_USE_STRINGS")); + var usedHashes = new Dictionary (); foreach (TypeMapGenerator.TypeMapDebugEntry entry in data.ManagedToJavaMap) { (int managedTypeNameOffset, int _) = managedTypeNames.Add (entry.ManagedName); (int javaTypeNameOffset, int _) = javaTypeNames.Add (entry.JavaName); @@ -211,12 +216,27 @@ protected override void Construct (LlvmIrModule module) from = (uint)managedTypeNameOffset, from_hash = MonoAndroidHelper.GetXxHash (entry.ManagedName, is64Bit: true), to = (uint)javaTypeNameOffset, - to_hash = MonoAndroidHelper.GetXxHash (entry.JavaName, is64Bit: true), }; managedToJavaMap.Add (new StructureInstance (typeMapEntryStructureInfo, m2j)); + + if (usedHashes.ContainsKey (m2j.from_hash)) { + typemap_uses_hashes = false; + // It could be a warning, but it's not really actionable - users might not be able to rename the clashing types + Log.LogMessage ($"Detected xxHash conflict between managed type names '{entry.ManagedName}' and '{usedHashes[m2j.from_hash]}' when mapping to Java type '{entry.JavaName}'."); + } else { + usedHashes[m2j.from_hash] = entry.ManagedName; + } + } + // Input is sorted on name, we need to re-sort it on hashes, if used + if (typemap_uses_hashes) { + managedToJavaMap.Sort ((StructureInstance a, StructureInstance b) => a.Instance.from_hash.CompareTo (b.Instance.from_hash)); + } + + if (!typemap_uses_hashes) { + Log.LogMessage ("Managed-to-java typemaps will use string-based matching."); } - managedToJavaMap.Sort ((StructureInstance a, StructureInstance b) => a.Instance.from_hash.CompareTo (b.Instance.from_hash)); + // Java-to-managed maps don't use hashes since many mappings have multiple instances foreach (TypeMapGenerator.TypeMapDebugEntry entry in data.JavaToManagedMap) { TypeMapGenerator.TypeMapDebugEntry managedEntry = entry.DuplicateForJavaToManaged != null ? entry.DuplicateForJavaToManaged : entry; (int managedTypeNameOffset, int _) = managedTypeNames.Add (entry.ManagedName); @@ -227,13 +247,11 @@ protected override void Construct (LlvmIrModule module) To = managedEntry.SkipInJavaToManaged ? String.Empty : entry.ManagedName, from = (uint)javaTypeNameOffset, - from_hash = MonoAndroidHelper.GetXxHash (entry.JavaName, is64Bit: true), + from_hash = 0, to = managedEntry.SkipInJavaToManaged ? uint.MaxValue : (uint)managedTypeNameOffset, - to_hash = managedEntry.SkipInJavaToManaged ? 0 : MonoAndroidHelper.GetXxHash (entry.ManagedName, is64Bit: true), }; javaToManagedMap.Add (new StructureInstance (typeMapEntryStructureInfo, j2m)); } - javaToManagedMap.Sort ((StructureInstance a, StructureInstance b) => a.Instance.from_hash.CompareTo (b.Instance.from_hash)); var assemblyNamesBlob = new LlvmIrStringBlob (); foreach (TypeMapGenerator.TypeMapDebugAssembly asm in data.UniqueAssemblies) { @@ -293,9 +311,15 @@ protected override void Construct (LlvmIrModule module) type_map = new StructureInstance (typeMapStructureInfo, map); module.AddGlobalVariable (TypeMapSymbol, type_map, LlvmIrVariableOptions.GlobalConstant); - module.AddGlobalVariable (ManagedToJavaSymbol, managedToJavaMap, LlvmIrVariableOptions.LocalConstant); - module.AddGlobalVariable (JavaToManagedSymbol, javaToManagedMap, LlvmIrVariableOptions.LocalConstant); - module.AddGlobalVariable (TypeMapManagedTypeInfoSymbol, managedTypeInfos, LlvmIrVariableOptions.GlobalConstant); + + if (managedToJavaMap.Count > 0) { + module.AddGlobalVariable (ManagedToJavaSymbol, managedToJavaMap, LlvmIrVariableOptions.LocalConstant); + } + + if (javaToManagedMap.Count > 0) { + module.AddGlobalVariable (JavaToManagedSymbol, javaToManagedMap, LlvmIrVariableOptions.LocalConstant); + } + module.AddGlobalVariable (TypeMapUsesHashesSymbol, typemap_uses_hashes, LlvmIrVariableOptions.GlobalConstant); module.AddGlobalVariable (UniqueAssembliesSymbol, uniqueAssemblies, LlvmIrVariableOptions.GlobalConstant); module.AddGlobalVariable (AssemblyNamesBlobSymbol, assemblyNamesBlob, LlvmIrVariableOptions.GlobalConstant); diff --git a/src/native/clr/host/internal-pinvokes.cc b/src/native/clr/host/internal-pinvokes.cc index 75dd45d3e59..6f0a62b3002 100644 --- a/src/native/clr/host/internal-pinvokes.cc +++ b/src/native/clr/host/internal-pinvokes.cc @@ -34,7 +34,7 @@ const char* clr_typemap_managed_to_java (const char *typeName, const uint8_t *mv bool clr_typemap_java_to_managed (const char *java_type_name, char const** assembly_name, uint32_t *managed_type_token_id) noexcept { log_debug (LOG_ASSEMBLY, __PRETTY_FUNCTION__); - return TypeMapper::typemap_java_to_managed (java_type_name, assembly_name, managed_type_token_id); + return TypeMapper::java_to_managed (java_type_name, assembly_name, managed_type_token_id); } void monodroid_log (LogLevel level, LogCategories category, const char *message) noexcept diff --git a/src/native/clr/host/typemap.cc b/src/native/clr/host/typemap.cc index 3e4326963da..c83b4935d6e 100644 --- a/src/native/clr/host/typemap.cc +++ b/src/native/clr/host/typemap.cc @@ -64,44 +64,30 @@ namespace { } #if defined(DEBUG) -[[gnu::always_inline]] -auto TypeMapper::typemap_type_to_type_debug (const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) noexcept -> const char* +[[gnu::always_inline, gnu::flatten]] +auto TypeMapper::find_index_by_name (const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) noexcept -> ssize_t { - log_debug (LOG_ASSEMBLY, "Looking up {} type '{}'", from_name, optional_string (typeName)); - auto equal = [](TypeMapEntry const& entry, hash_t key) -> bool { - if (entry.from == std::numeric_limits::max ()) { + log_debug (LOG_ASSEMBLY, "typemap: map {} -> {} uses strings", from_name, to_name); + + auto equal = [](TypeMapEntry const& entry, const char *key, const char (&name_map)[]) -> bool { + if (entry.from == std::numeric_limits::max ()) [[unlikely]] { return 1; } - return entry.from_hash == key; + const char *type_name = &name_map[entry.from]; + return strcmp (type_name, key) == 0; }; - auto less_than = [](TypeMapEntry const& entry, hash_t key) -> bool { - if (entry.from == std::numeric_limits::max ()) { + auto less_than = [](TypeMapEntry const& entry, const char *key, const char (&name_map)[]) -> bool { + if (entry.from == std::numeric_limits::max ()) [[unlikely]] { return 1; } - return entry.from_hash < key; + const char *type_name = &name_map[entry.from]; + return strcmp (type_name, key) < 0; }; - hash_t type_name_hash = xxhash::hash (typeName, strlen (typeName)); - ssize_t idx = Search::binary_search (type_name_hash, map, type_map.entry_count); - if (idx >= 0) [[likely]] { - TypeMapEntry const& entry = map[idx]; - const char *mapped_name = &name_map[entry.to]; - - log_debug ( - LOG_ASSEMBLY, - "{} type '{}' maps to {} type '{}'", - from_name, - optional_string (typeName), - to_name, - optional_string (mapped_name) - ); - return mapped_name; - } - - return nullptr; + return Search::binary_search (name_map, typeName, map, type_map.entry_count); } [[gnu::always_inline, gnu::flatten]] @@ -172,15 +158,20 @@ auto TypeMapper::managed_to_java_debug (const char *typeName, const uint8_t *mvi if (assm.name_offset < type_map.assembly_names_blob_size) [[likely]] { full_type_name.append (&type_map_assembly_names[assm.name_offset], assm.name_length); - log_debug (LOG_ASSEMBLY, "Fixed-up type name: '{}'", full_type_name.get ()); + log_debug (LOG_ASSEMBLY, "typemap: fixed-up type name: '{}'", full_type_name.get ()); } else { - log_warn (LOG_ASSEMBLY, "Invalid assembly name offset {}", assm.name_offset); + log_warn (LOG_ASSEMBLY, "typemap: fnvalid assembly name offset {}", assm.name_offset); } } else { log_warn (LOG_ASSEMBLY, "typemap: unable to look up assembly name for type '{}', trying without it.", typeName); } - return typemap_type_to_type_debug (full_type_name.get (), type_map.managed_to_java, type_map_java_type_names, MANAGED, JAVA); + // If hashes are used for matching, the type names array is not used. If, however, string-based matching is in + // effect, the managed type name is looked up and then... + idx = find_index_by_hash (full_type_name.get (), type_map.managed_to_java, type_map_managed_type_names, MANAGED, JAVA); + + // ...either method gives us index into the Java type names array + return index_to_name (idx, full_type_name.get (), type_map.managed_to_java, type_map_java_type_names, MANAGED, JAVA); } #endif // def DEBUG @@ -335,7 +326,7 @@ auto TypeMapper::managed_to_java (const char *typeName, const uint8_t *mvid) noe auto do_map = [&typeName, &mvid]() -> const char* { #if defined(RELEASE) - return managed_to_java_release (typeName, mvid); + return typemap_managed_to_java_release (typeName, mvid); #else return managed_to_java_debug (typeName, mvid); #endif @@ -356,7 +347,12 @@ auto TypeMapper::java_to_managed_debug (const char *java_type_name, char const** // FIXME: this is currently VERY broken *assembly_name = nullptr; *managed_type_token_id = 0; - return typemap_type_to_type_debug (java_type_name, type_map.java_to_managed, type_map_managed_type_names, JAVA, MANAGED); + + // We need to find entry matching the Java type name, which will then... + ssize_t idx = find_index_by_name (java_type_name, type_map.java_to_managed, type_map_java_type_names, JAVA, MANAGED); + + // ..provide us with the managed type name index + return index_to_name (idx, java_type_name, type_map.java_to_managed, type_map_managed_type_names, JAVA, MANAGED); } #else // def DEBUG diff --git a/src/native/clr/include/host/typemap.hh b/src/native/clr/include/host/typemap.hh index 0c4347c88e7..eea9564cc98 100644 --- a/src/native/clr/include/host/typemap.hh +++ b/src/native/clr/include/host/typemap.hh @@ -27,9 +27,11 @@ namespace xamarin::android { static auto find_java_to_managed_entry (hash_t name_hash) noexcept -> const TypeMapJava*; #else - static auto typemap_type_to_type_debug (const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) noexcept -> const char*; - static auto typemap_managed_to_java_debug (const char *typeName, const uint8_t *mvid) noexcept -> const char*; - static auto typemap_java_to_managed_debug (const char *java_type_name, char const** assembly_name, uint32_t *managed_type_token_id) noexcept -> bool; + static auto index_to_name (ssize_t index, const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) -> const char*; + static auto find_index_by_hash (const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) noexcept -> ssize_t; + static auto find_index_by_name (const char *typeName, const TypeMapEntry *map, const char (&name_map)[], std::string_view const& from_name, std::string_view const& to_name) noexcept -> ssize_t; + static auto managed_to_java_debug (const char *typeName, const uint8_t *mvid) noexcept -> const char*; + static auto java_to_managed_debug (const char *java_type_name, char const** assembly_name, uint32_t *managed_type_token_id) noexcept -> bool; #endif }; } diff --git a/src/native/clr/include/xamarin-app.hh b/src/native/clr/include/xamarin-app.hh index a7452ed450b..07912623be5 100644 --- a/src/native/clr/include/xamarin-app.hh +++ b/src/native/clr/include/xamarin-app.hh @@ -64,7 +64,6 @@ struct TypeMapEntry const uint32_t from; const xamarin::android::hash_t from_hash; const uint32_t to; - const xamarin::android::hash_t to_hash; }; // MUST match src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs diff --git a/src/native/clr/xamarin-app-stub/application_dso_stub.cc b/src/native/clr/xamarin-app-stub/application_dso_stub.cc index 7919083ef1f..4d87f7a7adb 100644 --- a/src/native/clr/xamarin-app-stub/application_dso_stub.cc +++ b/src/native/clr/xamarin-app-stub/application_dso_stub.cc @@ -21,7 +21,6 @@ const TypeMap type_map = { }; const bool typemap_use_hashes = true; -const TypeMapManagedTypeInfo type_map_managed_type_info[] = {}; const TypeMapAssembly type_map_unique_assemblies[] = {}; const char type_map_assembly_names[] = {}; const char type_map_managed_type_names[] = {}; From b9d04d16bb4bf7b3fe4fa7fb24ae05ae47c11dfb Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 28 Apr 2025 15:27:01 +0200 Subject: [PATCH 07/24] java-to-managed map should be fully functional now --- .../LlvmIrGenerator/LlvmIrStringBlob.cs | 13 +++++ ...eMappingDebugNativeAssemblyGeneratorCLR.cs | 54 ++++++++----------- src/native/clr/host/typemap.cc | 29 ++++++++-- src/native/clr/include/xamarin-app.hh | 9 ++++ .../xamarin-app-stub/application_dso_stub.cc | 1 + 5 files changed, 70 insertions(+), 36 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs index 0f74ecc8d1c..410efcdcb4d 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrStringBlob.cs @@ -54,6 +54,19 @@ public record struct StringInfo (int Offset, int Length, byte[] Bytes, string Va return (info.Offset, info.Length); } + public int GetIndexOf (string s) + { + if (String.IsNullOrEmpty (s)) { + return -1; + } + + if (!cache.TryGetValue (s, out StringInfo info)) { + return -1; + } + + return info.Offset; + } + public IEnumerable GetSegments () { foreach (StringInfo si in segments) { diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs index bf4ceee5467..c0d5f8ed83b 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs @@ -19,6 +19,7 @@ class TypeMappingDebugNativeAssemblyGeneratorCLR : LlvmIrComposer const string ManagedTypeNamesBlobSymbol = "type_map_managed_type_names"; const string JavaTypeNamesBlobSymbol = "type_map_java_type_names"; const string TypeMapUsesHashesSymbol = "typemap_use_hashes"; + const string TypeMapManagedTypeInfoSymbol = "type_map_managed_type_info"; sealed class TypeMapContextDataProvider : NativeAssemblerStructContextDataProvider { @@ -95,10 +96,6 @@ public override string GetComment (object data, string fieldName) return $" '{entry.AssemblyName}'"; } - if (String.Compare ("managed_type_token_id", fieldName, StringComparison.Ordinal) == 0) { - return $" '{entry.ManagedTypeName}'"; - } - return String.Empty; } } @@ -124,6 +121,21 @@ sealed class TypeMapEntry public uint to; }; + // Order of fields and their type must correspond *exactly* to that in + // src/native/clr/include/xamarin-app.hh TypeMapManagedTypeInfo structure + [NativeAssemblerStructContextDataProvider (typeof (TypeMapManagedTypeInfoContextDataProvider))] + sealed class TypeMapManagedTypeInfo + { + [NativeAssembler (Ignore = true)] + public string AssemblyName; + + [NativeAssembler (UsesDataProvider = true)] + public uint assembly_name_index; + + [NativeAssembler (NumberFormat = LlvmIrVariableNumberFormat.Hexadecimal)] + public uint managed_type_token_id; + }; + // Order of fields and their type must correspond *exactly* to that in // src/native/clr/include/xamarin-app.hh TypeMap structure [NativeAssemblerStructContextDataProvider (typeof (TypeMapContextDataProvider))] @@ -168,9 +180,11 @@ sealed class TypeMapAssembly StructureInfo typeMapEntryStructureInfo; StructureInfo typeMapStructureInfo; StructureInfo typeMapAssemblyStructureInfo; + StructureInfo typeMapManagedTypeInfoStructureInfo; List> javaToManagedMap; List> managedToJavaMap; List> uniqueAssemblies; + List> managedTypeInfos; StructureInstance type_map; public TypeMappingDebugNativeAssemblyGeneratorCLR (TaskLoggingHelper log, TypeMapGenerator.ModuleDebugData data) @@ -236,23 +250,6 @@ protected override void Construct (LlvmIrModule module) Log.LogMessage ("Managed-to-java typemaps will use string-based matching."); } - // Java-to-managed maps don't use hashes since many mappings have multiple instances - foreach (TypeMapGenerator.TypeMapDebugEntry entry in data.JavaToManagedMap) { - TypeMapGenerator.TypeMapDebugEntry managedEntry = entry.DuplicateForJavaToManaged != null ? entry.DuplicateForJavaToManaged : entry; - (int managedTypeNameOffset, int _) = managedTypeNames.Add (entry.ManagedName); - (int javaTypeNameOffset, int _) = javaTypeNames.Add (entry.JavaName); - - var j2m = new TypeMapEntry { - From = entry.JavaName, - To = managedEntry.SkipInJavaToManaged ? String.Empty : entry.ManagedName, - - from = (uint)javaTypeNameOffset, - from_hash = 0, - to = managedEntry.SkipInJavaToManaged ? uint.MaxValue : (uint)managedTypeNameOffset, - }; - javaToManagedMap.Add (new StructureInstance (typeMapEntryStructureInfo, j2m)); - } - var assemblyNamesBlob = new LlvmIrStringBlob (); foreach (TypeMapGenerator.TypeMapDebugAssembly asm in data.UniqueAssemblies) { (int assemblyNameOffset, int assemblyNameLength) = assemblyNamesBlob.Add (asm.Name); @@ -292,10 +289,9 @@ protected override void Construct (LlvmIrModule module) var typeInfo = new TypeMapManagedTypeInfo { AssemblyName = entry.AssemblyName, - ManagedTypeName = entry.ManagedName, assembly_name_index = (uint)assemblyNameOffset, - managed_type_token_id = entry.ManagedTypeTokenId, + managed_type_token_id = entry.TypeDefinition.MetadataToken.ToUInt32 (), }; managedTypeInfos.Add (new StructureInstance (typeMapManagedTypeInfoStructureInfo, typeInfo)); } @@ -311,15 +307,9 @@ protected override void Construct (LlvmIrModule module) type_map = new StructureInstance (typeMapStructureInfo, map); module.AddGlobalVariable (TypeMapSymbol, type_map, LlvmIrVariableOptions.GlobalConstant); - - if (managedToJavaMap.Count > 0) { - module.AddGlobalVariable (ManagedToJavaSymbol, managedToJavaMap, LlvmIrVariableOptions.LocalConstant); - } - - if (javaToManagedMap.Count > 0) { - module.AddGlobalVariable (JavaToManagedSymbol, javaToManagedMap, LlvmIrVariableOptions.LocalConstant); - } - + module.AddGlobalVariable (ManagedToJavaSymbol, managedToJavaMap, LlvmIrVariableOptions.LocalConstant); + module.AddGlobalVariable (JavaToManagedSymbol, javaToManagedMap, LlvmIrVariableOptions.LocalConstant); + module.AddGlobalVariable (TypeMapManagedTypeInfoSymbol, managedTypeInfos, LlvmIrVariableOptions.GlobalConstant); module.AddGlobalVariable (TypeMapUsesHashesSymbol, typemap_uses_hashes, LlvmIrVariableOptions.GlobalConstant); module.AddGlobalVariable (UniqueAssembliesSymbol, uniqueAssemblies, LlvmIrVariableOptions.GlobalConstant); module.AddGlobalVariable (AssemblyNamesBlobSymbol, assemblyNamesBlob, LlvmIrVariableOptions.GlobalConstant); diff --git a/src/native/clr/host/typemap.cc b/src/native/clr/host/typemap.cc index c83b4935d6e..f8c93bfbd4b 100644 --- a/src/native/clr/host/typemap.cc +++ b/src/native/clr/host/typemap.cc @@ -344,15 +344,36 @@ auto TypeMapper::managed_to_java (const char *typeName, const uint8_t *mvid) noe [[gnu::flatten]] auto TypeMapper::java_to_managed_debug (const char *java_type_name, char const** assembly_name, uint32_t *managed_type_token_id) noexcept -> bool { - // FIXME: this is currently VERY broken - *assembly_name = nullptr; - *managed_type_token_id = 0; + if (assembly_name == nullptr || managed_type_token_id == nullptr) [[unlikely]] { + log_warn (LOG_ASSEMBLY, "Managed land called java-to-managed mapping function with invalid pointers"); + return false; + } // We need to find entry matching the Java type name, which will then... ssize_t idx = find_index_by_name (java_type_name, type_map.java_to_managed, type_map_java_type_names, JAVA, MANAGED); // ..provide us with the managed type name index - return index_to_name (idx, java_type_name, type_map.java_to_managed, type_map_managed_type_names, JAVA, MANAGED); + const char *name = index_to_name (idx, java_type_name, type_map.java_to_managed, type_map_managed_type_names, JAVA, MANAGED); + if (name == nullptr) { + *assembly_name = nullptr; + *managed_type_token_id = 0; + return false; + } + + TypeMapManagedTypeInfo const& type_info = type_map_managed_type_info[idx]; + *assembly_name = &type_map_assembly_names[type_info.assembly_name_index]; + *managed_type_token_id = type_info.managed_type_token_id; + + log_debug ( + LOG_ASSEMBLY, + "Mapped Java type '{}' to managed type '{}' in assembly '{}' and with token '{:x}'", + optional_string (java_type_name), + name, + *assembly_name, + *managed_type_token_id + ); + + return true; } #else // def DEBUG diff --git a/src/native/clr/include/xamarin-app.hh b/src/native/clr/include/xamarin-app.hh index 07912623be5..8caa09323d4 100644 --- a/src/native/clr/include/xamarin-app.hh +++ b/src/native/clr/include/xamarin-app.hh @@ -56,6 +56,8 @@ struct TypeMapIndexHeader uint32_t module_file_name_width; }; +// MUST match src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs +// // If any of the members is set to maximum uint32_t value it means the entry is ignored (treated // as equivalent to `nullptr` if the member was a pointer). The reasoning is that no string could // begin at this offset (well, an empty string could, but we don't have those here) @@ -66,6 +68,13 @@ struct TypeMapEntry const uint32_t to; }; +// MUST match src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs +struct TypeMapManagedTypeInfo +{ + const uint32_t assembly_name_index; + const uint32_t managed_type_token_id; +}; + // MUST match src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs struct TypeMap { diff --git a/src/native/clr/xamarin-app-stub/application_dso_stub.cc b/src/native/clr/xamarin-app-stub/application_dso_stub.cc index 4d87f7a7adb..7919083ef1f 100644 --- a/src/native/clr/xamarin-app-stub/application_dso_stub.cc +++ b/src/native/clr/xamarin-app-stub/application_dso_stub.cc @@ -21,6 +21,7 @@ const TypeMap type_map = { }; const bool typemap_use_hashes = true; +const TypeMapManagedTypeInfo type_map_managed_type_info[] = {}; const TypeMapAssembly type_map_unique_assemblies[] = {}; const char type_map_assembly_names[] = {}; const char type_map_managed_type_names[] = {}; From 4fa897c553e2f1457788576ac1cf95edd3688edd Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 29 Apr 2025 10:57:56 +0200 Subject: [PATCH 08/24] A handful of tweaks for the Debug build --- ...eMappingDebugNativeAssemblyGeneratorCLR.cs | 23 ++++++++++++++----- src/native/clr/host/typemap.cc | 9 +++----- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs index c0d5f8ed83b..2e6e4e67fbb 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs @@ -57,11 +57,11 @@ public override string GetComment (object data, string fieldName) var entry = EnsureType (data); if (String.Compare ("from", fieldName, StringComparison.Ordinal) == 0) { - return $"from: {entry.From}"; + return $" from: '{entry.From}'"; } if (String.Compare ("to", fieldName, StringComparison.Ordinal) == 0) { - return $"to: {entry.To}"; + return $" to: '{entry.To}'"; } return String.Empty; @@ -96,6 +96,10 @@ public override string GetComment (object data, string fieldName) return $" '{entry.AssemblyName}'"; } + if (String.Compare ("managed_type_token_id", fieldName, StringComparison.Ordinal) == 0) { + return $" '{entry.ManagedTypeName}'"; + } + return String.Empty; } } @@ -129,10 +133,13 @@ sealed class TypeMapManagedTypeInfo [NativeAssembler (Ignore = true)] public string AssemblyName; + [NativeAssembler (Ignore = true)] + public string ManagedTypeName; + [NativeAssembler (UsesDataProvider = true)] public uint assembly_name_index; - [NativeAssembler (NumberFormat = LlvmIrVariableNumberFormat.Hexadecimal)] + [NativeAssembler (UsesDataProvider = true, NumberFormat = LlvmIrVariableNumberFormat.Hexadecimal)] public uint managed_type_token_id; }; @@ -228,11 +235,15 @@ protected override void Construct (LlvmIrModule module) To = entry.JavaName, from = (uint)managedTypeNameOffset, - from_hash = MonoAndroidHelper.GetXxHash (entry.ManagedName, is64Bit: true), + from_hash = typemap_uses_hashes ? MonoAndroidHelper.GetXxHash (entry.ManagedName, is64Bit: true) : 0, to = (uint)javaTypeNameOffset, }; managedToJavaMap.Add (new StructureInstance (typeMapEntryStructureInfo, m2j)); + if (!typemap_uses_hashes) { + continue; + } + if (usedHashes.ContainsKey (m2j.from_hash)) { typemap_uses_hashes = false; // It could be a warning, but it's not really actionable - users might not be able to rename the clashing types @@ -289,9 +300,10 @@ protected override void Construct (LlvmIrModule module) var typeInfo = new TypeMapManagedTypeInfo { AssemblyName = entry.AssemblyName, + ManagedTypeName = entry.ManagedName, assembly_name_index = (uint)assemblyNameOffset, - managed_type_token_id = entry.TypeDefinition.MetadataToken.ToUInt32 (), + managed_type_token_id = entry.ManagedTypeTokenId, }; managedTypeInfos.Add (new StructureInstance (typeMapManagedTypeInfoStructureInfo, typeInfo)); } @@ -302,7 +314,6 @@ protected override void Construct (LlvmIrModule module) entry_count = data.EntryCount, unique_assemblies_count = (ulong)data.UniqueAssemblies.Count, - assembly_names_blob_size = (ulong)assemblyNamesBlob.Size, }; type_map = new StructureInstance (typeMapStructureInfo, map); diff --git a/src/native/clr/host/typemap.cc b/src/native/clr/host/typemap.cc index f8c93bfbd4b..5d0184eec35 100644 --- a/src/native/clr/host/typemap.cc +++ b/src/native/clr/host/typemap.cc @@ -156,12 +156,8 @@ auto TypeMapper::managed_to_java_debug (const char *typeName, const uint8_t *mvi TypeMapAssembly const& assm = type_map_unique_assemblies[idx]; full_type_name.append (", "sv); - if (assm.name_offset < type_map.assembly_names_blob_size) [[likely]] { - full_type_name.append (&type_map_assembly_names[assm.name_offset], assm.name_length); - log_debug (LOG_ASSEMBLY, "typemap: fixed-up type name: '{}'", full_type_name.get ()); - } else { - log_warn (LOG_ASSEMBLY, "typemap: fnvalid assembly name offset {}", assm.name_offset); - } + // We explicitly trust the build process here, with regards to validity of offsets + full_type_name.append (&type_map_assembly_names[assm.name_offset], assm.name_length); } else { log_warn (LOG_ASSEMBLY, "typemap: unable to look up assembly name for type '{}', trying without it.", typeName); } @@ -360,6 +356,7 @@ auto TypeMapper::java_to_managed_debug (const char *java_type_name, char const** return false; } + // We explicitly trust the build process here, with regards to the size of the arrays TypeMapManagedTypeInfo const& type_info = type_map_managed_type_info[idx]; *assembly_name = &type_map_assembly_names[type_info.assembly_name_index]; *managed_type_token_id = type_info.managed_type_token_id; From 57d0b8014690cec3c4a4eedfea6eed7060a40b43 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 29 Apr 2025 11:07:53 +0200 Subject: [PATCH 09/24] Cleanup --- src/Mono.Android/Android.Runtime/AndroidRuntime.cs | 2 -- src/Mono.Android/Java.Interop/TypeManager.cs | 3 --- 2 files changed, 5 deletions(-) diff --git a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs index ec0f0891c86..3f6d8f86cf2 100644 --- a/src/Mono.Android/Android.Runtime/AndroidRuntime.cs +++ b/src/Mono.Android/Android.Runtime/AndroidRuntime.cs @@ -265,11 +265,9 @@ public AndroidTypeManager (bool jniAddNativeMethodRegistrationAttributePresent) protected override IEnumerable GetTypesForSimpleReference (string jniSimpleReference) { - RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"#1 GetTypesForSimpleReference (\"{jniSimpleReference}\")"); foreach (var ti in base.GetTypesForSimpleReference (jniSimpleReference)) yield return ti; - RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"#2 GetTypesForSimpleReference (\"{jniSimpleReference}\")"); var t = Java.Interop.TypeManager.GetJavaToManagedType (jniSimpleReference); if (t != null) yield return t; diff --git a/src/Mono.Android/Java.Interop/TypeManager.cs b/src/Mono.Android/Java.Interop/TypeManager.cs index e526c52bd39..adc07f0edb1 100644 --- a/src/Mono.Android/Java.Interop/TypeManager.cs +++ b/src/Mono.Android/Java.Interop/TypeManager.cs @@ -131,7 +131,6 @@ static Type[] GetParameterTypes (string? signature) [UnconditionalSuppressMessage ("Trimming", "IL2057", Justification = "Type.GetType() can never statically know the string value from parameter 'typename_ptr'.")] static void n_Activate (IntPtr jnienv, IntPtr jclass, IntPtr typename_ptr, IntPtr signature_ptr, IntPtr jobject, IntPtr parameters_ptr) { - RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"TypeManager.n_Activate"); if (!global::Java.Interop.JniEnvironment.BeginMarshalMethod (jnienv, out var __envp, out var __r)) return; @@ -254,7 +253,6 @@ static Type monovm_typemap_java_to_managed (string java_type_name) internal static Type? GetJavaToManagedType (string class_name) { - RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"TypeManager.GetJavaToManagedType (\"{class_name}\")"); Type? type = JNIEnvInit.RuntimeType switch { DotNetRuntimeType.MonoVM => monovm_typemap_java_to_managed (class_name), DotNetRuntimeType.CoreCLR => clr_typemap_java_to_managed (class_name), @@ -283,7 +281,6 @@ static Type monovm_typemap_java_to_managed (string java_type_name) [UnconditionalSuppressMessage ("Trimming", "IL2072", Justification = "TypeManager.CreateProxy() does not statically know the value of the 'type' local variable.")] internal static IJavaPeerable? CreateInstance (IntPtr handle, JniHandleOwnership transfer, Type? targetType) { - RuntimeNativeMethods.monodroid_log (LogLevel.Warn, LogCategories.Default, $"TypeManager.CreateInstance ()"); Type? type = null; IntPtr class_ptr = JNIEnv.GetObjectClass (handle); string? class_name = GetClassName (class_ptr); From 2f74c7a896dbfb5556c932b42f2b897c2dff4942 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 29 Apr 2025 11:40:26 +0200 Subject: [PATCH 10/24] Add on-device test for string-based typemaps --- .../Tests/InstallAndRunTests.cs | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs index 246287cae37..d179740e994 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs @@ -565,8 +565,18 @@ public void SingleProject_ApplicationId ([Values (false, true)] bool testOnly) } [Test] - public void AppWithStyleableUsageRuns ([Values (true, false)] bool isRelease, [Values (true, false)] bool linkResources) + public void AppWithStyleableUsageRuns ([Values (true, false)] bool useCLR, [Values (true, false)] bool isRelease, + [Values (true, false)] bool linkResources, [Values (true, false)] bool useStringTypeMaps) { + // Not all combinations are valid, ignore those that aren't + if (!useCLR && useStringTypeMaps) { + Assert.Ignore ("String-based typemaps mode is used only in CoreCLR apps"); + } + + if (useCLR && isRelease && useStringTypeMaps) { + Assert.Ignore ("String-based typemaps mode is available only in Debug CoreCLR builds"); + } + var rootPath = Path.Combine (Root, "temp", TestName); var lib = new XamarinAndroidLibraryProject () { ProjectName = "Styleable.Library" @@ -615,6 +625,7 @@ public MyLibraryLayout (Android.Content.Context context, Android.Util.IAttribute proj = new XamarinAndroidApplicationProject () { IsRelease = isRelease, }; + proj.SetProperty ("UseMonoRuntime", useCLR ? "false" : "true"); proj.AddReference (lib); proj.AndroidResources.Add (new AndroidItem.AndroidResource ("Resources\\values\\styleables.xml") { @@ -652,15 +663,27 @@ public MyLayout (Android.Content.Context context, Android.Util.IAttributeSet att } "); - var abis = new string [] { "armeabi-v7a", "arm64-v8a", "x86", "x86_64" }; + string[] abis = useCLR switch { + true => new string [] { "arm64-v8a", "x86_64" }, + false => new string [] { "armeabi-v7a", "arm64-v8a", "x86", "x86_64" }, + }; + proj.SetAndroidSupportedAbis (abis); var libBuilder = CreateDllBuilder (Path.Combine (rootPath, lib.ProjectName)); Assert.IsTrue (libBuilder.Build (lib), "Library should have built succeeded."); builder = CreateApkBuilder (Path.Combine (rootPath, proj.ProjectName)); - Assert.IsTrue (builder.Install (proj), "Install should have succeeded."); - RunProjectAndAssert (proj, builder); + + Dictionary? environmentVariables = null; + if (useCLR && !isRelease && useStringTypeMaps) { + // The variable must have content to enable string-based typemaps + environmentVariables = new (StringComparer.Ordinal) { + {"CI_TYPEMAP_DEBUG_USE_STRINGS", "yes"} + }; + } + + RunProjectAndAssert (proj, builder, environmentVariables: environmentVariables); var didStart = WaitForActivityToStart (proj.PackageName, "MainActivity", Path.Combine (Root, builder.ProjectDirectory, "startup-logcat.log")); From e943a4fa1488981671cf4b81998664a7d2fb124e Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Wed, 30 Apr 2025 20:38:33 +0200 Subject: [PATCH 11/24] Fix after rebase --- .../Utilities/TypeMapCecilAdapter.cs | 10 ++++------ .../Utilities/TypeMapGenerator.cs | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapCecilAdapter.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapCecilAdapter.cs index ab8288ff33c..e48923c4771 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapCecilAdapter.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapCecilAdapter.cs @@ -32,10 +32,8 @@ public static (TypeMapDebugDataSets dataSets, bool foundJniNativeRegistration) G var managedToJava = new List (); var foundJniNativeRegistration = false; - var javaDuplicates = new Dictionary> (StringComparer.Ordinal); - var uniqueAssemblies = needUniqueAssemblies ? new Dictionary (StringComparer.OrdinalIgnoreCase) : null; - foreach (TypeDefinition td in state.AllJavaTypes) { - UpdateApplicationConfig (state, td); + foreach (var td in types) { + foundJniNativeRegistration = JniAddNativeMethodRegistrationAttributeFound (foundJniNativeRegistration, td); TypeMapDebugEntry entry = GetDebugEntry (td, cache); HandleDebugDuplicates (javaDuplicates, entry, td, cache); @@ -62,11 +60,11 @@ public static (TypeMapDebugDataSets dataSets, bool foundJniNativeRegistration) G SyncDebugDuplicates (javaDuplicates); - return new TypeMapDebugDataSets { + return (new TypeMapDebugDataSets { JavaToManaged = javaToManaged, ManagedToJava = managedToJava, UniqueAssemblies = uniqueAssemblies != null ? new List (uniqueAssemblies.Values) : null - }; + }, foundJniNativeRegistration); } public static ReleaseGenerationState GetReleaseGenerationState (NativeCodeGenState state) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs index 3183f49154b..7a22e76f2a0 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs @@ -155,7 +155,7 @@ public void Generate (bool debugBuild, bool skipJniAddNativeMethodRegistrationAt void GenerateDebugNativeAssembly (string outputDirectory) { - TypeMapDebugDataSets dataSets = TypeMapCecilAdapter.GetDebugNativeEntries (state, needUniqueAssemblies: runtime == AndroidRuntime.CoreCLR); + TypeMapDebugDataSets dataSets = state.GetDebugNativeEntries (needUniqueAssemblies: runtime == AndroidRuntime.CoreCLR); var data = new ModuleDebugData { EntryCount = (uint)dataSets.JavaToManaged.Count, From 626796fd8045f3448afe8cbf74c30020e8ff1724 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 5 May 2025 14:24:47 +0200 Subject: [PATCH 12/24] Fix nullable issues after rebase on `main` --- ...eMappingDebugNativeAssemblyGeneratorCLR.cs | 51 ++++++++++++++----- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs index 2e6e4e67fbb..a6b76fc23da 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs @@ -34,7 +34,7 @@ public override ulong GetBufferSize (object data, string fieldName) return 0; } - public override string GetPointedToSymbolName (object data, string fieldName) + public override string? GetPointedToSymbolName (object data, string fieldName) { var map_module = EnsureType (data); @@ -110,10 +110,10 @@ public override string GetComment (object data, string fieldName) sealed class TypeMapEntry { [NativeAssembler (Ignore = true)] - public string From; + public string From = String.Empty; [NativeAssembler (Ignore = true)] - public string To; + public string To = String.Empty; [NativeAssembler (UsesDataProvider = true)] public uint from; @@ -131,10 +131,10 @@ sealed class TypeMapEntry sealed class TypeMapManagedTypeInfo { [NativeAssembler (Ignore = true)] - public string AssemblyName; + public string AssemblyName = String.Empty; [NativeAssembler (Ignore = true)] - public string ManagedTypeName; + public string ManagedTypeName = String.Empty; [NativeAssembler (UsesDataProvider = true)] public uint assembly_name_index; @@ -170,7 +170,7 @@ sealed class TypeMap sealed class TypeMapAssembly { [NativeAssembler (Ignore = true)] - public string Name; + public string Name = String.Empty; [NativeAssembler (Ignore = true)] public Guid MVID; @@ -184,15 +184,15 @@ sealed class TypeMapAssembly } readonly TypeMapGenerator.ModuleDebugData data; - StructureInfo typeMapEntryStructureInfo; - StructureInfo typeMapStructureInfo; - StructureInfo typeMapAssemblyStructureInfo; - StructureInfo typeMapManagedTypeInfoStructureInfo; + StructureInfo? typeMapEntryStructureInfo; + StructureInfo? typeMapStructureInfo; + StructureInfo? typeMapAssemblyStructureInfo; + StructureInfo? typeMapManagedTypeInfoStructureInfo; List> javaToManagedMap; List> managedToJavaMap; List> uniqueAssemblies; List> managedTypeInfos; - StructureInstance type_map; + StructureInstance? type_map; public TypeMappingDebugNativeAssemblyGeneratorCLR (TaskLoggingHelper log, TypeMapGenerator.ModuleDebugData data) : base (log) @@ -213,6 +213,10 @@ protected override void Construct (LlvmIrModule module) { module.DefaultStringGroup = "tmd"; + if (data.UniqueAssemblies == null) { + throw new InvalidOperationException ("Internal error: unique assemblies collection must be present"); + } + MapStructures (module); var managedTypeNames = new LlvmIrStringBlob (); @@ -254,7 +258,17 @@ protected override void Construct (LlvmIrModule module) } // Input is sorted on name, we need to re-sort it on hashes, if used if (typemap_uses_hashes) { - managedToJavaMap.Sort ((StructureInstance a, StructureInstance b) => a.Instance.from_hash.CompareTo (b.Instance.from_hash)); + managedToJavaMap.Sort ((StructureInstance a, StructureInstance b) => { + if (a.Instance == null) { + return b.Instance == null ? 0 : -1; + } + + if (b.Instance == null) { + return 1; + } + + return a.Instance.from_hash.CompareTo (b.Instance.from_hash); + }); } if (!typemap_uses_hashes) { @@ -274,7 +288,18 @@ protected override void Construct (LlvmIrModule module) }; uniqueAssemblies.Add (new StructureInstance (typeMapAssemblyStructureInfo, entry)); } - uniqueAssemblies.Sort ((StructureInstance a, StructureInstance b) => a.Instance.mvid_hash.CompareTo (b.Instance.mvid_hash)); + + uniqueAssemblies.Sort ((StructureInstance a, StructureInstance b) => { + if (a.Instance == null) { + return b.Instance == null ? 0 : -1; + } + + if (b.Instance == null) { + return 1; + } + + return a.Instance.mvid_hash.CompareTo (b.Instance.mvid_hash); + }); var managedTypeInfos = new List> (); // Java-to-managed maps don't use hashes since many mappings have multiple instances From 64068173df4139712a1ba47dcf7f1d0b478c0225 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 5 May 2025 13:11:41 +0200 Subject: [PATCH 13/24] This test requires more changes in the runtime, will be enabled in another PR --- .../Tests/InstallAndRunTests.cs | 31 +++---------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs index d179740e994..246287cae37 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs @@ -565,18 +565,8 @@ public void SingleProject_ApplicationId ([Values (false, true)] bool testOnly) } [Test] - public void AppWithStyleableUsageRuns ([Values (true, false)] bool useCLR, [Values (true, false)] bool isRelease, - [Values (true, false)] bool linkResources, [Values (true, false)] bool useStringTypeMaps) + public void AppWithStyleableUsageRuns ([Values (true, false)] bool isRelease, [Values (true, false)] bool linkResources) { - // Not all combinations are valid, ignore those that aren't - if (!useCLR && useStringTypeMaps) { - Assert.Ignore ("String-based typemaps mode is used only in CoreCLR apps"); - } - - if (useCLR && isRelease && useStringTypeMaps) { - Assert.Ignore ("String-based typemaps mode is available only in Debug CoreCLR builds"); - } - var rootPath = Path.Combine (Root, "temp", TestName); var lib = new XamarinAndroidLibraryProject () { ProjectName = "Styleable.Library" @@ -625,7 +615,6 @@ public MyLibraryLayout (Android.Content.Context context, Android.Util.IAttribute proj = new XamarinAndroidApplicationProject () { IsRelease = isRelease, }; - proj.SetProperty ("UseMonoRuntime", useCLR ? "false" : "true"); proj.AddReference (lib); proj.AndroidResources.Add (new AndroidItem.AndroidResource ("Resources\\values\\styleables.xml") { @@ -663,27 +652,15 @@ public MyLayout (Android.Content.Context context, Android.Util.IAttributeSet att } "); - string[] abis = useCLR switch { - true => new string [] { "arm64-v8a", "x86_64" }, - false => new string [] { "armeabi-v7a", "arm64-v8a", "x86", "x86_64" }, - }; - + var abis = new string [] { "armeabi-v7a", "arm64-v8a", "x86", "x86_64" }; proj.SetAndroidSupportedAbis (abis); var libBuilder = CreateDllBuilder (Path.Combine (rootPath, lib.ProjectName)); Assert.IsTrue (libBuilder.Build (lib), "Library should have built succeeded."); builder = CreateApkBuilder (Path.Combine (rootPath, proj.ProjectName)); - Assert.IsTrue (builder.Install (proj), "Install should have succeeded."); - - Dictionary? environmentVariables = null; - if (useCLR && !isRelease && useStringTypeMaps) { - // The variable must have content to enable string-based typemaps - environmentVariables = new (StringComparer.Ordinal) { - {"CI_TYPEMAP_DEBUG_USE_STRINGS", "yes"} - }; - } - RunProjectAndAssert (proj, builder, environmentVariables: environmentVariables); + Assert.IsTrue (builder.Install (proj), "Install should have succeeded."); + RunProjectAndAssert (proj, builder); var didStart = WaitForActivityToStart (proj.PackageName, "MainActivity", Path.Combine (Root, builder.ProjectDirectory, "startup-logcat.log")); From cf50b0d747cb45fd365e56101fc1a491d9a7a474 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 6 May 2025 19:19:22 +0200 Subject: [PATCH 14/24] Test the debug typemaps --- .../Tests/InstallAndRunTests.cs | 31 ++++++++++++++++--- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs index 246287cae37..d179740e994 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs @@ -565,8 +565,18 @@ public void SingleProject_ApplicationId ([Values (false, true)] bool testOnly) } [Test] - public void AppWithStyleableUsageRuns ([Values (true, false)] bool isRelease, [Values (true, false)] bool linkResources) + public void AppWithStyleableUsageRuns ([Values (true, false)] bool useCLR, [Values (true, false)] bool isRelease, + [Values (true, false)] bool linkResources, [Values (true, false)] bool useStringTypeMaps) { + // Not all combinations are valid, ignore those that aren't + if (!useCLR && useStringTypeMaps) { + Assert.Ignore ("String-based typemaps mode is used only in CoreCLR apps"); + } + + if (useCLR && isRelease && useStringTypeMaps) { + Assert.Ignore ("String-based typemaps mode is available only in Debug CoreCLR builds"); + } + var rootPath = Path.Combine (Root, "temp", TestName); var lib = new XamarinAndroidLibraryProject () { ProjectName = "Styleable.Library" @@ -615,6 +625,7 @@ public MyLibraryLayout (Android.Content.Context context, Android.Util.IAttribute proj = new XamarinAndroidApplicationProject () { IsRelease = isRelease, }; + proj.SetProperty ("UseMonoRuntime", useCLR ? "false" : "true"); proj.AddReference (lib); proj.AndroidResources.Add (new AndroidItem.AndroidResource ("Resources\\values\\styleables.xml") { @@ -652,15 +663,27 @@ public MyLayout (Android.Content.Context context, Android.Util.IAttributeSet att } "); - var abis = new string [] { "armeabi-v7a", "arm64-v8a", "x86", "x86_64" }; + string[] abis = useCLR switch { + true => new string [] { "arm64-v8a", "x86_64" }, + false => new string [] { "armeabi-v7a", "arm64-v8a", "x86", "x86_64" }, + }; + proj.SetAndroidSupportedAbis (abis); var libBuilder = CreateDllBuilder (Path.Combine (rootPath, lib.ProjectName)); Assert.IsTrue (libBuilder.Build (lib), "Library should have built succeeded."); builder = CreateApkBuilder (Path.Combine (rootPath, proj.ProjectName)); - Assert.IsTrue (builder.Install (proj), "Install should have succeeded."); - RunProjectAndAssert (proj, builder); + + Dictionary? environmentVariables = null; + if (useCLR && !isRelease && useStringTypeMaps) { + // The variable must have content to enable string-based typemaps + environmentVariables = new (StringComparer.Ordinal) { + {"CI_TYPEMAP_DEBUG_USE_STRINGS", "yes"} + }; + } + + RunProjectAndAssert (proj, builder, environmentVariables: environmentVariables); var didStart = WaitForActivityToStart (proj.PackageName, "MainActivity", Path.Combine (Root, builder.ProjectDirectory, "startup-logcat.log")); From a99cb007de38ff8b2c7ca0c3e2b0d73d36aba986 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Wed, 7 May 2025 17:39:57 +0200 Subject: [PATCH 15/24] Implement support for fastdev assemblies --- src/native/clr/host/CMakeLists.txt | 6 + src/native/clr/host/fastdev-assemblies.cc | 130 ++++++++++++++++++ src/native/clr/host/host.cc | 43 ++++-- src/native/clr/include/constants.hh | 1 + .../clr/include/host/fastdev-assemblies.hh | 29 ++++ src/native/clr/include/runtime-base/util.hh | 40 +++++- 6 files changed, 232 insertions(+), 17 deletions(-) create mode 100644 src/native/clr/host/fastdev-assemblies.cc create mode 100644 src/native/clr/include/host/fastdev-assemblies.hh diff --git a/src/native/clr/host/CMakeLists.txt b/src/native/clr/host/CMakeLists.txt index d53734eaf8c..d2338f4f072 100644 --- a/src/native/clr/host/CMakeLists.txt +++ b/src/native/clr/host/CMakeLists.txt @@ -41,6 +41,12 @@ set(XAMARIN_MONODROID_SOURCES xamarin_getifaddrs.cc ) +if(DEBUG_BUILD) + list(APPEND XAMARIN_MONODROID_SOURCES + fastdev-assemblies.cc + ) +endif() + list(APPEND LOCAL_CLANG_CHECK_SOURCES ${XAMARIN_MONODROID_SOURCES} ) diff --git a/src/native/clr/host/fastdev-assemblies.cc b/src/native/clr/host/fastdev-assemblies.cc new file mode 100644 index 00000000000..ad209070f64 --- /dev/null +++ b/src/native/clr/host/fastdev-assemblies.cc @@ -0,0 +1,130 @@ +#include +#include +#include +#include + +#include +#include +#include + +#include +#include +#include +#include + +using namespace xamarin::android; + +auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &size) noexcept -> void* +{ + size = 0; + + std::string const& override_dir_path = AndroidSystem::get_primary_override_dir (); + if (!Util::dir_exists (override_dir_path)) [[unlikely]] { + log_debug (LOG_ASSEMBLY, "Override directory '{}' does not exist", override_dir_path); + return nullptr; + } + + if (override_dir_fd == -1) [[unlikely]] { + std::lock_guard dir_lock { override_dir_lock }; + if (override_dir_fd == -1) [[likely]] { + override_dir = opendir (override_dir_path.c_str ()); + if (override_dir == nullptr) [[unlikely]] { + log_warn (LOG_ASSEMBLY, "Failed to open override dir '{}'. {}", override_dir_path, strerror (errno)); + return nullptr; + } + override_dir_fd = dirfd (override_dir); + } + } + + log_debug ( + LOG_ASSEMBLY, + "Attempting to load FastDev assembly '{}' from override directory '{}'", + name, + override_dir_path + ); + + if (!Util::file_exists (override_dir_fd, name)) { + log_warn ( + LOG_ASSEMBLY, + "FastDev assembly '{}' not found.", + name + ); + return nullptr; + } + + log_debug ( + LOG_ASSEMBLY, + "Found FastDev assembly '{}'", + name + ); + + auto file_size = Util::get_file_size_at (override_dir_fd, name); + if (!file_size) [[unlikely]] { + log_warn ( + LOG_ASSEMBLY, + "Unable to determine FastDev assembly '{}' file size", + name + ); + return nullptr; + } + + constexpr size_t MAX_SIZE = std::numeric_limits::max (); + if (file_size.value () > MAX_SIZE) [[unlikely]] { + Helpers::abort_application ( + LOG_ASSEMBLY, + std::format ( + "FastDev assembly '{}' size exceeds the maximum supported value of {}", + name, + MAX_SIZE + ) + ); + } + + size = static_cast(file_size.value ()); + int asm_fd = openat (override_dir_fd, name.data (), O_RDONLY); + if (asm_fd < 0) { + log_warn ( + LOG_ASSEMBLY, + "Failed to open FastDev assembly '{}' for reading. {}", + name, + strerror (errno) + ); + + size = 0; + return nullptr; + } + + // TODO: consider who owns the pointer - we allocate the data, but we have no way of knowing when + // the allocated space is no longer (if ever) needed by CoreCLR. Probably would be best if + // CoreCLR notified us when it wants to free the data, as that eliminates any races as well + // as ambiguity. + auto buffer = new uint8_t[file_size.value ()]; + ssize_t nread = 0; + do { + nread = read (asm_fd, reinterpret_cast(buffer), file_size.value ()); + } while (nread == -1 && errno == EINTR); + close (asm_fd); + + if (nread != size) [[unlikely]] { + delete[] buffer; + + log_warn ( + LOG_ASSEMBLY, + "Failed to read FastDev assembly '{}' data. {}", + name, + strerror (errno) + ); + + size = 0; + return nullptr; + } + + log_debug ( + LOG_ASSEMBLY, + "Read {} bytes of FastDev assembly '{}'", + nread, + name + ); + + return reinterpret_cast(buffer); +} diff --git a/src/native/clr/host/host.cc b/src/native/clr/host/host.cc index 8103c70030b..67cf27465dd 100644 --- a/src/native/clr/host/host.cc +++ b/src/native/clr/host/host.cc @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -46,22 +47,40 @@ bool Host::clr_external_assembly_probe (const char *path, void **data_start, int internal_timing.start_event (TimingEventKind::AssemblyLoad); } - *data_start = AssemblyStore::open_assembly (path, *size); + auto log_and_return = [](const char *name, void *data_start, int64_t size) { + if (FastTiming::enabled ()) [[unlikely]] { + internal_timing.end_event (true /* uses_more_info */); + internal_timing.add_more_info (name); + } - if (FastTiming::enabled ()) [[unlikely]] { - internal_timing.end_event (true /* uses_more_info */); - internal_timing.add_more_info (path); + log_debug ( + LOG_ASSEMBLY, + "Assembly '{}' data {}mapped ({:p}, {} bytes)", + optional_string (name), + data_start == nullptr ? "not "sv : ""sv, + data_start, + size + ); + + return data_start != nullptr && size > 0; + }; + + if constexpr (Constants::is_debug_build) { + *data_start = FastDevAssemblies::open_assembly (path, *size); + if (*data_start != nullptr && *size > 0) { + return log_and_return (path, *data_start, *size); + } + + log_warn ( + LOG_ASSEMBLY, + "Assembly '{}' not found in FastDev override directory. Attempting to load from assembly store", + optional_string (path) + ); } - log_debug ( - LOG_ASSEMBLY, - "Assembly data {}mapped ({:p}, {} bytes)", - *data_start == nullptr ? "not "sv : ""sv, - *data_start, - *size - ); + *data_start = AssemblyStore::open_assembly (path, *size); - return *data_start != nullptr && *size > 0; + return log_and_return (path, *data_start, *size); } auto Host::zip_scan_callback (std::string_view const& apk_path, int apk_fd, dynamic_local_string const& entry_name, uint32_t offset, uint32_t size) -> bool diff --git a/src/native/clr/include/constants.hh b/src/native/clr/include/constants.hh index 5e4d5adcab2..e29c2fa5c32 100644 --- a/src/native/clr/include/constants.hh +++ b/src/native/clr/include/constants.hh @@ -41,6 +41,7 @@ namespace xamarin::android { public: static constexpr std::string_view NEWLINE { "\n" }; static constexpr std::string_view EMPTY { "" }; + static constexpr std::string_view DIR_SEP { "/" }; // .data() must be used otherwise string_view length will include the trailing \0 in the array static constexpr std::string_view RUNTIME_CONFIG_BLOB_NAME { RUNTIME_CONFIG_BLOB_NAME_ARRAY.data () }; diff --git a/src/native/clr/include/host/fastdev-assemblies.hh b/src/native/clr/include/host/fastdev-assemblies.hh new file mode 100644 index 00000000000..51f1945fce3 --- /dev/null +++ b/src/native/clr/include/host/fastdev-assemblies.hh @@ -0,0 +1,29 @@ +#pragma once + +#include + +#include +#include +#include + +namespace xamarin::android { + class FastDevAssemblies + { + public: +#if defined(DEBUG) + static auto open_assembly (std::string_view const& name, int64_t &size) noexcept -> void*; +#else + static auto open_assembly ([[maybe_unused]] std::string_view const& name, [[maybe_unused]] int64_t &size) noexcept -> void* + { + return nullptr; + } +#endif + + private: +#if defined(DEBUG) + static inline DIR *override_dir = nullptr; + static inline int override_dir_fd = -1; + static inline std::mutex override_dir_lock {}; +#endif + }; +} diff --git a/src/native/clr/include/runtime-base/util.hh b/src/native/clr/include/runtime-base/util.hh index 0dc4316f24b..a3512304e11 100644 --- a/src/native/clr/include/runtime-base/util.hh +++ b/src/native/clr/include/runtime-base/util.hh @@ -73,19 +73,38 @@ namespace xamarin::android { return (log_categories & category) != 0; } - static auto file_exists (const char *file) noexcept -> bool + private: + static auto fs_entry_is_mode (struct stat const& s, mode_t mode) noexcept -> bool { - if (file == nullptr) { - return false; - } + return (s.st_mode & S_IFMT) == mode; + } + static auto exists_and_is_mode (std::string_view const& path, mode_t mode) noexcept -> bool + { struct stat s; - if (::stat (file, &s) == 0 && (s.st_mode & S_IFMT) == S_IFREG) { + + if (::stat (path.data (), &s) == 0 && fs_entry_is_mode (s, mode)) { return true; } + return false; } + public: + static auto dir_exists (std::string_view const& dir_path) noexcept -> bool + { + return exists_and_is_mode (dir_path, S_IFDIR); + } + + static auto file_exists (const char *file) noexcept -> bool + { + if (file == nullptr) { + return false; + } + + return exists_and_is_mode (file, S_IFREG); + } + template static auto file_exists (dynamic_local_string const& file) noexcept -> bool { @@ -96,6 +115,12 @@ namespace xamarin::android { return file_exists (file.get ()); } + static auto file_exists (int dirfd, std::string_view const& file) noexcept -> bool + { + struct stat sbuf; + return fstatat (dirfd, file.data (), &sbuf, 0) == 0 && fs_entry_is_mode (sbuf, S_IFREG); + } + static auto get_file_size_at (int dirfd, const char *file_name) noexcept -> std::optional { struct stat sbuf; @@ -107,6 +132,11 @@ namespace xamarin::android { return static_cast(sbuf.st_size); } + static auto get_file_size_at (int dirfd, std::string_view const& file_name) noexcept -> std::optional + { + return get_file_size_at (dirfd, file_name.data ()); + } + static void set_environment_variable (std::string_view const& name, jstring_wrapper& value) noexcept { ::setenv (name.data (), value.get_cstr (), 1); From b6cfb674c1ddf27b9410314a64c1b8b87fa67560 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Wed, 7 May 2025 16:08:32 +0000 Subject: [PATCH 16/24] Update src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Utilities/LlvmIrGenerator/LlvmIrGenerator.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs index 28f8a57809e..a0c31d0291d 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrGenerator.cs @@ -817,6 +817,7 @@ public void WriteValue (GeneratorWriteContext context, Type type, object? value, void WriteStringBlobArray (GeneratorWriteContext context, LlvmIrStringBlob blob) { + // The stride determines how many elements are written on a single line before a newline is added. const uint stride = 16; Type elementType = typeof(byte); From a728f2b9eb5ac14b67826ec5246533c6d2370451 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Wed, 7 May 2025 19:18:54 +0200 Subject: [PATCH 17/24] Be more generic (even though it looks fugly) --- src/native/clr/host/fastdev-assemblies.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/clr/host/fastdev-assemblies.cc b/src/native/clr/host/fastdev-assemblies.cc index ad209070f64..5d4d4691e84 100644 --- a/src/native/clr/host/fastdev-assemblies.cc +++ b/src/native/clr/host/fastdev-assemblies.cc @@ -68,7 +68,7 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si return nullptr; } - constexpr size_t MAX_SIZE = std::numeric_limits::max (); + constexpr size_t MAX_SIZE = std::numeric_limits>::max (); if (file_size.value () > MAX_SIZE) [[unlikely]] { Helpers::abort_application ( LOG_ASSEMBLY, From bb7b04b32f4d9a25ed2ae3589173cba3f48d3b04 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 8 May 2025 12:35:23 +0200 Subject: [PATCH 18/24] A handful of cosmetic changes --- src/native/clr/host/fastdev-assemblies.cc | 33 ++++++----------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/src/native/clr/host/fastdev-assemblies.cc b/src/native/clr/host/fastdev-assemblies.cc index 5d4d4691e84..2a38ef06f19 100644 --- a/src/native/clr/host/fastdev-assemblies.cc +++ b/src/native/clr/host/fastdev-assemblies.cc @@ -24,9 +24,11 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si return nullptr; } - if (override_dir_fd == -1) [[unlikely]] { + // NOTE: override_dir will be kept open, we have no way of knowing when it will be no longer + // needed + if (override_dir_fd < 0) [[unlikely]] { std::lock_guard dir_lock { override_dir_lock }; - if (override_dir_fd == -1) [[likely]] { + if (override_dir_fd < 0) [[likely]] { override_dir = opendir (override_dir_path.c_str ()); if (override_dir == nullptr) [[unlikely]] { log_warn (LOG_ASSEMBLY, "Failed to open override dir '{}'. {}", override_dir_path, strerror (errno)); @@ -44,27 +46,14 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si ); if (!Util::file_exists (override_dir_fd, name)) { - log_warn ( - LOG_ASSEMBLY, - "FastDev assembly '{}' not found.", - name - ); + log_warn (LOG_ASSEMBLY, "FastDev assembly '{}' not found.", name); return nullptr; } - - log_debug ( - LOG_ASSEMBLY, - "Found FastDev assembly '{}'", - name - ); + log_debug (LOG_ASSEMBLY, "Found FastDev assembly '{}'", name); auto file_size = Util::get_file_size_at (override_dir_fd, name); if (!file_size) [[unlikely]] { - log_warn ( - LOG_ASSEMBLY, - "Unable to determine FastDev assembly '{}' file size", - name - ); + log_warn (LOG_ASSEMBLY, "Unable to determine FastDev assembly '{}' file size", name); return nullptr; } @@ -118,13 +107,7 @@ auto FastDevAssemblies::open_assembly (std::string_view const& name, int64_t &si size = 0; return nullptr; } - - log_debug ( - LOG_ASSEMBLY, - "Read {} bytes of FastDev assembly '{}'", - nread, - name - ); + log_debug (LOG_ASSEMBLY, "Read {} bytes of FastDev assembly '{}'", nread, name); return reinterpret_cast(buffer); } From 37ed99ff57ba07befd3da00a37e4d99902aae178 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Thu, 8 May 2025 21:51:47 +0200 Subject: [PATCH 19/24] Use better looking code --- .../TypeMappingDebugNativeAssemblyGeneratorCLR.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs index a6b76fc23da..6ea4ba8b455 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs @@ -26,12 +26,11 @@ sealed class TypeMapContextDataProvider : NativeAssemblerStructContextDataProvid public override ulong GetBufferSize (object data, string fieldName) { var map_module = EnsureType (data); - if (String.Compare ("java_to_managed", fieldName, StringComparison.Ordinal) == 0 || - String.Compare ("managed_to_java", fieldName, StringComparison.Ordinal) == 0) { - return map_module.entry_count; - } - - return 0; + return fieldName switch { + "java_to_managed" => map_module.entry_count, + "managed_to_java" => map_module.entry_count, + _ => 0 + }; } public override string? GetPointedToSymbolName (object data, string fieldName) From a8e69254ef8f6af843921c6d30edaaaeb0f3dec1 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 12 May 2025 22:18:39 +0200 Subject: [PATCH 20/24] Drop the `typemap_` prefix --- src/native/clr/host/typemap.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/clr/host/typemap.cc b/src/native/clr/host/typemap.cc index 5d0184eec35..8320950fd5a 100644 --- a/src/native/clr/host/typemap.cc +++ b/src/native/clr/host/typemap.cc @@ -322,7 +322,7 @@ auto TypeMapper::managed_to_java (const char *typeName, const uint8_t *mvid) noe auto do_map = [&typeName, &mvid]() -> const char* { #if defined(RELEASE) - return typemap_managed_to_java_release (typeName, mvid); + return managed_to_java_release (typeName, mvid); #else return managed_to_java_debug (typeName, mvid); #endif From c096b28d037ae6d7daab08d823224d2fac6c03c5 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 13 May 2025 09:43:10 +0200 Subject: [PATCH 21/24] Fix after rebase --- src/native/clr/host/typemap.cc | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/src/native/clr/host/typemap.cc b/src/native/clr/host/typemap.cc index 8320950fd5a..012feaa59b2 100644 --- a/src/native/clr/host/typemap.cc +++ b/src/native/clr/host/typemap.cc @@ -475,31 +475,3 @@ auto TypeMapper::java_to_managed (const char *java_type_name, char const** assem return ret; } -#endif // ndef DEBUG - -[[gnu::flatten]] -auto TypeMapper::typemap_java_to_managed (const char *java_type_name, char const** assembly_name, uint32_t *managed_type_token_id) noexcept -> bool -{ - log_debug (LOG_ASSEMBLY, "typemap_java_to_managed: looking up type '{}'", optional_string (java_type_name)); - if (FastTiming::enabled ()) [[unlikely]] { - internal_timing.start_event (TimingEventKind::JavaToManaged); - } - - if (java_type_name == nullptr) [[unlikely]] { - log_warn (LOG_ASSEMBLY, "typemap: type name not specified in typemap_java_to_managed"); - return false; - } - - bool ret; -#if defined(RELEASE) - ret = typemap_java_to_managed_release (java_type_name, assembly_name, managed_type_token_id); -#else - ret = typemap_java_to_managed_debug (java_type_name, assembly_name, managed_type_token_id); -#endif - - if (FastTiming::enabled ()) [[unlikely]] { - internal_timing.end_event (); - } - - return ret; -} From aec41568eff288fb0ae09fa08d1f0c6389fcf96b Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 13 May 2025 09:18:14 +0200 Subject: [PATCH 22/24] Cleanup --- src/native/clr/host/internal-pinvokes.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/native/clr/host/internal-pinvokes.cc b/src/native/clr/host/internal-pinvokes.cc index 6f0a62b3002..2bcca553fd6 100644 --- a/src/native/clr/host/internal-pinvokes.cc +++ b/src/native/clr/host/internal-pinvokes.cc @@ -33,7 +33,6 @@ const char* clr_typemap_managed_to_java (const char *typeName, const uint8_t *mv bool clr_typemap_java_to_managed (const char *java_type_name, char const** assembly_name, uint32_t *managed_type_token_id) noexcept { - log_debug (LOG_ASSEMBLY, __PRETTY_FUNCTION__); return TypeMapper::java_to_managed (java_type_name, assembly_name, managed_type_token_id); } From e5a13a3bd02e20be6e7776c36edc392a221f6711 Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Tue, 13 May 2025 09:44:58 +0200 Subject: [PATCH 23/24] Cleanup --- .../Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs index 6ea4ba8b455..7990cd12e51 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGeneratorCLR.cs @@ -277,6 +277,7 @@ protected override void Construct (LlvmIrModule module) var assemblyNamesBlob = new LlvmIrStringBlob (); foreach (TypeMapGenerator.TypeMapDebugAssembly asm in data.UniqueAssemblies) { (int assemblyNameOffset, int assemblyNameLength) = assemblyNamesBlob.Add (asm.Name); + var entry = new TypeMapAssembly { Name = asm.Name, MVID = asm.MVID, From 29bef2d58bfd656e2ab7d66817b729e6bc8188fd Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Mon, 19 May 2025 15:36:22 +0200 Subject: [PATCH 24/24] Fix after rebase