From 1292ff70d950c6e992f2fea837a6ca3976d8f1c9 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Fri, 7 May 2021 21:48:47 +0200 Subject: [PATCH] Fix #2391: mark method as unsafe when passing `null` to a parameter of pointer type. --- .../TestCases/Pretty/UnsafeCode.cs | 18 ++++++++ .../Transforms/IntroduceUnsafeModifier.cs | 43 +++++++++++-------- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs index 0d20cba7b..784d7d8b2 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/UnsafeCode.cs @@ -561,5 +561,23 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty { 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 index d814221f2..d3bcef5cf 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/IntroduceUnsafeModifier.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/IntroduceUnsafeModifier.cs @@ -130,14 +130,8 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms 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; @@ -149,14 +143,8 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms 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; @@ -166,7 +154,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms { bool result = base.VisitStackAllocExpression(stackAllocExpression); var rr = stackAllocExpression.GetResolveResult(); - if (IsPointer(rr?.Type)) + if (IsUnsafeType(rr?.Type)) return true; return result; } @@ -175,8 +163,13 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms { 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; } @@ -186,7 +179,20 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms 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) { @@ -194,7 +200,8 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms 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; }