Skip to content

Commit

Permalink
Fix ILGenerator maxstack computation (#70388)
Browse files Browse the repository at this point in the history 
* Fix ILGenerator maxstack computation

When generating a dynamic method both with many unconditional jumps, the
ILGenerator's computation of maxstack is way to large - it adds the max
stack of each basic block ending with an unconditional transfer
(br, ret, throw, etc). If the generated code has many such bbs
(eg, from the "then" clauses of if-then-else constructs) this sum can
overflow 2^16. When it does, the maxstack recorded/used is the computed
value mod 2^16. When that is less than the correct value, the
JIT throws an InvalidProgramException.

Keep track of the stack depth at various positions and use it to
calculate an adjustment to the depth.

Fix #63805

* Fix System.Linq.Expressions tests

* Remove stack handling in MethodBuilder
  • Loading branch information
@panguye
panguye authored Jun 22, 2022
1 parent f20a046 commit f310367
Show file tree
Hide file tree
Showing 9 changed files with 538 additions and 143 deletions.
154 changes: 119 additions & 35 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/ILGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ internal static T[] EnlargeArray<T>(T[] incoming, int requiredSize)
private int m_length;
private byte[] m_ILStream;

private int[]? m_labelList;
private __LabelInfo[]? m_labelList;
private int m_labelCount;

private __FixupData[]? m_fixupData;
Expand All @@ -61,10 +61,13 @@ internal static T[] EnlargeArray<T>(T[] incoming, int requiredSize)
internal int m_localCount;
internal SignatureHelper m_localSignature;

private int m_maxStackSize; // Maximum stack size not counting the exceptions.
private int m_curDepth; // Current stack depth, with -1 meaning unknown.
private int m_targetDepth; // Stack depth at a target of the previous instruction (when it is branching).
private int m_maxDepth; // Running max of the stack depth.

private int m_maxMidStack; // Maximum stack size for a given basic block.
private int m_maxMidStackCur; // Running count of the maximum stack size for the current basic block.
// Adjustment to add to m_maxDepth for incorrect/invalid IL. For example, when branch instructions
// with different stack depths target the same label.
private long m_depthAdjustment;

internal int CurrExcStackCount => m_currExcStackCount;

Expand Down Expand Up @@ -131,28 +134,32 @@ internal void UpdateStackSize(OpCode opcode, int stackchange)
// requirements for the function. stackchange specifies the amount
// by which the stacksize needs to be updated.

// Special case for the Return. Returns pops 1 if there is a
// non-void return value.

// Update the running stacksize. m_maxMidStack specifies the maximum
// amount of stack required for the current basic block irrespective of
// where you enter the block.
m_maxMidStackCur += stackchange;
if (m_maxMidStackCur > m_maxMidStack)
m_maxMidStack = m_maxMidStackCur;
else if (m_maxMidStackCur < 0)
m_maxMidStackCur = 0;

// If the current instruction signifies end of a basic, which basically
// means an unconditional branch, add m_maxMidStack to m_maxStackSize.
// m_maxStackSize will eventually be the sum of the stack requirements for
// each basic block.
if (opcode.EndsUncondJmpBlk())
if (m_curDepth < 0)
{
// Current depth is "unknown". We get here when:
// * this is unreachable code.
// * the client uses explicit numeric offsets rather than Labels.
m_curDepth = 0;
}

m_curDepth += stackchange;
if (m_curDepth < 0)
{
m_maxStackSize += m_maxMidStack;
m_maxMidStack = 0;
m_maxMidStackCur = 0;
// Stack underflow. Assume our previous depth computation was flawed.
m_depthAdjustment -= m_curDepth;
m_curDepth = 0;
}
else if (m_maxDepth < m_curDepth)
m_maxDepth = m_curDepth;
Debug.Assert(m_depthAdjustment >= 0);
Debug.Assert(m_curDepth >= 0);

// Record the stack depth at a "target" of this instruction.
m_targetDepth = m_curDepth;

// If the current instruction can't fall through, set the depth to unknown.
if (opcode.EndsUncondJmpBlk())
m_curDepth = -1;
}

private int GetMethodToken(MethodBase method, Type[]? optionalParameterTypes, bool useMethodDef)
Expand Down Expand Up @@ -280,10 +287,11 @@ private int GetLabelPos(Label lbl)
if (index < 0 || index >= m_labelCount || m_labelList is null)
throw new ArgumentException(SR.Argument_BadLabel);

if (m_labelList[index] < 0)
int pos = m_labelList[index].m_pos;
if (pos < 0)
throw new ArgumentException(SR.Argument_BadLabelContent);

return m_labelList[index];
return pos;
}

private void AddFixup(Label lbl, int pos, int instSize)
Expand All @@ -306,11 +314,30 @@ private void AddFixup(Label lbl, int pos, int instSize)
m_fixupLabel = lbl,
m_fixupInstSize = instSize
};

int labelIndex = lbl.GetLabelValue();
if (labelIndex < 0 || labelIndex >= m_labelCount || m_labelList is null)
throw new ArgumentException(SR.Argument_BadLabel);

int depth = m_labelList[labelIndex].m_depth;
int targetDepth = m_targetDepth;
Debug.Assert(depth >= -1);
Debug.Assert(targetDepth >= -1);
if (depth < targetDepth)
{
// Either unknown depth for this label or this branch location has a larger depth than previously recorded.
// In the latter case, the IL is (likely) invalid, but we just compensate for it.
if (depth >= 0)
m_depthAdjustment += targetDepth - depth;
m_labelList[labelIndex].m_depth = targetDepth;
}
}

internal int GetMaxStackSize()
{
return m_maxStackSize;
// Limit the computed max stack to 2^16 - 1, since the value is mod`ed by 2^16 by other code.
Debug.Assert(m_depthAdjustment >= 0);
return (int)Math.Min(ushort.MaxValue, m_maxDepth + m_depthAdjustment);
}

private static void SortExceptions(__ExceptionInfo[] exceptions)
Expand Down Expand Up @@ -926,14 +953,18 @@ public virtual Label BeginExceptionBlock()
m_currExcStack = EnlargeArray(m_currExcStack);
}

Label endLabel = DefineLabel();
Label endLabel = DefineLabel(0);
__ExceptionInfo exceptionInfo = new __ExceptionInfo(m_length, endLabel);

// add the exception to the tracking list
m_exceptions[m_exceptionCount++] = exceptionInfo;

// Make this exception the current active exception
m_currExcStack[m_currExcStackCount++] = exceptionInfo;

// Stack depth for "try" starts at zero.
m_curDepth = 0;

return endLabel;
}

Expand Down Expand Up @@ -969,7 +1000,7 @@ public virtual void EndExceptionBlock()
// Check if we've already set this label.
// The only reason why we might have set this is if we have a finally block.

Label label = m_labelList![endLabel.GetLabelValue()] != -1
Label label = m_labelList![endLabel.GetLabelValue()].m_pos != -1
? current.m_finallyEndLabel
: endLabel;

Expand All @@ -990,6 +1021,9 @@ public virtual void BeginExceptFilterBlock()
Emit(OpCodes.Leave, current.GetEndLabel());

current.MarkFilterAddr(m_length);

// Stack depth for "filter" starts at one.
m_curDepth = 1;
}

public virtual void BeginCatchBlock(Type? exceptionType)
Expand Down Expand Up @@ -1020,6 +1054,9 @@ public virtual void BeginCatchBlock(Type? exceptionType)
}

current.MarkCatchAddr(m_length, exceptionType);

// Stack depth for "catch" starts at one.
m_curDepth = 1;
}

public virtual void BeginFaultBlock()
Expand All @@ -1034,6 +1071,9 @@ public virtual void BeginFaultBlock()
Emit(OpCodes.Leave, current.GetEndLabel());

current.MarkFaultAddr(m_length);

// Stack depth for "fault" starts at zero.
m_curDepth = 0;
}

public virtual void BeginFinallyBlock()
Expand All @@ -1055,33 +1095,44 @@ public virtual void BeginFinallyBlock()

MarkLabel(endLabel);

Label finallyEndLabel = DefineLabel();
Label finallyEndLabel = DefineLabel(0);
current.SetFinallyEndLabel(finallyEndLabel);

// generate leave for try clause
Emit(OpCodes.Leave, finallyEndLabel);
if (catchEndAddr == 0)
catchEndAddr = m_length;
current.MarkFinallyAddr(m_length, catchEndAddr);

// Stack depth for "finally" starts at zero.
m_curDepth = 0;
}

#endregion

#region Labels
public virtual Label DefineLabel()
{
// We don't know the stack depth at the label yet, so set it to -1.
return DefineLabel(-1);
}

private Label DefineLabel(int depth)
{
// Declares a new Label. This is just a token and does not yet represent any particular location
// within the stream. In order to set the position of the label within the stream, you must call
// Mark Label.
Debug.Assert(depth >= -1);

// Delay init the lable array in case we dont use it
m_labelList ??= new int[DefaultLabelArraySize];
// Delay init the label array in case we dont use it
m_labelList ??= new __LabelInfo[DefaultLabelArraySize];

if (m_labelCount >= m_labelList.Length)
{
m_labelList = EnlargeArray(m_labelList);
}
m_labelList[m_labelCount] = -1;
m_labelList[m_labelCount].m_pos = -1;
m_labelList[m_labelCount].m_depth = depth;
return new Label(m_labelCount++);
}

Expand All @@ -1098,12 +1149,39 @@ public virtual void MarkLabel(Label loc)
throw new ArgumentException(SR.Argument_InvalidLabel);
}

if (m_labelList[labelIndex] != -1)
if (m_labelList[labelIndex].m_pos != -1)
{
throw new ArgumentException(SR.Argument_RedefinedLabel);
}

m_labelList[labelIndex] = m_length;
m_labelList[labelIndex].m_pos = m_length;

int depth = m_labelList[labelIndex].m_depth;
if (depth < 0)
{
// Unknown depth for this label, indicating that it hasn't been used yet.
// If m_curDepth is unknown, we're in the Backward branch constraint case. See ECMA-335 III.1.7.5.
// The m_depthAdjustment field will compensate for violations of this constraint, as we
// discover them. That is, here we assume a depth of zero. If a (later) branch to this label
// has a positive stack depth, we'll record that as the new depth and add the delta into
// m_depthAdjustment.
if (m_curDepth < 0)
m_curDepth = 0;
m_labelList[labelIndex].m_depth = m_curDepth;
}
else if (depth < m_curDepth)
{
// A branch location with smaller stack targets this label. In this case, the IL is
// invalid, but we just compensate for it.
m_depthAdjustment += m_curDepth - depth;
m_labelList[labelIndex].m_depth = m_curDepth;
}
else if (depth > m_curDepth)
{
// Either the current depth is unknown, or a branch location with larger stack targets
// this label, so the IL is invalid. In either case, just adjust the current depth.
m_curDepth = depth;
}
}

#endregion
Expand Down Expand Up @@ -1287,6 +1365,12 @@ public virtual void EndScope()
#endregion
}

internal struct __LabelInfo
{
internal int m_pos; // Position in the il stream, with -1 meaning unknown.
internal int m_depth; // Stack depth, with -1 meaning unknown.
}

internal struct __FixupData
{
internal Label m_fixupLabel;
Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/MethodBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ internal int GetMaxStack()
{
if (m_ilGenerator != null)
{
return m_ilGenerator.GetMaxStackSize() + ExceptionHandlerCount;
return m_ilGenerator.GetMaxStackSize();
}
else
{
Expand All @@ -323,8 +323,6 @@ internal int GetMaxStack()
return m_exceptions;
}

internal int ExceptionHandlerCount => m_exceptions != null ? m_exceptions.Length : 0;

internal static int CalculateNumberOfExceptions(__ExceptionInfo[]? excp)
{
int num = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Linq.Expressions/tests/BinaryOperators/Coalesce/BinaryCoalesceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ public static void VerifyIL_NullableIntCoalesceToNullableInt()
f.VerifyIL(
@".method valuetype [System.Private.CoreLib]System.Nullable`1<int32> ::lambda_method(class [System.Linq.Expressions]System.Runtime.CompilerServices.Closure,valuetype [System.Private.CoreLib]System.Nullable`1<int32>,valuetype [System.Private.CoreLib]System.Nullable`1<int32>)
{
.maxstack 2
.maxstack 1
.locals init (
[0] valuetype [System.Private.CoreLib]System.Nullable`1<int32>
)
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Linq.Expressions/tests/CompilerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ public static void VerifyIL_Exceptions()
f.VerifyIL(
@".method int32 ::lambda_method(class [System.Linq.Expressions]System.Runtime.CompilerServices.Closure,int32)
{
.maxstack 4
.maxstack 2
.locals init (
[0] int32
)
Expand Down
Loading

0 comments on commit f310367

Please sign in to comment.