Skip to content

Commit

Permalink
Fixes issue dotnet#51612 (dotnet#52231)
Browse files Browse the repository at this point in the history
* Add regression test for dotnet#51612

* Allocate a dummy local when an inlinee needs GSCookie but the root method does not in src/coreclr/jit/fginline.cpp
  • Loading branch information
echesakov authored May 5, 2021
1 parent 4bbaa68 commit 64c679d
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 1 deletion.
10 changes: 9 additions & 1 deletion src/coreclr/jit/fginline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1343,7 +1343,6 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
compLocallocOptimized |= InlineeCompiler->compLocallocOptimized;
compQmarkUsed |= InlineeCompiler->compQmarkUsed;
compUnsafeCastUsed |= InlineeCompiler->compUnsafeCastUsed;
compNeedsGSSecurityCookie |= InlineeCompiler->compNeedsGSSecurityCookie;
compGSReorderStackLayout |= InlineeCompiler->compGSReorderStackLayout;
compHasBackwardJump |= InlineeCompiler->compHasBackwardJump;

Expand Down Expand Up @@ -1398,6 +1397,15 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo)
}
#endif

// If an inlinee needs GS cookie we need to make sure that the cookie will not be allocated at zero stack offset.
// Note that if the root method needs GS cookie then this has already been taken care of.
if (!getNeedsGSSecurityCookie() && InlineeCompiler->getNeedsGSSecurityCookie())
{
setNeedsGSSecurityCookie();
const unsigned dummy = lvaGrabTempWithImplicitUse(false DEBUGARG("GSCookie dummy for inlinee"));
lvaTable[dummy].lvType = TYP_INT;
}

// If there is non-NULL return, replace the GT_CALL with its return value expression,
// so later it will be picked up by the GT_RET_EXPR node.
if ((pInlineInfo->inlineCandidateInfo->fncRetType != TYP_VOID) || (iciCall->gtReturnType == TYP_STRUCT))
Expand Down
81 changes: 81 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_51612/Runtime_51612.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System;
using System.Runtime.CompilerServices;

namespace Runtime_51612
{
class Program
{
struct PassedViaReturnBuffer
{
public int _0;
public int _1;
}

abstract class Base
{
public unsafe abstract PassedViaReturnBuffer HasEspBasedFrame();
}

class Derived : Base
{
public Derived(Exception ex)
{
_ex = ex;
}

readonly Exception _ex;

public override PassedViaReturnBuffer HasEspBasedFrame()
{
DoesNotReturn();
PassedViaReturnBuffer retBuf;
retBuf = RequiresGuardStack();
return retBuf;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
static unsafe PassedViaReturnBuffer RequiresGuardStack()
{
int* p = stackalloc int[2];

p[0] = 1;
p[1] = 2;

PassedViaReturnBuffer retBuf;

retBuf._0 = p[0];
retBuf._1 = p[1];

return retBuf;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
void DoesNotReturn()
{
throw _ex;
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void AssertsThatGuardStackCookieOffsetIsInvalid(Base x)
{
x.HasEspBasedFrame();
}

static int Main(string[] args)
{
try
{
AssertsThatGuardStackCookieOffsetIsInvalid(new Derived(new Exception()));
}
catch (Exception ex)
{
return 100;
}

return 0;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
</PropertyGroup>
<PropertyGroup>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<DebugType>None</DebugType>
<Optimize>True</Optimize>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(MSBuildProjectName).cs" />
</ItemGroup>
</Project>

0 comments on commit 64c679d

Please sign in to comment.