From fd895f1c74712edd9dbee4cf265e7a4d39f55f4c Mon Sep 17 00:00:00 2001
From: Andy Ayers <andya@microsoft.com>
Date: Sat, 2 Jun 2018 00:25:23 -0700
Subject: [PATCH] JIT: fix issue with zero-length stackalloc (#18183)

Defer calling `setNeedsGSSecurityCookie` until we know for sure that
the jit won't do the zero-length stackalloc optimization.

Closes #18176.
---
 src/jit/importer.cpp                               | 15 +++++---
 .../JitBlue/GitHub_18176/GitHub_18176.il           | 45 ++++++++++++++++++++++
 .../JitBlue/GitHub_18176/GitHub_18176.ilproj       | 23 +++++++++++
 3 files changed, 78 insertions(+), 5 deletions(-)
 create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18176/GitHub_18176.il
 create mode 100644 tests/src/JIT/Regression/JitBlue/GitHub_18176/GitHub_18176.ilproj

diff --git a/src/jit/importer.cpp b/src/jit/importer.cpp
index 3e705a8..ef5798f 100644
--- a/src/jit/importer.cpp
+++ b/src/jit/importer.cpp
@@ -14305,8 +14305,6 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                     BADCODE("Localloc can't be inside handler");
                 }
 
-                setNeedsGSSecurityCookie();
-
                 // Get the size to allocate
 
                 op2 = impPopStack().val;
@@ -14355,9 +14353,13 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                                 lvaTable[stackallocAsLocal].lvType           = TYP_BLK;
                                 lvaTable[stackallocAsLocal].lvExactSize      = (unsigned)allocSize;
                                 lvaTable[stackallocAsLocal].lvIsUnsafeBuffer = true;
-                                op1                      = gtNewLclvNode(stackallocAsLocal, TYP_BLK);
-                                op1                      = gtNewOperNode(GT_ADDR, TYP_I_IMPL, op1);
-                                convertedToLocal         = true;
+                                op1              = gtNewLclvNode(stackallocAsLocal, TYP_BLK);
+                                op1              = gtNewOperNode(GT_ADDR, TYP_I_IMPL, op1);
+                                convertedToLocal = true;
+
+                                // Ensure we have stack security for this method.
+                                // Reorder layout since the converted localloc is treated as an unsafe buffer.
+                                setNeedsGSSecurityCookie();
                                 compGSReorderStackLayout = true;
                             }
                         }
@@ -14382,6 +14384,9 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                         // May throw a stack overflow exception. Obviously, we don't want locallocs to be CSE'd.
                         op1->gtFlags |= (GTF_EXCEPT | GTF_DONT_CSE);
 
+                        // Ensure we have stack security for this method.
+                        setNeedsGSSecurityCookie();
+
                         /* The FP register may not be back to the original value at the end
                            of the method, even if the frame size is 0, as localloc may
                            have modified it. So we will HAVE to reset it */
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18176/GitHub_18176.il b/tests/src/JIT/Regression/JitBlue/GitHub_18176/GitHub_18176.il
new file mode 100644
index 0000000..c4b33d8
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_18176/GitHub_18176.il
@@ -0,0 +1,45 @@
+// 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.
+
+.assembly extern mscorlib { auto }
+.assembly GitHub_18176 { }
+
+// Ensure the jit is not tripped up by a foldable zero-length localloc
+
+.class private auto ansi beforefieldinit X
+       extends [mscorlib]System.Object
+{
+  .method private hidebysig static void  Foo(int32* x) cil managed noinlining
+  {
+    ret
+  }
+
+  .method public hidebysig static int32  Main() cil managed
+  {
+    .entrypoint
+    .maxstack  2
+    .locals init (int32* V_0)
+    ldc.i4.0
+    conv.u
+    ldc.i4.4
+    mul.ovf.un
+    localloc
+    stloc.0
+    ldloc.0
+    call       void X::Foo(int32*)
+    ldc.i4.s   100
+    ret
+  }
+
+  .method public hidebysig specialname rtspecialname 
+          instance void  .ctor() cil managed
+  {
+    // Code size       7 (0x7)
+    .maxstack  8
+    ldarg.0
+    call       instance void [mscorlib]System.Object::.ctor()
+    ret
+  }
+
+}
diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_18176/GitHub_18176.ilproj b/tests/src/JIT/Regression/JitBlue/GitHub_18176/GitHub_18176.ilproj
new file mode 100644
index 0000000..5934cf6
--- /dev/null
+++ b/tests/src/JIT/Regression/JitBlue/GitHub_18176/GitHub_18176.ilproj
@@ -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>
-- 
2.7.4