From a997f3cc4e40bf0f73825c1ad726fbc17fe1c398 Mon Sep 17 00:00:00 2001 From: Adeel Mujahid <3840695+am11@users.noreply.github.com> Date: Fri, 13 Aug 2021 13:24:30 +0300 Subject: [PATCH] Add support for function pointers in ilverify (#57163) * Provide logic verifying access to function pointer. Extend AccessVerificationHelpers.CanAccess method so that it is able to handle FunctionPointerType by making sure that a current class can access all the types returned or accepted by the method pointed by the given function pointer. Fix #43502 * Add support for function pointers in ilverify importer * Add ilverify tests for function pointers * Trim trailing whitespace * Use public surface of FunctionPointerType * Check both return type and arguments of signature * Delete CanAccessMethodSignature and simplify * Add a test case for inaccessible return type --- .../ILVerification/AccessVerificationHelpers.cs | 48 ++-------- .../tools/ILVerification/ILImporter.StackValue.cs | 27 +++--- .../tools/ILVerification/ILImporter.Verify.cs | 3 +- src/tests/ilverify/ILMethodTester.cs | 2 +- src/tests/ilverify/ILTests/FunctionPointerTests.il | 103 +++++++++++++++++++++ .../ilverify/ILTests/FunctionPointerTests.ilproj | 3 + 6 files changed, 132 insertions(+), 54 deletions(-) create mode 100644 src/tests/ilverify/ILTests/FunctionPointerTests.il create mode 100644 src/tests/ilverify/ILTests/FunctionPointerTests.ilproj diff --git a/src/coreclr/tools/ILVerification/AccessVerificationHelpers.cs b/src/coreclr/tools/ILVerification/AccessVerificationHelpers.cs index 82cf528..ef694cb 100644 --- a/src/coreclr/tools/ILVerification/AccessVerificationHelpers.cs +++ b/src/coreclr/tools/ILVerification/AccessVerificationHelpers.cs @@ -22,19 +22,8 @@ namespace ILVerify if (targetClass.IsParameterizedType) return currentClass.CanAccess(((ParameterizedType)targetClass).ParameterType); -#if false - // perform transparency check on the type, if the caller is transparent - if ((NULL != pCurrentMD) && Security::IsTransparentMethod(pCurrentMD)) - { - // check if type is visible outside the assembly - if (!IsTypeVisibleOutsideAssembly(pTargetClass)) - { - // check transparent/critical on type - if (!Security::CheckNonPublicCriticalAccess(pCurrentMD, NULL, NULL, pTargetClass)) - return FALSE; - } - } -#endif + if (targetClass.IsFunctionPointer) + return currentClass.CanAccessSignature(((FunctionPointerType)targetClass).Signature); // Check access to class instantiations if generic class if (targetClass.HasInstantiation && !currentClass.CanAccessInstantiation(targetClass.Instantiation)) @@ -80,7 +69,7 @@ namespace ILVerify return false; } - return currentTypeDef.CanAccessMethodSignature(targetMethod); + return currentTypeDef.CanAccessSignature(targetMethod.Signature); } /// @@ -180,33 +169,14 @@ namespace ILVerify return true; } - private static bool CanAccessMethodSignature(this TypeDesc currentType, MethodDesc targetMethod) + private static bool CanAccessSignature(this TypeDesc currentType, MethodSignature signature) { - var methodSig = targetMethod.Signature; - - // Check return type - var returnType = methodSig.ReturnType; - if (returnType.IsParameterizedType) - returnType = ((ParameterizedType)returnType).ParameterType; - - if (!returnType.IsGenericParameter && !returnType.IsSignatureVariable // Generic parameters are always accessible - && !returnType.IsVoid) - { - if (!currentType.CanAccess(returnType)) - return false; - } + if (!currentType.CanAccess(signature.ReturnType)) + return false; - // Check arguments - for (int i = 0; i < methodSig.Length; ++i) + for (int i = 0; i < signature.Length; ++i) { - var param = methodSig[i]; - if (param.IsByRef) - param = ((ByRefType)param).ParameterType; - - if (param.IsGenericParameter || param.IsSignatureVariable) - continue; // Generic parameters are always accessible - - if (!currentType.CanAccess(param)) + if (!currentType.CanAccess(signature[i])) return false; } @@ -258,7 +228,7 @@ namespace ILVerify private static EcmaAssembly ToEcmaAssembly(this ModuleDesc module) { return module.Assembly as EcmaAssembly; - } + } private static bool GrantsFriendAccessTo(this ModuleDesc module, ModuleDesc friendModule) { diff --git a/src/coreclr/tools/ILVerification/ILImporter.StackValue.cs b/src/coreclr/tools/ILVerification/ILImporter.StackValue.cs index f6ba568..b3005d4 100644 --- a/src/coreclr/tools/ILVerification/ILImporter.StackValue.cs +++ b/src/coreclr/tools/ILVerification/ILImporter.StackValue.cs @@ -93,8 +93,8 @@ namespace Internal.IL static public StackValue CreatePrimitive(StackValueKind kind) { - Debug.Assert(kind == StackValueKind.Int32 || - kind == StackValueKind.Int64 || + Debug.Assert(kind == StackValueKind.Int32 || + kind == StackValueKind.Int64 || kind == StackValueKind.NativeInt || kind == StackValueKind.Float); @@ -114,7 +114,7 @@ namespace Internal.IL static public StackValue CreateByRef(TypeDesc type, bool readOnly = false, bool permanentHome = false) { return new StackValue(StackValueKind.ByRef, type, null, - (readOnly ? StackValueFlags.ReadOnly : StackValueFlags.None) | + (readOnly ? StackValueFlags.ReadOnly : StackValueFlags.None) | (permanentHome ? StackValueFlags.PermanentHome : StackValueFlags.None)); } @@ -145,6 +145,7 @@ namespace Internal.IL case TypeFlags.IntPtr: case TypeFlags.UIntPtr: case TypeFlags.Pointer: + case TypeFlags.FunctionPointer: return CreatePrimitive(StackValueKind.NativeInt); case TypeFlags.Enum: return CreateFromType(type.UnderlyingType); @@ -241,7 +242,7 @@ namespace Internal.IL partial class ILImporter { /// - /// Merges two stack values to a common stack value as defined in the ECMA-335 + /// Merges two stack values to a common stack value as defined in the ECMA-335 /// standard III.1.8.1.3 (Merging stack states). /// /// The value to be merged with . @@ -278,7 +279,7 @@ namespace Internal.IL return true; // Merging classes always succeeds since System.Object always works - merged = StackValue.CreateFromType(MergeObjectReferences(valueA.Type, valueB.Type)); + merged = StackValue.CreateFromType(MergeObjectReferences(valueA.Type, valueB.Type)); return true; } @@ -443,7 +444,7 @@ namespace Internal.IL mergedElementType = MergeArrayTypes(arrayTypeA, arrayTypeB); } //Both array element types are ObjRefs - else if ((!arrayTypeA.ElementType.IsValueType && !arrayTypeA.ElementType.IsByRef) && + else if ((!arrayTypeA.ElementType.IsValueType && !arrayTypeA.ElementType.IsByRef) && (!arrayTypeB.ElementType.IsValueType && !arrayTypeB.ElementType.IsByRef)) { // Find common ancestor of the element types @@ -544,9 +545,9 @@ namespace Internal.IL } // Normally we just let the runtime sort it out but we wish to be more strict - // than the runtime wants to be. For backwards compatibility, the runtime considers + // than the runtime wants to be. For backwards compatibility, the runtime considers // int32[] and nativeInt[] to be the same on 32-bit machines. It also is OK with - // int64[] and nativeInt[] on a 64-bit machine. + // int64[] and nativeInt[] on a 64-bit machine. if (child.IsType(TI_REF) && parent.IsType(TI_REF) && jitInfo->isSDArray(child.GetClassHandleForObjRef()) @@ -568,16 +569,16 @@ namespace Internal.IL cType = CORINFO_TYPE_NATIVEINT; if (pType == CORINFO_TYPE_NATIVEUINT) pType = CORINFO_TYPE_NATIVEINT; - + if (cType == CORINFO_TYPE_NATIVEINT) return pType == CORINFO_TYPE_NATIVEINT; - + if (pType == CORINFO_TYPE_NATIVEINT) return cType == CORINFO_TYPE_NATIVEINT; return runtime_OK; } - + if (parent.IsUnboxedGenericTypeVar() || child.IsUnboxedGenericTypeVar()) { return (FALSE); // need to have had child == parent @@ -634,8 +635,8 @@ namespace Internal.IL { case StackValueKind.ObjRef: // ECMA-335 III.1.5 Operand type table, P. 303: - // __cgt.un__ is allowed and verifiable on ObjectRefs (O). This is commonly used when - // comparing an ObjectRef with null(there is no "compare - not - equal" instruction, which + // __cgt.un__ is allowed and verifiable on ObjectRefs (O). This is commonly used when + // comparing an ObjectRef with null(there is no "compare - not - equal" instruction, which // would otherwise be a more obvious solution) return op == ILOpcode.beq || op == ILOpcode.beq_s || op == ILOpcode.bne_un || op == ILOpcode.bne_un_s || diff --git a/src/coreclr/tools/ILVerification/ILImporter.Verify.cs b/src/coreclr/tools/ILVerification/ILImporter.Verify.cs index 6540aaf..2341e70 100644 --- a/src/coreclr/tools/ILVerification/ILImporter.Verify.cs +++ b/src/coreclr/tools/ILVerification/ILImporter.Verify.cs @@ -535,7 +535,8 @@ again: var args = new ErrorArgument[] { new ErrorArgument("Offset", _currentInstructionOffset), - new ErrorArgument("Found", found.ToString()) + new ErrorArgument("Found", found.ToString()), + new ErrorArgument("Expected", expected.ToString()) }; ReportVerificationError(args, error); } diff --git a/src/tests/ilverify/ILMethodTester.cs b/src/tests/ilverify/ILMethodTester.cs index cb5a316..83865a9 100644 --- a/src/tests/ilverify/ILMethodTester.cs +++ b/src/tests/ilverify/ILMethodTester.cs @@ -48,7 +48,7 @@ namespace ILVerification.Tests foreach (var item in invalidIL.ExpectedVerifierErrors) { var actual = results.Select(e => e.Code.ToString()); - Assert.True(results.Where(r => r.Code == item).Count() > 0, $"Actual errors where: {string.Join(",", actual)}"); + Assert.True(results.Where(r => r.Code == item).Count() > 0, $"Actual errors were: {string.Join(",", actual)}"); } } } diff --git a/src/tests/ilverify/ILTests/FunctionPointerTests.il b/src/tests/ilverify/ILTests/FunctionPointerTests.il new file mode 100644 index 0000000..e7ac6b6 --- /dev/null +++ b/src/tests/ilverify/ILTests/FunctionPointerTests.il @@ -0,0 +1,103 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +.assembly extern System.Runtime +{ +} + +.assembly FunctionPointerTests +{ +} + +.class public auto ansi beforefieldinit C + extends [System.Runtime]System.Object +{ + .method private hidebysig instance void MyDispatcher(method void *(int32) f) cil managed + { + .locals init ( + [0] method void *(int32) + ) + + ldarg.1 + stloc.0 + ldc.i4.s 42 + ldloc.0 + calli void(int32) + ret + } + + .method private hidebysig static void StaticMethod(int32 x) cil managed + { + ldarg.0 + ldc.i4 571 + add + starg.s x + ret + } + + .method public hidebysig instance void Call.Dispatcher_Valid() cil managed + { + newobj instance void C::.ctor() + ldftn void C::StaticMethod(int32) + call instance void C::MyDispatcher(method void *(int32)) + ret + } + + .method public hidebysig instance void Call.Dispatcher_Invalid_MethodAccess() cil managed + { + newobj instance void C::.ctor() + ldftn void D::PrivateStaticMethodOfOtherType(int32) + call instance void C::MyDispatcher(method void *(int32)) + ret + } + + .method private hidebysig instance void ReceiverOfPrivateType(method class D/D_Private *(int32) ptr) cil managed + { + ret + } + + .method public hidebysig instance void Call.SendFunctionPointerWithInaccessibleReturnType_Invalid_MethodAccess_MethodAccess() cil managed + { + newobj instance void C::.ctor() + ldftn class D/D_Private D::StaticMethodReturningPrivateTypeInstance(int32) + call instance void C::ReceiverOfPrivateType(method class D/D_Private *(int32)) + ret + } + + .method public hidebysig specialname rtspecialname instance void .ctor() cil managed + { + ldarg.0 + call instance void [System.Runtime]System.Object::.ctor() + ret + } +} + +.class public auto ansi beforefieldinit D + extends [System.Runtime]System.Object +{ + .method private hidebysig static void PrivateStaticMethodOfOtherType(int32 x) cil managed + { + ldarg.0 + ldc.i4 571 + add + starg.s x + ret + } + + .method public hidebysig static class D/D_Private StaticMethodReturningPrivateTypeInstance(int32 x) cil managed + { + newobj instance void D/D_Private::.ctor() + ret + } + + .class nested private auto ansi beforefieldinit D_Private + extends [System.Runtime]System.Object + { + .method public hidebysig specialname rtspecialname instance void .ctor() cil managed + { + ldarg.0 + call instance void [System.Runtime]System.Object::.ctor() + ret + } + } +} diff --git a/src/tests/ilverify/ILTests/FunctionPointerTests.ilproj b/src/tests/ilverify/ILTests/FunctionPointerTests.ilproj new file mode 100644 index 0000000..8e8765d --- /dev/null +++ b/src/tests/ilverify/ILTests/FunctionPointerTests.ilproj @@ -0,0 +1,3 @@ + + + -- 2.7.4