From 39d2f90381a67222fd79d3890c0b33a3a28f6ee6 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 11 Jun 2022 15:19:24 +0800 Subject: [PATCH 01/10] P/Invoke definition --- .../Windows/Kernel32/Interop.GetCommandLine.cs | 13 +++++++++++++ .../Windows/Shell32/Interop.CommandLineToArgv.cs | 13 +++++++++++++ .../src/System.Private.CoreLib.Shared.projitems | 6 ++++++ 3 files changed, 32 insertions(+) create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetCommandLine.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Shell32/Interop.CommandLineToArgv.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetCommandLine.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetCommandLine.cs new file mode 100644 index 0000000000000..748967ff53344 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetCommandLine.cs @@ -0,0 +1,13 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +internal static unsafe partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, EntryPoint = "GetCommandLineW")] + internal static partial char* GetCommandLine(); + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Shell32/Interop.CommandLineToArgv.cs b/src/libraries/Common/src/Interop/Windows/Shell32/Interop.CommandLineToArgv.cs new file mode 100644 index 0000000000000..ecc73f56005e0 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Shell32/Interop.CommandLineToArgv.cs @@ -0,0 +1,13 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; + +internal static unsafe partial class Interop +{ + internal static partial class Shell32 + { + [LibraryImport(Libraries.Shell32, EntryPoint = "CommandLineToArgvW")] + internal static partial char** CommandLineToArgv(char* lpCommandLine, int* pNumArgs); + } +} diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index d3d733b73f569..8bdda94bfb811 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1558,6 +1558,9 @@ Common\Interop\Windows\Kernel32\Interop.GET_FILEEX_INFO_LEVELS.cs + + Common\Interop\Windows\Kernel32\Interop.GetCommandLine.cs + Common\Interop\Windows\Kernel32\Interop.GetComputerName.cs @@ -1828,6 +1831,9 @@ Common\Interop\Windows\Secur32\Interop.GetUserNameExW.cs + + Common\Interop\Windows\Shell32\Interop.CommandLineToArgv.cs + Common\Interop\Windows\Shell32\Interop.SHGetKnownFolderPath.cs From bc6540ac02fa11c5f533a43afeaa7f1dbad6906f Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 11 Jun 2022 15:32:03 +0800 Subject: [PATCH 02/10] Use P/Invoke in managed code --- .../src/System/Environment.CoreCLR.cs | 3 --- .../src/System/Environment.Unix.cs | 8 ++++++ .../src/System/Environment.Windows.cs | 25 +++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Environment.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Environment.CoreCLR.cs index ce4ab04249aaa..150348ec16ab5 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Environment.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Environment.CoreCLR.cs @@ -61,9 +61,6 @@ public static extern int ExitCode [MethodImpl(MethodImplOptions.InternalCall)] public static extern void FailFast(string? message, Exception? exception, string? errorMessage); - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern string[] GetCommandLineArgsNative(); - public static string[] GetCommandLineArgs() { // There are multiple entry points to a hosted app. The host could diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Unix.cs index e1ecca767122d..046a8ab06dbed 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Unix.cs @@ -31,5 +31,13 @@ public static string MachineName [MethodImplAttribute(MethodImplOptions.NoInlining)] // Avoid inlining PInvoke frame into the hot path private static string? GetProcessPath() => Interop.Sys.GetProcessPath(); + + private static string[] GetCommandLineArgsNative() + { + // This is only used for delegate created from native host + + // Consider to use /proc/self/cmdline to get command line + return Array.Empty(); + } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs index 7ac8ecd5bc1d2..3020ce9cedf61 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs @@ -180,5 +180,30 @@ public static unsafe long WorkingSet return (long)memoryCounters.WorkingSetSize; } } + + private static unsafe string[] GetCommandLineArgsNative() + { + char* lpCmdLine = Interop.Kernel32.GetCommandLine(); + if (lpCmdLine == null) + { + ThrowHelper.ThrowOutOfMemoryException(); + } + + int numArgs = 0; + char** argvW = Interop.Shell32.CommandLineToArgv(lpCmdLine, &numArgs); + if (argvW == null) + { + ThrowHelper.ThrowOutOfMemoryException(); + } + + string[] result = new string[numArgs]; + for (int i = 0; i < result.Length; i++) + { + result[i] = new string(*(argvW + i)); + } + + Interop.Kernel32.LocalFree((nint)argvW); + return result; + } } } From 9711a33ed6eeb58b2679f2e32284b16a8dc63df7 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 11 Jun 2022 15:33:19 +0800 Subject: [PATCH 03/10] Delete FCall definition --- src/coreclr/classlibnative/bcltype/system.cpp | 37 ------------------- src/coreclr/classlibnative/bcltype/system.h | 1 - src/coreclr/vm/ecalllist.h | 1 - 3 files changed, 39 deletions(-) diff --git a/src/coreclr/classlibnative/bcltype/system.cpp b/src/coreclr/classlibnative/bcltype/system.cpp index 2d517279fad6f..2f1387ddd2fcf 100644 --- a/src/coreclr/classlibnative/bcltype/system.cpp +++ b/src/coreclr/classlibnative/bcltype/system.cpp @@ -90,43 +90,6 @@ FCIMPL0(INT32, SystemNative::GetExitCode) } FCIMPLEND -FCIMPL0(Object*, SystemNative::GetCommandLineArgs) -{ - FCALL_CONTRACT; - - PTRARRAYREF strArray = NULL; - - HELPER_METHOD_FRAME_BEGIN_RET_1(strArray); - - LPWSTR commandLine; - - commandLine = WszGetCommandLine(); - if (commandLine==NULL) - COMPlusThrowOM(); - - DWORD numArgs = 0; - LPWSTR* argv = SegmentCommandLine(commandLine, &numArgs); - if (!argv) - COMPlusThrowOM(); - - _ASSERTE(numArgs > 0); - - strArray = (PTRARRAYREF) AllocateObjectArray(numArgs, g_pStringClass); - // Copy each argument into new Strings. - for(unsigned int i=0; iGetDataPtr())) + i; - SetObjectReference((OBJECTREF*)destData, (OBJECTREF)str); - } - delete [] argv; - - HELPER_METHOD_FRAME_END(); - - return OBJECTREFToObject(strArray); -} -FCIMPLEND - // Return a method info for the method were the exception was thrown FCIMPL1(ReflectMethodObject*, SystemNative::GetMethodFromStackTrace, ArrayBase* pStackTraceUNSAFE) { diff --git a/src/coreclr/classlibnative/bcltype/system.h b/src/coreclr/classlibnative/bcltype/system.h index 27e772be2af07..b4a773a847c39 100644 --- a/src/coreclr/classlibnative/bcltype/system.h +++ b/src/coreclr/classlibnative/bcltype/system.h @@ -43,7 +43,6 @@ class SystemNative static FCDECL1(VOID,SetExitCode,INT32 exitcode); static FCDECL0(INT32, GetExitCode); - static FCDECL0(Object*, GetCommandLineArgs); static FCDECL1(VOID, FailFast, StringObject* refMessageUNSAFE); static FCDECL2(VOID, FailFastWithExitCode, StringObject* refMessageUNSAFE, UINT exitCode); static FCDECL2(VOID, FailFastWithException, StringObject* refMessageUNSAFE, ExceptionObject* refExceptionUNSAFE); diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 0bfdff8c6a10e..e050bcae62228 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -103,7 +103,6 @@ FCFuncStart(gEnvironmentFuncs) FCFuncElement("get_TickCount64", SystemNative::GetTickCount64) FCFuncElement("set_ExitCode", SystemNative::SetExitCode) FCFuncElement("get_ExitCode", SystemNative::GetExitCode) - FCFuncElement("GetCommandLineArgsNative", SystemNative::GetCommandLineArgs) FCFuncElementSig("FailFast", &gsig_SM_Str_RetVoid, SystemNative::FailFast) FCFuncElementSig("FailFast", &gsig_SM_Str_Exception_RetVoid, SystemNative::FailFastWithException) From e36103c1e4fcb93afbdb786249b0a69c852501b9 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 12 Jun 2022 03:05:52 +0800 Subject: [PATCH 04/10] Update managed call site --- .../src/System/Environment.Windows.cs | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs index 3020ce9cedf61..ef49a597a5796 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; using System.IO; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -184,10 +185,7 @@ public static unsafe long WorkingSet private static unsafe string[] GetCommandLineArgsNative() { char* lpCmdLine = Interop.Kernel32.GetCommandLine(); - if (lpCmdLine == null) - { - ThrowHelper.ThrowOutOfMemoryException(); - } + Debug.Assert(lpCmdLine != null); int numArgs = 0; char** argvW = Interop.Shell32.CommandLineToArgv(lpCmdLine, &numArgs); @@ -196,14 +194,19 @@ private static unsafe string[] GetCommandLineArgsNative() ThrowHelper.ThrowOutOfMemoryException(); } - string[] result = new string[numArgs]; - for (int i = 0; i < result.Length; i++) + try + { + string[] result = new string[numArgs]; + for (int i = 0; i < result.Length; i++) + { + result[i] = new string(*(argvW + i)); + } + return result; + } + finally { - result[i] = new string(*(argvW + i)); + Interop.Kernel32.LocalFree((nint)argvW); } - - Interop.Kernel32.LocalFree((nint)argvW); - return result; } } } From aaa5defdf0655b41e4b61472c58b7c2fe5f4c718 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 15 Jun 2022 17:01:37 +0800 Subject: [PATCH 05/10] Add test using private reflection --- .../Interop/CommandLine/CommandLineTest.cs | 34 +++++++++++++++++++ .../CommandLine/CommandLineTest.csproj | 12 +++++++ 2 files changed, 46 insertions(+) create mode 100644 src/tests/Interop/CommandLine/CommandLineTest.cs create mode 100644 src/tests/Interop/CommandLine/CommandLineTest.csproj diff --git a/src/tests/Interop/CommandLine/CommandLineTest.cs b/src/tests/Interop/CommandLine/CommandLineTest.cs new file mode 100644 index 0000000000000..4c78b6e1c8787 --- /dev/null +++ b/src/tests/Interop/CommandLine/CommandLineTest.cs @@ -0,0 +1,34 @@ +using System; +using System.IO; +using System.Reflection; +using TestLibrary; +using Xunit; + +namespace CommandLineTest +{ + class Program + { + int Main() + { + // Currently native command line is only implemented on CoreCLR Windows + if (!TestLibrary.Utilities.IsMonoRuntime && !TestLibrary.Utilities.IsNativeAot) + { + // Clear the command line args set for managed entry point + var field = typeof(Environment).GetField("s_commandLineArgs", BindingFlags.Static | BindingFlags.NonPublic); + Assert.NotNull(field); + field.SetValue(null, null); + + string[] args = Environment.GetCommandLineArgs(); + if (OperatingSystem.IsWindows()) + { + // The command line should be "corerun assemblyname.dll" for coreclr test + Assert.Equal(2, args.Length); + Assert.Equal("corerun", Path.GetFileNameWithoutExtension(args[0])); + Assert.Equal(typeof(Program).Assembly.GetName().Name, Path.GetFileNameWithoutExtension(args[1])); + } + } + + return 100; + } + } +} \ No newline at end of file diff --git a/src/tests/Interop/CommandLine/CommandLineTest.csproj b/src/tests/Interop/CommandLine/CommandLineTest.csproj new file mode 100644 index 0000000000000..df25278d2bd86 --- /dev/null +++ b/src/tests/Interop/CommandLine/CommandLineTest.csproj @@ -0,0 +1,12 @@ + + + Exe + true + + + + + + + + From 03d582a0d421c148607191b2b677c7bd8d1a1772 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Wed, 15 Jun 2022 09:46:12 -0700 Subject: [PATCH 06/10] Apply suggestions from code review --- .../System.Private.CoreLib/src/System/Environment.Windows.cs | 2 +- src/tests/Interop/CommandLine/CommandLineTest.cs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs index ef49a597a5796..e95f41a312bea 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs @@ -205,7 +205,7 @@ private static unsafe string[] GetCommandLineArgsNative() } finally { - Interop.Kernel32.LocalFree((nint)argvW); + Interop.Kernel32.LocalFree((IntPtr)argvW); } } } diff --git a/src/tests/Interop/CommandLine/CommandLineTest.cs b/src/tests/Interop/CommandLine/CommandLineTest.cs index 4c78b6e1c8787..b1f5f82a1adda 100644 --- a/src/tests/Interop/CommandLine/CommandLineTest.cs +++ b/src/tests/Interop/CommandLine/CommandLineTest.cs @@ -1,3 +1,6 @@ +// 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.IO; using System.Reflection; From a3b3db52f23795f2cda6e98744baafb1c9e48e0f Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 19 Jun 2022 14:36:44 +0800 Subject: [PATCH 07/10] Revert "Add test using private reflection" This reverts commit aaa5defdf0655b41e4b61472c58b7c2fe5f4c718. --- .../Interop/CommandLine/CommandLineTest.cs | 37 ------------------- .../CommandLine/CommandLineTest.csproj | 12 ------ 2 files changed, 49 deletions(-) delete mode 100644 src/tests/Interop/CommandLine/CommandLineTest.cs delete mode 100644 src/tests/Interop/CommandLine/CommandLineTest.csproj diff --git a/src/tests/Interop/CommandLine/CommandLineTest.cs b/src/tests/Interop/CommandLine/CommandLineTest.cs deleted file mode 100644 index b1f5f82a1adda..0000000000000 --- a/src/tests/Interop/CommandLine/CommandLineTest.cs +++ /dev/null @@ -1,37 +0,0 @@ -// 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.IO; -using System.Reflection; -using TestLibrary; -using Xunit; - -namespace CommandLineTest -{ - class Program - { - int Main() - { - // Currently native command line is only implemented on CoreCLR Windows - if (!TestLibrary.Utilities.IsMonoRuntime && !TestLibrary.Utilities.IsNativeAot) - { - // Clear the command line args set for managed entry point - var field = typeof(Environment).GetField("s_commandLineArgs", BindingFlags.Static | BindingFlags.NonPublic); - Assert.NotNull(field); - field.SetValue(null, null); - - string[] args = Environment.GetCommandLineArgs(); - if (OperatingSystem.IsWindows()) - { - // The command line should be "corerun assemblyname.dll" for coreclr test - Assert.Equal(2, args.Length); - Assert.Equal("corerun", Path.GetFileNameWithoutExtension(args[0])); - Assert.Equal(typeof(Program).Assembly.GetName().Name, Path.GetFileNameWithoutExtension(args[1])); - } - } - - return 100; - } - } -} \ No newline at end of file diff --git a/src/tests/Interop/CommandLine/CommandLineTest.csproj b/src/tests/Interop/CommandLine/CommandLineTest.csproj deleted file mode 100644 index df25278d2bd86..0000000000000 --- a/src/tests/Interop/CommandLine/CommandLineTest.csproj +++ /dev/null @@ -1,12 +0,0 @@ - - - Exe - true - - - - - - - - From 01eeb9ebfa9fa0891340bcf71b1dfc7114f543f2 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Mon, 20 Jun 2022 00:44:58 +0800 Subject: [PATCH 08/10] Add managed test --- .../System/Environment.GetCommandLineArgs.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs b/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs index 695af6b8f2567..e85b7113ebe95 100644 --- a/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs +++ b/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs @@ -70,5 +70,30 @@ public static int CheckCommandLineArgs(string[] args) return RemoteExecutor.SuccessExitCode; } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void GetCommandLineArgs_Fallback_Returns() + { + if (PlatformDetection.IsNotMonoRuntime + && PlatformDetection.IsNotNativeAot + && PlatformDetection.IsWindows) + { + // Currently fallback command line is only implemented on Windows coreclr + RemoteExecutor.Invoke(CheckCommandLineArgsFallback); + } + } + + public static int CheckCommandLineArgsFallback() + { + // Clear the command line args set for managed entry point + var field = typeof(Environment).GetField("s_commandLineArgs", BindingFlags.Static | BindingFlags.NonPublic); + Assert.NotNull(field); + field.SetValue(null, null); + + string[] args = Environment.GetCommandLineArgs(); + Assert.NotEmpty(args); + + return RemoteExecutor.SuccessExitCode; + } } } From 4ff89e1bc85fd6915ac658900826c32d193fec02 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Mon, 20 Jun 2022 11:21:16 +0800 Subject: [PATCH 09/10] Dispose result of RemoteExecutor --- .../tests/System/Environment.GetCommandLineArgs.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs b/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs index e85b7113ebe95..396f04356cd17 100644 --- a/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs +++ b/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs @@ -79,7 +79,7 @@ public void GetCommandLineArgs_Fallback_Returns() && PlatformDetection.IsWindows) { // Currently fallback command line is only implemented on Windows coreclr - RemoteExecutor.Invoke(CheckCommandLineArgsFallback); + RemoteExecutor.Invoke(CheckCommandLineArgsFallback).Dispose(); } } From 9ebedfed9c454f4ad1d3f6d5522b8d26cbfaf360 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Mon, 20 Jun 2022 21:24:07 +0800 Subject: [PATCH 10/10] Native command line should be superset of managed --- .../tests/System/Environment.GetCommandLineArgs.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs b/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs index 396f04356cd17..dc07a79090975 100644 --- a/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs +++ b/src/libraries/System.Runtime.Extensions/tests/System/Environment.GetCommandLineArgs.cs @@ -85,6 +85,8 @@ public void GetCommandLineArgs_Fallback_Returns() public static int CheckCommandLineArgsFallback() { + string[] oldArgs = Environment.GetCommandLineArgs(); + // Clear the command line args set for managed entry point var field = typeof(Environment).GetField("s_commandLineArgs", BindingFlags.Static | BindingFlags.NonPublic); Assert.NotNull(field); @@ -93,6 +95,12 @@ public static int CheckCommandLineArgsFallback() string[] args = Environment.GetCommandLineArgs(); Assert.NotEmpty(args); + // The native command line should be superset of managed command line + foreach (string arg in oldArgs) + { + Assert.Contains(arg, args); + } + return RemoteExecutor.SuccessExitCode; } }