From 703d58e9a1a60d79ef86070b9b1ef420cbe5010b Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 22 Nov 2019 11:38:55 +0100 Subject: [PATCH 1/2] Fix crossgen2 handling of direct calls to abstract methods This change fixes two differences between old and new crossgen in the getCallInfo method. Direct calls to abstract methods were being compiled as if they were possible and lead to runtime crash in the call chain from the PreStubWorker. --- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index d38ca882b1dc09..498e7b53a6e08f 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -915,7 +915,7 @@ private void ceeInfoGetCallInfo( // a static method would have never found an instance method. if (originalMethod.Signature.IsStatic && (flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_CALLVIRT) != 0) { - throw new BadImageFormatException(); + throw new RequiresRuntimeJitException("CallVirt to static method"); } exactType = type; @@ -1037,15 +1037,15 @@ private void ceeInfoGetCallInfo( // Static methods are always direct calls directCall = true; } + else if ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_CALLVIRT) == 0 || resolvedConstraint) + { + directCall = true; + } else if (targetMethod.OwningType.IsInterface && targetMethod.IsAbstract) { // Backwards compat: calls to abstract interface methods are treated as callvirt directCall = false; } - else if ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_CALLVIRT) == 0 || resolvedConstraint) - { - directCall = true; - } else { bool devirt; @@ -1090,6 +1090,14 @@ private void ceeInfoGetCallInfo( if (directCall) { + // Direct calls to abstract methods are not allowed + if (targetMethod.IsAbstract && + // Compensate for always treating delegates as direct calls above + !(((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) != 0) && ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_CALLVIRT) != 0) && !resolvedCallVirt)) + { + throw new RequiresRuntimeJitException("Direct calls to abstract methods are not allowed"); + } + bool allowInstParam = (flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_ALLOWINSTPARAM) != 0; if (!allowInstParam && canonMethod.RequiresInstArg()) From dc75b2713ddde8f8405a2c302cafdb9127979353 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Fri, 22 Nov 2019 12:21:24 +0100 Subject: [PATCH 2/2] Change the exception thrown for invalid abstract calls It seems that the InvalidProgramException is more appropriate. --- .../crossgen2/Common/TypeSystem/Common/ExceptionStringID.cs | 2 ++ .../JitInterface/CorInfoImpl.ReadyToRun.cs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Common/ExceptionStringID.cs b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Common/ExceptionStringID.cs index 14800f0e739f96..7de12aa4fe0168 100644 --- a/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Common/ExceptionStringID.cs +++ b/src/coreclr/src/tools/crossgen2/Common/TypeSystem/Common/ExceptionStringID.cs @@ -32,6 +32,8 @@ public enum ExceptionStringID InvalidProgramVararg, InvalidProgramCallVirtFinalize, InvalidProgramNativeCallable, + InvalidProgramCallAbstractMethod, + InvalidProgramCallVirtStatic, // BadImageFormatException BadImageFormatGeneric, diff --git a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 498e7b53a6e08f..70d61f327e35c5 100644 --- a/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/src/tools/crossgen2/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -915,7 +915,7 @@ private void ceeInfoGetCallInfo( // a static method would have never found an instance method. if (originalMethod.Signature.IsStatic && (flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_CALLVIRT) != 0) { - throw new RequiresRuntimeJitException("CallVirt to static method"); + ThrowHelper.ThrowInvalidProgramException(ExceptionStringID.InvalidProgramCallVirtStatic, originalMethod); } exactType = type; @@ -1095,7 +1095,7 @@ private void ceeInfoGetCallInfo( // Compensate for always treating delegates as direct calls above !(((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_LDFTN) != 0) && ((flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_CALLVIRT) != 0) && !resolvedCallVirt)) { - throw new RequiresRuntimeJitException("Direct calls to abstract methods are not allowed"); + ThrowHelper.ThrowInvalidProgramException(ExceptionStringID.InvalidProgramCallAbstractMethod, targetMethod); } bool allowInstParam = (flags & CORINFO_CALLINFO_FLAGS.CORINFO_CALLINFO_ALLOWINSTPARAM) != 0;