From aec9b6941dbfe44aec179cbba346c32f9b14c891 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Fri, 17 Apr 2020 16:28:49 -0700 Subject: [PATCH 1/7] Add app-local support for ICU --- .../PinvokeAnalyzerExceptionList.analyzerdata | 1 + .../Globalization/GlobalizationMode.Unix.cs | 43 +++- .../GlobalizationMode.Windows.cs | 46 +++- .../System/Globalization/GlobalizationMode.cs | 102 ++++++++- .../src/libraries-native/entrypoints.c | 1 + .../Common/src/Interop/Interop.Collation.cs | 1 + .../Common/src/Interop/Interop.ICU.cs | 4 + .../System.Globalization.Native/pal_icushim.c | 213 ++++++++++++------ .../System.Globalization.Native/pal_icushim.h | 2 + .../pal_icushim_internal.h | 10 +- .../tests/NlsTests/NlsSwitchTests.cs | 2 +- 11 files changed, 348 insertions(+), 77 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/PinvokeAnalyzerExceptionList.analyzerdata b/src/coreclr/src/System.Private.CoreLib/PinvokeAnalyzerExceptionList.analyzerdata index c53e2b766ad136..f2570c7c958eea 100644 --- a/src/coreclr/src/System.Private.CoreLib/PinvokeAnalyzerExceptionList.analyzerdata +++ b/src/coreclr/src/System.Private.CoreLib/PinvokeAnalyzerExceptionList.analyzerdata @@ -36,6 +36,7 @@ libSystem.Globalization.Native!GlobalizationNative_GetSortVersion libSystem.Globalization.Native!GlobalizationNative_GetTimeZoneDisplayName libSystem.Globalization.Native!GlobalizationNative_IndexOf libSystem.Globalization.Native!GlobalizationNative_IndexOfOrdinalIgnoreCase +libSystem.Globalization.Native!GlobalizationNative_InitICUFunctions libSystem.Globalization.Native!GlobalizationNative_IsNormalized libSystem.Globalization.Native!GlobalizationNative_IsPredefinedLocale libSystem.Globalization.Native!GlobalizationNative_LastIndexOf diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs index 6cb513abc23d09..b7173aa8614f99 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Reflection; + namespace System.Globalization { internal static partial class GlobalizationMode @@ -17,7 +19,7 @@ private static bool GetGlobalizationInvariantMode() bool invariantEnabled = GetInvariantSwitchValue(); if (!invariantEnabled) { - if (Interop.Globalization.LoadICU() == 0) + if (!LoadIcu()) { string message = "Couldn't find a valid ICU package installed on the system. " + "Set the configuration flag System.Globalization.Invariant to true if you want to run with no globalization support."; @@ -26,5 +28,44 @@ private static bool GetGlobalizationInvariantMode() } return invariantEnabled; } + + private static bool LoadIcu() + { + if (!TryGetAppLocalIcuSwitchValue(out ReadOnlySpan version, out ReadOnlySpan suffix)) + { + return Interop.Globalization.LoadICU() != 0; + } + +#if TARGET_OSX + string extension = ".dylib"; + bool versionAtEnd = false; +#else + string extension = "so."; + bool versionAtEnd = true; +#endif + + // Append '.' to suffix since in Unix the version is separated by '.' + int suffixLength = suffix.Length + 1; + Span suffixWithSeparator = stackalloc char[suffixLength]; + suffix.CopyTo(suffixWithSeparator); + suffixWithSeparator[suffixLength - 1] = '.'; + + Assembly assembly = Assembly.GetExecutingAssembly(); + +#if !TARGET_OSX + // In Linux we need to load libicudata first because libicuuc and libicui18n depend on it. In order for the loader to find + // it on the same path, we load it before loading the other two libraries. + string icudataBase = "libicudata"; + LoadLibrary(CreateLibraryName(icudataBase, suffixWithSeparator, extension, version, versionAtEnd), assembly, failOnLoadFailure: true); +#endif + + string icuucBase = "libicuuc"; + string icuinBase = "libicui18n"; + IntPtr icuucLib = LoadLibrary(CreateLibraryName(icuucBase, suffixWithSeparator, extension, version, versionAtEnd), assembly, failOnLoadFailure: true); + IntPtr icuinLib = LoadLibrary(CreateLibraryName(icuinBase, suffixWithSeparator, extension, version, versionAtEnd), assembly, failOnLoadFailure: true); + + Interop.Globalization.InitICU(icuucLib, icuinLib, version.ToString(), suffix.Length > 0 ? suffix.ToString() : null); + return true; + } } } diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Windows.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Windows.cs index 50285f01da2028..de07f766f4deb6 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Windows.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Windows.cs @@ -2,6 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Reflection; +using System.Runtime.InteropServices; + namespace System.Globalization { internal static partial class GlobalizationMode @@ -12,6 +15,47 @@ internal static partial class GlobalizationMode internal static bool UseNls { get; } = !Invariant && (GetSwitchValue("System.Globalization.UseNls", "DOTNET_SYSTEM_GLOBALIZATION_USENLS") || - Interop.Globalization.LoadICU() == 0); + !LoadIcu()); + + private static bool LoadIcu() + { + if (!TryGetAppLocalIcuSwitchValue(out ReadOnlySpan version, out ReadOnlySpan suffix)) + { + return Interop.Globalization.LoadICU() != 0; + } + + string extension = ".dll"; + string icuucBase = "icuuc"; + string icuinBase = "icuin"; + IntPtr icuucLib = IntPtr.Zero; + IntPtr icuinLib = IntPtr.Zero; + Assembly assembly = Assembly.GetExecutingAssembly(); + + int index = version.ToString().IndexOf('.', StringComparison.Ordinal); + + if (index != -1) + { + ReadOnlySpan truncatedVersion = version.Slice(0, index); + icuucLib = LoadLibrary(CreateLibraryName(icuucBase, suffix, extension, truncatedVersion), assembly, failOnLoadFailure: false); + + if (icuucLib != IntPtr.Zero) + { + icuinLib = LoadLibrary(CreateLibraryName(icuinBase, suffix, extension, truncatedVersion), assembly, failOnLoadFailure: false); + } + } + + if (icuucLib == IntPtr.Zero) + { + icuucLib = LoadLibrary(CreateLibraryName(icuucBase, suffix, extension, version), assembly, failOnLoadFailure: true); + } + + if (icuinLib == IntPtr.Zero) + { + icuinLib = LoadLibrary(CreateLibraryName(icuinBase, suffix, extension, version), assembly, failOnLoadFailure: true); + } + + Interop.Globalization.InitICU(icuucLib, icuinLib, version.ToString(), suffix.Length > 0 ? suffix.ToString() : null); + return true; + } } } diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs index 1433e5220b976d..e6b90d259e1665 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs @@ -2,6 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Diagnostics.CodeAnalysis; +using System.Reflection; +using System.Runtime.InteropServices; + namespace System.Globalization { internal static partial class GlobalizationMode @@ -9,13 +13,51 @@ internal static partial class GlobalizationMode private static bool GetInvariantSwitchValue() => GetSwitchValue("System.Globalization.Invariant", "DOTNET_SYSTEM_GLOBALIZATION_INVARIANT"); + private static bool TryGetAppLocalIcuSwitchValue(out ReadOnlySpan version, out ReadOnlySpan icuSuffix) + { + icuSuffix = default; + + if (!TryGetStringValue("System.Globalization.AppLocalIcu", "DOTNET_SYSTEM_GLOBALIZATION_APPLOCALICU", out string? value)) + { + version = default; + return false; + } + + // Custom built ICU can have a suffix on the name, i.e: libicuucmyapp.so.67.1 + // So users would set the runtime switch as: myapp:67.1 + + int indexOfSeparator = value.IndexOf(':', StringComparison.Ordinal); + if (indexOfSeparator != -1) + { + ReadOnlySpan valueAsSpan = value.AsSpan(); + icuSuffix = valueAsSpan.Slice(0, indexOfSeparator); + + if (icuSuffix.Length > 20) + { + Environment.FailFast($"The resolved \"{icuSuffix.ToString()}\" suffix from System.Globalization.AppLocalIcu switch has to be < 20 chars long."); + } + + version = valueAsSpan.Slice(icuSuffix.Length + 1); + } + else + { + version = value; + } + + if (version.Length > 33) + { + Environment.FailFast($"The resolved version \"{version.ToString()}\" from System.Globalization.AppLocalIcu switch has to be < 33 chars long."); + } + + return true; + } + // GetSwitchValue calls CLRConfig first to detect if the switch is defined in the config file. // if the switch is defined we just use the value of this switch. otherwise, we'll try to get the switch // value from the environment variable if it is defined. private static bool GetSwitchValue(string switchName, string envVariable) { - bool ret = CLRConfig.GetBoolValue(switchName, out bool exist); - if (!exist) + if (!AppContext.TryGetSwitch(switchName, out bool ret)) { string? switchValue = Environment.GetEnvironmentVariable(envVariable); if (switchValue != null) @@ -26,5 +68,61 @@ private static bool GetSwitchValue(string switchName, string envVariable) return ret; } + + private static bool TryGetStringValue(string switchName, string envVariable, [NotNullWhen(true)] out string? value) + { + value = AppContext.GetData(switchName) as string; + if (string.IsNullOrEmpty(value)) + { + value = Environment.GetEnvironmentVariable(envVariable); + if (string.IsNullOrEmpty(value)) + { + return false; + } + } + + return true; + } + + private static string CreateLibraryName(ReadOnlySpan baseName, ReadOnlySpan suffix, ReadOnlySpan extension, ReadOnlySpan version, bool versionAtEnd = false) + { + int length = baseName.Length + suffix.Length + version.Length + extension.Length; + + // We validate that suffix and version are not larger than 53 characters. + Span result = stackalloc char[length]; + baseName.CopyTo(result); + + Span secondPart = result.Slice(baseName.Length); + suffix.CopyTo(secondPart); + + Span middle = secondPart.Slice(suffix.Length); + + if (!versionAtEnd) + { + version.CopyTo(middle); + + Span end = middle.Slice(version.Length); + extension.CopyTo(end); + } + else + { + extension.CopyTo(middle); + + Span end = middle.Slice(extension.Length); + version.CopyTo(end); + } + + return result.ToString(); + } + + private static IntPtr LoadLibrary(string library, Assembly assembly, bool failOnLoadFailure) + { + if (!NativeLibrary.TryLoad(library, assembly, DllImportSearchPath.ApplicationDirectory, out IntPtr lib) && failOnLoadFailure) + { + Environment.FailFast($"Failed to load app-local ICU: {library}"); + } + + return lib; + } } } diff --git a/src/coreclr/src/libraries-native/entrypoints.c b/src/coreclr/src/libraries-native/entrypoints.c index 68553e99997af6..72d95e68be2638 100644 --- a/src/coreclr/src/libraries-native/entrypoints.c +++ b/src/coreclr/src/libraries-native/entrypoints.c @@ -51,6 +51,7 @@ FCFuncStart(gPalGlobalizationNative) QCFuncElement("GetTimeZoneDisplayName", GlobalizationNative_GetTimeZoneDisplayName) QCFuncElement("IndexOf", GlobalizationNative_IndexOf) QCFuncElement("IndexOfOrdinalIgnoreCase", GlobalizationNative_IndexOfOrdinalIgnoreCase) + QCFuncElement("InitICU", GlobalizationNative_InitICUFunctions) QCFuncElement("IsNormalized", GlobalizationNative_IsNormalized) QCFuncElement("IsPredefinedLocale", GlobalizationNative_IsPredefinedLocale) QCFuncElement("LastIndexOf", GlobalizationNative_LastIndexOf) diff --git a/src/libraries/Common/src/Interop/Interop.Collation.cs b/src/libraries/Common/src/Interop/Interop.Collation.cs index a59292e0fca1df..9ac819ece014e9 100644 --- a/src/libraries/Common/src/Interop/Interop.Collation.cs +++ b/src/libraries/Common/src/Interop/Interop.Collation.cs @@ -27,6 +27,7 @@ internal static partial class Globalization [DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_IndexOfOrdinalIgnoreCase")] internal static extern unsafe int IndexOfOrdinalIgnoreCase(string target, int cwTargetLength, char* pSource, int cwSourceLength, bool findLast); + [DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_IndexOfOrdinalIgnoreCase")] internal static extern unsafe int IndexOfOrdinalIgnoreCase(char* target, int cwTargetLength, char* pSource, int cwSourceLength, bool findLast); diff --git a/src/libraries/Common/src/Interop/Interop.ICU.cs b/src/libraries/Common/src/Interop/Interop.ICU.cs index 0dce072958a7fa..7e0661c4c21f34 100644 --- a/src/libraries/Common/src/Interop/Interop.ICU.cs +++ b/src/libraries/Common/src/Interop/Interop.ICU.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Runtime.InteropServices; internal static partial class Interop @@ -11,6 +12,9 @@ internal static partial class Globalization [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_LoadICU")] internal static extern int LoadICU(); + [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_InitICUFunctions")] + internal static extern void InitICU(IntPtr icuuc, IntPtr icuin, string version, string? suffix); + [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_GetICUVersion")] internal static extern int GetICUVersion(); } diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c index fbea9cf6ca87f4..387f913f5c7f6e 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c @@ -23,11 +23,76 @@ FOR_ALL_ICU_FUNCTIONS #undef PER_FUNCTION_BLOCK +// 20 for the actual suffix, 1 for _ and 1 for '\0' +#define SYMBOL_CUSTOM_SUFFIX_SIZE 22 +#define SYMBOL_NAME_SIZE (128 + SYMBOL_CUSTOM_SUFFIX_SIZE) +#define MaxICUVersionStringWithSuffixLength (MaxICUVersionStringLength + SYMBOL_CUSTOM_SUFFIX_SIZE) + + +#if defined(TARGET_WINDOWS) || defined(TARGET_OSX) || defined(TARGET_ANDROID) + +#define MaxICUVersionStringLength 33 + +#endif + static void* libicuuc = NULL; static void* libicui18n = NULL; +#if defined (TARGET_UNIX) + +#define PER_FUNCTION_BLOCK(fn, lib) \ + c_static_assert_msg((sizeof(#fn) + MaxICUVersionStringWithSuffixLength + 1) <= sizeof(symbolName), "The symbolName is too small for symbol " #fn); \ + sprintf(symbolName, #fn "%s", symbolVersion); \ + fn##_ptr = (__typeof(fn)*)dlsym(lib, symbolName); \ + if (fn##_ptr == NULL) { fprintf(stderr, "Cannot get symbol %s from " #lib "\nError: %s\n", symbolName, dlerror()); abort(); } + +static int FindSymbolVersion(int majorVer, int minorVer, int subVer, char* symbolName, char* symbolVersion, char* suffix) +{ + // Find out the format of the version string added to each symbol + // First try just the unversioned symbol + if (dlsym(libicuuc, "u_strlen") == NULL) + { + // Now try just the _majorVer added + sprintf(symbolVersion, "_%d%s", majorVer, suffix); + sprintf(symbolName, "u_strlen%s", symbolVersion); + if (dlsym(libicuuc, symbolName) == NULL) + { + if (minorVer == -1) + return FALSE; + + // Now try the _majorVer_minorVer added + sprintf(symbolVersion, "_%d_%d%s", majorVer, minorVer, suffix); + sprintf(symbolName, "u_strlen%s", symbolVersion); + if (dlsym(libicuuc, symbolName) == NULL) + { + if (subVer == -1) + return FALSE; + + // Finally, try the _majorVer_minorVer_subVer added + sprintf(symbolVersion, "_%d_%d_%d%s", majorVer, minorVer, subVer, suffix); + sprintf(symbolName, "u_strlen%s", symbolVersion); + if (dlsym(libicuuc, symbolName) == NULL) + { + return FALSE; + } + } + } + } + + return TRUE; +} + +#endif // TARGET_UNIX + #if defined(TARGET_WINDOWS) +#define sscanf sscanf_s + +#define PER_FUNCTION_BLOCK(fn, lib) \ + sprintf_s(symbolName, SYMBOL_NAME_SIZE, #fn "%s", symbolVersion); \ + fn##_ptr = (__typeof(fn)*)GetProcAddress((HMODULE)lib, symbolName); \ + if (fn##_ptr == NULL) { fprintf(stderr, "Cannot get symbol %s from " #lib "\nError: %u\n", symbolName, GetLastError()); abort(); } + static int FindICULibs() { libicuuc = LoadLibraryExW(L"icu.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); @@ -41,6 +106,42 @@ static int FindICULibs() return TRUE; } +static int FindSymbolVersion(int majorVer, int minorVer, int subVer, char* symbolName, char* symbolVersion, char* suffix) +{ + HMODULE lib = (HMODULE)libicuuc; + // Find out the format of the version string added to each symbol + // First try just the unversioned symbol + if (GetProcAddress(lib, "u_strlen") == NULL) + { + // Now try just the _majorVer added + sprintf_s(symbolVersion, MaxICUVersionStringWithSuffixLength,"_%d%s", majorVer, suffix); + sprintf_s(symbolName, SYMBOL_NAME_SIZE, "u_strlen%s", symbolVersion); + if (GetProcAddress(lib, symbolName) == NULL) + { + if (minorVer == -1) + return FALSE; + + // Now try the _majorVer_minorVer added + sprintf_s(symbolVersion, MaxICUVersionStringWithSuffixLength, "_%d_%d%s", majorVer, minorVer, suffix); + sprintf_s(symbolName, SYMBOL_NAME_SIZE, "u_strlen%s", symbolVersion); + if (GetProcAddress(lib, symbolName) == NULL) + { + if (subVer == -1) + return FALSE; + // Finally, try the _majorVer_minorVer_subVer added + sprintf_s(symbolVersion, MaxICUVersionStringWithSuffixLength, "_%d_%d_%d%s", majorVer, minorVer, subVer, suffix); + sprintf_s(symbolName, SYMBOL_NAME_SIZE, "u_strlen%s", symbolVersion); + if (GetProcAddress(lib, symbolName) == NULL) + { + return FALSE; + } + } + } + } + + return TRUE; +} + #elif defined(TARGET_OSX) static int FindICULibs() @@ -68,15 +169,13 @@ static int FindICULibs() // support ICU versions from 50-255 #define MinICUVersion 50 #define MaxICUVersion 255 -#define MaxICUVersionStringLength 4 static int FindSymbolVersion(char* symbolName, char* symbolVersion) { + char symbolSuffix[SYMBOL_CUSTOM_SUFFIX_SIZE]=""; for (int i = MinICUVersion; i <= MaxICUVersion; i++) { - sprintf(symbolVersion, "_%d", i); - sprintf(symbolName, "u_strlen%s", symbolVersion); - if (dlsym(libicuuc, symbolName) != NULL) + if (FindSymbolVersion(i, -1, -1, symbolName, symbolVersion, symbolSuffix)) { return TRUE; } @@ -153,36 +252,6 @@ static void GetVersionedLibFileName(const char* baseFileName, int majorVer, int } } -static int FindSymbolVersion(int majorVer, int minorVer, int subVer, char* symbolName, char* symbolVersion) -{ - // Find out the format of the version string added to each symbol - // First try just the unversioned symbol - if (dlsym(libicuuc, "u_strlen") == NULL) - { - // Now try just the _majorVer added - sprintf(symbolVersion, "_%d", majorVer); - sprintf(symbolName, "u_strlen%s", symbolVersion); - if ((dlsym(libicuuc, symbolName) == NULL) && (minorVer != -1)) - { - // Now try the _majorVer_minorVer added - sprintf(symbolVersion, "_%d_%d", majorVer, minorVer); - sprintf(symbolName, "u_strlen%s", symbolVersion); - if ((dlsym(libicuuc, symbolName) == NULL) && (subVer != -1)) - { - // Finally, try the _majorVer_minorVer_subVer added - sprintf(symbolVersion, "_%d_%d_%d", majorVer, minorVer, subVer); - sprintf(symbolName, "u_strlen%s", symbolVersion); - if (dlsym(libicuuc, symbolName) == NULL) - { - return FALSE; - } - } - } - } - - return TRUE; -} - // Try to open the necessary ICU libraries static int OpenICULibraries(int majorVer, int minorVer, int subVer, const char* versionPrefix, char* symbolName, char* symbolVersion) { @@ -198,7 +267,8 @@ static int OpenICULibraries(int majorVer, int minorVer, int subVer, const char* libicuuc = dlopen(libicuucName, RTLD_LAZY); if (libicuuc != NULL) { - if (FindSymbolVersion(majorVer, minorVer, subVer, symbolName, symbolVersion)) + char symbolSuffix[SYMBOL_CUSTOM_SUFFIX_SIZE]=""; + if (FindSymbolVersion(majorVer, minorVer, subVer, symbolName, symbolVersion, symbolSuffix)) { libicui18n = dlopen(libicui18nName, RTLD_LAZY); } @@ -312,35 +382,17 @@ static int FindICULibs(const char* versionPrefix, char* symbolName, char* symbol // return 0 if failed to load ICU and 1 otherwise int32_t GlobalizationNative_LoadICU() { -#if defined(TARGET_WINDOWS) - - if (!FindICULibs()) - { - return FALSE; - } + char symbolName[SYMBOL_NAME_SIZE]; + char symbolVersion[MaxICUVersionStringLength + 1]=""; -#define PER_FUNCTION_BLOCK(fn, lib) \ - fn##_ptr = (__typeof(fn)*)GetProcAddress((HMODULE)lib, #fn); \ - if (fn##_ptr == NULL) { fprintf(stderr, "Cannot get symbol %s from " #lib "\nError: %u\n", #fn, GetLastError()); abort(); } - -#elif defined(TARGET_OSX) +#if defined(TARGET_WINDOWS) || defined(TARGET_OSX) if (!FindICULibs()) { return FALSE; } - // Get pointers to all the ICU functions that are needed -#define PER_FUNCTION_BLOCK(fn, lib) \ - fn##_ptr = (__typeof(fn)*)dlsym(lib, #fn); \ - if (fn##_ptr == NULL) { fprintf(stderr, "Cannot get symbol %s from " #lib "\nError: %s\n", #fn, dlerror()); abort(); } - -#else // !TARGET_WINDOWS && !TARGET_OSX - - char symbolName[128]; - char symbolVersion[MaxICUVersionStringLength + 1] = ""; - -#if defined(TARGET_ANDROID) +#elif defined(TARGET_ANDROID) if (!FindICULibs(symbolName, symbolVersion)) { return FALSE; @@ -353,23 +405,50 @@ int32_t GlobalizationNative_LoadICU() return FALSE; } } -#endif +#endif // TARGET_WINDOWS || TARGET_OSX - // Get pointers to all the ICU functions that are needed -#define PER_FUNCTION_BLOCK(fn, lib) \ - c_static_assert_msg((sizeof(#fn) + MaxICUVersionStringLength + 1) <= sizeof(symbolName), "The symbolName is too small for symbol " #fn); \ - sprintf(symbolName, #fn "%s", symbolVersion); \ - fn##_ptr = (__typeof(fn)*)dlsym(lib, symbolName); \ - if (fn##_ptr == NULL) { fprintf(stderr, "Cannot get symbol %s from " #lib "\nError: %s\n", symbolName, dlerror()); abort(); } + FOR_ALL_ICU_FUNCTIONS + return TRUE; +} +void GlobalizationNative_InitICUFunctions(void* icuuc, void* icuin, const char* version, const char* suffix) +{ + assert(icuuc != NULL); + assert(icuin != NULL); + assert(version != NULL); + + libicuuc = icuuc; + libicui18n = icuin; + int major = -1; + int minor = -1; + int build = -1; + + char symbolName[SYMBOL_NAME_SIZE]; + char symbolVersion[MaxICUVersionStringWithSuffixLength + 1]=""; + char symbolSuffix[SYMBOL_CUSTOM_SUFFIX_SIZE]=""; + + sscanf(version, "%d.%d.%d", &major, &minor, &build); + + if (suffix != NULL) + { +#if defined(TARGET_WINDOWS) + sprintf_s(symbolSuffix, SYMBOL_CUSTOM_SUFFIX_SIZE, "_%s", suffix); +#else + sprintf(symbolSuffix, "_%s", suffix); #endif + } - FOR_ALL_ICU_FUNCTIONS -#undef PER_FUNCTION_BLOCK + if(!FindSymbolVersion(major, minor, build, symbolName, symbolVersion, symbolSuffix)) + { + fprintf(stderr, "Could not find symbol: %s from libicuuc\n", symbolName); + abort(); + } - return TRUE; + FOR_ALL_ICU_FUNCTIONS } +#undef PER_FUNCTION_BLOCK + // GlobalizationNative_GetICUVersion // return the current loaded ICU version int32_t GlobalizationNative_GetICUVersion() 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 49db55214bf8dd..ef940e2509355a 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.h +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.h @@ -5,4 +5,6 @@ PALEXPORT int32_t GlobalizationNative_LoadICU(void); +PALEXPORT void GlobalizationNative_InitICUFunctions(void* icuuc, void* icuin, const char* version, const char* suffix); + PALEXPORT int32_t GlobalizationNative_GetICUVersion(void); diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h index a13ea5f7ad9219..1175aef21614ee 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h @@ -87,8 +87,8 @@ PER_FUNCTION_BLOCK(ucol_safeClone, libicui18n) \ PER_FUNCTION_BLOCK(ucol_setAttribute, libicui18n) \ PER_FUNCTION_BLOCK(ucol_strcoll, libicui18n) \ - PER_FUNCTION_BLOCK(ucurr_forLocale, libicui18n) \ - PER_FUNCTION_BLOCK(ucurr_getName, libicui18n) \ + PER_FUNCTION_BLOCK(ucurr_forLocale, libicuuc) \ + PER_FUNCTION_BLOCK(ucurr_getName, libicuuc) \ PER_FUNCTION_BLOCK(udat_close, libicui18n) \ PER_FUNCTION_BLOCK(udat_countSymbols, libicui18n) \ PER_FUNCTION_BLOCK(udat_getSymbols, libicui18n) \ @@ -105,9 +105,9 @@ PER_FUNCTION_BLOCK(uidna_nameToASCII, libicuuc) \ PER_FUNCTION_BLOCK(uidna_nameToUnicode, libicuuc) \ PER_FUNCTION_BLOCK(uidna_openUTS46, libicuuc) \ - PER_FUNCTION_BLOCK(uldn_close, libicui18n) \ - PER_FUNCTION_BLOCK(uldn_keyValueDisplayName, libicui18n) \ - PER_FUNCTION_BLOCK(uldn_open, libicui18n) \ + PER_FUNCTION_BLOCK(uldn_close, libicuuc) \ + PER_FUNCTION_BLOCK(uldn_keyValueDisplayName, libicuuc) \ + PER_FUNCTION_BLOCK(uldn_open, libicuuc) \ PER_FUNCTION_BLOCK(uloc_canonicalize, libicuuc) \ PER_FUNCTION_BLOCK(uloc_countAvailable, libicuuc) \ PER_FUNCTION_BLOCK(uloc_getAvailable, libicuuc) \ diff --git a/src/libraries/System.Globalization/tests/NlsTests/NlsSwitchTests.cs b/src/libraries/System.Globalization/tests/NlsTests/NlsSwitchTests.cs index c7885ac41774ef..58da0a9dae525f 100644 --- a/src/libraries/System.Globalization/tests/NlsTests/NlsSwitchTests.cs +++ b/src/libraries/System.Globalization/tests/NlsTests/NlsSwitchTests.cs @@ -30,7 +30,7 @@ public static void NlsRuntimeSwitchIsHonored() [Fact] public static void IcuShouldNotBeLoaded() { - Assert.False(PlatformDetection.IsIcuGlobalization); + Assert.False(PlatformDetection.IsIcuGlobalization, $"Found ICU: {PlatformDetection.ICUVersion}"); } } } From ec9b9bf40fddaf1f99ade642526bdc2ebe364cbd Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Thu, 23 Apr 2020 18:51:59 -0700 Subject: [PATCH 2/7] Fix android build --- .../System.Globalization.Native/pal_icushim.c | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c index 387f913f5c7f6e..ef79be0c754ec0 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c @@ -170,20 +170,6 @@ static int FindICULibs() #define MinICUVersion 50 #define MaxICUVersion 255 -static int FindSymbolVersion(char* symbolName, char* symbolVersion) -{ - char symbolSuffix[SYMBOL_CUSTOM_SUFFIX_SIZE]=""; - for (int i = MinICUVersion; i <= MaxICUVersion; i++) - { - if (FindSymbolVersion(i, -1, -1, symbolName, symbolVersion, symbolSuffix)) - { - return TRUE; - } - } - - return FALSE; -} - static int FindICULibs(char* symbolName, char* symbolVersion) { libicui18n = dlopen("libicui18n.so", RTLD_LAZY); @@ -200,13 +186,17 @@ static int FindICULibs(char* symbolName, char* symbolVersion) return FALSE; } - if (!FindSymbolVersion(symbolName, symbolVersion)) + char symbolSuffix[SYMBOL_CUSTOM_SUFFIX_SIZE]=""; + for (int i = MinICUVersion; i <= MaxICUVersion; i++) { - fprintf(stderr, "Cannot determine ICU version."); - return FALSE; + if (FindSymbolVersion(i, -1, -1, symbolName, symbolVersion, symbolSuffix)) + { + return TRUE; + } } - return TRUE; + fprintf(stderr, "Cannot determine ICU version."); + return FALSE; } #else // !TARGET_WINDOWS && !TARGET_OSX && !TARGET_ANDROID From 29290350c344a125b455e34292380c4f98ce0abe Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Fri, 24 Apr 2020 17:23:58 -0700 Subject: [PATCH 3/7] Fix Unix distros with an old ICU version --- .../pal_icushim_internal.h | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h index 1175aef21614ee..dc53b829626715 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h @@ -87,8 +87,6 @@ PER_FUNCTION_BLOCK(ucol_safeClone, libicui18n) \ PER_FUNCTION_BLOCK(ucol_setAttribute, libicui18n) \ PER_FUNCTION_BLOCK(ucol_strcoll, libicui18n) \ - PER_FUNCTION_BLOCK(ucurr_forLocale, libicuuc) \ - PER_FUNCTION_BLOCK(ucurr_getName, libicuuc) \ PER_FUNCTION_BLOCK(udat_close, libicui18n) \ PER_FUNCTION_BLOCK(udat_countSymbols, libicui18n) \ PER_FUNCTION_BLOCK(udat_getSymbols, libicui18n) \ @@ -105,9 +103,6 @@ PER_FUNCTION_BLOCK(uidna_nameToASCII, libicuuc) \ PER_FUNCTION_BLOCK(uidna_nameToUnicode, libicuuc) \ PER_FUNCTION_BLOCK(uidna_openUTS46, libicuuc) \ - PER_FUNCTION_BLOCK(uldn_close, libicuuc) \ - PER_FUNCTION_BLOCK(uldn_keyValueDisplayName, libicuuc) \ - PER_FUNCTION_BLOCK(uldn_open, libicuuc) \ PER_FUNCTION_BLOCK(uloc_canonicalize, libicuuc) \ PER_FUNCTION_BLOCK(uloc_countAvailable, libicuuc) \ PER_FUNCTION_BLOCK(uloc_getAvailable, libicuuc) \ @@ -150,16 +145,37 @@ PER_FUNCTION_BLOCK(usearch_openFromCollator, libicui18n) #if HAVE_SET_MAX_VARIABLE -#define FOR_ALL_ICU_FUNCTIONS \ - FOR_ALL_UNCONDITIONAL_ICU_FUNCTIONS \ +#define FOR_ALL_SET_VARIABLE_ICU_FUNCTIONS \ PER_FUNCTION_BLOCK(ucol_setMaxVariable, libicui18n) #else -#define FOR_ALL_ICU_FUNCTIONS \ - FOR_ALL_UNCONDITIONAL_ICU_FUNCTIONS \ +#define FOR_ALL_SET_VARIABLE_ICU_FUNCTIONS \ PER_FUNCTION_BLOCK(ucol_setVariableTop, libicui18n) #endif +#if defined(TARGET_WINDOWS) +#define FOR_ALL_OS_CONDITIONAL_ICU_FUNCTIONS \ + PER_FUNCTION_BLOCK(ucurr_forLocale, libicuuc) \ + PER_FUNCTION_BLOCK(ucurr_getName, libicuuc) \ + PER_FUNCTION_BLOCK(uldn_close, libicuuc) \ + PER_FUNCTION_BLOCK(uldn_keyValueDisplayName, libicuuc) \ + PER_FUNCTION_BLOCK(uldn_open, libicuuc) +#else + // Unix ICU is dynamically resolved at runtime and these APIs in old versions + // of ICU were in libicui18n +#define FOR_ALL_OS_CONDITIONAL_ICU_FUNCTIONS \ + PER_FUNCTION_BLOCK(ucurr_forLocale, libicui18n) \ + PER_FUNCTION_BLOCK(ucurr_getName, libicui18n) \ + PER_FUNCTION_BLOCK(uldn_close, libicui18n) \ + PER_FUNCTION_BLOCK(uldn_keyValueDisplayName, libicui18n) \ + PER_FUNCTION_BLOCK(uldn_open, libicui18n) +#endif + +#define FOR_ALL_ICU_FUNCTIONS \ + FOR_ALL_UNCONDITIONAL_ICU_FUNCTIONS \ + FOR_ALL_SET_VARIABLE_ICU_FUNCTIONS \ + FOR_ALL_OS_CONDITIONAL_ICU_FUNCTIONS + // Declare pointers to all the used ICU functions #define PER_FUNCTION_BLOCK(fn, lib) EXTERN_C __typeof(fn)* fn##_ptr; FOR_ALL_ICU_FUNCTIONS From 1c3ff8a86e37cc205452551d8458ee0d479c79ca Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Mon, 27 Apr 2020 19:19:22 -0700 Subject: [PATCH 4/7] PR Feedback --- .../Globalization/GlobalizationMode.Unix.cs | 38 +++---- .../GlobalizationMode.Windows.cs | 34 +++---- .../System/Globalization/GlobalizationMode.cs | 99 ++++++++----------- .../src/libraries-native/entrypoints.c | 2 +- .../Common/src/Interop/Interop.ICU.cs | 11 ++- .../System.Globalization.Native/pal_icushim.c | 10 +- 6 files changed, 86 insertions(+), 108 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs index b7173aa8614f99..14c7699d1b7133 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Unix.cs @@ -2,8 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Reflection; - namespace System.Globalization { internal static partial class GlobalizationMode @@ -19,7 +17,11 @@ private static bool GetGlobalizationInvariantMode() bool invariantEnabled = GetInvariantSwitchValue(); if (!invariantEnabled) { - if (!LoadIcu()) + if (TryGetAppLocalIcuSwitchValue(out string? icuSuffixAndVersion)) + { + LoadAppLocalIcu(icuSuffixAndVersion, suffixWithSeparator: true); + } + else if (Interop.Globalization.LoadICU() == 0) { string message = "Couldn't find a valid ICU package installed on the system. " + "Set the configuration flag System.Globalization.Invariant to true if you want to run with no globalization support."; @@ -29,43 +31,27 @@ private static bool GetGlobalizationInvariantMode() return invariantEnabled; } - private static bool LoadIcu() + private static void LoadAppLocalIcuCore(ReadOnlySpan version, ReadOnlySpan suffix) { - if (!TryGetAppLocalIcuSwitchValue(out ReadOnlySpan version, out ReadOnlySpan suffix)) - { - return Interop.Globalization.LoadICU() != 0; - } #if TARGET_OSX - string extension = ".dylib"; + const string extension = ".dylib"; bool versionAtEnd = false; #else - string extension = "so."; + string extension = version.Length > 0 ? "so." : "so"; bool versionAtEnd = true; #endif - // Append '.' to suffix since in Unix the version is separated by '.' - int suffixLength = suffix.Length + 1; - Span suffixWithSeparator = stackalloc char[suffixLength]; - suffix.CopyTo(suffixWithSeparator); - suffixWithSeparator[suffixLength - 1] = '.'; - - Assembly assembly = Assembly.GetExecutingAssembly(); - #if !TARGET_OSX // In Linux we need to load libicudata first because libicuuc and libicui18n depend on it. In order for the loader to find // it on the same path, we load it before loading the other two libraries. - string icudataBase = "libicudata"; - LoadLibrary(CreateLibraryName(icudataBase, suffixWithSeparator, extension, version, versionAtEnd), assembly, failOnLoadFailure: true); + LoadLibrary(CreateLibraryName("libicudata", suffix, extension, version, versionAtEnd), failOnLoadFailure: true); #endif - string icuucBase = "libicuuc"; - string icuinBase = "libicui18n"; - IntPtr icuucLib = LoadLibrary(CreateLibraryName(icuucBase, suffixWithSeparator, extension, version, versionAtEnd), assembly, failOnLoadFailure: true); - IntPtr icuinLib = LoadLibrary(CreateLibraryName(icuinBase, suffixWithSeparator, extension, version, versionAtEnd), assembly, failOnLoadFailure: true); + IntPtr icuucLib = LoadLibrary(CreateLibraryName("libicuuc", suffix, extension, version, versionAtEnd), failOnLoadFailure: true); + IntPtr icuinLib = LoadLibrary(CreateLibraryName("libicui18n", suffix, extension, version, versionAtEnd), failOnLoadFailure: true); - Interop.Globalization.InitICU(icuucLib, icuinLib, version.ToString(), suffix.Length > 0 ? suffix.ToString() : null); - return true; + Interop.Globalization.InitICUFunctions(icuucLib, icuinLib, version, suffix); } } } diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Windows.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Windows.cs index de07f766f4deb6..1fa21bc928b6d3 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Windows.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.Windows.cs @@ -2,9 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Reflection; -using System.Runtime.InteropServices; - namespace System.Globalization { internal static partial class GlobalizationMode @@ -19,43 +16,46 @@ internal static partial class GlobalizationMode private static bool LoadIcu() { - if (!TryGetAppLocalIcuSwitchValue(out ReadOnlySpan version, out ReadOnlySpan suffix)) + if (!TryGetAppLocalIcuSwitchValue(out string? icuSuffixAndVersion)) { return Interop.Globalization.LoadICU() != 0; } - string extension = ".dll"; - string icuucBase = "icuuc"; - string icuinBase = "icuin"; + LoadAppLocalIcu(icuSuffixAndVersion); + return true; + } + + private static void LoadAppLocalIcuCore(ReadOnlySpan version, ReadOnlySpan suffix) + { + const string extension = ".dll"; + const string icuucBase = "icuuc"; + const string icuinBase = "icuin"; IntPtr icuucLib = IntPtr.Zero; IntPtr icuinLib = IntPtr.Zero; - Assembly assembly = Assembly.GetExecutingAssembly(); - - int index = version.ToString().IndexOf('.', StringComparison.Ordinal); - if (index != -1) + int index = version.IndexOf('.'); + if (index > 0) { ReadOnlySpan truncatedVersion = version.Slice(0, index); - icuucLib = LoadLibrary(CreateLibraryName(icuucBase, suffix, extension, truncatedVersion), assembly, failOnLoadFailure: false); + icuucLib = LoadLibrary(CreateLibraryName(icuucBase, suffix, extension, truncatedVersion), failOnLoadFailure: false); if (icuucLib != IntPtr.Zero) { - icuinLib = LoadLibrary(CreateLibraryName(icuinBase, suffix, extension, truncatedVersion), assembly, failOnLoadFailure: false); + icuinLib = LoadLibrary(CreateLibraryName(icuinBase, suffix, extension, truncatedVersion), failOnLoadFailure: false); } } if (icuucLib == IntPtr.Zero) { - icuucLib = LoadLibrary(CreateLibraryName(icuucBase, suffix, extension, version), assembly, failOnLoadFailure: true); + icuucLib = LoadLibrary(CreateLibraryName(icuucBase, suffix, extension, version), failOnLoadFailure: true); } if (icuinLib == IntPtr.Zero) { - icuinLib = LoadLibrary(CreateLibraryName(icuinBase, suffix, extension, version), assembly, failOnLoadFailure: true); + icuinLib = LoadLibrary(CreateLibraryName(icuinBase, suffix, extension, version), failOnLoadFailure: true); } - Interop.Globalization.InitICU(icuucLib, icuinLib, version.ToString(), suffix.Length > 0 ? suffix.ToString() : null); - return true; + Interop.Globalization.InitICUFunctions(icuucLib, icuinLib, version, suffix); } } } diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs index e6b90d259e1665..9c2723f659756f 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs @@ -13,44 +13,8 @@ internal static partial class GlobalizationMode private static bool GetInvariantSwitchValue() => GetSwitchValue("System.Globalization.Invariant", "DOTNET_SYSTEM_GLOBALIZATION_INVARIANT"); - private static bool TryGetAppLocalIcuSwitchValue(out ReadOnlySpan version, out ReadOnlySpan icuSuffix) - { - icuSuffix = default; - - if (!TryGetStringValue("System.Globalization.AppLocalIcu", "DOTNET_SYSTEM_GLOBALIZATION_APPLOCALICU", out string? value)) - { - version = default; - return false; - } - - // Custom built ICU can have a suffix on the name, i.e: libicuucmyapp.so.67.1 - // So users would set the runtime switch as: myapp:67.1 - - int indexOfSeparator = value.IndexOf(':', StringComparison.Ordinal); - if (indexOfSeparator != -1) - { - ReadOnlySpan valueAsSpan = value.AsSpan(); - icuSuffix = valueAsSpan.Slice(0, indexOfSeparator); - - if (icuSuffix.Length > 20) - { - Environment.FailFast($"The resolved \"{icuSuffix.ToString()}\" suffix from System.Globalization.AppLocalIcu switch has to be < 20 chars long."); - } - - version = valueAsSpan.Slice(icuSuffix.Length + 1); - } - else - { - version = value; - } - - if (version.Length > 33) - { - Environment.FailFast($"The resolved version \"{version.ToString()}\" from System.Globalization.AppLocalIcu switch has to be < 33 chars long."); - } - - return true; - } + private static bool TryGetAppLocalIcuSwitchValue([NotNullWhen(true)] out string? value) => + TryGetStringValue("System.Globalization.AppLocalIcu", "DOTNET_SYSTEM_GLOBALIZATION_APPLOCALICU", out value); // GetSwitchValue calls CLRConfig first to detect if the switch is defined in the config file. // if the switch is defined we just use the value of this switch. otherwise, we'll try to get the switch @@ -84,40 +48,57 @@ private static bool TryGetStringValue(string switchName, string envVariable, [No return true; } - private static string CreateLibraryName(ReadOnlySpan baseName, ReadOnlySpan suffix, ReadOnlySpan extension, ReadOnlySpan version, bool versionAtEnd = false) + private static void LoadAppLocalIcu(string icuSuffixAndVersion, bool suffixWithSeparator = false) { - int length = baseName.Length + suffix.Length + version.Length + extension.Length; - - // We validate that suffix and version are not larger than 53 characters. - Span result = stackalloc char[length]; - baseName.CopyTo(result); - - Span secondPart = result.Slice(baseName.Length); - suffix.CopyTo(secondPart); + ReadOnlySpan icuSuffix = default; + ReadOnlySpan version = default; - Span middle = secondPart.Slice(suffix.Length); - - if (!versionAtEnd) + // Custom built ICU can have a suffix on the name, i.e: libicuucmyapp.so.67.1 + // So users would set the runtime switch as: myapp:67.1 + int indexOfSeparator = icuSuffixAndVersion.IndexOf(':'); + if (indexOfSeparator >= 0) { - version.CopyTo(middle); + icuSuffix = icuSuffixAndVersion.AsSpan().Slice(0, indexOfSeparator); - Span end = middle.Slice(version.Length); - extension.CopyTo(end); + if (icuSuffix.Length > 35) + { + Environment.FailFast($"The resolved \"{icuSuffix.ToString()}\" suffix from System.Globalization.AppLocalIcu switch has to be < 20 chars long."); + } + + if (icuSuffix.Length + 1 <= icuSuffixAndVersion.Length) + version = icuSuffixAndVersion.AsSpan().Slice(icuSuffix.Length + 1); } else { - extension.CopyTo(middle); + version = icuSuffixAndVersion; + } + + if (version.Length > 33) + { + Environment.FailFast($"The resolved version \"{version.ToString()}\" from System.Globalization.AppLocalIcu switch has to be < 33 chars long."); + } - Span end = middle.Slice(extension.Length); - version.CopyTo(end); + if (suffixWithSeparator) + { + int suffixLength = icuSuffix.Length + 1; + Span finalSuffix = stackalloc char[suffixLength]; + icuSuffix.CopyTo(finalSuffix); + finalSuffix[suffixLength - 1] = '.'; + LoadAppLocalIcuCore(version, finalSuffix); + return; } - return result.ToString(); + LoadAppLocalIcuCore(version, icuSuffix); } - private static IntPtr LoadLibrary(string library, Assembly assembly, bool failOnLoadFailure) + private static string CreateLibraryName(ReadOnlySpan baseName, ReadOnlySpan suffix, ReadOnlySpan extension, ReadOnlySpan version, bool versionAtEnd = false) => + versionAtEnd ? + string.Concat(baseName, suffix, extension, version) : + string.Concat(baseName, suffix, version, extension); + + private static IntPtr LoadLibrary(string library, bool failOnLoadFailure) { - if (!NativeLibrary.TryLoad(library, assembly, DllImportSearchPath.ApplicationDirectory, out IntPtr lib) && failOnLoadFailure) + if (!NativeLibrary.TryLoad(library, typeof(object).Assembly, DllImportSearchPath.ApplicationDirectory, out IntPtr lib) && failOnLoadFailure) { Environment.FailFast($"Failed to load app-local ICU: {library}"); } diff --git a/src/coreclr/src/libraries-native/entrypoints.c b/src/coreclr/src/libraries-native/entrypoints.c index 72d95e68be2638..c0dc63961b9b4b 100644 --- a/src/coreclr/src/libraries-native/entrypoints.c +++ b/src/coreclr/src/libraries-native/entrypoints.c @@ -51,7 +51,7 @@ FCFuncStart(gPalGlobalizationNative) QCFuncElement("GetTimeZoneDisplayName", GlobalizationNative_GetTimeZoneDisplayName) QCFuncElement("IndexOf", GlobalizationNative_IndexOf) QCFuncElement("IndexOfOrdinalIgnoreCase", GlobalizationNative_IndexOfOrdinalIgnoreCase) - QCFuncElement("InitICU", GlobalizationNative_InitICUFunctions) + QCFuncElement("InitICUFunctions", GlobalizationNative_InitICUFunctions) QCFuncElement("IsNormalized", GlobalizationNative_IsNormalized) QCFuncElement("IsPredefinedLocale", GlobalizationNative_IsPredefinedLocale) QCFuncElement("LastIndexOf", GlobalizationNative_LastIndexOf) diff --git a/src/libraries/Common/src/Interop/Interop.ICU.cs b/src/libraries/Common/src/Interop/Interop.ICU.cs index 7e0661c4c21f34..fb8b56e41d35f2 100644 --- a/src/libraries/Common/src/Interop/Interop.ICU.cs +++ b/src/libraries/Common/src/Interop/Interop.ICU.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Diagnostics; using System.Runtime.InteropServices; internal static partial class Interop @@ -12,8 +13,16 @@ internal static partial class Globalization [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_LoadICU")] internal static extern int LoadICU(); + internal static void InitICUFunctions(IntPtr icuuc, IntPtr icuin, ReadOnlySpan version, ReadOnlySpan suffix) + { + Debug.Assert(icuuc != IntPtr.Zero); + Debug.Assert(icuin != IntPtr.Zero); + + InitICUFunctions(icuuc, icuin, version.ToString(), suffix.Length > 0 ? suffix.ToString() : null); + } + [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_InitICUFunctions")] - internal static extern void InitICU(IntPtr icuuc, IntPtr icuin, string version, string? suffix); + internal static extern void InitICUFunctions(IntPtr icuuc, IntPtr icuin, string version, string? suffix); [DllImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_GetICUVersion")] internal static extern int GetICUVersion(); diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c index ef79be0c754ec0..5bedc116b09032 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c @@ -23,8 +23,8 @@ FOR_ALL_ICU_FUNCTIONS #undef PER_FUNCTION_BLOCK -// 20 for the actual suffix, 1 for _ and 1 for '\0' -#define SYMBOL_CUSTOM_SUFFIX_SIZE 22 +// 35 for the actual suffix, 1 for _ and 1 for '\0' +#define SYMBOL_CUSTOM_SUFFIX_SIZE 37 #define SYMBOL_NAME_SIZE (128 + SYMBOL_CUSTOM_SUFFIX_SIZE) #define MaxICUVersionStringWithSuffixLength (MaxICUVersionStringLength + SYMBOL_CUSTOM_SUFFIX_SIZE) @@ -421,10 +421,12 @@ void GlobalizationNative_InitICUFunctions(void* icuuc, void* icuin, const char* if (suffix != NULL) { + assert(strlen(suffix) + 1 <= SYMBOL_CUSTOM_SUFFIX_SIZE); + #if defined(TARGET_WINDOWS) - sprintf_s(symbolSuffix, SYMBOL_CUSTOM_SUFFIX_SIZE, "_%s", suffix); + sprintf_s(symbolSuffix, SYMBOL_CUSTOM_SUFFIX_SIZE, "_%s", suffix); #else - sprintf(symbolSuffix, "_%s", suffix); + sprintf(symbolSuffix, "_%s", suffix); #endif } From 2f39284c1fde5939d4570df0cf548ffd56997743 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Fri, 1 May 2020 15:15:00 -0700 Subject: [PATCH 5/7] PR Feedback --- .../src/System/Globalization/GlobalizationMode.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs index 9c2723f659756f..dd460b178945f6 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Globalization/GlobalizationMode.cs @@ -65,8 +65,7 @@ private static void LoadAppLocalIcu(string icuSuffixAndVersion, bool suffixWithS Environment.FailFast($"The resolved \"{icuSuffix.ToString()}\" suffix from System.Globalization.AppLocalIcu switch has to be < 20 chars long."); } - if (icuSuffix.Length + 1 <= icuSuffixAndVersion.Length) - version = icuSuffixAndVersion.AsSpan().Slice(icuSuffix.Length + 1); + version = icuSuffixAndVersion.AsSpan().Slice(icuSuffix.Length + 1); } else { @@ -80,12 +79,7 @@ private static void LoadAppLocalIcu(string icuSuffixAndVersion, bool suffixWithS if (suffixWithSeparator) { - int suffixLength = icuSuffix.Length + 1; - Span finalSuffix = stackalloc char[suffixLength]; - icuSuffix.CopyTo(finalSuffix); - finalSuffix[suffixLength - 1] = '.'; - LoadAppLocalIcuCore(version, finalSuffix); - return; + icuSuffix = string.Concat(icuSuffix, "."); } LoadAppLocalIcuCore(version, icuSuffix); From 6420026a89d1a74764d092a7f80fc32a669a9ad0 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Fri, 1 May 2020 16:59:57 -0700 Subject: [PATCH 6/7] Add ICU data validation when initializing --- .../System.Globalization.Native/pal_icushim.c | 15 +++++++++++++++ .../pal_icushim_internal.h | 2 ++ .../pal_icushim_internal_android.h | 1 + 3 files changed, 18 insertions(+) diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c index 5bedc116b09032..f7ccb2f73a5617 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c @@ -366,6 +366,19 @@ static int FindICULibs(const char* versionPrefix, char* symbolName, char* symbol #endif +void ValidateICUDataCanLoad() +{ + UVersionInfo version; + UErrorCode err = U_ZERO_ERROR; + ulocdata_getCLDRVersion(version, &err); + + if (U_FAILURE(err)) + { + fprintf(stderr, "Could not load ICU data. UErrorCode: %d\n", err); + abort(); + } +} + // GlobalizationNative_LoadICU // This method get called from the managed side during the globalization initialization. // This method shouldn't get called at all if we are running in globalization invariant mode @@ -398,6 +411,7 @@ int32_t GlobalizationNative_LoadICU() #endif // TARGET_WINDOWS || TARGET_OSX FOR_ALL_ICU_FUNCTIONS + ValidateICUDataCanLoad(); return TRUE; } @@ -437,6 +451,7 @@ void GlobalizationNative_InitICUFunctions(void* icuuc, void* icuin, const char* } FOR_ALL_ICU_FUNCTIONS + ValidateICUDataCanLoad(); } #undef PER_FUNCTION_BLOCK diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h index dc53b829626715..33617e7f5d039d 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal.h @@ -121,6 +121,7 @@ PER_FUNCTION_BLOCK(uloc_getName, libicuuc) \ PER_FUNCTION_BLOCK(uloc_getParent, libicuuc) \ PER_FUNCTION_BLOCK(uloc_setKeywordValue, libicuuc) \ + PER_FUNCTION_BLOCK(ulocdata_getCLDRVersion, libicui18n) \ PER_FUNCTION_BLOCK(ulocdata_getMeasurementSystem, libicui18n) \ PER_FUNCTION_BLOCK(unorm2_getNFCInstance, libicuuc) \ PER_FUNCTION_BLOCK(unorm2_getNFDInstance, libicuuc) \ @@ -256,6 +257,7 @@ FOR_ALL_ICU_FUNCTIONS #define uloc_getName(...) uloc_getName_ptr(__VA_ARGS__) #define uloc_getParent(...) uloc_getParent_ptr(__VA_ARGS__) #define uloc_setKeywordValue(...) uloc_setKeywordValue_ptr(__VA_ARGS__) +#define ulocdata_getCLDRVersion(...) ulocdata_getCLDRVersion_ptr(__VA_ARGS__) #define ulocdata_getMeasurementSystem(...) ulocdata_getMeasurementSystem_ptr(__VA_ARGS__) #define unorm2_getNFCInstance(...) unorm2_getNFCInstance_ptr(__VA_ARGS__) #define unorm2_getNFDInstance(...) unorm2_getNFDInstance_ptr(__VA_ARGS__) diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal_android.h b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal_android.h index 5a3ac55072e860..17e1b8d8c8ad03 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal_android.h +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim_internal_android.h @@ -490,6 +490,7 @@ uint32_t uloc_getLCID(const char * localeID); int32_t uloc_getName(const char * localeID, char * name, int32_t nameCapacity, UErrorCode * err); int32_t uloc_getParent(const char * localeID, char * parent, int32_t parentCapacity, UErrorCode * err); int32_t uloc_setKeywordValue(const char * keywordName, const char * keywordValue, char * buffer, int32_t bufferCapacity, UErrorCode * status); +void ulocdata_getCLDRVersion(UVersionInfo versionArray, UErrorCode * status); UMeasurementSystem ulocdata_getMeasurementSystem(const char * localeID, UErrorCode * status); const UNormalizer2 * unorm2_getNFCInstance(UErrorCode * pErrorCode); const UNormalizer2 * unorm2_getNFDInstance(UErrorCode * pErrorCode); From 81296371bf6151cfc956299542513c6697903292 Mon Sep 17 00:00:00 2001 From: Santiago Fernandez Madero Date: Tue, 5 May 2020 11:24:48 -0700 Subject: [PATCH 7/7] Fix linux build --- .../Native/Unix/System.Globalization.Native/pal_icushim.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c index f7ccb2f73a5617..1a594c461c0ed9 100644 --- a/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c +++ b/src/libraries/Native/Unix/System.Globalization.Native/pal_icushim.c @@ -366,7 +366,7 @@ static int FindICULibs(const char* versionPrefix, char* symbolName, char* symbol #endif -void ValidateICUDataCanLoad() +static void ValidateICUDataCanLoad() { UVersionInfo version; UErrorCode err = U_ZERO_ERROR;