From 7d697e7c7685e124c9d1a203bd7f5acad48e9f35 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 26 Aug 2021 10:14:58 +0200 Subject: [PATCH 01/11] Implement mono_icall_get_environment_variable_names for Apple platforms using official API --- src/mono/mono/metadata/icall.c | 95 ++++++++++++++++++++++--------- src/mono/mono/mini/CMakeLists.txt | 2 +- 2 files changed, 70 insertions(+), 27 deletions(-) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 21fd4a523bdaa..ba56abbc1a002 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -36,6 +36,14 @@ #if defined (HAVE_WCHAR_H) #include #endif +#if defined (HOST_DARWIN) +#define BOOL OBJC_BOOL +#include +#include +#include +#undef BOOL +#undef bool +#endif #include "mono/metadata/icall-internals.h" #include "mono/utils/mono-membar.h" @@ -346,7 +354,7 @@ array_set_value_impl (MonoArrayHandle arr_handle, MonoObjectHandle value_handle, if (mono_class_is_nullable (ec)) { if (vc && m_class_is_primitive (vc) && vc != m_class_get_nullable_elem_class (ec)) { - // T -> Nullable T must be exact + // T -> Nullable T must be exact set_invalid_cast (error, vc, ec); goto leave; } @@ -3069,11 +3077,11 @@ ves_icall_RuntimeType_GetCorrespondingInflatedMethod (MonoReflectionTypeHandle r MonoMethod *method; gpointer iter = NULL; while ((method = mono_class_get_methods (klass, &iter))) { - if (method->token == generic_method->token) { + if (method->token == generic_method->token) { ret = mono_method_get_object_handle (method, klass, error); return_val_if_nok (error, MONO_HANDLE_CAST (MonoReflectionMethod, NULL_HANDLE)); } - } + } return ret; } @@ -4638,7 +4646,7 @@ g_concat_dir_and_file (const char *dir, const char *file) g_return_val_if_fail (dir != NULL, NULL); g_return_val_if_fail (file != NULL, NULL); - /* + /* * If the directory name doesn't have a / on the end, we need * to add one so we get a proper path to the file */ @@ -5553,11 +5561,11 @@ ves_icall_AssemblyExtensions_ApplyUpdate (MonoAssembly *assm, g_assert (image_base); #ifndef HOST_WASM - if (mono_is_debugger_attached ()) { - mono_error_set_not_supported (error, "Cannot use System.Reflection.Metadata.MetadataUpdater.ApplyChanges while debugger is attached"); - mono_error_set_pending_exception (error); - return; - } + if (mono_is_debugger_attached ()) { + mono_error_set_not_supported (error, "Cannot use System.Reflection.Metadata.MetadataUpdater.ApplyChanges while debugger is attached"); + mono_error_set_pending_exception (error); + return; + } #endif mono_image_load_enc_delta (MONO_ENC_DELTA_API, image_base, dmeta_bytes, dmeta_len, dil_bytes, dil_len, dpdb_bytes, dpdb_len, error); @@ -6268,21 +6276,7 @@ ves_icall_System_Environment_GetEnvironmentVariable_native (const gchar *utf8_na */ #ifndef _MSC_VER #ifndef __MINGW32_VERSION -#if defined(__APPLE__) -#if defined (TARGET_OSX) -/* Apple defines this in crt_externs.h but doesn't provide that header for - * arm-apple-darwin9. We'll manually define the symbol on Apple as it does - * in fact exist on all implementations (so far) - */ -G_BEGIN_DECLS -gchar ***_NSGetEnviron(void); -G_END_DECLS -#define environ (*_NSGetEnviron()) -#else -static char *mono_environ[1] = { NULL }; -#define environ mono_environ -#endif /* defined (TARGET_OSX) */ -#else +#ifndef __APPLE__ G_BEGIN_DECLS extern char **environ; @@ -6298,7 +6292,55 @@ ves_icall_System_Environment_GetCommandLineArgs (MonoError *error) return result; } -#ifdef HOST_WIN32 +#if defined(HOST_DARWIN) + +static MonoArrayHandle +mono_icall_get_environment_variable_names (MonoError *error) +{ + MonoArrayHandle names; + MonoStringHandle str; + int n, i; + id *keys; + CFDictionaryRef environ; + char *name; + + id id_NSProcessInfo = (id) objc_getClass ("NSProcessInfo"); + SEL sel_processInfo = sel_registerName ("processInfo"); + SEL sel_environment = sel_registerName ("environment"); + SEL sel_UTF8String = sel_registerName ("UTF8String"); + + id (*id_objc_msgSend)(id, SEL) = (id (*)(id, SEL)) objc_msgSend; + CFDictionaryRef (*dict_objc_msgSend)(id, SEL) = (CFDictionaryRef (*)(id, SEL)) objc_msgSend; + char* (*charptr_objc_msgSend)(id, SEL) = (char* (*)(id, SEL)) objc_msgSend; + + environ = dict_objc_msgSend (id_objc_msgSend (id_NSProcessInfo, sel_processInfo), sel_environment); + + n = CFDictionaryGetCount (environ); + keys = g_malloc (n * sizeof(id)); + CFDictionaryGetKeysAndValues (environ, (const void **) keys, NULL); + + names = mono_array_new_handle (mono_defaults.string_class, n, error); + return_val_if_nok (error, NULL_HANDLE_ARRAY); + + str = MONO_HANDLE_NEW (MonoString, NULL); + for (i = 0; i < n; i++) { + name = (char *) charptr_objc_msgSend (keys[i], sel_UTF8String); + MonoString *s = mono_string_new_checked (name, error); + MONO_HANDLE_ASSIGN_RAW (str, s); + if (!is_ok (error)) { + g_free (keys); + return NULL_HANDLE_ARRAY; + } + mono_array_handle_setref (names, n, str); + } + + g_free (keys); + + return names; +} + +#elif defined (HOST_WIN32) + static MonoArrayHandle mono_icall_get_environment_variable_names (MonoError *error) { @@ -6395,7 +6437,8 @@ mono_icall_get_environment_variable_names (MonoError *error) return names; } -#endif /* !HOST_WIN32 */ + +#endif /* !HOST_WIN32 && !HOST_DARWIN */ MonoArrayHandle ves_icall_System_Environment_GetEnvironmentVariableNames (MonoError *error) diff --git a/src/mono/mono/mini/CMakeLists.txt b/src/mono/mono/mini/CMakeLists.txt index 3e9ca497336ab..c392138b17349 100644 --- a/src/mono/mono/mini/CMakeLists.txt +++ b/src/mono/mono/mini/CMakeLists.txt @@ -23,7 +23,7 @@ if(HOST_DARWIN) set(OS_LIBS "-framework CoreFoundation" "-framework Foundation") if(CMAKE_SYSTEM_VARIANT STREQUAL "MacCatalyst") - set(OS_LIBS "-lobjc" "-lc++") + set(OS_LIBS ${OS_LIBS} "-lobjc" "-lc++") endif() elseif(HOST_IOS) set(OS_LIBS "-framework CoreFoundation" "-lobjc" "-lc++") From 33e5b5df98223950e6ded0b15ed994e503a83e94 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 26 Aug 2021 13:18:23 +0200 Subject: [PATCH 02/11] Move environment handling from Mono runtime to System.Native --- .../Unix/System.Native/Interop.GetEnv.cs | 14 ++ .../Unix/System.Native/Interop.GetEnviron.cs | 14 ++ .../Native/Unix/System.Native/CMakeLists.txt | 4 + .../Native/Unix/System.Native/entrypoints.c | 3 + .../Unix/System.Native/pal_environment.c | 29 +++ .../Unix/System.Native/pal_environment.h | 11 + .../Unix/System.Native/pal_environment.m | 59 ++++++ .../System.Private.CoreLib.csproj | 6 + .../src/System/Environment.Unix.Mono.cs | 71 +++++-- .../src/System/Environment.iOS.cs | 2 +- src/mono/mono/metadata/icall-def-netcore.h | 2 - src/mono/mono/metadata/icall.c | 194 ------------------ 12 files changed, 190 insertions(+), 219 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnv.cs create mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnviron.cs create mode 100644 src/libraries/Native/Unix/System.Native/pal_environment.c create mode 100644 src/libraries/Native/Unix/System.Native/pal_environment.h create mode 100644 src/libraries/Native/Unix/System.Native/pal_environment.m diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnv.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnv.cs new file mode 100644 index 0000000000000..d83d6700b5162 --- /dev/null +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnv.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 unsafe partial class Sys + { + [DllImport(Interop.Libraries.SystemNative, EntryPoint = "SystemNative_GetEnv")] + internal static extern unsafe IntPtr GetEnv(string name); + } +} \ No newline at end of file diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnviron.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnviron.cs new file mode 100644 index 0000000000000..2afa918d23b37 --- /dev/null +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnviron.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 unsafe partial class Sys + { + [DllImport(Interop.Libraries.SystemNative, EntryPoint = "SystemNative_GetEnviron")] + internal static extern unsafe IntPtr GetEnviron(); + } +} \ No newline at end of file diff --git a/src/libraries/Native/Unix/System.Native/CMakeLists.txt b/src/libraries/Native/Unix/System.Native/CMakeLists.txt index a28952750b3ef..19266ebed5b75 100644 --- a/src/libraries/Native/Unix/System.Native/CMakeLists.txt +++ b/src/libraries/Native/Unix/System.Native/CMakeLists.txt @@ -11,6 +11,7 @@ endif () include_directories("${CLR_SRC_NATIVE_DIR}/common") set(NATIVE_SOURCES + pal_environment.c pal_errno.c pal_interfaceaddresses.c pal_io.c @@ -36,8 +37,11 @@ set(NATIVE_SOURCES if (CLR_CMAKE_TARGET_OSX OR CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS) list (APPEND NATIVE_SOURCES pal_autoreleasepool.m) set_source_files_properties(pal_autoreleasepool.m PROPERTIES COMPILE_FLAGS -fno-objc-arc) + list (APPEND NATIVE_SOURCES pal_environment.m) + add_definitions(-DUSE_APPLE_GETENVIRON=1) else() list (APPEND NATIVE_SOURCES pal_autoreleasepool.c) + add_definitions(-DUSE_APPLE_GETENVIRON=0) endif() if (CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS) diff --git a/src/libraries/Native/Unix/System.Native/entrypoints.c b/src/libraries/Native/Unix/System.Native/entrypoints.c index b3ae64f96f91e..4a6d4302f12ee 100644 --- a/src/libraries/Native/Unix/System.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Native/entrypoints.c @@ -7,6 +7,7 @@ #include "pal_autoreleasepool.h" #include "pal_console.h" #include "pal_datetime.h" +#include "pal_environment.h" #include "pal_errno.h" #include "pal_interfaceaddresses.h" #include "pal_io.h" @@ -258,6 +259,8 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_SetPosixSignalHandler) DllImportEntry(SystemNative_GetPlatformSignalNumber) DllImportEntry(SystemNative_GetGroups) + DllImportEntry(SystemNative_GetEnv) + DllImportEntry(SystemNative_GetEnviron) }; EXTERN_C const void* SystemResolveDllImport(const char* name); diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.c b/src/libraries/Native/Unix/System.Native/pal_environment.c new file mode 100644 index 0000000000000..b436e16b9c58b --- /dev/null +++ b/src/libraries/Native/Unix/System.Native/pal_environment.c @@ -0,0 +1,29 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include "pal_config.h" +#include "pal_environment.h" + +#include +#include +#if HAVE_NSGETENVIRON +#include +#endif + +char* SystemNative_GetEnv(const char* variable) +{ + return getenv(variable); +} + +char** SystemNative_GetEnviron() +{ +#if USE_APPLE_GETENVIRON + extern char** GetEnvironApple(); + return GetEnvironApple(); +#elif HAVE_NSGETENVIRON + return *(_NSGetEnviron()); +#else + extern char **environ; + return environ; +#endif +} diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.h b/src/libraries/Native/Unix/System.Native/pal_environment.h new file mode 100644 index 0000000000000..8fbd978be6ed1 --- /dev/null +++ b/src/libraries/Native/Unix/System.Native/pal_environment.h @@ -0,0 +1,11 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#pragma once + +#include "pal_compiler.h" +#include "pal_types.h" + +PALEXPORT char* SystemNative_GetEnv(const char* variable); + +PALEXPORT char** SystemNative_GetEnviron(void); diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.m b/src/libraries/Native/Unix/System.Native/pal_environment.m new file mode 100644 index 0000000000000..1d035c3398db4 --- /dev/null +++ b/src/libraries/Native/Unix/System.Native/pal_environment.m @@ -0,0 +1,59 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#include "pal_config.h" +#include "pal_environment.h" + +#include +#include +#include +#include + +static void get_environ_helper(const void *key, const void *value, void *context) +{ + char ***temp_environ_ptr = (char***)context; + const char *utf8_key = [(NSString *)key UTF8String]; + const char *utf8_value = [(NSString *)value UTF8String]; + int utf8_key_length = strlen(utf8_key); + int utf8_value_length = strlen(utf8_value); + char *key_value_pair; + + key_value_pair = malloc(utf8_key_length + utf8_value_length + 2); + if (key_value_pair != NULL) + { + strcpy(key_value_pair, utf8_key); + key_value_pair[utf8_key_length] = '='; + strcpy(key_value_pair + utf8_key_length + 1, utf8_value); + } + + **temp_environ_ptr = key_value_pair; + (*temp_environ_ptr)++; +} + +char** GetEnvironApple() +{ + static char **environ; + + // NOTE: This function is not thread-safe and it leaks one array per process. This is + // intentional behavior and the managed code is expected to take additional guards + // around this call. + if (environ == NULL) + { + int environ_size = 1; + char **temp_environ; + char **temp_environ_ptr; + + CFDictionaryRef environment = (CFDictionaryRef)[[NSProcessInfo processInfo] environment]; + int count = CFDictionaryGetCount(environment); + temp_environ = (char **)malloc((count + 1) * sizeof(char *)); + if (temp_environ != NULL) + { + temp_environ_ptr = temp_environ; + CFDictionaryApplyFunction(environment, get_environ_helper, &temp_environ_ptr); + *temp_environ_ptr = NULL; + environ = temp_environ; + } + } + + return environ; +} \ No newline at end of file diff --git a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj index 7aa4413aad70d..3e032e76483b2 100644 --- a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -265,6 +265,12 @@ + + Common\Interop\Unix\System.Native\Interop.GetEnv.cs + + + Common\Interop\Unix\System.Native\Interop.GetEnviron.cs + diff --git a/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs index 1b75ae0abbc2b..148b170df0449 100644 --- a/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs @@ -5,8 +5,8 @@ using System.Collections.Generic; using System.Diagnostics; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Threading; -using Mono; namespace System { @@ -20,7 +20,7 @@ public partial class Environment if (s_environment == null) { - return InternalGetEnvironmentVariable(variable); + return Marshal.PtrToStringAnsi(Interop.Sys.GetEnv(variable)); } variable = TrimStringOnFirstZero(variable); @@ -31,23 +31,15 @@ public partial class Environment } } - private static string InternalGetEnvironmentVariable(string name) - { - using (SafeStringMarshal handle = RuntimeMarshal.MarshalString(name)) - { - return internalGetEnvironmentVariable_native(handle.Value); - } - } - private static unsafe void SetEnvironmentVariableCore(string variable, string? value) { Debug.Assert(variable != null); EnsureEnvironmentCached(); + variable = TrimStringOnFirstZero(variable); + value = value == null ? null : TrimStringOnFirstZero(value); lock (s_environment!) { - variable = TrimStringOnFirstZero(variable); - value = value == null ? null : TrimStringOnFirstZero(value); if (string.IsNullOrEmpty(value)) { s_environment.Remove(variable); @@ -97,22 +89,57 @@ private static Dictionary GetSystemEnvironmentVariables() { var results = new Dictionary(); - foreach (string name in GetEnvironmentVariableNames()) + IntPtr block = Interop.Sys.GetEnviron(); + if (block != IntPtr.Zero) { - if (name != null) + // Per man page, environment variables come back as an array of pointers to strings + // Parse each pointer of strings individually + while (ParseEntry(block, out string? key, out string? value)) { - results.Add(name, InternalGetEnvironmentVariable(name)); + if (key != null && value != null) + results.Add(key, value); + + // Increment to next environment variable entry + block += IntPtr.Size; } } return results; - } - - - [MethodImplAttribute(MethodImplOptions.InternalCall)] - private static extern string internalGetEnvironmentVariable_native(IntPtr variable); - [MethodImplAttribute(MethodImplOptions.InternalCall)] - private static extern string[] GetEnvironmentVariableNames(); + // Use a local, unsafe function since we cannot use `yield return` inside of an `unsafe` block + static unsafe bool ParseEntry(IntPtr current, out string? key, out string? value) + { + // Setup + key = null; + value = null; + + // Point to current entry + byte* entry = *(byte**)current; + + // Per man page, "The last pointer in this array has the value NULL" + // Therefore, if entry is null then we're at the end and can bail + if (entry == null) + return false; + + // Parse each byte of the entry until we hit either the separator '=' or '\0'. + // This finds the split point for creating key/value strings below. + // On some old OS, the environment block can be corrupted. + // Some will not have '=', so we need to check for '\0'. + byte* splitpoint = entry; + while (*splitpoint != '=' && *splitpoint != '\0') + splitpoint++; + + // Skip over entries starting with '=' and entries with no value (just a null-terminating char '\0') + if (splitpoint == entry || *splitpoint == '\0') + return true; + + // The key is the bytes from start (0) until our splitpoint + key = new string((sbyte*)entry, 0, checked((int)(splitpoint - entry))); + // The value is the rest of the bytes starting after the splitpoint + value = new string((sbyte*)(splitpoint + 1)); + + return true; + } + } } } diff --git a/src/mono/System.Private.CoreLib/src/System/Environment.iOS.cs b/src/mono/System.Private.CoreLib/src/System/Environment.iOS.cs index 01ceb05bba7dc..affed35d7b452 100644 --- a/src/mono/System.Private.CoreLib/src/System/Environment.iOS.cs +++ b/src/mono/System.Private.CoreLib/src/System/Environment.iOS.cs @@ -80,7 +80,7 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio return Interop.Sys.SearchPath(NSSearchPathDirectory.NSCachesDirectory); case SpecialFolder.UserProfile: - return InternalGetEnvironmentVariable("HOME"); + return GetEnvironmentVariable("HOME"); case SpecialFolder.CommonApplicationData: return "/usr/share"; diff --git a/src/mono/mono/metadata/icall-def-netcore.h b/src/mono/mono/metadata/icall-def-netcore.h index 0a249b1521e61..1b826c7737196 100644 --- a/src/mono/mono/metadata/icall-def-netcore.h +++ b/src/mono/mono/metadata/icall-def-netcore.h @@ -82,12 +82,10 @@ ICALL_TYPE(ENV, "System.Environment", ENV_1) NOHANDLES(ICALL(ENV_1, "Exit", ves_icall_System_Environment_Exit)) HANDLES(ENV_1a, "FailFast", ves_icall_System_Environment_FailFast, void, 3, (MonoString, MonoException, MonoString)) HANDLES(ENV_2, "GetCommandLineArgs", ves_icall_System_Environment_GetCommandLineArgs, MonoArray, 0, ()) -HANDLES(ENV_3, "GetEnvironmentVariableNames", ves_icall_System_Environment_GetEnvironmentVariableNames, MonoArray, 0, ()) NOHANDLES(ICALL(ENV_4, "GetProcessorCount", ves_icall_System_Environment_get_ProcessorCount)) NOHANDLES(ICALL(ENV_9, "get_ExitCode", mono_environment_exitcode_get)) NOHANDLES(ICALL(ENV_15, "get_TickCount", ves_icall_System_Environment_get_TickCount)) NOHANDLES(ICALL(ENV_15a, "get_TickCount64", ves_icall_System_Environment_get_TickCount64)) -HANDLES(ENV_17, "internalGetEnvironmentVariable_native", ves_icall_System_Environment_GetEnvironmentVariable_native, MonoString, 1, (const_char_ptr)) NOHANDLES(ICALL(ENV_20, "set_ExitCode", mono_environment_exitcode_set)) ICALL_TYPE(GC, "System.GC", GC_13) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index ba56abbc1a002..67eabc171fa62 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -36,14 +36,6 @@ #if defined (HAVE_WCHAR_H) #include #endif -#if defined (HOST_DARWIN) -#define BOOL OBJC_BOOL -#include -#include -#include -#undef BOOL -#undef bool -#endif #include "mono/metadata/icall-internals.h" #include "mono/utils/mono-membar.h" @@ -6253,38 +6245,6 @@ mono_array_get_byte_length (MonoArrayHandle array) /* System.Environment */ -MonoStringHandle -ves_icall_System_Environment_GetEnvironmentVariable_native (const gchar *utf8_name, MonoError *error) -{ - gchar *value; - - if (utf8_name == NULL) - return NULL_HANDLE_STRING; - - value = g_getenv (utf8_name); - - if (value == 0) - return NULL_HANDLE_STRING; - - MonoStringHandle res = mono_string_new_handle (value, error); - g_free (value); - return res; -} - -/* - * There is no standard way to get at environ. - */ -#ifndef _MSC_VER -#ifndef __MINGW32_VERSION -#ifndef __APPLE__ -G_BEGIN_DECLS -extern -char **environ; -G_END_DECLS -#endif -#endif -#endif - MonoArrayHandle ves_icall_System_Environment_GetCommandLineArgs (MonoError *error) { @@ -6292,160 +6252,6 @@ ves_icall_System_Environment_GetCommandLineArgs (MonoError *error) return result; } -#if defined(HOST_DARWIN) - -static MonoArrayHandle -mono_icall_get_environment_variable_names (MonoError *error) -{ - MonoArrayHandle names; - MonoStringHandle str; - int n, i; - id *keys; - CFDictionaryRef environ; - char *name; - - id id_NSProcessInfo = (id) objc_getClass ("NSProcessInfo"); - SEL sel_processInfo = sel_registerName ("processInfo"); - SEL sel_environment = sel_registerName ("environment"); - SEL sel_UTF8String = sel_registerName ("UTF8String"); - - id (*id_objc_msgSend)(id, SEL) = (id (*)(id, SEL)) objc_msgSend; - CFDictionaryRef (*dict_objc_msgSend)(id, SEL) = (CFDictionaryRef (*)(id, SEL)) objc_msgSend; - char* (*charptr_objc_msgSend)(id, SEL) = (char* (*)(id, SEL)) objc_msgSend; - - environ = dict_objc_msgSend (id_objc_msgSend (id_NSProcessInfo, sel_processInfo), sel_environment); - - n = CFDictionaryGetCount (environ); - keys = g_malloc (n * sizeof(id)); - CFDictionaryGetKeysAndValues (environ, (const void **) keys, NULL); - - names = mono_array_new_handle (mono_defaults.string_class, n, error); - return_val_if_nok (error, NULL_HANDLE_ARRAY); - - str = MONO_HANDLE_NEW (MonoString, NULL); - for (i = 0; i < n; i++) { - name = (char *) charptr_objc_msgSend (keys[i], sel_UTF8String); - MonoString *s = mono_string_new_checked (name, error); - MONO_HANDLE_ASSIGN_RAW (str, s); - if (!is_ok (error)) { - g_free (keys); - return NULL_HANDLE_ARRAY; - } - mono_array_handle_setref (names, n, str); - } - - g_free (keys); - - return names; -} - -#elif defined (HOST_WIN32) - -static MonoArrayHandle -mono_icall_get_environment_variable_names (MonoError *error) -{ - MonoArrayHandle names; - MonoStringHandle str; - WCHAR* env_strings; - WCHAR* env_string; - WCHAR* equal_str; - int n = 0; - - env_strings = GetEnvironmentStrings(); - - if (env_strings) { - env_string = env_strings; - while (*env_string != '\0') { - /* weird case that MS seems to skip */ - if (*env_string != '=') - n++; - while (*env_string != '\0') - env_string++; - env_string++; - } - } - - names = mono_array_new_handle (mono_defaults.string_class, n, error); - return_val_if_nok (error, NULL_HANDLE_ARRAY); - - if (env_strings) { - n = 0; - str = MONO_HANDLE_NEW (MonoString, NULL); - env_string = env_strings; - while (*env_string != '\0') { - /* weird case that MS seems to skip */ - if (*env_string != '=') { - equal_str = wcschr(env_string, '='); - g_assert(equal_str); - MonoString *s = mono_string_new_utf16_checked (env_string, (gint32)(equal_str - env_string), error); - goto_if_nok (error, cleanup); - MONO_HANDLE_ASSIGN_RAW (str, s); - - mono_array_handle_setref (names, n, str); - n++; - } - while (*env_string != '\0') - env_string++; - env_string++; - } - - } - -cleanup: - if (env_strings) - FreeEnvironmentStrings (env_strings); - if (!is_ok (error)) - return NULL_HANDLE_ARRAY; - return names; -} - -#else - -static MonoArrayHandle -mono_icall_get_environment_variable_names (MonoError *error) -{ - MonoArrayHandle names; - MonoStringHandle str; - gchar **e, **parts; - int n; - - n = 0; - for (e = environ; *e != 0; ++ e) - ++ n; - - names = mono_array_new_handle (mono_defaults.string_class, n, error); - return_val_if_nok (error, NULL_HANDLE_ARRAY); - - str = MONO_HANDLE_NEW (MonoString, NULL); - n = 0; - for (e = environ; *e != 0; ++ e) { - parts = g_strsplit (*e, "=", 2); - if (*parts != 0) { - MonoString *s = mono_string_new_checked (*parts, error); - MONO_HANDLE_ASSIGN_RAW (str, s); - if (!is_ok (error)) { - g_strfreev (parts); - return NULL_HANDLE_ARRAY; - } - mono_array_handle_setref (names, n, str); - } - - g_strfreev (parts); - - ++ n; - } - - return names; -} - -#endif /* !HOST_WIN32 && !HOST_DARWIN */ - -MonoArrayHandle -ves_icall_System_Environment_GetEnvironmentVariableNames (MonoError *error) -{ - return mono_icall_get_environment_variable_names (error); -} - void ves_icall_System_Environment_Exit (int result) { From 5eb8dc2f352ef04fd1fd9d229d55cbbcd2964a00 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 26 Aug 2021 17:04:06 +0200 Subject: [PATCH 03/11] Revert unnecessary changes --- src/mono/mono/metadata/icall.c | 18 +++++++++--------- src/mono/mono/mini/CMakeLists.txt | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 67eabc171fa62..09593c4bb68a6 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -346,7 +346,7 @@ array_set_value_impl (MonoArrayHandle arr_handle, MonoObjectHandle value_handle, if (mono_class_is_nullable (ec)) { if (vc && m_class_is_primitive (vc) && vc != m_class_get_nullable_elem_class (ec)) { - // T -> Nullable T must be exact + // T -> Nullable T must be exact set_invalid_cast (error, vc, ec); goto leave; } @@ -3069,11 +3069,11 @@ ves_icall_RuntimeType_GetCorrespondingInflatedMethod (MonoReflectionTypeHandle r MonoMethod *method; gpointer iter = NULL; while ((method = mono_class_get_methods (klass, &iter))) { - if (method->token == generic_method->token) { + if (method->token == generic_method->token) { ret = mono_method_get_object_handle (method, klass, error); return_val_if_nok (error, MONO_HANDLE_CAST (MonoReflectionMethod, NULL_HANDLE)); } - } + } return ret; } @@ -4638,7 +4638,7 @@ g_concat_dir_and_file (const char *dir, const char *file) g_return_val_if_fail (dir != NULL, NULL); g_return_val_if_fail (file != NULL, NULL); - /* + /* * If the directory name doesn't have a / on the end, we need * to add one so we get a proper path to the file */ @@ -5553,11 +5553,11 @@ ves_icall_AssemblyExtensions_ApplyUpdate (MonoAssembly *assm, g_assert (image_base); #ifndef HOST_WASM - if (mono_is_debugger_attached ()) { - mono_error_set_not_supported (error, "Cannot use System.Reflection.Metadata.MetadataUpdater.ApplyChanges while debugger is attached"); - mono_error_set_pending_exception (error); - return; - } + if (mono_is_debugger_attached ()) { + mono_error_set_not_supported (error, "Cannot use System.Reflection.Metadata.MetadataUpdater.ApplyChanges while debugger is attached"); + mono_error_set_pending_exception (error); + return; + } #endif mono_image_load_enc_delta (MONO_ENC_DELTA_API, image_base, dmeta_bytes, dmeta_len, dil_bytes, dil_len, dpdb_bytes, dpdb_len, error); diff --git a/src/mono/mono/mini/CMakeLists.txt b/src/mono/mono/mini/CMakeLists.txt index c392138b17349..3e9ca497336ab 100644 --- a/src/mono/mono/mini/CMakeLists.txt +++ b/src/mono/mono/mini/CMakeLists.txt @@ -23,7 +23,7 @@ if(HOST_DARWIN) set(OS_LIBS "-framework CoreFoundation" "-framework Foundation") if(CMAKE_SYSTEM_VARIANT STREQUAL "MacCatalyst") - set(OS_LIBS ${OS_LIBS} "-lobjc" "-lc++") + set(OS_LIBS "-lobjc" "-lc++") endif() elseif(HOST_IOS) set(OS_LIBS "-framework CoreFoundation" "-lobjc" "-lc++") From 33460363d8aa3f320d8fbcffdc35394104b3037b Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 26 Aug 2021 17:52:33 +0200 Subject: [PATCH 04/11] Split pal_environment, remove HAVE_NSGETENVIRON --- src/libraries/Native/Unix/Common/pal_config.h.in | 1 - src/libraries/Native/Unix/System.Native/CMakeLists.txt | 4 +--- .../Native/Unix/System.Native/pal_environment.c | 10 ---------- .../Native/Unix/System.Native/pal_environment.m | 7 ++++++- src/libraries/Native/Unix/configure.cmake | 9 --------- 5 files changed, 7 insertions(+), 24 deletions(-) diff --git a/src/libraries/Native/Unix/Common/pal_config.h.in b/src/libraries/Native/Unix/Common/pal_config.h.in index 9a02610534101..cfc7a7770a68f 100644 --- a/src/libraries/Native/Unix/Common/pal_config.h.in +++ b/src/libraries/Native/Unix/Common/pal_config.h.in @@ -102,7 +102,6 @@ #cmakedefine01 HAVE_GSS_SPNEGO_MECHANISM #cmakedefine01 HAVE_GSS_KRB5_CRED_NO_CI_FLAGS_X #cmakedefine01 HAVE_HEIMDAL_HEADERS -#cmakedefine01 HAVE_NSGETENVIRON #cmakedefine01 HAVE_GETAUXVAL #cmakedefine01 HAVE_CRT_EXTERNS_H #cmakedefine01 HAVE_GETDOMAINNAME diff --git a/src/libraries/Native/Unix/System.Native/CMakeLists.txt b/src/libraries/Native/Unix/System.Native/CMakeLists.txt index 19266ebed5b75..8f468f19306df 100644 --- a/src/libraries/Native/Unix/System.Native/CMakeLists.txt +++ b/src/libraries/Native/Unix/System.Native/CMakeLists.txt @@ -11,7 +11,6 @@ endif () include_directories("${CLR_SRC_NATIVE_DIR}/common") set(NATIVE_SOURCES - pal_environment.c pal_errno.c pal_interfaceaddresses.c pal_io.c @@ -38,10 +37,9 @@ if (CLR_CMAKE_TARGET_OSX OR CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS list (APPEND NATIVE_SOURCES pal_autoreleasepool.m) set_source_files_properties(pal_autoreleasepool.m PROPERTIES COMPILE_FLAGS -fno-objc-arc) list (APPEND NATIVE_SOURCES pal_environment.m) - add_definitions(-DUSE_APPLE_GETENVIRON=1) else() list (APPEND NATIVE_SOURCES pal_autoreleasepool.c) - add_definitions(-DUSE_APPLE_GETENVIRON=0) + list (APPEND NATIVE_SOURCES pal_environment.c) endif() if (CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS) diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.c b/src/libraries/Native/Unix/System.Native/pal_environment.c index b436e16b9c58b..f44f18f846140 100644 --- a/src/libraries/Native/Unix/System.Native/pal_environment.c +++ b/src/libraries/Native/Unix/System.Native/pal_environment.c @@ -6,9 +6,6 @@ #include #include -#if HAVE_NSGETENVIRON -#include -#endif char* SystemNative_GetEnv(const char* variable) { @@ -17,13 +14,6 @@ char* SystemNative_GetEnv(const char* variable) char** SystemNative_GetEnviron() { -#if USE_APPLE_GETENVIRON - extern char** GetEnvironApple(); - return GetEnvironApple(); -#elif HAVE_NSGETENVIRON - return *(_NSGetEnviron()); -#else extern char **environ; return environ; -#endif } diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.m b/src/libraries/Native/Unix/System.Native/pal_environment.m index 1d035c3398db4..d3f00192c6d6c 100644 --- a/src/libraries/Native/Unix/System.Native/pal_environment.m +++ b/src/libraries/Native/Unix/System.Native/pal_environment.m @@ -9,6 +9,11 @@ #include #include +char* SystemNative_GetEnv(const char* variable) +{ + return getenv(variable); +} + static void get_environ_helper(const void *key, const void *value, void *context) { char ***temp_environ_ptr = (char***)context; @@ -30,7 +35,7 @@ static void get_environ_helper(const void *key, const void *value, void *context (*temp_environ_ptr)++; } -char** GetEnvironApple() +char** SystemNative_GetEnviron() { static char **environ; diff --git a/src/libraries/Native/Unix/configure.cmake b/src/libraries/Native/Unix/configure.cmake index 18afacc2dbc59..e9d00dafc4330 100644 --- a/src/libraries/Native/Unix/configure.cmake +++ b/src/libraries/Native/Unix/configure.cmake @@ -1047,15 +1047,6 @@ endif () check_symbol_exists(getauxval sys/auxv.h HAVE_GETAUXVAL) check_include_files(crt_externs.h HAVE_CRT_EXTERNS_H) -if (HAVE_CRT_EXTERNS_H) - check_c_source_compiles( - " - #include - int main(void) { char** e = *(_NSGetEnviron()); return 0; } - " - HAVE_NSGETENVIRON) -endif() - set (CMAKE_REQUIRED_LIBRARIES) check_c_source_compiles( From 0e520abe32c4f96029eaf24916ccb5896f5d8d52 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 26 Aug 2021 18:29:24 +0200 Subject: [PATCH 05/11] Bring back _NSGetEnviron --- src/libraries/Native/Unix/Common/pal_config.h.in | 1 + src/libraries/Native/Unix/System.Native/CMakeLists.txt | 6 +++++- .../Native/Unix/System.Native/pal_environment.c | 7 +++++++ src/libraries/Native/Unix/configure.cmake | 9 +++++++++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/libraries/Native/Unix/Common/pal_config.h.in b/src/libraries/Native/Unix/Common/pal_config.h.in index cfc7a7770a68f..9a02610534101 100644 --- a/src/libraries/Native/Unix/Common/pal_config.h.in +++ b/src/libraries/Native/Unix/Common/pal_config.h.in @@ -102,6 +102,7 @@ #cmakedefine01 HAVE_GSS_SPNEGO_MECHANISM #cmakedefine01 HAVE_GSS_KRB5_CRED_NO_CI_FLAGS_X #cmakedefine01 HAVE_HEIMDAL_HEADERS +#cmakedefine01 HAVE_NSGETENVIRON #cmakedefine01 HAVE_GETAUXVAL #cmakedefine01 HAVE_CRT_EXTERNS_H #cmakedefine01 HAVE_GETDOMAINNAME diff --git a/src/libraries/Native/Unix/System.Native/CMakeLists.txt b/src/libraries/Native/Unix/System.Native/CMakeLists.txt index 8f468f19306df..188ca5fc6bde8 100644 --- a/src/libraries/Native/Unix/System.Native/CMakeLists.txt +++ b/src/libraries/Native/Unix/System.Native/CMakeLists.txt @@ -36,9 +36,13 @@ set(NATIVE_SOURCES if (CLR_CMAKE_TARGET_OSX OR CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS) list (APPEND NATIVE_SOURCES pal_autoreleasepool.m) set_source_files_properties(pal_autoreleasepool.m PROPERTIES COMPILE_FLAGS -fno-objc-arc) - list (APPEND NATIVE_SOURCES pal_environment.m) else() list (APPEND NATIVE_SOURCES pal_autoreleasepool.c) +endif() + +if (CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS) + list (APPEND NATIVE_SOURCES pal_environment.m) +else() list (APPEND NATIVE_SOURCES pal_environment.c) endif() diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.c b/src/libraries/Native/Unix/System.Native/pal_environment.c index f44f18f846140..1ba12109454bc 100644 --- a/src/libraries/Native/Unix/System.Native/pal_environment.c +++ b/src/libraries/Native/Unix/System.Native/pal_environment.c @@ -6,6 +6,9 @@ #include #include +#if HAVE_NSGETENVIRON +#include +#endif char* SystemNative_GetEnv(const char* variable) { @@ -14,6 +17,10 @@ char* SystemNative_GetEnv(const char* variable) char** SystemNative_GetEnviron() { +#if HAVE_NSGETENVIRON + return *(_NSGetEnviron()); +#else extern char **environ; return environ; +#endif } diff --git a/src/libraries/Native/Unix/configure.cmake b/src/libraries/Native/Unix/configure.cmake index e9d00dafc4330..18afacc2dbc59 100644 --- a/src/libraries/Native/Unix/configure.cmake +++ b/src/libraries/Native/Unix/configure.cmake @@ -1047,6 +1047,15 @@ endif () check_symbol_exists(getauxval sys/auxv.h HAVE_GETAUXVAL) check_include_files(crt_externs.h HAVE_CRT_EXTERNS_H) +if (HAVE_CRT_EXTERNS_H) + check_c_source_compiles( + " + #include + int main(void) { char** e = *(_NSGetEnviron()); return 0; } + " + HAVE_NSGETENVIRON) +endif() + set (CMAKE_REQUIRED_LIBRARIES) check_c_source_compiles( From d9f527ba1959fb5be53ef1b18af73a07898460cf Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 26 Aug 2021 19:07:56 +0200 Subject: [PATCH 06/11] Remove the memory leak --- .../Unix/System.Native/Interop.GetEnviron.cs | 3 ++ .../Native/Unix/System.Native/entrypoints.c | 1 + .../Unix/System.Native/pal_environment.c | 5 ++ .../Unix/System.Native/pal_environment.h | 4 +- .../Unix/System.Native/pal_environment.m | 52 ++++++++++++------- .../src/System/Environment.Unix.Mono.cs | 8 ++- 6 files changed, 51 insertions(+), 22 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnviron.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnviron.cs index 2afa918d23b37..abe8ff0e1916e 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnviron.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnviron.cs @@ -10,5 +10,8 @@ internal unsafe partial class Sys { [DllImport(Interop.Libraries.SystemNative, EntryPoint = "SystemNative_GetEnviron")] internal static extern unsafe IntPtr GetEnviron(); + + [DllImport(Interop.Libraries.SystemNative, EntryPoint = "SystemNative_FreeEnviron")] + internal static extern unsafe void FreeEnviron(IntPtr environ); } } \ No newline at end of file diff --git a/src/libraries/Native/Unix/System.Native/entrypoints.c b/src/libraries/Native/Unix/System.Native/entrypoints.c index 4a6d4302f12ee..721be5eee63de 100644 --- a/src/libraries/Native/Unix/System.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Native/entrypoints.c @@ -261,6 +261,7 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_GetGroups) DllImportEntry(SystemNative_GetEnv) DllImportEntry(SystemNative_GetEnviron) + DllImportEntry(SystemNative_FreeEnviron) }; EXTERN_C const void* SystemResolveDllImport(const char* name); diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.c b/src/libraries/Native/Unix/System.Native/pal_environment.c index 1ba12109454bc..502c7a66c392a 100644 --- a/src/libraries/Native/Unix/System.Native/pal_environment.c +++ b/src/libraries/Native/Unix/System.Native/pal_environment.c @@ -24,3 +24,8 @@ char** SystemNative_GetEnviron() return environ; #endif } + +void SystemNative_FreeEnviron(char** environ) +{ + // no op +} diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.h b/src/libraries/Native/Unix/System.Native/pal_environment.h index 8fbd978be6ed1..bf123ee3e64fe 100644 --- a/src/libraries/Native/Unix/System.Native/pal_environment.h +++ b/src/libraries/Native/Unix/System.Native/pal_environment.h @@ -8,4 +8,6 @@ PALEXPORT char* SystemNative_GetEnv(const char* variable); -PALEXPORT char** SystemNative_GetEnviron(void); +PALEXPORT char** SystemNative_GetEnviron(int32_t *releaseMemory); + +PALEXPORT void SystemNative_FreeEnviron(char** environ); diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.m b/src/libraries/Native/Unix/System.Native/pal_environment.m index d3f00192c6d6c..ea4edc6ccd319 100644 --- a/src/libraries/Native/Unix/System.Native/pal_environment.m +++ b/src/libraries/Native/Unix/System.Native/pal_environment.m @@ -14,6 +14,8 @@ return getenv(variable); } +static char *empty_key_value_pair = "="; + static void get_environ_helper(const void *key, const void *value, void *context) { char ***temp_environ_ptr = (char***)context; @@ -30,6 +32,12 @@ static void get_environ_helper(const void *key, const void *value, void *context key_value_pair[utf8_key_length] = '='; strcpy(key_value_pair + utf8_key_length + 1, utf8_value); } + else + { + // In case of failed allocation add pointer to preallocated entry. This is + // ignored on the managed side and skipped over in SystemNative_FreeEnviron. + key_value_pair = empty_key_value_pair; + } **temp_environ_ptr = key_value_pair; (*temp_environ_ptr)++; @@ -37,28 +45,34 @@ static void get_environ_helper(const void *key, const void *value, void *context char** SystemNative_GetEnviron() { - static char **environ; + char **temp_environ; + char **temp_environ_ptr; - // NOTE: This function is not thread-safe and it leaks one array per process. This is - // intentional behavior and the managed code is expected to take additional guards - // around this call. - if (environ == NULL) + CFDictionaryRef environment = (CFDictionaryRef)[[NSProcessInfo processInfo] environment]; + int count = CFDictionaryGetCount(environment); + temp_environ = (char **)malloc((count + 1) * sizeof(char *)); + if (temp_environ != NULL) { - int environ_size = 1; - char **temp_environ; - char **temp_environ_ptr; + temp_environ_ptr = temp_environ; + CFDictionaryApplyFunction(environment, get_environ_helper, &temp_environ_ptr); + *temp_environ_ptr = NULL; + } + + return temp_environ; +} - CFDictionaryRef environment = (CFDictionaryRef)[[NSProcessInfo processInfo] environment]; - int count = CFDictionaryGetCount(environment); - temp_environ = (char **)malloc((count + 1) * sizeof(char *)); - if (temp_environ != NULL) +void SystemNative_FreeEnviron(char** environ) +{ + if (environ != NULL) + { + for (char** environ_ptr = environ; *environ_ptr != NULL; environ_ptr++) { - temp_environ_ptr = temp_environ; - CFDictionaryApplyFunction(environment, get_environ_helper, &temp_environ_ptr); - *temp_environ_ptr = NULL; - environ = temp_environ; + if (*environ_ptr != empty_key_value_pair) + { + free(*environ_ptr); + } } - } - return environ; -} \ No newline at end of file + free(environ); + } +} diff --git a/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs index 148b170df0449..a2a0252179e2b 100644 --- a/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs @@ -92,16 +92,20 @@ private static Dictionary GetSystemEnvironmentVariables() IntPtr block = Interop.Sys.GetEnviron(); if (block != IntPtr.Zero) { + IntPtr blockIterator = block; + // Per man page, environment variables come back as an array of pointers to strings // Parse each pointer of strings individually - while (ParseEntry(block, out string? key, out string? value)) + while (ParseEntry(blockIterator, out string? key, out string? value)) { if (key != null && value != null) results.Add(key, value); // Increment to next environment variable entry - block += IntPtr.Size; + blockIterator += IntPtr.Size; } + + Interop.Sys.FreeEnviron(block); } return results; From d55d4ad3d6c9afad5d787e87e81807f042454f6d Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 26 Aug 2021 19:17:41 +0200 Subject: [PATCH 07/11] Fix error with unused variable --- src/libraries/Native/Unix/System.Native/pal_environment.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.c b/src/libraries/Native/Unix/System.Native/pal_environment.c index 502c7a66c392a..fd9e68f023028 100644 --- a/src/libraries/Native/Unix/System.Native/pal_environment.c +++ b/src/libraries/Native/Unix/System.Native/pal_environment.c @@ -28,4 +28,5 @@ char** SystemNative_GetEnviron() void SystemNative_FreeEnviron(char** environ) { // no op + (void)environ; } From 30b72b4d0e307f874971db487fcaa204d9038908 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 26 Aug 2021 20:03:40 +0200 Subject: [PATCH 08/11] Add try-finally block --- .../src/System/Environment.Unix.Mono.cs | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs index a2a0252179e2b..893f9afb2a8c8 100644 --- a/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs @@ -92,20 +92,25 @@ private static Dictionary GetSystemEnvironmentVariables() IntPtr block = Interop.Sys.GetEnviron(); if (block != IntPtr.Zero) { - IntPtr blockIterator = block; - - // Per man page, environment variables come back as an array of pointers to strings - // Parse each pointer of strings individually - while (ParseEntry(blockIterator, out string? key, out string? value)) + try { - if (key != null && value != null) - results.Add(key, value); - - // Increment to next environment variable entry - blockIterator += IntPtr.Size; + IntPtr blockIterator = block; + + // Per man page, environment variables come back as an array of pointers to strings + // Parse each pointer of strings individually + while (ParseEntry(blockIterator, out string? key, out string? value)) + { + if (key != null && value != null) + results.Add(key, value); + + // Increment to next environment variable entry + blockIterator += IntPtr.Size; + } + } + finally + { + Interop.Sys.FreeEnviron(block); } - - Interop.Sys.FreeEnviron(block); } return results; From 8759796e1365fe38acdaebd16020b778973907b1 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 26 Aug 2021 20:08:30 +0200 Subject: [PATCH 09/11] Match Windows behavior for duplicate keys --- .../src/System/Environment.Unix.Mono.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs index 893f9afb2a8c8..5858d78f4c972 100644 --- a/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs @@ -101,7 +101,16 @@ private static Dictionary GetSystemEnvironmentVariables() while (ParseEntry(blockIterator, out string? key, out string? value)) { if (key != null && value != null) - results.Add(key, value); + { + try + { + // Add may throw if the environment block was corrupted leading to duplicate entries. + // We allow such throws and eat them (rather than proactively checking for duplication) + // to provide a non-fatal notification about the corruption. + results.Add(key, value); + } + catch (ArgumentException) { } + } // Increment to next environment variable entry blockIterator += IntPtr.Size; From 64744d68d1519b016a1309b2e1b871d6fdd32f76 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Thu, 26 Aug 2021 21:01:27 +0200 Subject: [PATCH 10/11] Update src/libraries/Native/Unix/System.Native/pal_environment.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alexander Köplinger --- src/libraries/Native/Unix/System.Native/pal_environment.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.h b/src/libraries/Native/Unix/System.Native/pal_environment.h index bf123ee3e64fe..eb62af0ce1cc0 100644 --- a/src/libraries/Native/Unix/System.Native/pal_environment.h +++ b/src/libraries/Native/Unix/System.Native/pal_environment.h @@ -8,6 +8,6 @@ PALEXPORT char* SystemNative_GetEnv(const char* variable); -PALEXPORT char** SystemNative_GetEnviron(int32_t *releaseMemory); +PALEXPORT char** SystemNative_GetEnviron(); PALEXPORT void SystemNative_FreeEnviron(char** environ); From 316b2f117694292e4683fb9f6d8516b38a9b1760 Mon Sep 17 00:00:00 2001 From: Filip Navara Date: Fri, 27 Aug 2021 04:18:21 +0200 Subject: [PATCH 11/11] Update src/libraries/Native/Unix/System.Native/pal_environment.h Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com> --- src/libraries/Native/Unix/System.Native/pal_environment.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Native/Unix/System.Native/pal_environment.h b/src/libraries/Native/Unix/System.Native/pal_environment.h index eb62af0ce1cc0..dee7a10f3aecd 100644 --- a/src/libraries/Native/Unix/System.Native/pal_environment.h +++ b/src/libraries/Native/Unix/System.Native/pal_environment.h @@ -8,6 +8,6 @@ PALEXPORT char* SystemNative_GetEnv(const char* variable); -PALEXPORT char** SystemNative_GetEnviron(); +PALEXPORT char** SystemNative_GetEnviron(void); PALEXPORT void SystemNative_FreeEnviron(char** environ);