diff --git a/DllImportGenerator/Ancillary.Interop/Ancillary.Interop.csproj b/DllImportGenerator/Ancillary.Interop/Ancillary.Interop.csproj index e9063db196b..642e04fe9e5 100644 --- a/DllImportGenerator/Ancillary.Interop/Ancillary.Interop.csproj +++ b/DllImportGenerator/Ancillary.Interop/Ancillary.Interop.csproj @@ -5,6 +5,7 @@ 8.0 System.Runtime.InteropServices enable + true diff --git a/DllImportGenerator/Ancillary.Interop/MarshalEx.cs b/DllImportGenerator/Ancillary.Interop/MarshalEx.cs index 95be96d8685..9c3ad649573 100644 --- a/DllImportGenerator/Ancillary.Interop/MarshalEx.cs +++ b/DllImportGenerator/Ancillary.Interop/MarshalEx.cs @@ -25,5 +25,79 @@ public static void SetHandle(SafeHandle safeHandle, IntPtr handle) { typeof(SafeHandle).GetMethod("SetHandle", BindingFlags.NonPublic | BindingFlags.Instance)!.Invoke(safeHandle, new object[] { handle }); } + + /// + /// Set the last platform invoke error on the thread + /// + public static void SetLastWin32Error(int error) + { + typeof(Marshal).GetMethod("SetLastWin32Error", BindingFlags.NonPublic | BindingFlags.Static)!.Invoke(null, new object[] { error }); + } + + /// + /// Get the last system error on the current thread (errno on Unix, GetLastError on Windows) + /// + public static unsafe int GetLastSystemError() + { + // Would be internal call that handles getting the last error for the thread using the PAL + + if (OperatingSystem.IsWindows()) + { + return Kernel32.GetLastError(); + } + else if (OperatingSystem.IsMacOS()) + { + return *libc.__error(); + } + else if (OperatingSystem.IsLinux()) + { + return *libc.__errno_location(); + } + + throw new NotImplementedException(); + } + + /// + /// Set the last system error on the current thread (errno on Unix, SetLastError on Windows) + /// + public static unsafe void SetLastSystemError(int error) + { + // Would be internal call that handles setting the last error for the thread using the PAL + + if (OperatingSystem.IsWindows()) + { + Kernel32.SetLastError(error); + } + else if (OperatingSystem.IsMacOS()) + { + *libc.__error() = error; + } + else if (OperatingSystem.IsLinux()) + { + *libc.__errno_location() = error; + } + else + { + throw new NotImplementedException(); + } + } + + private class Kernel32 + { + [DllImport(nameof(Kernel32))] + public static extern void SetLastError(int error); + + [DllImport(nameof(Kernel32))] + public static extern int GetLastError(); + } + + private class libc + { + [DllImport(nameof(libc))] + internal static unsafe extern int* __errno_location(); + + [DllImport(nameof(libc))] + internal static unsafe extern int* __error(); + } } } diff --git a/DllImportGenerator/DllImportGenerator.IntegrationTests/SetLastErrorTests.cs b/DllImportGenerator/DllImportGenerator.IntegrationTests/SetLastErrorTests.cs new file mode 100644 index 00000000000..6533889e2b2 --- /dev/null +++ b/DllImportGenerator/DllImportGenerator.IntegrationTests/SetLastErrorTests.cs @@ -0,0 +1,60 @@ +using System; +using System.Runtime.InteropServices; + +using Xunit; + +namespace DllImportGenerator.IntegrationTests +{ + partial class NativeExportsNE + { + public partial class SetLastError + { + [GeneratedDllImport(nameof(NativeExportsNE), EntryPoint = "set_error", SetLastError = true)] + public static partial void SetError(int error, byte shouldSetError); + + [GeneratedDllImport(nameof(NativeExportsNE), EntryPoint = "set_error_return_string", SetLastError = true)] + [return: MarshalAs(UnmanagedType.LPWStr)] + public static partial string SetError_NonBlittableSignature(int error, [MarshalAs(UnmanagedType.U1)] bool shouldSetError, [MarshalAs(UnmanagedType.LPWStr)] string errorString); + } + } + + public class SetLastErrorTests + { + [Theory] + [InlineData(0)] + [InlineData(2)] + [InlineData(-5)] + public void LastWin32Error_HasExpectedValue(int error) + { + string errorString = error.ToString(); + string ret = NativeExportsNE.SetLastError.SetError_NonBlittableSignature(error, shouldSetError: true, errorString); + Assert.Equal(error, Marshal.GetLastWin32Error()); + Assert.Equal(errorString, ret); + + // Clear the last error + MarshalEx.SetLastWin32Error(0); + + NativeExportsNE.SetLastError.SetError(error, shouldSetError: 1); + Assert.Equal(error, Marshal.GetLastWin32Error()); + } + + [Fact] + public void ClearPreviousError() + { + int error = 100; + MarshalEx.SetLastWin32Error(error); + + // Don't actually set the error in the native call. SetLastError=true should clear any existing error. + string errorString = error.ToString(); + string ret = NativeExportsNE.SetLastError.SetError_NonBlittableSignature(error, shouldSetError: false, errorString); + Assert.Equal(0, Marshal.GetLastWin32Error()); + Assert.Equal(errorString, ret); + + MarshalEx.SetLastWin32Error(error); + + // Don't actually set the error in the native call. SetLastError=true should clear any existing error. + NativeExportsNE.SetLastError.SetError(error, shouldSetError: 0); + Assert.Equal(0, Marshal.GetLastWin32Error()); + } + } +} diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/CompileFails.cs b/DllImportGenerator/DllImportGenerator.UnitTests/CompileFails.cs index 51464147487..50e430b22de 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/CompileFails.cs +++ b/DllImportGenerator/DllImportGenerator.UnitTests/CompileFails.cs @@ -41,8 +41,7 @@ public static IEnumerable CodeSnippetsToCompile() // Unsupported named arguments // * BestFitMapping, ThrowOnUnmappableChar - // [TODO]: Expected diagnostic count should be 2 once we support SetLastError - yield return new object[] { CodeSnippets.AllDllImportNamedArguments, 3, 0 }; + yield return new object[] { CodeSnippets.AllDllImportNamedArguments, 2, 0 }; // LCIDConversion yield return new object[] { CodeSnippets.LCIDConversionAttribute, 1, 0 }; diff --git a/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs b/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs index d41dc38eb18..f79c6201854 100644 --- a/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs +++ b/DllImportGenerator/DllImportGenerator.UnitTests/Compiles.cs @@ -9,7 +9,7 @@ namespace DllImportGenerator.UnitTests { public class Compiles { - public static IEnumerable CodeSnippetsToCompile_NoDiagnostics() + public static IEnumerable CodeSnippetsToCompile() { yield return new[] { CodeSnippets.TrivialClassDeclarations }; yield return new[] { CodeSnippets.TrivialStructDeclarations }; @@ -17,7 +17,7 @@ public static IEnumerable CodeSnippetsToCompile_NoDiagnostics() yield return new[] { CodeSnippets.NestedNamespace }; yield return new[] { CodeSnippets.NestedTypes }; yield return new[] { CodeSnippets.UserDefinedEntryPoint }; - //yield return new[] { CodeSnippets.AllSupportedDllImportNamedArguments }; + yield return new[] { CodeSnippets.AllSupportedDllImportNamedArguments }; yield return new[] { CodeSnippets.DefaultParameters }; yield return new[] { CodeSnippets.UseCSharpFeaturesForConstants }; @@ -151,14 +151,9 @@ public static IEnumerable CodeSnippetsToCompile_NoDiagnostics() yield return new[] { CodeSnippets.ArrayPreserveSigFalse() }; } - public static IEnumerable CodeSnippetsToCompile_WithDiagnostics() - { - yield return new[] { CodeSnippets.AllSupportedDllImportNamedArguments }; - } - [Theory] - [MemberData(nameof(CodeSnippetsToCompile_NoDiagnostics))] - public async Task ValidateSnippets_NoDiagnostics(string source) + [MemberData(nameof(CodeSnippetsToCompile))] + public async Task ValidateSnippets(string source) { Compilation comp = await TestUtils.CreateCompilation(source); TestUtils.AssertPreSourceGeneratorCompilation(comp); @@ -169,20 +164,5 @@ public async Task ValidateSnippets_NoDiagnostics(string source) var newCompDiags = newComp.GetDiagnostics(); Assert.Empty(newCompDiags); } - - [Theory] - [MemberData(nameof(CodeSnippetsToCompile_WithDiagnostics))] - public async Task ValidateSnippets_WithDiagnostics(string source) - { - Compilation comp = await TestUtils.CreateCompilation(source); - TestUtils.AssertPreSourceGeneratorCompilation(comp); - - var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.DllImportGenerator()); - Assert.NotEmpty(generatorDiags); - Assert.All(generatorDiags, d => Assert.StartsWith(Microsoft.Interop.GeneratorDiagnostics.Ids.Prefix, d.Id)); - - var newCompDiags = newComp.GetDiagnostics(); - Assert.Empty(newCompDiags); - } } } diff --git a/DllImportGenerator/DllImportGenerator/DllImportGenerator.cs b/DllImportGenerator/DllImportGenerator/DllImportGenerator.cs index 73da797fafb..d8bb6caea7e 100644 --- a/DllImportGenerator/DllImportGenerator/DllImportGenerator.cs +++ b/DllImportGenerator/DllImportGenerator/DllImportGenerator.cs @@ -103,12 +103,6 @@ public void Execute(GeneratorExecutionContext context) generatorDiagnostics.ReportConfigurationNotSupported(generatedDllImportAttr, nameof(DllImportStub.GeneratedDllImportData.ThrowOnUnmappableChar)); } - // [TODO] Remove once we support SetLastError=true - if (dllImportData.SetLastError) - { - generatorDiagnostics.ReportConfigurationNotSupported(generatedDllImportAttr, nameof(DllImportStub.GeneratedDllImportData.SetLastError), "true"); - } - if (lcidConversionAttr != null) { // Using LCIDConversion with GeneratedDllImport is not supported diff --git a/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs b/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs index 7b5dc53f922..87653bd923a 100644 --- a/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs +++ b/DllImportGenerator/DllImportGenerator/StubCodeGenerator.cs @@ -235,11 +235,41 @@ public override (string managed, string native) GetIdentifiers(TypePositionInfo invoke)); } + if (this.dllImportData.SetLastError) + { + // Marshal.SetLastSystemError(0); + var clearLastError = ExpressionStatement( + InvocationExpression( + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + ParseName(TypeNames.System_Runtime_InteropServices_MarshalEx), + IdentifierName("SetLastSystemError")), + ArgumentList(SingletonSeparatedList( + Argument(LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0))))))); + + // Marshal.SetLastWin32Error(Marshal.GetLastSystemError()); + var updateLastError = ExpressionStatement( + InvocationExpression( + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + ParseName(TypeNames.System_Runtime_InteropServices_MarshalEx), + IdentifierName("SetLastWin32Error")), + ArgumentList(SingletonSeparatedList( + Argument( + InvocationExpression( + MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + ParseName(TypeNames.System_Runtime_InteropServices_MarshalEx), + IdentifierName("GetLastSystemError")))))))); + + invokeStatement = Block(clearLastError, invokeStatement, updateLastError); + } + // Nest invocation in fixed statements if (fixedStatements.Any()) { fixedStatements.Reverse(); - invokeStatement = fixedStatements.First().WithStatement(Block(invokeStatement)); + invokeStatement = fixedStatements.First().WithStatement(invokeStatement); foreach (var fixedStatement in fixedStatements.Skip(1)) { invokeStatement = fixedStatement.WithStatement(Block(invokeStatement)); diff --git a/DllImportGenerator/TestAssets/NativeExports/Error.cs b/DllImportGenerator/TestAssets/NativeExports/Error.cs new file mode 100644 index 00000000000..7af05e28a49 --- /dev/null +++ b/DllImportGenerator/TestAssets/NativeExports/Error.cs @@ -0,0 +1,64 @@ +using System; +using System.Runtime.InteropServices; + +namespace NativeExports +{ + public static unsafe class Error + { + private class Kernel32 + { + [DllImport(nameof(Kernel32))] + public static extern void SetLastError(int error); + + [DllImport(nameof(Kernel32))] + public static extern int GetLastError(); + } + + private class libc + { + [DllImport(nameof(libc))] + internal static unsafe extern int* __errno_location(); + + [DllImport(nameof(libc))] + internal static unsafe extern int* __error(); + } + + [UnmanagedCallersOnly(EntryPoint = "set_error")] + public static void SetError(int error, byte shouldSetError) + { + if (shouldSetError != 0) + SetLastError(error); + } + + [UnmanagedCallersOnly(EntryPoint = "set_error_return_string")] + public static ushort* SetErrorReturnString(int error, byte shouldSetError, ushort* errorString) + { + ushort* ret = (ushort*)Marshal.StringToCoTaskMemUni(new string((char*)errorString)); + + if (shouldSetError != 0) + SetLastError(error); + + return ret; + } + + private static void SetLastError(int error) + { + if (OperatingSystem.IsWindows()) + { + Kernel32.SetLastError(error); + } + else if (OperatingSystem.IsMacOS()) + { + *libc.__error() = error; + } + else if (OperatingSystem.IsLinux()) + { + *libc.__errno_location() = error; + } + else + { + throw new NotImplementedException(); + } + } + } +} diff --git a/DllImportGenerator/designs/Pipeline.md b/DllImportGenerator/designs/Pipeline.md index 6ff67b39ad6..a751257bcee 100644 --- a/DllImportGenerator/designs/Pipeline.md +++ b/DllImportGenerator/designs/Pipeline.md @@ -139,6 +139,21 @@ These various scenarios have different levels of support for these three feature | non-blittable array marshalling in a P/Invoke | unsupported | unsupported (supportable with https://github.com/dotnet/runtime/issues/25423) | unuspported | | non-blittable array marshalling not in a P/Invoke | unsupported | unsupported | unuspported | +### `SetLastError=true` + +The stub code generation also handles [`SetLastError=true`][SetLastError] behaviour. This configuration indicates that system error code ([`errno`](https://en.wikipedia.org/wiki/Errno.h) on Unix, [`GetLastError`](https://docs.microsoft.com/windows/win32/api/errhandlingapi/nf-errhandlingapi-getlasterror) on Windows) should be stored after the native invocation, such that it can be retrieved using [`Marshal.GetLastWin32Error`](https://docs.microsoft.com/dotnet/api/system.runtime.interopservices.marshal.getlastwin32error). + +This means that, rather than simply invoke the native method, the generated stub will: + +1. Clear the system error by setting it to 0 +2. Invoke the native method +3. Get the system error +4. Set the stored error for the P/Invoke (accessible via `Marshal.GetLastWin32Error`) + +A core requirement of this functionality is that the P/Invoke called in (2) is blittable (the purpose of the P/Invoke source generator), such that there will be no additional operations (e.g unmarshalling) after the invocation that could change the system error that is retrieved in (3). Similarly, (3) must not involve any operations before getting the system error that could change the system error. This also relies on the runtime itself handling preserving the last error (see `BEGIN/END_PRESERVE_LAST_ERROR` macros) during JIT and P/Invoke resolution. + +Clearing the system error (1) is necessary because the native method may not set the error at all on success and the system error would retain its value from a previous operation. The developer should be able to check `Marshal.GetLastWin32Error` after a P/Inovke to determine success or failure, so the stub explicitly clears the error before the native invocation, such that the last error will indicate success if the native call does not change it. + ## P/Invoke The P/Invoke called by the stub is created based on the user's original declaration of the stub. The signature is generated using the syntax returned by `AsNativeType` and `AsParameter` of the marshalling generators for the return and parameters. Any marshalling attributes on the return and parameters of the managed method - [`MarshalAsAttribute`][MarshalAsAttribute], [`InAttribute`][InAttribute], [`OutAttribute`][OutAttribute] - are dropped.