diff --git a/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs b/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs index 433c06957c02ef..db981a83ae974d 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs @@ -1241,7 +1241,8 @@ internal static IntPtr SafeHandleAddRef(SafeHandle pHandle, ref bool success) { if (pHandle == null) { - ThrowHelper.ThrowArgumentNullException(ExceptionArgument.pHandle, ExceptionResource.ArgumentNull_SafeHandle); + success = false; + return IntPtr.Zero; } pHandle.DangerousAddRef(ref success); @@ -1251,11 +1252,7 @@ internal static IntPtr SafeHandleAddRef(SafeHandle pHandle, ref bool success) // Releases the SH (to be called from finally block). internal static void SafeHandleRelease(SafeHandle pHandle) { - if (pHandle == null) - { - ThrowHelper.ThrowArgumentNullException(ExceptionArgument.pHandle, ExceptionResource.ArgumentNull_SafeHandle); - } - + Debug.Assert(pHandle != null); pHandle.DangerousRelease(); } diff --git a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs index 804c0c576d79da..1620e0caeee115 100644 --- a/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs +++ b/src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs @@ -1742,7 +1742,19 @@ protected override void EmitMarshalArgumentManagedToNative() if (IsManagedByRef) PropagateFromByRefArg(marshallingCodeStream, _managedHome); + // Boolean by-ref for DangerousAddRef(). var vAddRefed = emitter.NewLocal(Context.GetWellKnownType(WellKnownType.Boolean)); + marshallingCodeStream.EmitLdc(0); + marshallingCodeStream.EmitStLoc(vAddRefed); + + // Initialize native value to null. + marshallingCodeStream.EmitLdc(0); + marshallingCodeStream.Emit(ILOpcode.conv_i); + StoreNativeValue(marshallingCodeStream); + + ILCodeLabel lHandleIsNull = emitter.NewCodeLabel(); + LoadManagedValue(marshallingCodeStream); + marshallingCodeStream.Emit(ILOpcode.brfalse, lHandleIsNull); LoadManagedValue(marshallingCodeStream); marshallingCodeStream.EmitLdLoca(vAddRefed); marshallingCodeStream.Emit(ILOpcode.call, emitter.NewToken( @@ -1755,6 +1767,7 @@ protected override void EmitMarshalArgumentManagedToNative() safeHandleType.GetKnownMethod("DangerousGetHandle", new MethodSignature(0, 0, Context.GetWellKnownType(WellKnownType.IntPtr), TypeDesc.EmptyTypes)))); StoreNativeValue(marshallingCodeStream); + marshallingCodeStream.EmitLabel(lHandleIsNull); ILCodeLabel lNotAddrefed = emitter.NewCodeLabel(); cleanupCodeStream.EmitLdLoc(vAddRefed); diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.SecKeyRef.Export.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.SecKeyRef.Export.cs index e0f99e87474908..d327d4249e7797 100644 --- a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.SecKeyRef.Export.cs +++ b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.SecKeyRef.Export.cs @@ -13,13 +13,11 @@ internal static partial class Interop { internal static partial class AppleCrypto { - private static readonly SafeCreateHandle s_nullExportString = new SafeCreateHandle(); - [DllImport(Libraries.AppleCryptoNative)] private static extern int AppleCryptoNative_SecKeyExport( SafeSecKeyRefHandle? key, int exportPrivate, - SafeCreateHandle cfExportPassphrase, + SafeCreateHandle? cfExportPassphrase, out SafeCFDataHandle cfDataOut, out int pOSStatus); @@ -28,9 +26,9 @@ internal static SafeCFDataHandle SecKeyExportData( bool exportPrivate, ReadOnlySpan password) { - SafeCreateHandle exportPassword = exportPrivate + SafeCreateHandle? exportPassword = exportPrivate ? CoreFoundation.CFStringCreateFromSpan(password) - : s_nullExportString; + : null; int ret; SafeCFDataHandle cfData; @@ -47,10 +45,7 @@ internal static SafeCFDataHandle SecKeyExportData( } finally { - if (exportPassword != s_nullExportString) - { - exportPassword.Dispose(); - } + exportPassword?.Dispose(); } if (ret == 1) diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs index 87eb51a3512a20..fe9255d30414c7 100644 --- a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs +++ b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs @@ -21,7 +21,7 @@ internal static partial class AppleCrypto private static int AppleCryptoNative_X509ImportCertificate( ReadOnlySpan keyBlob, X509ContentType contentType, - SafeCreateHandle cfPfxPassphrase, + SafeCreateHandle? cfPfxPassphrase, SafeKeychainHandle tmpKeychain, int exportable, out SafeSecCertificateHandle pCertOut, @@ -45,7 +45,7 @@ private static extern int AppleCryptoNative_X509ImportCertificate( ref byte pbKeyBlob, int cbKeyBlob, X509ContentType contentType, - SafeCreateHandle cfPfxPassphrase, + SafeCreateHandle? cfPfxPassphrase, SafeKeychainHandle tmpKeychain, int exportable, out SafeSecCertificateHandle pCertOut, @@ -57,7 +57,7 @@ private static extern int AppleCryptoNative_X509ImportCollection( ref byte pbKeyBlob, int cbKeyBlob, X509ContentType contentType, - SafeCreateHandle cfPfxPassphrase, + SafeCreateHandle? cfPfxPassphrase, SafeKeychainHandle tmpKeychain, int exportable, out SafeCFArrayHandle pCollectionOut, @@ -98,7 +98,7 @@ private static extern int AppleCryptoNative_X509DemuxAndRetainHandle( private static extern int AppleCryptoNative_X509ExportData( SafeCreateHandle data, X509ContentType type, - SafeCreateHandle cfExportPassphrase, + SafeCreateHandle? cfExportPassphrase, out SafeCFDataHandle pExportOut, out int pOSStatus); @@ -191,12 +191,10 @@ private static SafeSecCertificateHandle X509ImportCertificate( SafeSecCertificateHandle certHandle; int osStatus; - SafeCreateHandle cfPassphrase = importPassword ?? s_nullExportString; - int ret = AppleCryptoNative_X509ImportCertificate( bytes, contentType, - cfPassphrase, + importPassword, keychain, exportable ? 1 : 0, out certHandle, @@ -238,7 +236,7 @@ internal static SafeCFArrayHandle X509ImportCollection( SafeKeychainHandle keychain, bool exportable) { - SafeCreateHandle cfPassphrase = s_nullExportString; + SafeCreateHandle? cfPassphrase = null; bool releasePassword = false; int ret; @@ -280,10 +278,7 @@ ref MemoryMarshal.GetReference(bytes), importPassword.DangerousRelease(); } - if (cfPassphrase != s_nullExportString) - { - cfPassphrase.Dispose(); - } + cfPassphrase?.Dispose(); } collectionHandle.Dispose(); @@ -477,7 +472,7 @@ internal static SafeSecIdentityHandle X509CopyWithPrivateKey( return null; } - private static byte[] X509Export(X509ContentType contentType, SafeCreateHandle cfPassphrase, IntPtr[] certHandles) + private static byte[] X509Export(X509ContentType contentType, SafeCreateHandle? cfPassphrase, IntPtr[] certHandles) { Debug.Assert(contentType == X509ContentType.Pkcs7 || contentType == X509ContentType.Pkcs12); @@ -514,7 +509,7 @@ private static byte[] X509Export(X509ContentType contentType, SafeCreateHandle c internal static byte[] X509ExportPkcs7(IntPtr[] certHandles) { - return X509Export(X509ContentType.Pkcs7, s_nullExportString, certHandles); + return X509Export(X509ContentType.Pkcs7, null, certHandles); } internal static byte[] X509ExportPfx(IntPtr[] certHandles, SafePasswordHandle exportPassword) diff --git a/src/libraries/Common/src/Interop/Windows/WinSock/Interop.TransmitFile.cs b/src/libraries/Common/src/Interop/Windows/WinSock/Interop.TransmitFile.cs index d6e290713361e2..d097205cde4cb6 100644 --- a/src/libraries/Common/src/Interop/Windows/WinSock/Interop.TransmitFile.cs +++ b/src/libraries/Common/src/Interop/Windows/WinSock/Interop.TransmitFile.cs @@ -14,7 +14,7 @@ internal static partial class Mswsock [DllImport(Interop.Libraries.Mswsock, SetLastError = true)] internal static extern unsafe bool TransmitFile( SafeHandle socket, - IntPtr fileHandle, + SafeHandle? fileHandle, int numberOfBytesToWrite, int numberOfBytesPerSend, NativeOverlapped* overlapped, diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs index 09858566bc28eb..8bfd91c446ad9b 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs @@ -1052,27 +1052,9 @@ private static unsafe bool TransmitFileHelper( transmitFileBuffers.TailLength = postBufferLength; } - bool releaseRef = false; - IntPtr fileHandlePtr = IntPtr.Zero; - try - { - if (fileHandle != null) - { - fileHandle.DangerousAddRef(ref releaseRef); - fileHandlePtr = fileHandle.DangerousGetHandle(); - } - - return Interop.Mswsock.TransmitFile( - socket, fileHandlePtr, 0, 0, overlapped, - needTransmitFileBuffers ? &transmitFileBuffers : null, flags); - } - finally - { - if (releaseRef) - { - fileHandle!.DangerousRelease(); - } - } + return Interop.Mswsock.TransmitFile( + socket, fileHandle, 0, 0, overlapped, + needTransmitFileBuffers ? &transmitFileBuffers : null, flags); } public static unsafe SocketError SendFileAsync(SafeSocketHandle handle, FileStream? fileStream, byte[]? preBuffer, byte[]? postBuffer, TransmitFileOptions flags, TransmitFileAsyncResult asyncResult) diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index 7186e5b4f63a30..140c8861fdec27 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -1585,9 +1585,6 @@ Path cannot be null. - - SafeHandle cannot be null. - Stream cannot be null. diff --git a/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs b/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs index 8667c198913bcb..6562396488e40f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs @@ -831,8 +831,6 @@ private static string GetResourceString(ExceptionResource resource) return SR.ArgumentException_OtherNotArrayOfCorrectLength; case ExceptionResource.ArgumentNull_Array: return SR.ArgumentNull_Array; - case ExceptionResource.ArgumentNull_SafeHandle: - return SR.ArgumentNull_SafeHandle; case ExceptionResource.ArgumentOutOfRange_EndIndexStartIndex: return SR.ArgumentOutOfRange_EndIndexStartIndex; case ExceptionResource.ArgumentOutOfRange_Enum: @@ -1029,7 +1027,6 @@ internal enum ExceptionResource Task_WaitMulti_NullTask, ArgumentException_OtherNotArrayOfCorrectLength, ArgumentNull_Array, - ArgumentNull_SafeHandle, ArgumentOutOfRange_EndIndexStartIndex, ArgumentOutOfRange_Enum, ArgumentOutOfRange_HugeArrayNotSupported, diff --git a/src/mono/mono/metadata/marshal-ilgen.c b/src/mono/mono/metadata/marshal-ilgen.c index ae8b2b72bd76f5..bd7d5330e94d71 100644 --- a/src/mono/mono/metadata/marshal-ilgen.c +++ b/src/mono/mono/metadata/marshal-ilgen.c @@ -5343,7 +5343,7 @@ emit_marshal_safehandle_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, switch (action){ case MARSHAL_ACTION_CONV_IN: { - int dar_release_slot, pos; + int dar_release_slot, pos_done; conv_arg = mono_mb_add_local (mb, int_type); *conv_arg_type = int_type; @@ -5351,12 +5351,6 @@ emit_marshal_safehandle_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, if (!sh_dangerous_add_ref) init_safe_handle (); - mono_mb_emit_ldarg (mb, argnum); - pos = mono_mb_emit_branch (mb, CEE_BRTRUE); - mono_mb_emit_exception (mb, "ArgumentNullException", NULL); - - mono_mb_patch_branch (mb, pos); - /* Create local to hold the ref parameter to DangerousAddRef */ dar_release_slot = mono_mb_add_local (mb, boolean_type); @@ -5364,13 +5358,20 @@ emit_marshal_safehandle_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, mono_mb_emit_icon (mb, 0); mono_mb_emit_stloc (mb, dar_release_slot); + /* set conv = IntPtr.Zero; */ + mono_mb_emit_icon (mb, 0); + mono_mb_emit_byte (mb, CEE_CONV_I); + mono_mb_emit_stloc (mb, conv_arg); + if (t->byref) { int old_handle_value_slot = mono_mb_add_local (mb, int_type); - if (!is_in (t)) { - mono_mb_emit_icon (mb, 0); - mono_mb_emit_stloc (mb, conv_arg); - } else { + if (is_in (t)) { + /* Load and check the byref SafeHandle */ + mono_mb_emit_ldarg(mb, argnum); + mono_mb_emit_byte(mb, CEE_LDIND_REF); + pos_done = mono_mb_emit_branch(mb, CEE_BRFALSE); + /* safehandle.DangerousAddRef (ref release) */ mono_mb_emit_ldarg (mb, argnum); mono_mb_emit_byte (mb, CEE_LDIND_REF); @@ -5385,8 +5386,14 @@ emit_marshal_safehandle_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, mono_mb_emit_byte (mb, CEE_DUP); mono_mb_emit_stloc (mb, conv_arg); mono_mb_emit_stloc (mb, old_handle_value_slot); + + mono_mb_patch_branch(mb, pos_done); } } else { + /* Load and check the SafeHandle */ + mono_mb_emit_ldarg(mb, argnum); + pos_done = mono_mb_emit_branch(mb, CEE_BRFALSE); + /* safehandle.DangerousAddRef (ref release) */ mono_mb_emit_ldarg (mb, argnum); mono_mb_emit_ldloc_addr (mb, dar_release_slot); @@ -5397,6 +5404,8 @@ emit_marshal_safehandle_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, mono_mb_emit_ldflda (mb, MONO_STRUCT_OFFSET (MonoSafeHandle, handle)); mono_mb_emit_byte (mb, CEE_LDIND_I); mono_mb_emit_stloc (mb, conv_arg); + + mono_mb_patch_branch(mb, pos_done); } break; @@ -5482,7 +5491,7 @@ emit_marshal_safehandle_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, } break; } - + case MARSHAL_ACTION_CONV_RESULT: { ERROR_DECL (error); MonoMethod *ctor = NULL; @@ -5516,7 +5525,7 @@ emit_marshal_safehandle_ilgen (EmitMarshalContext *m, int argnum, MonoType *t, mono_mb_emit_byte (mb, CEE_STIND_I); break; } - + case MARSHAL_ACTION_MANAGED_CONV_IN: fprintf (stderr, "mono/marshal: SafeHandles missing MANAGED_CONV_IN\n"); break; diff --git a/src/tests/Interop/PInvoke/SafeHandles/SafeHandleTests.cs b/src/tests/Interop/PInvoke/SafeHandles/SafeHandleTests.cs index b1179aa2d768d4..4587968e428416 100644 --- a/src/tests/Interop/PInvoke/SafeHandles/SafeHandleTests.cs +++ b/src/tests/Interop/PInvoke/SafeHandles/SafeHandleTests.cs @@ -16,14 +16,23 @@ public static void RunTest() { var testHandle = new TestSafeHandle(initialValue); Assert.IsTrue(SafeHandleNative.SafeHandleByValue(testHandle, initialValue)); + Assert.IsTrue(SafeHandleNative.SafeHandleByValue(null, IntPtr.Zero)); Assert.IsTrue(SafeHandleNative.SafeHandleByRef(ref testHandle, initialValue, newValue)); Assert.AreEqual(newValue, testHandle.DangerousGetHandle()); + testHandle = null; + Assert.IsTrue(SafeHandleNative.SafeHandleByRef(ref testHandle, IntPtr.Zero, newValue)); + Assert.AreEqual(newValue, testHandle.DangerousGetHandle()); + AbstractDerivedSafeHandle abstrHandle = new AbstractDerivedSafeHandleImplementation(initialValue); Assert.IsTrue(SafeHandleNative.SafeHandleInByRef(abstrHandle, initialValue)); Assert.Throws(() => SafeHandleNative.SafeHandleByRef(ref abstrHandle, initialValue, newValue)); + abstrHandle = null; + Assert.IsTrue(SafeHandleNative.SafeHandleInByRef(abstrHandle, IntPtr.Zero)); + Assert.AreEqual(null, abstrHandle); + NoDefaultConstructorSafeHandle noDefaultCtorHandle = new NoDefaultConstructorSafeHandle(initialValue); Assert.Throws(() => SafeHandleNative.SafeHandleByRef(ref noDefaultCtorHandle, initialValue, newValue));