Implement Marshal.Get(Last)PInvokeErrorMessage() APIs. (#70685)

* Implement Marshal.GetPInvokeErrorMessage() and Marshal.GetLastPInvokeErrorMessage() APIs. The behavior of this API is to return the same value as new Win32Exception(error).Message, while avoiding the instantiation of an exception. Consume new API in places where possible.
dotnet · Jun 24, 2022 · 318018e · 318018e
1 parent f3c63d8
commit 318018e
Show file tree

Hide file tree

Showing 17 changed files with 144 additions and 50 deletions.
diff --git a/src/libraries/Common/src/System/IO/Win32Marshal.cs b/src/libraries/Common/src/System/IO/Win32Marshal.cs
@@ -58,10 +58,22 @@ internal static Exception GetExceptionForWin32Error(int errorCode, string? path
                     return new OperationCanceledException();
                 case Interop.Errors.ERROR_INVALID_PARAMETER:
                 default:
+                    string msg = string.IsNullOrEmpty(path)
+                        ? GetPInvokeErrorMessage(errorCode)
+                        : $"{GetPInvokeErrorMessage(errorCode)} : '{path}'";
                     return new IOException(
-                        string.IsNullOrEmpty(path) ? GetMessage(errorCode) : $"{GetMessage(errorCode)} : '{path}'",
+                        msg,
                         MakeHRFromErrorCode(errorCode));
             }
+
+            static string GetPInvokeErrorMessage(int errorCode)
+            {
+#if NET7_0_OR_GREATER
+                return Marshal.GetPInvokeErrorMessage(errorCode);
+#else
+                return Interop.Kernel32.GetMessage(errorCode);
+#endif
+            }
         }
 
         /// <summary>
@@ -90,10 +102,5 @@ internal static int TryMakeWin32ErrorCodeFromHR(int hr)
 
             return hr;
         }
-
-        /// <summary>
-        /// Returns a string message for the specified Win32 error code.
-        /// </summary>
-        internal static string GetMessage(int errorCode) => Interop.Kernel32.GetMessage(errorCode);
     }
 }
diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs
@@ -203,7 +203,7 @@ private string GetMainWindowTitle()
 #if DEBUG
                 // We never used to throw here, want to surface possible mistakes on our part
                 int error = Marshal.GetLastWin32Error();
-                Debug.Assert(error == 0, $"Failed GetWindowTextLengthW(): { new Win32Exception(error).Message }");
+                Debug.Assert(error == 0, $"Failed GetWindowTextLengthW(): { Marshal.GetPInvokeErrorMessage(error) }");
 #endif
                 return string.Empty;
             }
@@ -222,7 +222,7 @@ private string GetMainWindowTitle()
             {
                 // We never used to throw here, want to surface possible mistakes on our part
                 int error = Marshal.GetLastWin32Error();
-                Debug.Assert(error == 0, $"Failed GetWindowTextW(): { new Win32Exception(error).Message }");
+                Debug.Assert(error == 0, $"Failed GetWindowTextW(): { Marshal.GetPInvokeErrorMessage(error) }");
             }
 #endif
             return title.Slice(0, length).ToString();

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
@@ -1881,7 +1881,6 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\DateTime.Windows.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Environment.Win32.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Environment.Windows.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\ComponentModel\Win32Exception.Windows.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\DebugProvider.Windows.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\Stopwatch.Windows.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\Tracing\RuntimeEventSourceHelper.Windows.cs" Condition="'$(FeaturePerfTracing)' == 'true'" />
@@ -2176,7 +2175,6 @@
     <Compile Include="$(MSBuildThisFileDirectory)Microsoft\Win32\SafeHandles\SafeFileHandle.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\AppDomain.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Buffer.Unix.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\ComponentModel\Win32Exception.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\DateTime.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\DebugProvider.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Diagnostics\Stopwatch.Unix.cs" />

diff --git a/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Unix.cs
diff --git a/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Windows.cs
diff --git a/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.cs b/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.cs
@@ -13,7 +13,7 @@ namespace System.ComponentModel
     /// </summary>
     [Serializable]
     [System.Runtime.CompilerServices.TypeForwardedFrom("System, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")]
-    public partial class Win32Exception : ExternalException, ISerializable
+    public class Win32Exception : ExternalException, ISerializable
     {
         private const int E_FAIL = unchecked((int)0x80004005);
 
@@ -28,7 +28,7 @@ public Win32Exception() : this(Marshal.GetLastPInvokeError())
         /// <summary>
         /// Initializes a new instance of the <see cref='System.ComponentModel.Win32Exception'/> class with the specified error.
         /// </summary>
-        public Win32Exception(int error) : this(error, GetErrorMessage(error))
+        public Win32Exception(int error) : this(error, Marshal.GetPInvokeErrorMessage(error))
         {
         }
         /// <summary>

diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs
@@ -48,10 +48,10 @@ private static void SetEnvironmentVariableCore(string variable, string? value)
 
                     case Interop.Errors.ERROR_NOT_ENOUGH_MEMORY:
                     case Interop.Errors.ERROR_NO_SYSTEM_RESOURCES:
-                        throw new OutOfMemoryException(Interop.Kernel32.GetMessage(errorCode));
+                        throw new OutOfMemoryException(Marshal.GetPInvokeErrorMessage(errorCode));
 
                     default:
-                        throw new ArgumentException(Interop.Kernel32.GetMessage(errorCode));
+                        throw new ArgumentException(Marshal.GetPInvokeErrorMessage(errorCode));
                 }
             }
         }

diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs
@@ -191,7 +191,7 @@ public static string UserDomainName
                     // The docs don't call this out clearly, but experimenting shows that the error returned is the following.
                     if (error != Interop.Errors.ERROR_INSUFFICIENT_BUFFER)
                     {
-                        throw new InvalidOperationException(Win32Marshal.GetMessage(error));
+                        throw new InvalidOperationException(Marshal.GetPInvokeErrorMessage(error));
                     }
 
                     domainBuilder.EnsureCapacity((int)length);

diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Unix.cs
@@ -195,5 +195,15 @@ public static void SetLastSystemError(int error)
         {
             Interop.Sys.SetErrNo(error);
         }
+
+        /// <summary>
+        /// Gets the system error message for the supplied error code.
+        /// </summary>
+        /// <param name="error">The error code.</param>
+        /// <returns>The error message associated with <paramref name="error"/>.</returns>
+        public static string GetPInvokeErrorMessage(int error)
+        {
+            return Interop.Sys.StrError(error);
+        }
     }
 }
diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.Windows.cs
@@ -256,5 +256,15 @@ public static void SetLastSystemError(int error)
         {
             Interop.Kernel32.SetLastError(error);
         }
+
+        /// <summary>
+        /// Gets the system error message for the supplied error code.
+        /// </summary>
+        /// <param name="error">The error code.</param>
+        /// <returns>The error message associated with <paramref name="error"/>.</returns>
+        public static string GetPInvokeErrorMessage(int error)
+        {
+            return Interop.Kernel32.GetMessage(error);
+        }
     }
 }
diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/Marshal.cs
@@ -1296,5 +1296,14 @@ public static int GetLastWin32Error()
         {
             return GetLastPInvokeError();
         }
+
+        /// <summary>
+        /// Gets the system error message for the last PInvoke error code.
+        /// </summary>
+        /// <returns>The error message associated with the last PInvoke error code.</returns>
+        public static string GetLastPInvokeErrorMessage()
+        {
+            return GetPInvokeErrorMessage(GetLastPInvokeError());
+        }
     }
 }
diff --git a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs
@@ -586,6 +586,8 @@ public static void FreeHGlobal(System.IntPtr hglobal) { }
         public static int GetLastPInvokeError() { throw null; }
         public static int GetLastSystemError() { throw null; }
         public static int GetLastWin32Error() { throw null; }
+        public static string GetLastPInvokeErrorMessage() { throw null; }
+        public static string GetPInvokeErrorMessage(int error) { throw null; }
         [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")]
         [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)]
         public static void GetNativeVariantForObject(object? obj, System.IntPtr pDstNativeVariant) { }

diff --git a/...ests/System.Runtime.InteropServices.UnitTests/System.Runtime.InteropServices.Tests.csproj b/...ests/System.Runtime.InteropServices.UnitTests/System.Runtime.InteropServices.Tests.csproj
@@ -99,6 +99,7 @@
     <Compile Include="System\Runtime\InteropServices\Marshal\IsComObjectTests.cs" />
     <Compile Include="System\Runtime\InteropServices\Marshal\IsComObjectTests.Windows.cs" Condition="'$(TargetPlatformIdentifier)' == 'windows'" />
     <Compile Include="System\Runtime\InteropServices\Marshal\LastErrorTests.cs" />
+    <Compile Include="System\Runtime\InteropServices\Marshal\PInvokeErrorMessageTests.cs" />
     <Compile Include="System\Runtime\InteropServices\Marshal\ReleaseTests.cs" />
     <Compile Include="System\Runtime\InteropServices\Marshal\ReleaseComObjectTests.cs" />
     <Compile Include="System\Runtime\InteropServices\Marshal\ReleaseComObjectTests.Windows.cs" Condition="'$(TargetPlatformIdentifier)' == 'windows'" />

diff --git a/...eropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs b/...eropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs
@@ -0,0 +1,76 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+using System.ComponentModel;
+using Xunit;
+
+namespace System.Runtime.InteropServices.Tests
+{
+    public class PInvokeErrorMessageTests
+    {
+        public static IEnumerable<object[]> GetErrorCode_TestData()
+        {
+            yield return new object[] { 0 };
+            yield return new object[] { 1 };
+
+            // errno values
+            yield return new object[] { 0x10002 };
+            yield return new object[] { 0x10003 };
+            yield return new object[] { 0x10014 };
+            yield return new object[] { 0x1001D };
+
+            // HRESULT values
+            yield return new object[] { unchecked((int)0x80004001) };
+            yield return new object[] { unchecked((int)0x80004005) };
+            yield return new object[] { unchecked((int)0x80070057) };
+            yield return new object[] { unchecked((int)0x8000FFFF) };
+        }
+
+        [Theory]
+        [MemberData(nameof(GetErrorCode_TestData))]
+        public void PInvokeErrorMessage_Returns_Win32Exception_Message(int error)
+        {
+            // The Win32Exception represents the canonical system exception on
+            // all platforms. The GetPInvokeErrorMessage API is about providing
+            // this message in a manner that avoid the instantiation of a Win32Exception
+            // instance and querying for the message.
+            string expected = new Win32Exception(error).Message;
+            Assert.Equal(expected, Marshal.GetPInvokeErrorMessage(error));
+        }
+
+        [Theory]
+        [MemberData(nameof(GetErrorCode_TestData))]
+        public void LastPInvokeErrorMessage_Returns_Correct_Message(int error)
+        {
+            string expected = Marshal.GetPInvokeErrorMessage(error);
+
+            Marshal.SetLastPInvokeError(error);
+            Assert.Equal(expected, Marshal.GetLastPInvokeErrorMessage());
+        }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public void PInvokeErrorMessage_Returns_UniqueMessage_Windows()
+        {
+            var err1 = Marshal.GetPInvokeErrorMessage(unchecked((int)0x80070057)); // E_INVALIDARG
+            var err2 = Marshal.GetPInvokeErrorMessage(unchecked((int)0x80004001)); // E_NOTIMPL
+            Assert.NotNull(err1);
+            Assert.NotNull(err2);
+            Assert.NotEqual(err1, err2);
+        }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.AnyUnix)]
+        public void PInvokeErrorMessage_Returns_UniqueMessage_Unix()
+        {
+            var err1 = Marshal.GetPInvokeErrorMessage(0x10002); // EACCES
+            var err2 = Marshal.GetPInvokeErrorMessage(0x10014); // EEXIST
+            Assert.NotNull(err1);
+            Assert.NotNull(err2);
+            // It is difficult to determine which error codes do or do not have
+            // translations on non-Windows. For now, we will just confirm the
+            // message is non-null.
+        }
+    }
+}
diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs
@@ -612,7 +612,7 @@ public SecurityIdentifier(WellKnownSidType sidType, SecurityIdentifier? domainSi
             if (error == Interop.Errors.ERROR_INVALID_PARAMETER)
             {
 #pragma warning disable CA2208 // Instantiate argument exceptions correctly, combination of arguments used
-                throw new ArgumentException(new Win32Exception(error).Message, "sidType/domainSid");
+                throw new ArgumentException(Marshal.GetPInvokeErrorMessage(error), "sidType/domainSid");
 #pragma warning restore CS2208
             }
             else if (error != Interop.Errors.ERROR_SUCCESS)

diff --git a/...raries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs b/...raries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs
@@ -131,7 +131,7 @@ public WindowsIdentity(string sUserPrincipalName)
                 unsafe
                 {
                     if (!Interop.Advapi32.AllocateLocallyUniqueId(&sourceContext.SourceIdentifier))
-                        throw new SecurityException(new Win32Exception().Message);
+                        throw new SecurityException(Marshal.GetLastPInvokeErrorMessage());
 
                     sourceName.AsSpan().CopyTo(new Span<byte>(sourceContext.SourceName, TOKEN_SOURCE.TOKEN_SOURCE_LENGTH));
                 }
@@ -264,7 +264,7 @@ private static SafeAccessTokenHandle DuplicateAccessToken(IntPtr accessToken)
                     true,
                     Interop.DuplicateHandleOptions.DUPLICATE_SAME_ACCESS))
             {
-                throw new SecurityException(new Win32Exception().Message);
+                throw new SecurityException(Marshal.GetLastPInvokeErrorMessage());
             }
 
             return duplicateAccessToken;
@@ -465,15 +465,15 @@ private bool CheckNtTokenForSid(SecurityIdentifier sid)
                                                       (uint)TokenImpersonationLevel.Identification,
                                                       (uint)TokenType.TokenImpersonation,
                                                       ref token))
-                        throw new SecurityException(new Win32Exception().Message);
+                        throw new SecurityException(Marshal.GetLastPInvokeErrorMessage());
                 }
 
 
                 // CheckTokenMembership will check if the SID is both present and enabled in the access token.
                 if (!Interop.Advapi32.CheckTokenMembership((til != TokenImpersonationLevel.None ? _safeTokenHandle : token),
                                                       sid.BinaryForm,
                                                       ref isMember))
-                    throw new SecurityException(new Win32Exception().Message);
+                    throw new SecurityException(Marshal.GetLastPInvokeErrorMessage());
 
 
             }
@@ -743,7 +743,7 @@ private static void RunImpersonatedInternal(SafeAccessTokenHandle token, Action
 
             SafeAccessTokenHandle previousToken = GetCurrentToken(TokenAccessLevels.MaximumAllowed, false, out bool isImpersonating, out int hr);
             if (previousToken == null || previousToken.IsInvalid)
-                throw new SecurityException(new Win32Exception(hr).Message);
+                throw new SecurityException(Marshal.GetPInvokeErrorMessage(hr));
 
             s_currentImpersonatedToken.Value = isImpersonating ? previousToken : null;
 
@@ -756,7 +756,7 @@ private static void RunImpersonatedInternal(SafeAccessTokenHandle token, Action
                 delegate
                 {
                     if (!Interop.Advapi32.RevertToSelf())
-                        Environment.FailFast(new Win32Exception().Message);
+                        Environment.FailFast(Marshal.GetLastPInvokeErrorMessage());
 
                     s_currentImpersonatedToken.Value = null;
 
@@ -776,12 +776,12 @@ private static void CurrentImpersonatedTokenChanged(AsyncLocalValueChangedArgs<S
                 return; // we handle explicit Value property changes elsewhere.
 
             if (!Interop.Advapi32.RevertToSelf())
-                Environment.FailFast(new Win32Exception().Message);
+                Environment.FailFast(Marshal.GetLastPInvokeErrorMessage());
 
             if (args.CurrentValue != null && !args.CurrentValue.IsInvalid)
             {
                 if (!Interop.Advapi32.ImpersonateLoggedOnUser(args.CurrentValue))
-                    Environment.FailFast(new Win32Exception().Message);
+                    Environment.FailFast(Marshal.GetLastPInvokeErrorMessage());
             }
         }
 
@@ -794,7 +794,7 @@ private static void CurrentImpersonatedTokenChanged(AsyncLocalValueChangedArgs<S
                 if (threadOnly && !isImpersonating)
                     return null;
                 // or there was an error
-                throw new SecurityException(new Win32Exception(hr).Message);
+                throw new SecurityException(Marshal.GetPInvokeErrorMessage(hr));
             }
             WindowsIdentity wi = new WindowsIdentity();
             wi._safeTokenHandle.Dispose();
@@ -822,7 +822,7 @@ private static Exception GetExceptionFromNtStatus(int status)
                 return new OutOfMemoryException();
 
             uint win32ErrorCode = Interop.Advapi32.LsaNtStatusToWinError((uint)status);
-            return new SecurityException(new Win32Exception(unchecked((int)win32ErrorCode)).Message);
+            return new SecurityException(Marshal.GetPInvokeErrorMessage((int)win32ErrorCode));
         }
 
         private static SafeAccessTokenHandle GetCurrentToken(TokenAccessLevels desiredAccess, bool threadOnly, out bool isImpersonating, out int hr)
@@ -899,7 +899,7 @@ private static Interop.LUID GetLogonAuthId(SafeAccessTokenHandle safeTokenHandle
                                                              dwLength,
                                                              out _);
                     if (!result)
-                        throw new SecurityException(new Win32Exception().Message);
+                        throw new SecurityException(Marshal.GetLastPInvokeErrorMessage());
                     break;
                 case Interop.Errors.ERROR_INVALID_HANDLE:
                     throw new ArgumentException(SR.Argument_InvalidImpersonationToken);
@@ -913,7 +913,7 @@ private static Interop.LUID GetLogonAuthId(SafeAccessTokenHandle safeTokenHandle
                     // Throw the exception.
                     goto default;
                 default:
-                    throw new SecurityException(new Win32Exception(dwErrorCode).Message);
+                    throw new SecurityException(Marshal.GetPInvokeErrorMessage(dwErrorCode));
             }
             return safeLocalAllocHandle;
         }