Fix resolveVirtualMethodHelper to check for explicit override cases
authorAndy Ayers <andya@microsoft.com>
Fri, 3 Aug 2018 02:20:48 +0000 (19:20 -0700)
committerAndy Ayers <andya@microsoft.com>
Fri, 3 Aug 2018 02:24:51 +0000 (19:24 -0700)
Explicit method overrides in a class can override a virtual final method
with different method. This is only possible if the different method
first is introduced in a new slot. So when devirtualizing, verify that the
slot of the derived method matches the slot of the base method.

If they don't match, just bail on devirtualizing.

Fixes #19222.

src/vm/jitinterface.cpp
tests/src/JIT/Regression/JitBlue/GitHub_19222/GitHub_19222.il [new file with mode: 0644]
tests/src/JIT/Regression/JitBlue/GitHub_19222/GitHub_19222.ilproj [new file with mode: 0644]

index 42cbc01..114b073 100644 (file)
@@ -8900,6 +8900,20 @@ CORINFO_METHOD_HANDLE CEEInfo::resolveVirtualMethodHelper(CORINFO_METHOD_HANDLE
         // exactly derived class. It is up to the jit to determine whether
         // directly calling this method is correct.
         pDevirtMD = pDerivedMT->GetMethodDescForSlot(slot);
+
+        // If the derived method's slot does not match the vtable slot,
+        // bail on devirtualization, as the method was installed into
+        // the vtable slot via an explicit override and even if the
+        // method is final, the slot may not be.
+        //
+        // Note the jit could still safely devirtualize if it had an exact
+        // class, but such cases are likely rare.
+        WORD dslot = pDevirtMD->GetSlot();
+
+        if (dslot != slot)
+        {
+            return nullptr;
+        }
     }
 
     _ASSERTE(pDevirtMD->IsRestored());
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_19222/GitHub_19222.il b/tests/src/JIT/Regression/JitBlue/GitHub_19222/GitHub_19222.il
new file mode 100644 (file)
index 0000000..d54e1b9
--- /dev/null
@@ -0,0 +1,112 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+// Ensure that the jit doesn't rely on final methods for devirtualization
+// since a final method can be overridden by an explicit .override
+
+.assembly extern mscorlib { auto }
+
+.assembly GitHub_19222 { }
+
+.method assembly static int32 modopt([mscorlib]System.Runtime.CompilerServices.CallConvCdecl) 
+        main() cil managed
+{
+  .entrypoint
+  .maxstack  2
+  .locals ([0] class C c)
+  newobj     instance void C::.ctor()
+  stloc.0
+  newobj     instance void D::.ctor()
+  stloc.0
+  ldloc.0
+  // Must invoke D::f
+  callvirt   instance int32 modopt([mscorlib]System.Runtime.CompilerServices.IsLong) A::f()
+  ret
+}
+
+.class public auto ansi beforefieldinit A
+       extends [mscorlib]System.Object
+{
+  .method public hidebysig newslot virtual 
+          instance int32 modopt([mscorlib]System.Runtime.CompilerServices.IsLong) 
+          f() cil managed
+  {
+    ldc.i4.0
+    ret
+  }
+
+  .method public hidebysig specialname rtspecialname 
+          instance void  .ctor() cil managed
+  {
+    ldarg.0
+    call       instance void [mscorlib]System.Object::.ctor()
+    ret
+  }
+
+}
+
+.class public auto ansi beforefieldinit B
+       extends A
+{
+  .method public hidebysig virtual instance int32 modopt([mscorlib]System.Runtime.CompilerServices.IsLong) 
+          f() cil managed
+  {
+    IL_0000:  ldc.i4.1
+    IL_0001:  ret
+  }
+
+  .method public hidebysig specialname rtspecialname 
+          instance void  .ctor() cil managed
+  {
+    ldarg.0
+    call       instance void [mscorlib]System.Object::.ctor()
+    ret
+  }
+
+}
+
+.class public auto ansi beforefieldinit C
+       extends B
+{
+  .method public hidebysig newslot virtual final 
+          instance int32 modopt([mscorlib]System.Runtime.CompilerServices.IsLong) 
+          f() cil managed
+  {
+    .override A::f
+    ldc.i4.2
+    ret
+  }
+
+  .method public hidebysig specialname rtspecialname 
+          instance void  .ctor() cil managed
+  {
+    ldarg.0
+    call       instance void [mscorlib]System.Object::.ctor()
+    ret
+  }
+
+}
+
+.class public auto ansi beforefieldinit D
+       extends C
+{
+  .method public hidebysig newslot virtual final 
+          instance int32 modopt([mscorlib]System.Runtime.CompilerServices.IsLong) 
+          f() cil managed
+  {
+    .override A::f
+    ldc.i4 100
+    ret
+  }
+
+  .method public hidebysig specialname rtspecialname 
+          instance void  .ctor() cil managed
+  {
+    ldarg.0
+    call       instance void [mscorlib]System.Object::.ctor()
+    ret
+  }
+
+}
+
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_19222/GitHub_19222.ilproj b/tests/src/JIT/Regression/JitBlue/GitHub_19222/GitHub_19222.ilproj
new file mode 100644 (file)
index 0000000..5934cf6
--- /dev/null
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <AssemblyName>$(MSBuildProjectName)</AssemblyName>
+    <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).il" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
+</Project>