diff options
author | Matt Mitchell <mmitche@microsoft.com> | 2015-07-27 12:02:28 -0700 |
---|---|---|
committer | Matt Mitchell <mmitche@microsoft.com> | 2015-07-28 16:05:01 -0700 |
commit | 99dc0f533465a3fab44e62618f584646ee022e6b (patch) | |
tree | f9c789cc461e0806173929889087468adb9bc79e | |
parent | 6b020fe3f7a83aa28f57edd936e1b25ce4b50ca7 (diff) | |
download | coreclr-99dc0f533465a3fab44e62618f584646ee022e6b.tar.gz coreclr-99dc0f533465a3fab44e62618f584646ee022e6b.tar.bz2 coreclr-99dc0f533465a3fab44e62618f584646ee022e6b.zip |
Fix for potential bad tailcall code generation
The JIT can potentially pass an incorrect parameter to a callee when optimizing a tail call. While updating a GT_LCL* node with lcl var num of temp passed to the tail call, we also need to change its opererand to GT_LCL_VAR.
-rw-r--r-- | src/jit/lower.cpp | 2 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_1296/GitHub_1296.cs | 89 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_1296/GitHub_1296.csproj | 50 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_1296/app.config | 27 |
4 files changed, 168 insertions, 0 deletions
diff --git a/src/jit/lower.cpp b/src/jit/lower.cpp index 318649567f..a517732011 100644 --- a/src/jit/lower.cpp +++ b/src/jit/lower.cpp @@ -1727,6 +1727,8 @@ void Lowering::LowerFastTailCall(GenTreeCall *call) } lcl->SetLclNum(tmpLclNum); + lcl->SetOper(GT_LCL_VAR); + } } } diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_1296/GitHub_1296.cs b/tests/src/JIT/Regression/JitBlue/GitHub_1296/GitHub_1296.cs new file mode 100644 index 0000000000..7ad3534a1b --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_1296/GitHub_1296.cs @@ -0,0 +1,89 @@ +using System; + +// Simple struct containing two integers (size 8). +struct MyStruct +{ + public MyStruct(int a, int b) + { + A = a; + B = b; + } + + public int A; + public int B; + public int C { get { return A + B; } } +} + +class Program +{ + + static int Pass = 100; + static int Fail = -1; + static int Main(string[] args) + { + // Entry point for our repro. + // Pass in a bunch of integers. The 5th parameter is a MyStruct, a value type of size 8. + int result = Caller(0, 1, 2, 3, new MyStruct(4, 5)); + if (result != 9) + { + Console.WriteLine("Fail"); + return Fail; + } + else + { + Console.WriteLine("Pass"); + return Pass; + } + } + + // Caller method takes 5 parameters. 4 in register and 1 on the stack. The important details here are that + // * Must take a value type as a parameter + // * The value type must be of size 1, 2, 4 or 8 + // * That parameter must be passed on the stack. Typically this is the 5th parameter and beyond for + // static methods, or 4th and beyond for instance methods. + static int Caller(int regParam1, int regParam2, int regParam3, int regParam4, MyStruct stackParam1) + { + // Add random calls to block inlining of this call into the parent frame. + Console.Write("Let's "); + Console.Write("Discourage "); + Console.Write("Inlining "); + Console.Write("Of "); + Console.Write("Caller "); + Console.Write("Into "); + Console.WriteLine("Main."); + + // Avoid touching stackParam1 except to pass it off to the callee. Any non-trivial usage of + // stackParam1 or other code within this method will likely eliminate the potential for tail + // call optimization. + // if (stackParam1.C == 9) Console.WriteLine("C == 9"); + + // Now make our call. + // The keys here are: + // * That the incoming value type stack parameter must be passed to the callee in register. + // * Tail call optimizations must be enabled + // * The tail call optimization must fire (see above for an example of what might block it). + // The JIT will incorrectly load the outgoing parameter from the incorrect stack location + // on the local frame. + return Callee(stackParam1, regParam1, regParam2, regParam3, regParam4); + } + + // Callee method takes 5 parameters. 4 in register and 1 on the stack. The important details here are that + // * Must take a value type as a parameter + // * The value type must be of size 1, 2, 4 or 8 + // * That parameter must be passed in register. Typically this is the 4th parameter or before for + // static methods, or 3rd or before for instance methods. + static int Callee(MyStruct regParam1, int regParam2, int regParam3, int regParam4, int stackParam1) + { + // If all conditions are met, Callee enters with an incorrect value for regParam1 + // This should print "9 0 1 2 3". If the tail call is made incorrectly, + // the result is (typically) "418858424 0 1 2 3". + System.Console.WriteLine("Printing Outputs: {0} {1} {2} {3} {4}", + regParam1.C, + regParam2, + regParam3, + regParam4, + stackParam1); + + return regParam1.C; + } +}
\ No newline at end of file diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_1296/GitHub_1296.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_1296/GitHub_1296.csproj new file mode 100644 index 0000000000..8f33be95db --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_1296/GitHub_1296.csproj @@ -0,0 +1,50 @@ +<?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> + <SchemaVersion>2.0</SchemaVersion> + <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid> + <OutputType>Exe</OutputType> + <AppDesignerFolder>Properties</AppDesignerFolder> + <FileAlignment>512</FileAlignment> + <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids> + <ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\11.0\UITestExtensionPackages</ReferencePath> + <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir> + + <NuGetPackageImportStamp>7a9bfb7d</NuGetPackageImportStamp> + </PropertyGroup> + <!-- Default configurations to help VS understand the configurations --> + <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "> + </PropertyGroup> + <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "> + </PropertyGroup> + <ItemGroup> + <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies"> + <Visible>False</Visible> + </CodeAnalysisDependentAssemblyPaths> + </ItemGroup> + <PropertyGroup> + <DebugType></DebugType> + <Optimize>True</Optimize> + </PropertyGroup> + <ItemGroup> + <Compile Include="$(MSBuildProjectName).cs" /> + </ItemGroup> + <ItemGroup> + <None Include="$(JitPackagesConfigFileDirectory)minimal\project.json" /> + <None Include="app.config" /> + </ItemGroup> + <ItemGroup> + <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" /> + </ItemGroup> + <PropertyGroup> + <ProjectJson>$(JitPackagesConfigFileDirectory)minimal\project.json</ProjectJson> + <ProjectLockJson>$(JitPackagesConfigFileDirectory)minimal\project.lock.json</ProjectLockJson> + </PropertyGroup> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" /> + <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "> + </PropertyGroup> +</Project> diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_1296/app.config b/tests/src/JIT/Regression/JitBlue/GitHub_1296/app.config new file mode 100644 index 0000000000..62803f5972 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_1296/app.config @@ -0,0 +1,27 @@ +<?xml version="1.0" encoding="utf-8"?> +<configuration> + <runtime> + <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1"> + <dependentAssembly> + <assemblyIdentity name="System.Runtime" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> + <bindingRedirect oldVersion="0.0.0.0-4.0.20.0" newVersion="4.0.20.0" /> + </dependentAssembly> + <dependentAssembly> + <assemblyIdentity name="System.Text.Encoding" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> + <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" /> + </dependentAssembly> + <dependentAssembly> + <assemblyIdentity name="System.Threading.Tasks" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> + <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" /> + </dependentAssembly> + <dependentAssembly> + <assemblyIdentity name="System.IO" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> + <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" /> + </dependentAssembly> + <dependentAssembly> + <assemblyIdentity name="System.Reflection" publicKeyToken="b03f5f7f11d50a3a" culture="neutral" /> + <bindingRedirect oldVersion="0.0.0.0-4.0.10.0" newVersion="4.0.10.0" /> + </dependentAssembly> + </assemblyBinding> + </runtime> +</configuration>
\ No newline at end of file |