Verify if COM dangerous during call site check (#3009)

The motivation of the change is to reduce the number of unnecessary warnings around PInvokes. The change makes the RequiresReflectionMethodBodyScannerForCallSite check for COM to decide whether dataflow analysis is needed for PInvokes.

2 files changed, 54 insertions, 12 deletions

diff --git a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
index adac45faf..c11c86a6c 100644
--- a/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
+++ b/src/linker/Linker.Dataflow/ReflectionMethodBodyScanner.cs
@@ -33,7 +33,7 @@ namespace Mono.Linker.Dataflow
 			return Intrinsics.GetIntrinsicIdForMethod (methodDefinition) > IntrinsicId.RequiresReflectionBodyScanner_Sentinel ||
 				context.Annotations.FlowAnnotations.RequiresDataFlowAnalysis (methodDefinition) ||
 				context.Annotations.DoesMethodRequireUnreferencedCode (methodDefinition, out _) ||
-				methodDefinition.IsPInvokeImpl;
+				methodDefinition.IsPInvokeImpl && ComDangerousMethod (methodDefinition, context);
 		}
 
 		public static bool RequiresReflectionMethodBodyScannerForMethodBody (LinkContext context, MethodDefinition methodDefinition)
@@ -245,17 +245,8 @@ namespace Mono.Linker.Dataflow
 				}
 
 			case IntrinsicId.None: {
-					if (calledMethodDefinition.IsPInvokeImpl) {
-						// Is the PInvoke dangerous?
-						bool comDangerousMethod = IsComInterop (calledMethodDefinition.MethodReturnType, calledMethodDefinition.ReturnType, context);
-						foreach (ParameterDefinition pd in calledMethodDefinition.Parameters) {
-							comDangerousMethod |= IsComInterop (pd, pd.ParameterType, context);
-						}
-
-						if (comDangerousMethod) {
-							diagnosticContext.AddDiagnostic (DiagnosticId.CorrectnessOfCOMCannotBeGuaranteed, calledMethodDefinition.GetDisplayName ());
-						}
-					}
+					if (calledMethodDefinition.IsPInvokeImpl && ComDangerousMethod (calledMethodDefinition, context))
+						diagnosticContext.AddDiagnostic (DiagnosticId.CorrectnessOfCOMCannotBeGuaranteed, calledMethodDefinition.GetDisplayName ());
 					if (context.Annotations.DoesMethodRequireUnreferencedCode (calledMethodDefinition, out RequiresUnreferencedCodeAttribute? requiresUnreferencedCode))
 						MarkStep.ReportRequiresUnreferencedCode (calledMethodDefinition.GetDisplayName (), requiresUnreferencedCode, diagnosticContext);
 
@@ -449,5 +440,15 @@ namespace Mono.Linker.Dataflow
 		{
 			TrimAnalysisPatterns.Add (new TrimAnalysisAssignmentPattern (value, targetValue, origin));
 		}
+
+		private static bool ComDangerousMethod (MethodDefinition methodDefinition, LinkContext context)
+		{
+			bool comDangerousMethod = IsComInterop (methodDefinition.MethodReturnType, methodDefinition.ReturnType, context);
+			foreach (ParameterDefinition pd in methodDefinition.Parameters) {
+				comDangerousMethod |= IsComInterop (pd, pd.ParameterType, context);
+			}
+
+			return comDangerousMethod;
+		}
 	}
 }
diff --git a/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs
index 347358f90..64c31a813 100644
--- a/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs
+++ b/test/Mono.Linker.Tests.Cases/DataFlow/CompilerGeneratedCodeAccessedViaReflection.cs
@@ -344,6 +344,22 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 				lambda ();
 			}
 
+			static void LambdaCallsPInvokeTakingPrimitiveType ()
+			{
+				var lambda = () => MethodTakingPrimitiveType (42);
+				lambda ();
+			}
+
+			static void LambdaCallsPInvokeTakingObject ()
+			{
+				var lambda =
+				[ExpectedWarning ("IL2050")]
+				[ExpectedWarning ("IL2119", "<" + nameof (LambdaCallsPInvokeTakingObject) + ">",
+					ProducedBy = ProducedBy.Trimmer)]
+				() => MethodTakingObject (null);
+				lambda ();
+			}
+
 			[ExpectedWarning ("IL2112", nameof (RUCTypeWithLambdas) + "()", "--RUCTypeWithLambdas--", CompilerGeneratedCode = true,
 				ProducedBy = ProducedBy.Trimmer)]
 			[RequiresUnreferencedCode ("--RUCTypeWithLambdas--")]
@@ -374,6 +390,8 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 				}
 			}
 
+			[ExpectedWarning ("IL2118", "<" + nameof (LambdaCallsPInvokeTakingObject) + ">",
+				ProducedBy = ProducedBy.Trimmer)]
 			[ExpectedWarning ("IL2118", "<" + nameof (LambdaCallsMethodWithRequires) + ">",
 				ProducedBy = ProducedBy.Trimmer)]
 			[ExpectedWarning ("IL2118", "<" + nameof (LambdaWithCorrectDataflow) + ">",
@@ -463,6 +481,21 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 				LocalFunction ();
 			}
 
+			static void LocalFunctionCallsPInvokeTakingPrimitiveType ()
+			{
+				void LocalFunction () => MethodTakingPrimitiveType (42);
+				LocalFunction ();
+			}
+
+			static void LocalFunctionCallsPInvokeTakingObject ()
+			{
+				[ExpectedWarning ("IL2050")]
+				[ExpectedWarning ("IL2119", "<" + nameof (LocalFunctionCallsPInvokeTakingObject) + ">",
+					ProducedBy = ProducedBy.Trimmer)]
+				void LocalFunction () => MethodTakingObject (null);
+				LocalFunction ();
+			}
+
 			[ExpectedWarning ("IL2112", nameof (RUCTypeWithLocalFunctions) + "()", CompilerGeneratedCode = true,
 				ProducedBy = ProducedBy.Trimmer)]
 			[RequiresUnreferencedCode ("--RUCTypeWithLocalFunctions--")]
@@ -499,6 +532,8 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 				}
 			}
 
+			[ExpectedWarning ("IL2118", nameof (LocalFunctionCallsPInvokeTakingObject),
+				ProducedBy = ProducedBy.Trimmer)]
 			[ExpectedWarning ("IL2118", nameof (LocalFunctionCallsMethodWithRequires),
 				ProducedBy = ProducedBy.Trimmer)]
 			[ExpectedWarning ("IL2118", nameof (LocalFunctionWithCorrectDataflow),
@@ -644,5 +679,11 @@ namespace Mono.Linker.Tests.Cases.DataFlow
 
 		[return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)]
 		static Type GetAll () => null;
+
+		[DllImport ("Foo")]
+		static extern int MethodTakingPrimitiveType (int num);
+
+		[DllImport ("Foo")]
+		static extern void MethodTakingObject ([MarshalAs (UnmanagedType.IUnknown)] object obj);
 	}
 }