Convert fallback path of GetCommandLineArgs to managed (#70608)

* P/Invoke definition * Use P/Invoke in managed code * Update managed call site * Add test using private reflection * Native command line should be superset of managed Co-authored-by: Jan Kotas <[email protected]>
dotnet · Jun 20, 2022 · eba3636 · eba3636
1 parent 220ac84
commit eba3636
Show file tree

Hide file tree

Showing 10 changed files with 101 additions and 42 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
@@ -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/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; i<numArgs; i++)
-    {
-        STRINGREF str = StringObject::NewString(argv[i]);
-        STRINGREF * destData = ((STRINGREF*)(strArray->GetDataPtr())) + 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
@@ -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
@@ -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)

diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetCommandLine.cs 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
@@ -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
@@ -1558,6 +1558,9 @@
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GET_FILEEX_INFO_LEVELS.cs">
       <Link>Common\Interop\Windows\Kernel32\Interop.GET_FILEEX_INFO_LEVELS.cs</Link>
     </Compile>
+    <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetCommandLine.cs">
+      <Link>Common\Interop\Windows\Kernel32\Interop.GetCommandLine.cs</Link>
+    </Compile>
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetComputerName.cs">
       <Link>Common\Interop\Windows\Kernel32\Interop.GetComputerName.cs</Link>
     </Compile>
@@ -1822,6 +1825,9 @@
     <Compile Include="$(CommonPath)Interop\Windows\Secur32\Interop.GetUserNameExW.cs">
       <Link>Common\Interop\Windows\Secur32\Interop.GetUserNameExW.cs</Link>
     </Compile>
+    <Compile Include="$(CommonPath)Interop\Windows\Shell32\Interop.CommandLineToArgv.cs">
+      <Link>Common\Interop\Windows\Shell32\Interop.CommandLineToArgv.cs</Link>
+    </Compile>
     <Compile Include="$(CommonPath)Interop\Windows\Shell32\Interop.SHGetKnownFolderPath.cs">
       <Link>Common\Interop\Windows\Shell32\Interop.SHGetKnownFolderPath.cs</Link>
     </Compile>

diff --git 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<string>();
+        }
     }
 }
diff --git 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;
@@ -180,5 +181,32 @@ public static unsafe long WorkingSet
                 return (long)memoryCounters.WorkingSetSize;
             }
         }
+
+        private static unsafe string[] GetCommandLineArgsNative()
+        {
+            char* lpCmdLine = Interop.Kernel32.GetCommandLine();
+            Debug.Assert(lpCmdLine != null);
+
+            int numArgs = 0;
+            char** argvW = Interop.Shell32.CommandLineToArgv(lpCmdLine, &numArgs);
+            if (argvW == null)
+            {
+                ThrowHelper.ThrowOutOfMemoryException();
+            }
+
+            try
+            {
+                string[] result = new string[numArgs];
+                for (int i = 0; i < result.Length; i++)
+                {
+                    result[i] = new string(*(argvW + i));
+                }
+                return result;
+            }
+            finally
+            {
+                Interop.Kernel32.LocalFree((IntPtr)argvW);
+            }
+        }
     }
 }
diff --git 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,38 @@ 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).Dispose();
+            }
+        }
+
+        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);
+            field.SetValue(null, null);
+
+            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;
+        }
     }
 }