Revert "Support marshalling of null SafeHandle. (#47944)" (#48193)

This reverts commit c3ae456.

src/coreclr/System.Private.CoreLib/src/System/StubHelpers.cs

@@ -1241,8 +1241,7 @@ internal static IntPtr SafeHandleAddRef(SafeHandle pHandle, ref bool success)
         {
             if (pHandle == null)
             {
-                success = false;
-                return IntPtr.Zero;
+                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.pHandle, ExceptionResource.ArgumentNull_SafeHandle);
             }
 
             pHandle.DangerousAddRef(ref success);
@@ -1252,7 +1251,11 @@ internal static IntPtr SafeHandleAddRef(SafeHandle pHandle, ref bool success)
         // Releases the SH (to be called from finally block).
         internal static void SafeHandleRelease(SafeHandle pHandle)
         {
-            Debug.Assert(pHandle != null);
+            if (pHandle == null)
+            {
+                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.pHandle, ExceptionResource.ArgumentNull_SafeHandle);
+            }
+
             pHandle.DangerousRelease();
         }
 

src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs

@@ -1742,19 +1742,7 @@ 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(
@@ -1767,7 +1755,6 @@ 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);

src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.SecKeyRef.Export.cs

@@ -12,11 +12,13 @@ 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);
 
@@ -25,9 +27,9 @@ internal static SafeCFDataHandle SecKeyExportData(
             bool exportPrivate,
             ReadOnlySpan<char> password)
         {
-            SafeCreateHandle? exportPassword = exportPrivate
+            SafeCreateHandle exportPassword = exportPrivate
                 ? CoreFoundation.CFStringCreateFromSpan(password)
-                : null;
+                : s_nullExportString;
 
             int ret;
             SafeCFDataHandle cfData;
@@ -44,7 +46,10 @@ internal static SafeCFDataHandle SecKeyExportData(
             }
             finally
             {
-                exportPassword?.Dispose();
+                if (exportPassword != s_nullExportString)
+                {
+                    exportPassword.Dispose();
+                }
             }
 
             if (ret == 1)

src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.cs

@@ -20,7 +20,7 @@ internal static partial class AppleCrypto
         private static int AppleCryptoNative_X509ImportCertificate(
             ReadOnlySpan<byte> keyBlob,
             X509ContentType contentType,
-            SafeCreateHandle? cfPfxPassphrase,
+            SafeCreateHandle cfPfxPassphrase,
             SafeKeychainHandle tmpKeychain,
             int exportable,
             out SafeSecCertificateHandle pCertOut,
@@ -44,7 +44,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,
@@ -56,7 +56,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,
@@ -97,7 +97,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);
 
@@ -190,10 +190,12 @@ private static SafeSecCertificateHandle X509ImportCertificate(
             SafeSecCertificateHandle certHandle;
             int osStatus;
 
+            SafeCreateHandle cfPassphrase = importPassword ?? s_nullExportString;
+
             int ret = AppleCryptoNative_X509ImportCertificate(
                 bytes,
                 contentType,
-                importPassword,
+                cfPassphrase,
                 keychain,
                 exportable ? 1 : 0,
                 out certHandle,
@@ -235,7 +237,7 @@ internal static SafeCFArrayHandle X509ImportCollection(
             SafeKeychainHandle keychain,
             bool exportable)
         {
-            SafeCreateHandle? cfPassphrase = null;
+            SafeCreateHandle cfPassphrase = s_nullExportString;
             bool releasePassword = false;
 
             int ret;
@@ -277,7 +279,10 @@ ref MemoryMarshal.GetReference(bytes),
                     importPassword.DangerousRelease();
                 }
 
-                cfPassphrase?.Dispose();
+                if (cfPassphrase != s_nullExportString)
+                {
+                    cfPassphrase.Dispose();
+                }
             }
 
             collectionHandle.Dispose();
@@ -471,7 +476,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);
 
@@ -508,7 +513,7 @@ private static byte[] X509Export(X509ContentType contentType, SafeCreateHandle?
 
         internal static byte[] X509ExportPkcs7(IntPtr[] certHandles)
         {
-            return X509Export(X509ContentType.Pkcs7, null, certHandles);
+            return X509Export(X509ContentType.Pkcs7, s_nullExportString, certHandles);
         }
 
         internal static byte[] X509ExportPfx(IntPtr[] certHandles, SafePasswordHandle exportPassword)

src/libraries/Common/src/Interop/Windows/WinSock/Interop.TransmitFile.cs

@@ -13,7 +13,7 @@ internal static partial class Mswsock
         [DllImport(Interop.Libraries.Mswsock, SetLastError = true)]
         internal static extern unsafe bool TransmitFile(
             SafeHandle socket,
-            SafeHandle? fileHandle,
+            IntPtr fileHandle,
             int numberOfBytesToWrite,
             int numberOfBytesPerSend,
             NativeOverlapped* overlapped,

src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Windows.cs

@@ -1052,9 +1052,27 @@ private static unsafe bool TransmitFileHelper(
                 transmitFileBuffers.TailLength = postBufferLength;
             }
 
-            return Interop.Mswsock.TransmitFile(
-                socket, fileHandle, 0, 0, overlapped,
-                needTransmitFileBuffers ? &transmitFileBuffers : null, flags);
+            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();
+                }
+            }
         }
 
         public static unsafe SocketError SendFileAsync(SafeSocketHandle handle, FileStream? fileStream, byte[]? preBuffer, byte[]? postBuffer, TransmitFileOptions flags, TransmitFileAsyncResult asyncResult)

src/libraries/System.Private.CoreLib/src/Resources/Strings.resx

@@ -1585,6 +1585,9 @@
   <data name="ArgumentNull_Path" xml:space="preserve">
     <value>Path cannot be null.</value>
   </data>
+  <data name="ArgumentNull_SafeHandle" xml:space="preserve">
+    <value>SafeHandle cannot be null.</value>
+  </data>
   <data name="ArgumentNull_Stream" xml:space="preserve">
     <value>Stream cannot be null.</value>
   </data>

src/libraries/System.Private.CoreLib/src/System/ThrowHelper.cs

@@ -831,6 +831,8 @@ 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:
@@ -1027,6 +1029,7 @@ internal enum ExceptionResource
         Task_WaitMulti_NullTask,
         ArgumentException_OtherNotArrayOfCorrectLength,
         ArgumentNull_Array,
+        ArgumentNull_SafeHandle,
         ArgumentOutOfRange_EndIndexStartIndex,
         ArgumentOutOfRange_Enum,
         ArgumentOutOfRange_HugeArrayNotSupported,

src/mono/mono/metadata/marshal-ilgen.c

@@ -5343,35 +5343,34 @@ emit_marshal_safehandle_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
 
 	switch (action){
 	case MARSHAL_ACTION_CONV_IN: {
-		int dar_release_slot, pos_done;
+		int dar_release_slot, pos;
 
 		conv_arg = mono_mb_add_local (mb, int_type);
 		*conv_arg_type = int_type;
 
 		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);
 
 		/* set release = false; */
 		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)) {
-				/* 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);
-
+			if (!is_in (t)) {
+				mono_mb_emit_icon (mb, 0);
+				mono_mb_emit_stloc (mb, conv_arg);
+			} else {
 				/* safehandle.DangerousAddRef (ref release) */
 				mono_mb_emit_ldarg (mb, argnum);
 				mono_mb_emit_byte (mb, CEE_LDIND_REF);
@@ -5386,14 +5385,8 @@ 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);
@@ -5404,8 +5397,6 @@ 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;
@@ -5491,7 +5482,7 @@ emit_marshal_safehandle_ilgen (EmitMarshalContext *m, int argnum, MonoType *t,
 		}
 		break;
 	}
-
+		
 	case MARSHAL_ACTION_CONV_RESULT: {
 		ERROR_DECL (error);
 		MonoMethod *ctor = NULL;
@@ -5525,7 +5516,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;

src/tests/Interop/PInvoke/SafeHandles/SafeHandleTests.cs

@@ -16,23 +16,14 @@ 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<MarshalDirectiveException>(() => 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<MissingMethodException>(() => SafeHandleNative.SafeHandleByRef(ref noDefaultCtorHandle, initialValue, newValue));