From 61de8be751dd1ee783c6892b9e352e183d90b600 Mon Sep 17 00:00:00 2001 From: Steve Pfister Date: Tue, 9 Mar 2021 21:23:03 -0500 Subject: [PATCH 1/4] Support loading ICU data from managed Interop In iOS we support loading a custom dat file when working with ICU. The way this worked originally was the mono runtime exported a function that native code would call into (similar to wasm). After thinking about it a bit, it makes more sense to load this the same way we do on desktop, but with the ability to provide the path to an ICU dat file via an AppContext key `ICU_DAT_FILE_PATH`. This can be provided before Xamarin iOS calls `monovm_initialize` and they won't have to worry about calling some special function. --- .../Common/src/Interop/Interop.ICU.cs | 11 ++++- .../System.Globalization.Native/entrypoints.c | 1 + .../System.Globalization.Native/pal_icushim.h | 4 +- .../pal_icushim_static.c | 9 +++- src/tasks/AppleAppBuilder/AppleAppBuilder.cs | 2 +- src/tasks/AppleAppBuilder/Templates/runtime.m | 42 +++++++++++++------ src/tasks/AppleAppBuilder/Xcode.cs | 6 ++- 7 files changed, 56 insertions(+), 19 deletions(-) diff --git a/src/libraries/Common/src/Interop/Interop.ICU.cs b/src/libraries/Common/src/Interop/Interop.ICU.cs index 957b201aa2222c..4e1d2598678372 100644 --- a/src/libraries/Common/src/Interop/Interop.ICU.cs +++ b/src/libraries/Common/src/Interop/Interop.ICU.cs @@ -10,7 +10,16 @@ internal static partial class Interop internal static partial class Globalization { [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_LoadICU")] - internal static extern int LoadICU(); + private static extern int LoadICUDirect(); + + [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_LoadICUData")] + private static extern int LoadICUData(string? path); + + internal static int LoadICU() + { + object? datPath = AppContext.GetData("ICU_DAT_FILE_PATH"); + return (datPath != null) ? LoadICUData(datPath.ToString()) : LoadICUDirect(); + } internal static void InitICUFunctions(IntPtr icuuc, IntPtr icuin, ReadOnlySpan version, ReadOnlySpan suffix) { diff --git a/src/libraries/Native/Unix/System.Globalization.Native/entrypoints.c b/src/libraries/Native/Unix/System.Globalization.Native/entrypoints.c index 85a39085270b46..cf698f8361a5a0 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Globalization.Native/entrypoints.c @@ -49,6 +49,7 @@ static const Entry s_globalizationNative[] = DllImportEntry(GlobalizationNative_IsPredefinedLocale) DllImportEntry(GlobalizationNative_LastIndexOf) DllImportEntry(GlobalizationNative_LoadICU) + DllImportEntry(GlobalizationNative_LoadICUData) DllImportEntry(GlobalizationNative_NormalizeString) DllImportEntry(GlobalizationNative_StartsWith) DllImportEntry(GlobalizationNative_ToAscii) diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.h b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.h index 5f0123d8ef5212..0bfc6bfff6b125 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.h +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.h @@ -9,9 +9,9 @@ PALEXPORT void GlobalizationNative_InitICUFunctions(void* icuuc, void* icuin, co PALEXPORT int32_t GlobalizationNative_GetICUVersion(void); -#if defined(STATIC_ICU) +PALEXPORT int32_t GlobalizationNative_LoadICUData(const char* path); -PALEXPORT int32_t GlobalizationNative_LoadICUData(char* path); +#if defined(STATIC_ICU) PALEXPORT const char* GlobalizationNative_GetICUDTName(const char* culture); diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c index 49dd64df6507cb..57564b370a0457 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c @@ -89,7 +89,7 @@ static int32_t load_icu_data(void* pData) } } -int32_t GlobalizationNative_LoadICUData(char* path) +int32_t GlobalizationNative_LoadICUData(const char* path) { int32_t ret = -1; char* icu_data; @@ -127,7 +127,12 @@ int32_t GlobalizationNative_LoadICUData(char* path) fclose (fp); - return load_icu_data (icu_data); + if (load_icu_data (icu_data) == 0) { + fprintf (stderr, "ICU BAD EXIT %d.", ret); + return ret; + } + + return GlobalizationNative_LoadICU (); } const char* GlobalizationNative_GetICUDTName(const char* culture) diff --git a/src/tasks/AppleAppBuilder/AppleAppBuilder.cs b/src/tasks/AppleAppBuilder/AppleAppBuilder.cs index fc688ba239509c..d2e04a49598f74 100644 --- a/src/tasks/AppleAppBuilder/AppleAppBuilder.cs +++ b/src/tasks/AppleAppBuilder/AppleAppBuilder.cs @@ -182,7 +182,7 @@ public override bool Execute() if (GenerateXcodeProject) { - Xcode generator = new Xcode(TargetOS); + Xcode generator = new Xcode(TargetOS, Arch); generator.EnableRuntimeLogging = EnableRuntimeLogging; XcodeProjectPath = generator.GenerateXCode(ProjectName, MainLibraryFileName, assemblerFiles, diff --git a/src/tasks/AppleAppBuilder/Templates/runtime.m b/src/tasks/AppleAppBuilder/Templates/runtime.m index fe2845a10ad6de..46de1e97087791 100644 --- a/src/tasks/AppleAppBuilder/Templates/runtime.m +++ b/src/tasks/AppleAppBuilder/Templates/runtime.m @@ -24,6 +24,8 @@ #define MONO_ENTER_GC_UNSAFE #define MONO_EXIT_GC_UNSAFE +#define APPLE_RUNTIME_IDENTIFIER "//%APPLE_RUNTIME_IDENTIFIER%" + const char * get_bundle_path (void) { @@ -237,16 +239,6 @@ static int32_t load_icu_data () setenv ("MONO_LOG_MASK", "all", TRUE); #endif -#if !INVARIANT_GLOBALIZATION - int32_t ret = load_icu_data (); - - if (ret == 0) { - os_log_info (OS_LOG_DEFAULT, "ICU BAD EXIT %d.", ret); - exit (ret); - return; - } -#endif - id args_array = [[NSProcessInfo processInfo] arguments]; assert ([args_array count] <= 128); const char *managed_argv [128]; @@ -261,8 +253,34 @@ static int32_t load_icu_data () const char* bundle = get_bundle_path (); chdir (bundle); + char icu_dat_path[1024]; + int res; + + res = snprintf (icu_dat_path, sizeof (icu_dat_path) - 1, "%s/%s", bundle, "icudt.dat"); + assert (res > 0); + // TODO: set TRUSTED_PLATFORM_ASSEMBLIES, APP_PATHS and NATIVE_DLL_SEARCH_DIRECTORIES - monovm_initialize(0, NULL, NULL); +#if !INVARIANT_GLOBALIZATION + const char* appctx_keys[3]; + appctx_keys[0] = "RUNTIME_IDENTIFIER"; + appctx_keys[1] = "APP_CONTEXT_BASE_DIRECTORY"; + appctx_keys[2] = "ICU_DAT_FILE_PATH"; + + const char* appctx_values[3]; + appctx_values[0] = APPLE_RUNTIME_IDENTIFIER; + appctx_values[1] = bundle; + appctx_values[2] = icu_dat_path; + monovm_initialize(3, appctx_keys, appctx_values); +#else + const char* appctx_keys[2]; + appctx_keys[0] = "RUNTIME_IDENTIFIER"; + appctx_keys[1] = "APP_CONTEXT_BASE_DIRECTORY"; + + const char* appctx_values[2]; + appctx_values[0] = APPLE_RUNTIME_IDENTIFIER; + appctx_values[1] = bundle; + monovm_initialize(2, appctx_keys, appctx_values); +#endif #if FORCE_INTERPRETER os_log_info (OS_LOG_DEFAULT, "INTERP Enabled"); @@ -300,7 +318,7 @@ static int32_t load_icu_data () assert (assembly); os_log_info (OS_LOG_DEFAULT, "Executable: %{public}s", executable); - int res = mono_jit_exec (mono_domain_get (), assembly, argi, managed_argv); + res = mono_jit_exec (mono_domain_get (), assembly, argi, managed_argv); // Print this so apps parsing logs can detect when we exited os_log_info (OS_LOG_DEFAULT, "Exit code: %d.", res); diff --git a/src/tasks/AppleAppBuilder/Xcode.cs b/src/tasks/AppleAppBuilder/Xcode.cs index 96fab4b703579b..1c14860eb6c7cb 100644 --- a/src/tasks/AppleAppBuilder/Xcode.cs +++ b/src/tasks/AppleAppBuilder/Xcode.cs @@ -9,10 +9,11 @@ internal class Xcode { + private string RuntimeIdentifier { get; set; } private string SysRoot { get; set; } private string Target { get; set; } - public Xcode(string target) + public Xcode(string target, string arch) { Target = target; switch (Target) @@ -27,6 +28,8 @@ public Xcode(string target) SysRoot = Utils.RunProcess("xcrun", "--sdk macosx --show-sdk-path"); break; } + + RuntimeIdentifier = $"{Target}-{arch}"; } public bool EnableRuntimeLogging { get; set; } @@ -175,6 +178,7 @@ public string GenerateXCode( File.WriteAllText(Path.Combine(binDir, "runtime.m"), Utils.GetEmbeddedResource("runtime.m") .Replace("//%DllMap%", dllMap.ToString()) + .Replace("//%APPLE_RUNTIME_IDENTIFIER%", RuntimeIdentifier) .Replace("%EntryPointLibName%", Path.GetFileName(entryPointLib))); Utils.RunProcess("cmake", cmakeArgs.ToString(), workingDir: binDir); From 1fc8cde91f02dadbefa319e8b1780e049336aeb5 Mon Sep 17 00:00:00 2001 From: Steve Pfister Date: Wed, 10 Mar 2021 18:09:01 -0500 Subject: [PATCH 2/4] Address feedback and move LoadICUData to iOS only --- .../Common/src/Interop/Interop.ICU.cs | 10 +---- .../System.Globalization.Native/entrypoints.c | 2 + .../System.Globalization.Native/pal_icushim.h | 4 +- .../System.Private.CoreLib.Shared.projitems | 2 + .../GlobalizationMode.LoadICU.Unix.cs | 13 +++++++ .../GlobalizationMode.LoadICU.iOS.cs | 14 +++++++ .../Globalization/GlobalizationMode.Unix.cs | 2 +- src/mono/mono/mini/CMakeLists.txt | 1 + src/tasks/AppleAppBuilder/Templates/runtime.m | 37 ++++++++----------- 9 files changed, 53 insertions(+), 32 deletions(-) create mode 100644 src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs create mode 100644 src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs diff --git a/src/libraries/Common/src/Interop/Interop.ICU.cs b/src/libraries/Common/src/Interop/Interop.ICU.cs index 4e1d2598678372..28c77dc0a32b5b 100644 --- a/src/libraries/Common/src/Interop/Interop.ICU.cs +++ b/src/libraries/Common/src/Interop/Interop.ICU.cs @@ -10,16 +10,10 @@ internal static partial class Interop internal static partial class Globalization { [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_LoadICU")] - private static extern int LoadICUDirect(); + internal static extern int LoadICU(); [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_LoadICUData")] - private static extern int LoadICUData(string? path); - - internal static int LoadICU() - { - object? datPath = AppContext.GetData("ICU_DAT_FILE_PATH"); - return (datPath != null) ? LoadICUData(datPath.ToString()) : LoadICUDirect(); - } + internal static extern int LoadICUData(string? path); internal static void InitICUFunctions(IntPtr icuuc, IntPtr icuin, ReadOnlySpan version, ReadOnlySpan suffix) { diff --git a/src/libraries/Native/Unix/System.Globalization.Native/entrypoints.c b/src/libraries/Native/Unix/System.Globalization.Native/entrypoints.c index cf698f8361a5a0..30d5bb37748ba7 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Globalization.Native/entrypoints.c @@ -49,7 +49,9 @@ static const Entry s_globalizationNative[] = DllImportEntry(GlobalizationNative_IsPredefinedLocale) DllImportEntry(GlobalizationNative_LastIndexOf) DllImportEntry(GlobalizationNative_LoadICU) +#if defined(STATIC_ICU) DllImportEntry(GlobalizationNative_LoadICUData) +#endif DllImportEntry(GlobalizationNative_NormalizeString) DllImportEntry(GlobalizationNative_StartsWith) DllImportEntry(GlobalizationNative_ToAscii) diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.h b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.h index 0bfc6bfff6b125..51e117113de1c2 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.h +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.h @@ -9,10 +9,10 @@ PALEXPORT void GlobalizationNative_InitICUFunctions(void* icuuc, void* icuin, co PALEXPORT int32_t GlobalizationNative_GetICUVersion(void); -PALEXPORT int32_t GlobalizationNative_LoadICUData(const char* path); - #if defined(STATIC_ICU) +PALEXPORT int32_t GlobalizationNative_LoadICUData(const char* path); + PALEXPORT const char* GlobalizationNative_GetICUDTName(const char* culture); #endif diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index a6b2c16f6a37c3..5ee8ff70f4e549 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1838,6 +1838,8 @@ + + diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs new file mode 100644 index 00000000000000..6c6c8b783ff334 --- /dev/null +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs @@ -0,0 +1,13 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Globalization +{ + internal static partial class GlobalizationMode + { + private static int LoadICUCore() + { + return Interop.Globalization.LoadICU(); + } + } +} \ No newline at end of file diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs new file mode 100644 index 00000000000000..ec7ff248ca37d4 --- /dev/null +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Globalization +{ + internal static partial class GlobalizationMode + { + private static int LoadICUCore() + { + object? datPath = AppContext.GetData("ICU_DAT_FILE_PATH"); + return (datPath != null) ? Interop.Globalization.LoadICUData(datPath.ToString()) : Interop.Globalization.LoadICU(); + } + } +} \ No newline at end of file diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs index 0149463c6aa0eb..8b9de2298814a9 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs @@ -22,7 +22,7 @@ private static bool GetGlobalizationInvariantMode() } else { - int loaded = Interop.Globalization.LoadICU(); + int loaded = LoadICUCore(); if (loaded == 0 && !OperatingSystem.IsBrowser()) { // This can't go into resources, because a resource lookup requires globalization, which requires ICU diff --git a/src/mono/mono/mini/CMakeLists.txt b/src/mono/mono/mini/CMakeLists.txt index 4f9f05d68c15a1..a2af151915179c 100644 --- a/src/mono/mono/mini/CMakeLists.txt +++ b/src/mono/mono/mini/CMakeLists.txt @@ -50,6 +50,7 @@ if(HAVE_SYS_ICU) if(STATIC_ICU) set(pal_icushim_sources_base pal_icushim_static.c) + add_definitions(-DSTATIC_ICU=1) else() set(pal_icushim_sources_base pal_icushim.c) diff --git a/src/tasks/AppleAppBuilder/Templates/runtime.m b/src/tasks/AppleAppBuilder/Templates/runtime.m index 46de1e97087791..0123c74a21d664 100644 --- a/src/tasks/AppleAppBuilder/Templates/runtime.m +++ b/src/tasks/AppleAppBuilder/Templates/runtime.m @@ -260,27 +260,22 @@ static int32_t load_icu_data () assert (res > 0); // TODO: set TRUSTED_PLATFORM_ASSEMBLIES, APP_PATHS and NATIVE_DLL_SEARCH_DIRECTORIES -#if !INVARIANT_GLOBALIZATION - const char* appctx_keys[3]; - appctx_keys[0] = "RUNTIME_IDENTIFIER"; - appctx_keys[1] = "APP_CONTEXT_BASE_DIRECTORY"; - appctx_keys[2] = "ICU_DAT_FILE_PATH"; - - const char* appctx_values[3]; - appctx_values[0] = APPLE_RUNTIME_IDENTIFIER; - appctx_values[1] = bundle; - appctx_values[2] = icu_dat_path; - monovm_initialize(3, appctx_keys, appctx_values); -#else - const char* appctx_keys[2]; - appctx_keys[0] = "RUNTIME_IDENTIFIER"; - appctx_keys[1] = "APP_CONTEXT_BASE_DIRECTORY"; - - const char* appctx_values[2]; - appctx_values[0] = APPLE_RUNTIME_IDENTIFIER; - appctx_values[1] = bundle; - monovm_initialize(2, appctx_keys, appctx_values); -#endif + const char *appctx_keys[] = { + "RUNTIME_IDENTIFIER", + #ifndef INVARIANT_GLOBALIZATION + "ICU_DAT_FILE_PATH", + #endif + "APP_CONTEXT_BASE_DIRECTORY" + }; + const char *appctx_values [] = { + APPLE_RUNTIME_IDENTIFIER, + #ifndef INVARIANT_GLOBALIZATION + icu_dat_path, + #endif + bundle + }; + + monovm_initialize(sizeof (appctx_keys) / sizeof (appctx_keys [0]), appctx_keys, appctx_values); #if FORCE_INTERPRETER os_log_info (OS_LOG_DEFAULT, "INTERP Enabled"); From a3dbbe1e285f88758a0cfaa91bee98f2929ee61c Mon Sep 17 00:00:00 2001 From: Steve Pfister Date: Thu, 11 Mar 2021 13:04:19 -0500 Subject: [PATCH 3/4] More feedback --- .../Common/src/Interop/Interop.ICU.cs | 3 -- .../Common/src/Interop/Interop.ICU.iOS.cs | 14 ++++++++ .../pal_icushim_static.c | 32 +++++++++---------- .../System.Private.CoreLib.Shared.projitems | 3 ++ .../GlobalizationMode.LoadICU.Unix.cs | 4 +-- .../GlobalizationMode.LoadICU.iOS.cs | 4 +-- .../Globalization/GlobalizationMode.Unix.cs | 2 +- src/tasks/AppleAppBuilder/Templates/runtime.m | 31 ++++-------------- 8 files changed, 45 insertions(+), 48 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Interop.ICU.iOS.cs diff --git a/src/libraries/Common/src/Interop/Interop.ICU.cs b/src/libraries/Common/src/Interop/Interop.ICU.cs index 28c77dc0a32b5b..957b201aa2222c 100644 --- a/src/libraries/Common/src/Interop/Interop.ICU.cs +++ b/src/libraries/Common/src/Interop/Interop.ICU.cs @@ -12,9 +12,6 @@ internal static partial class Globalization [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_LoadICU")] internal static extern int LoadICU(); - [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_LoadICUData")] - internal static extern int LoadICUData(string? path); - internal static void InitICUFunctions(IntPtr icuuc, IntPtr icuin, ReadOnlySpan version, ReadOnlySpan suffix) { Debug.Assert(icuuc != IntPtr.Zero); diff --git a/src/libraries/Common/src/Interop/Interop.ICU.iOS.cs b/src/libraries/Common/src/Interop/Interop.ICU.iOS.cs new file mode 100644 index 00000000000000..7b770fa061e3e9 --- /dev/null +++ b/src/libraries/Common/src/Interop/Interop.ICU.iOS.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Globalization + { + [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_LoadICUData")] + internal static extern int LoadICUData(string? path); + } +} diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c index 57564b370a0457..124f63fe816446 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c @@ -94,45 +94,45 @@ int32_t GlobalizationNative_LoadICUData(const char* path) int32_t ret = -1; char* icu_data; - FILE *fp = fopen (path, "rb"); + FILE *fp = fopen(path, "rb"); if (fp == NULL) { - fprintf (stderr, "Unable to load ICU dat file '%s'.", path); + fprintf(stderr, "Unable to load ICU dat file '%s'.", path); return ret; } - if (fseek (fp, 0L, SEEK_END) != 0) { - fprintf (stderr, "Unable to determine size of the dat file"); + if (fseek(fp, 0L, SEEK_END) != 0) { + fprintf(stderr, "Unable to determine size of the dat file"); return ret; } - long bufsize = ftell (fp); + long bufsize = ftell(fp); if (bufsize == -1) { - fprintf (stderr, "Unable to determine size of the ICU dat file."); + fprintf(stderr, "Unable to determine size of the ICU dat file."); return ret; } - icu_data = malloc (sizeof (char) * (bufsize + 1)); + icu_data = malloc(sizeof(char) * (bufsize + 1)); - if (fseek (fp, 0L, SEEK_SET) != 0) { - fprintf (stderr, "Unable to seek ICU dat file."); + if (fseek(fp, 0L, SEEK_SET) != 0) { + fprintf(stderr, "Unable to seek ICU dat file."); return ret; } - fread (icu_data, sizeof (char), bufsize, fp); - if (ferror ( fp ) != 0 ) { - fprintf (stderr, "Unable to read ICU dat file"); + fread(icu_data, sizeof(char), bufsize, fp); + if (ferror( fp ) != 0 ) { + fprintf(stderr, "Unable to read ICU dat file"); return ret; } - fclose (fp); + fclose(fp); - if (load_icu_data (icu_data) == 0) { - fprintf (stderr, "ICU BAD EXIT %d.", ret); + if (load_icu_data(icu_data) == 0) { + fprintf(stderr, "ICU BAD EXIT %d.", ret); return ret; } - return GlobalizationNative_LoadICU (); + return GlobalizationNative_LoadICU(); } const char* GlobalizationNative_GetICUDTName(const char* culture) diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 5ee8ff70f4e549..cb45561d48d9c4 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1084,6 +1084,9 @@ Common\Interop\Interop.ICU.cs + + Common\Interop\Interop.ICU.iOS.cs + Common\Interop\Interop.Idna.cs diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs index 6c6c8b783ff334..7f48afc30cb281 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs @@ -5,9 +5,9 @@ namespace System.Globalization { internal static partial class GlobalizationMode { - private static int LoadICUCore() + private static int LoadICU() { return Interop.Globalization.LoadICU(); } } -} \ No newline at end of file +} diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs index ec7ff248ca37d4..42b149dbf85b5d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs @@ -5,10 +5,10 @@ namespace System.Globalization { internal static partial class GlobalizationMode { - private static int LoadICUCore() + private static int LoadICU() { object? datPath = AppContext.GetData("ICU_DAT_FILE_PATH"); return (datPath != null) ? Interop.Globalization.LoadICUData(datPath.ToString()) : Interop.Globalization.LoadICU(); } } -} \ No newline at end of file +} diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs index 8b9de2298814a9..cce3ae3a870dee 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs @@ -22,7 +22,7 @@ private static bool GetGlobalizationInvariantMode() } else { - int loaded = LoadICUCore(); + int loaded = LoadICU(); if (loaded == 0 && !OperatingSystem.IsBrowser()) { // This can't go into resources, because a resource lookup requires globalization, which requires ICU diff --git a/src/tasks/AppleAppBuilder/Templates/runtime.m b/src/tasks/AppleAppBuilder/Templates/runtime.m index 0123c74a21d664..4abf3871c831bc 100644 --- a/src/tasks/AppleAppBuilder/Templates/runtime.m +++ b/src/tasks/AppleAppBuilder/Templates/runtime.m @@ -205,23 +205,6 @@ //%DllMap% } -int32_t GlobalizationNative_LoadICUData(char *path); - -static int32_t load_icu_data () -{ - char path [1024]; - int res; - - const char *dname = "icudt.dat"; - const char *bundle = get_bundle_path (); - - os_log_info (OS_LOG_DEFAULT, "Loading ICU data file '%s'.", dname); - res = snprintf (path, sizeof (path) - 1, "%s/%s", bundle, dname); - assert (res > 0); - - return GlobalizationNative_LoadICUData(path); -} - #if FORCE_INTERPRETER || FORCE_AOT || (!TARGET_OS_SIMULATOR && !TARGET_OS_MACCATALYST) void mono_jit_set_aot_mode (MonoAotMode mode); void register_aot_modules (void); @@ -253,29 +236,29 @@ static int32_t load_icu_data () const char* bundle = get_bundle_path (); chdir (bundle); - char icu_dat_path[1024]; + char icu_dat_path [1024]; int res; res = snprintf (icu_dat_path, sizeof (icu_dat_path) - 1, "%s/%s", bundle, "icudt.dat"); assert (res > 0); // TODO: set TRUSTED_PLATFORM_ASSEMBLIES, APP_PATHS and NATIVE_DLL_SEARCH_DIRECTORIES - const char *appctx_keys[] = { + const char *appctx_keys [] = { "RUNTIME_IDENTIFIER", - #ifndef INVARIANT_GLOBALIZATION +#ifndef INVARIANT_GLOBALIZATION "ICU_DAT_FILE_PATH", - #endif +#endif "APP_CONTEXT_BASE_DIRECTORY" }; const char *appctx_values [] = { APPLE_RUNTIME_IDENTIFIER, - #ifndef INVARIANT_GLOBALIZATION +#ifndef INVARIANT_GLOBALIZATION icu_dat_path, - #endif +#endif bundle }; - monovm_initialize(sizeof (appctx_keys) / sizeof (appctx_keys [0]), appctx_keys, appctx_values); + monovm_initialize (sizeof (appctx_keys) / sizeof (appctx_keys [0]), appctx_keys, appctx_values); #if FORCE_INTERPRETER os_log_info (OS_LOG_DEFAULT, "INTERP Enabled"); From 88a99b0a87985f7da1be8b4c889520f31e2f62f3 Mon Sep 17 00:00:00 2001 From: Steve Pfister Date: Fri, 12 Mar 2021 14:56:02 -0500 Subject: [PATCH 4/4] Address additional good feedback --- .../Common/src/Interop/Interop.ICU.iOS.cs | 2 +- .../pal_icushim_static.c | 33 +++++++++++++++---- .../System.Private.CoreLib.Shared.projitems | 9 ++--- .../GlobalizationMode.LoadICU.Unix.cs | 5 +-- .../GlobalizationMode.LoadICU.iOS.cs | 2 +- src/tasks/AppleAppBuilder/Templates/runtime.m | 8 ++--- 6 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/libraries/Common/src/Interop/Interop.ICU.iOS.cs b/src/libraries/Common/src/Interop/Interop.ICU.iOS.cs index 7b770fa061e3e9..8eb0ee177ab8d4 100644 --- a/src/libraries/Common/src/Interop/Interop.ICU.iOS.cs +++ b/src/libraries/Common/src/Interop/Interop.ICU.iOS.cs @@ -9,6 +9,6 @@ internal static partial class Interop internal static partial class Globalization { [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_LoadICUData")] - internal static extern int LoadICUData(string? path); + internal static extern int LoadICUData(string path); } } diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c index 124f63fe816446..c81ce59e50acad 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_static.c @@ -22,10 +22,19 @@ static int32_t isLoaded = 0; static int32_t isDataSet = 0; +static void log_shim_error(const char* format, ...) +{ + va_list args; + + va_start(args, format); + vfprintf(stderr, format, args); + va_end(args); +} + static void log_icu_error(const char* name, UErrorCode status) { const char * statusText = u_errorName(status); - fprintf(stderr, "ICU call %s failed with error #%d '%s'.\n", name, status, statusText); + log_shim_error("ICU call %s failed with error #%d '%s'.\n", name, status, statusText); } static void U_CALLCONV icu_trace_data(const void* context, int32_t fnNumber, int32_t level, const char* fmt, va_list args) @@ -96,39 +105,49 @@ int32_t GlobalizationNative_LoadICUData(const char* path) FILE *fp = fopen(path, "rb"); if (fp == NULL) { - fprintf(stderr, "Unable to load ICU dat file '%s'.", path); + log_shim_error("Unable to load ICU dat file '%s'.", path); return ret; } if (fseek(fp, 0L, SEEK_END) != 0) { - fprintf(stderr, "Unable to determine size of the dat file"); + fclose(fp); + log_shim_error("Unable to determine size of the dat file"); return ret; } long bufsize = ftell(fp); if (bufsize == -1) { - fprintf(stderr, "Unable to determine size of the ICU dat file."); + fclose(fp); + log_shim_error("Unable to determine size of the ICU dat file."); return ret; } icu_data = malloc(sizeof(char) * (bufsize + 1)); + if (icu_data == NULL) { + fclose(fp); + log_shim_error("Unable to allocate enough to read the ICU dat file"); + return ret; + } + if (fseek(fp, 0L, SEEK_SET) != 0) { - fprintf(stderr, "Unable to seek ICU dat file."); + fclose(fp); + log_shim_error("Unable to seek ICU dat file."); return ret; } fread(icu_data, sizeof(char), bufsize, fp); if (ferror( fp ) != 0 ) { - fprintf(stderr, "Unable to read ICU dat file"); + fclose(fp); + log_shim_error("Unable to read ICU dat file"); return ret; } fclose(fp); if (load_icu_data(icu_data) == 0) { - fprintf(stderr, "ICU BAD EXIT %d.", ret); + log_shim_error("ICU BAD EXIT %d.", ret); return ret; } diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index b62b92598daa29..6dc1e58a9d7aff 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -11,6 +11,7 @@ enable true + true true true $(MSBuildThisFileDirectory)ILLink\ @@ -1082,7 +1083,7 @@ Common\Interop\Interop.ICU.cs - + Common\Interop\Interop.ICU.iOS.cs @@ -1834,13 +1835,13 @@ - + - - + + diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs index 7f48afc30cb281..c91596d3dee993 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.Unix.cs @@ -5,9 +5,6 @@ namespace System.Globalization { internal static partial class GlobalizationMode { - private static int LoadICU() - { - return Interop.Globalization.LoadICU(); - } + private static int LoadICU() => Interop.Globalization.LoadICU(); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs index 42b149dbf85b5d..08c5feb8ad1898 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.LoadICU.iOS.cs @@ -8,7 +8,7 @@ internal static partial class GlobalizationMode private static int LoadICU() { object? datPath = AppContext.GetData("ICU_DAT_FILE_PATH"); - return (datPath != null) ? Interop.Globalization.LoadICUData(datPath.ToString()) : Interop.Globalization.LoadICU(); + return (datPath != null) ? Interop.Globalization.LoadICUData(datPath!.ToString()!) : Interop.Globalization.LoadICU(); } } } diff --git a/src/tasks/AppleAppBuilder/Templates/runtime.m b/src/tasks/AppleAppBuilder/Templates/runtime.m index 4abf3871c831bc..ef8bdbd4dfce5c 100644 --- a/src/tasks/AppleAppBuilder/Templates/runtime.m +++ b/src/tasks/AppleAppBuilder/Templates/runtime.m @@ -245,17 +245,17 @@ // TODO: set TRUSTED_PLATFORM_ASSEMBLIES, APP_PATHS and NATIVE_DLL_SEARCH_DIRECTORIES const char *appctx_keys [] = { "RUNTIME_IDENTIFIER", + "APP_CONTEXT_BASE_DIRECTORY", #ifndef INVARIANT_GLOBALIZATION - "ICU_DAT_FILE_PATH", + "ICU_DAT_FILE_PATH" #endif - "APP_CONTEXT_BASE_DIRECTORY" }; const char *appctx_values [] = { APPLE_RUNTIME_IDENTIFIER, + bundle, #ifndef INVARIANT_GLOBALIZATION - icu_dat_path, + icu_dat_path #endif - bundle }; monovm_initialize (sizeof (appctx_keys) / sizeof (appctx_keys [0]), appctx_keys, appctx_values);