Fix icsharpcode#2391: mark method as unsafe when passing null to a …

…parameter of pointer type.
dnSpyEx · Aug 11, 2021 · c6f1a0a · c6f1a0a
1 parent 4dbab34
commit c6f1a0a
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 16 deletions.
diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs
@@ -540,5 +540,22 @@ public unsafe void ConditionalPointer(bool a, int* ptr)
 		{
 			UsePointer(a ? ptr : null);
 		}
+
+		public unsafe void UseArrayOfPointers(int*[] arr)
+		{
+			for (int i = 0; i < arr.Length; i++) {
+				arr[i] = null;
+			}
+		}
+
+		public unsafe void PassNullPointer1()
+		{
+			PointerReferenceExpression(null);
+		}
+
+		public unsafe void PassNullPointer2()
+		{
+			UseArrayOfPointers(null);
+		}
 	}
 }
diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/IntroduceUnsafeModifier.cs b/ICSharpCode.Decompiler/CSharp/Transforms/IntroduceUnsafeModifier.cs
@@ -121,13 +121,8 @@ public override bool VisitMemberReferenceExpression(MemberReferenceExpression me
 			var rr = memberReferenceExpression.GetResolveResult();
 			if (rr != null)
 			{
-				if (IsPointer(rr.Type))
+				if (IsUnsafeType(rr.Type))
 					return true;
-				if (rr is MemberResolveResult mrr && mrr.Member.ReturnType.Kind == TypeKind.Delegate) {
-					var method = mrr.Member.ReturnType.GetDefinition()?.GetDelegateInvokeMethod();
-					if (method != null && (IsPointer(method.ReturnType) || method.Parameters.Any(p => IsPointer(p.Type))))
-						return true;
-				}
 			}
 
 			return result;
@@ -139,13 +134,8 @@ public override bool VisitIdentifierExpression(IdentifierExpression identifierEx
 			var rr = identifierExpression.GetResolveResult();
 			if (rr != null)
 			{
-				if (IsPointer(rr.Type))
+				if (IsUnsafeType(rr.Type))
 					return true;
-				if (rr is MemberResolveResult mrr && mrr.Member.ReturnType.Kind == TypeKind.Delegate) {
-					var method = mrr.Member.ReturnType.GetDefinition()?.GetDelegateInvokeMethod();
-					if (method != null && (IsPointer(method.ReturnType) || method.Parameters.Any(p => IsPointer(p.Type))))
-						return true;
-				}
 			}
 
 			return result;
@@ -155,7 +145,7 @@ public override bool VisitStackAllocExpression(StackAllocExpression stackAllocEx
 		{
 			bool result = base.VisitStackAllocExpression(stackAllocExpression);
 			var rr = stackAllocExpression.GetResolveResult();
-			if (IsPointer(rr?.Type))
+			if (IsUnsafeType(rr?.Type))
 				return true;
 			return result;
 		}
@@ -164,8 +154,13 @@ public override bool VisitInvocationExpression(InvocationExpression invocationEx
 		{
 			bool result = base.VisitInvocationExpression(invocationExpression);
 			var rr = invocationExpression.GetResolveResult();
-			if (IsPointer(rr?.Type))
+			if (IsUnsafeType(rr?.Type))
 				return true;
+			if ((rr as MemberResolveResult)?.Member is IParameterizedMember pm)
+			{
+				if (pm.Parameters.Any(p => IsUnsafeType(p.Type)))
+					return true;
+			}
 			return result;
 		}
 
@@ -175,15 +170,29 @@ public override bool VisitFixedVariableInitializer(FixedVariableInitializer fixe
 			return true;
 		}
 
-		private bool IsPointer(IType type)
+		private bool IsUnsafeType(IType type)
+		{
+			if (type?.Kind == TypeKind.Delegate)
+			{
+				// Using a delegate which involves pointers in its signature needs the unsafe modifier
+				// https://github.com/icsharpcode/ILSpy/issues/949
+				IMethod invoke = type.GetDelegateInvokeMethod();
+				if (invoke != null && (ContainsPointer(invoke.ReturnType) || invoke.Parameters.Any(p => ContainsPointer(p.Type))))
+					return true;
+			}
+			return ContainsPointer(type);
+		}
+
+		private bool ContainsPointer(IType type)
 		{
 			switch (type?.Kind)
 			{
 				case TypeKind.Pointer:
 				case TypeKind.FunctionPointer:
 					return true;
 				case TypeKind.ByReference:
-					return IsPointer(((ByReferenceType)type).ElementType);
+				case TypeKind.Array:
+					return IsUnsafeType(((Decompiler.TypeSystem.Implementation.TypeWithElementType)type).ElementType);
 				default:
 					return false;
 			}