From 55959cfd08d4bf10f2ccafa26e498dca5ad184a3 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Mon, 13 Jun 2022 11:43:01 -0700 Subject: [PATCH 1/7] Implement Marshal.GetPInvokeErrorMessage() API. The behavior of this API is to return the same value as new Win32Exception(error).Message, while avoiding the instantion of an exception. --- .../ComponentModel/Win32Exception.Unix.cs | 2 +- .../ComponentModel/Win32Exception.Windows.cs | 2 +- .../System/Runtime/InteropServices/Marshal.cs | 10 +++++ .../ref/System.Runtime.InteropServices.cs | 1 + ...ystem.Runtime.InteropServices.Tests.csproj | 1 + .../Marshal/PInvokeErrorMessageTests.cs | 42 +++++++++++++++++++ 6 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.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 index 4caf3bf65e82c6..f0ca445a357643 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Unix.cs @@ -5,6 +5,6 @@ namespace System.ComponentModel { public partial class Win32Exception { - private static string GetErrorMessage(int error) => Interop.Sys.StrError(error); + internal static string GetErrorMessage(int error) => Interop.Sys.StrError(error); } } 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 index 58a47b061b2c60..d0cf217096736a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Windows.cs @@ -5,6 +5,6 @@ namespace System.ComponentModel { public partial class Win32Exception { - private static string GetErrorMessage(int error) => Interop.Kernel32.GetMessage(error); + internal static string GetErrorMessage(int error) => 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 index ed30d503147fe4..bd31ae698dc978 100644 --- 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,15 @@ public static int GetLastWin32Error() { return GetLastPInvokeError(); } + + /// + /// Gets the system error message for the supplied error code. + /// + /// The error code. + /// The error message associated with . + public static string GetPInvokeErrorMessage(int error) + { + return Win32Exception.GetErrorMessage(error); + } } } diff --git a/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs b/src/libraries/System.Runtime.InteropServices/ref/System.Runtime.InteropServices.cs index ba4ccb2687d385..a7c8a06c3e8d29 100644 --- 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,7 @@ 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 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/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System.Runtime.InteropServices.Tests.csproj b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System.Runtime.InteropServices.Tests.csproj index 5216e69b0d12ed..d39b569ec036f5 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System.Runtime.InteropServices.Tests.csproj +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System.Runtime.InteropServices.Tests.csproj @@ -99,6 +99,7 @@ + diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs new file mode 100644 index 00000000000000..da25ddd66a3540 --- /dev/null +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs @@ -0,0 +1,42 @@ +// 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 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)); + } + } +} From 1308eb224c3be05396a9901ffffc31c9ef50c8c9 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Mon, 13 Jun 2022 14:19:57 -0700 Subject: [PATCH 2/7] Invert the implementation for Win32Exception and move the Marshal implementation to handle the P/Invoke call. Consume new API in places where possible. --- .../Common/src/System/IO/Win32Marshal.cs | 10 ++++---- .../src/System/Diagnostics/Process.Win32.cs | 4 ++-- .../System.Private.CoreLib.Shared.projitems | 2 -- .../ComponentModel/Win32Exception.Unix.cs | 10 -------- .../ComponentModel/Win32Exception.Windows.cs | 10 -------- .../System/ComponentModel/Win32Exception.cs | 4 ++-- .../System/Environment.Variables.Windows.cs | 4 ++-- .../src/System/Environment.Win32.cs | 2 +- .../Runtime/InteropServices/Marshal.Unix.cs | 10 ++++++++ .../InteropServices/Marshal.Windows.cs | 10 ++++++++ .../System/Runtime/InteropServices/Marshal.cs | 10 -------- .../src/System/Security/Principal/SID.cs | 2 +- .../Security/Principal/WindowsIdentity.cs | 24 +++++++++---------- .../Security/Principal/WindowsPrincipal.cs | 5 ++-- 14 files changed, 47 insertions(+), 60 deletions(-) delete mode 100644 src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Unix.cs delete mode 100644 src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Windows.cs diff --git a/src/libraries/Common/src/System/IO/Win32Marshal.cs b/src/libraries/Common/src/System/IO/Win32Marshal.cs index 2bb328411d993c..8e81892bc6d777 100644 --- a/src/libraries/Common/src/System/IO/Win32Marshal.cs +++ b/src/libraries/Common/src/System/IO/Win32Marshal.cs @@ -58,8 +58,11 @@ internal static Exception GetExceptionForWin32Error(int errorCode, string? path return new OperationCanceledException(); case Interop.Errors.ERROR_INVALID_PARAMETER: default: + string msg = string.IsNullOrEmpty(path) + ? Marshal.GetPInvokeErrorMessage(errorCode) + : $"{Marshal.GetPInvokeErrorMessage(errorCode)} : '{path}'"; return new IOException( - string.IsNullOrEmpty(path) ? GetMessage(errorCode) : $"{GetMessage(errorCode)} : '{path}'", + msg, MakeHRFromErrorCode(errorCode)); } } @@ -90,10 +93,5 @@ internal static int TryMakeWin32ErrorCodeFromHR(int hr) return hr; } - - /// - /// Returns a string message for the specified Win32 error code. - /// - 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 index 84fc7fb2b54eb3..a8ab0cf8a97391 100644 --- 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 index 6a4a0e062698cd..a079ceb5f11ab7 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 @@ -1881,7 +1881,6 @@ - @@ -2176,7 +2175,6 @@ - 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 deleted file mode 100644 index f0ca445a357643..00000000000000 --- a/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Unix.cs +++ /dev/null @@ -1,10 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.ComponentModel -{ - public partial class Win32Exception - { - internal static string GetErrorMessage(int error) => Interop.Sys.StrError(error); - } -} 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 deleted file mode 100644 index d0cf217096736a..00000000000000 --- a/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.Windows.cs +++ /dev/null @@ -1,10 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.ComponentModel -{ - public partial class Win32Exception - { - internal static string GetErrorMessage(int error) => Interop.Kernel32.GetMessage(error); - } -} diff --git a/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.cs b/src/libraries/System.Private.CoreLib/src/System/ComponentModel/Win32Exception.cs index 0fde7ca4f87b3e..dfc7955362fc01 100644 --- 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 /// [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()) /// /// Initializes a new instance of the class with the specified error. /// - public Win32Exception(int error) : this(error, GetErrorMessage(error)) + public Win32Exception(int error) : this(error, Marshal.GetPInvokeErrorMessage(error)) { } /// 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 index 4531aa133b7eae..a9431b299777c0 100644 --- 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 index 032f4b16ad3b6d..eb9b0adaab5250 100644 --- 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 index 8a02bd29654f5e..a3c7bb4f4349bf 100644 --- 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); } + + /// + /// Gets the system error message for the supplied error code. + /// + /// The error code. + /// The error message associated with . + 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 index 1ea992b0a7f2e8..146e2011bf28c0 100644 --- 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); } + + /// + /// Gets the system error message for the supplied error code. + /// + /// The error code. + /// The error message associated with . + 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 index bd31ae698dc978..ed30d503147fe4 100644 --- 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,15 +1296,5 @@ public static int GetLastWin32Error() { return GetLastPInvokeError(); } - - /// - /// Gets the system error message for the supplied error code. - /// - /// The error code. - /// The error message associated with . - public static string GetPInvokeErrorMessage(int error) - { - return Win32Exception.GetErrorMessage(error); - } } } 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 index 8b16387ca2274f..fc456e219bb167 100644 --- 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/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs index 190042eb2954d8..c807b47de40d00 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs +++ b/src/libraries/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.GetPInvokeErrorMessage(Marshal.GetLastPInvokeError())); sourceName.AsSpan().CopyTo(new Span(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.GetPInvokeErrorMessage(Marshal.GetLastPInvokeError())); } return duplicateAccessToken; @@ -465,7 +465,7 @@ private bool CheckNtTokenForSid(SecurityIdentifier sid) (uint)TokenImpersonationLevel.Identification, (uint)TokenType.TokenImpersonation, ref token)) - throw new SecurityException(new Win32Exception().Message); + throw new SecurityException(Marshal.GetPInvokeErrorMessage(Marshal.GetLastPInvokeError())); } @@ -473,7 +473,7 @@ private bool CheckNtTokenForSid(SecurityIdentifier sid) if (!Interop.Advapi32.CheckTokenMembership((til != TokenImpersonationLevel.None ? _safeTokenHandle : token), sid.BinaryForm, ref isMember)) - throw new SecurityException(new Win32Exception().Message); + throw new SecurityException(Marshal.GetPInvokeErrorMessage(Marshal.GetLastPInvokeError())); } @@ -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.GetPInvokeErrorMessage(Marshal.GetLastPInvokeError())); s_currentImpersonatedToken.Value = null; @@ -776,12 +776,12 @@ private static void CurrentImpersonatedTokenChanged(AsyncLocalValueChangedArgs Date: Mon, 13 Jun 2022 15:17:23 -0700 Subject: [PATCH 3/7] Add additional testing. --- .../Marshal/PInvokeErrorMessageTests.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs index da25ddd66a3540..92833b7f2fbc41 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs @@ -38,5 +38,27 @@ public void PInvokeErrorMessage_Returns_Win32Exception_Message(int error) string expected = new Win32Exception(error).Message; Assert.Equal(expected, Marshal.GetPInvokeErrorMessage(error)); } + + [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); + Assert.NotEqual(err1, err2); + } } } From 1979b4c1187e1cdb29ecf0cca251cff95d1f73e5 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Mon, 13 Jun 2022 17:15:47 -0700 Subject: [PATCH 4/7] Handle unknown error codes on linux. --- .../InteropServices/Marshal/PInvokeErrorMessageTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs index 92833b7f2fbc41..9f9fb9cae49d72 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs @@ -58,7 +58,9 @@ public void PInvokeErrorMessage_Returns_UniqueMessage_Unix() var err2 = Marshal.GetPInvokeErrorMessage(0x10014); // EEXIST Assert.NotNull(err1); Assert.NotNull(err2); - Assert.NotEqual(err1, 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. } } } From c37247ce51f073cd675dafafee66b74389c4837b Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Tue, 14 Jun 2022 09:07:21 -0700 Subject: [PATCH 5/7] Condition usage of new API for NET7+. --- src/libraries/Common/src/System/IO/Win32Marshal.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/src/System/IO/Win32Marshal.cs b/src/libraries/Common/src/System/IO/Win32Marshal.cs index 8e81892bc6d777..cbc75603c428b6 100644 --- a/src/libraries/Common/src/System/IO/Win32Marshal.cs +++ b/src/libraries/Common/src/System/IO/Win32Marshal.cs @@ -59,12 +59,21 @@ internal static Exception GetExceptionForWin32Error(int errorCode, string? path case Interop.Errors.ERROR_INVALID_PARAMETER: default: string msg = string.IsNullOrEmpty(path) - ? Marshal.GetPInvokeErrorMessage(errorCode) - : $"{Marshal.GetPInvokeErrorMessage(errorCode)} : '{path}'"; + ? GetPInvokeErrorMessage(errorCode) + : $"{GetPInvokeErrorMessage(errorCode)} : '{path}'"; return new IOException( msg, MakeHRFromErrorCode(errorCode)); } + + static string GetPInvokeErrorMessage(int errorCode) + { +#if NET7_0_OR_GREATER + return Marshal.GetPInvokeErrorMessage(errorCode); +#else + return Interop.Kernel32.GetMessage(errorCode); +#endif + } } /// From f498fcc9369443718d25428cbe7e648105b6003f Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 23 Jun 2022 13:20:13 -0700 Subject: [PATCH 6/7] Added additional GetLastPInvokeErrorMessage() API --- .../System/Runtime/InteropServices/Marshal.cs | 9 +++++++++ .../ref/System.Runtime.InteropServices.cs | 1 + .../Marshal/PInvokeErrorMessageTests.cs | 10 ++++++++++ .../System/Security/Principal/WindowsIdentity.cs | 16 ++++++++-------- .../Security/Principal/WindowsPrincipal.cs | 4 ++-- 5 files changed, 30 insertions(+), 10 deletions(-) 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 index ed30d503147fe4..6d5a7050577f16 100644 --- 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(); } + + /// + /// Gets the system error message for the last PInvoke error code. + /// + /// The error message associated with the last PInvoke error code. + 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 index a7c8a06c3e8d29..5f3fa859c43e49 100644 --- 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,7 @@ 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)] diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs index 9f9fb9cae49d72..3ba0a261a667d4 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/PInvokeErrorMessageTests.cs @@ -39,6 +39,16 @@ public void PInvokeErrorMessage_Returns_Win32Exception_Message(int error) 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() diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs index c807b47de40d00..28e5a7d1d69ea8 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs +++ b/src/libraries/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(Marshal.GetPInvokeErrorMessage(Marshal.GetLastPInvokeError())); + throw new SecurityException(Marshal.GetLastPInvokeErrorMessage()); sourceName.AsSpan().CopyTo(new Span(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(Marshal.GetPInvokeErrorMessage(Marshal.GetLastPInvokeError())); + throw new SecurityException(Marshal.GetLastPInvokeErrorMessage()); } return duplicateAccessToken; @@ -465,7 +465,7 @@ private bool CheckNtTokenForSid(SecurityIdentifier sid) (uint)TokenImpersonationLevel.Identification, (uint)TokenType.TokenImpersonation, ref token)) - throw new SecurityException(Marshal.GetPInvokeErrorMessage(Marshal.GetLastPInvokeError())); + throw new SecurityException(Marshal.GetLastPInvokeErrorMessage()); } @@ -473,7 +473,7 @@ private bool CheckNtTokenForSid(SecurityIdentifier sid) if (!Interop.Advapi32.CheckTokenMembership((til != TokenImpersonationLevel.None ? _safeTokenHandle : token), sid.BinaryForm, ref isMember)) - throw new SecurityException(Marshal.GetPInvokeErrorMessage(Marshal.GetLastPInvokeError())); + throw new SecurityException(Marshal.GetLastPInvokeErrorMessage()); } @@ -756,7 +756,7 @@ private static void RunImpersonatedInternal(SafeAccessTokenHandle token, Action delegate { if (!Interop.Advapi32.RevertToSelf()) - Environment.FailFast(Marshal.GetPInvokeErrorMessage(Marshal.GetLastPInvokeError())); + Environment.FailFast(Marshal.GetLastPInvokeErrorMessage()); s_currentImpersonatedToken.Value = null; @@ -776,12 +776,12 @@ private static void CurrentImpersonatedTokenChanged(AsyncLocalValueChangedArgs Date: Fri, 24 Jun 2022 10:53:08 -0700 Subject: [PATCH 7/7] Apply suggestions from code review --- .../src/System/Security/Principal/WindowsIdentity.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs index 28e5a7d1d69ea8..1a21a722e6dc83 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs +++ b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs @@ -822,7 +822,7 @@ private static Exception GetExceptionFromNtStatus(int status) return new OutOfMemoryException(); uint win32ErrorCode = Interop.Advapi32.LsaNtStatusToWinError((uint)status); - return new SecurityException(Marshal.GetPInvokeErrorMessage(unchecked((int)win32ErrorCode))); + return new SecurityException(Marshal.GetPInvokeErrorMessage((int)win32ErrorCode)); } private static SafeAccessTokenHandle GetCurrentToken(TokenAccessLevels desiredAccess, bool threadOnly, out bool isImpersonating, out int hr)