Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix break in try-catch-finally #1279

Closed
wants to merge 4 commits into from
Closed

fix break in try-catch-finally #1279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
980c6e7
fix break in try-catch-finally
Hecate2 Jan 15, 2025
1eceb00
break from multiple try
Hecate2 Jan 16, 2025
e0ed16d
remove comments
Hecate2 Jan 16, 2025
4dca045
control exception in test
Hecate2 Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions src/Neo.Compiler.CSharp/MethodConvert/Statement/BreakStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

using Microsoft.CodeAnalysis.CSharp.Syntax;
using Neo.VM;
using System.Collections.Generic;
using System.Linq;

namespace Neo.Compiler
{
Expand Down Expand Up @@ -44,10 +46,38 @@ internal partial class MethodConvert
private void ConvertBreakStatement(BreakStatementSyntax syntax)
{
using (InsertSequencePoint(syntax))
if (_tryStack.TryPeek(out ExceptionHandling? result) && result.BreakTargetCount == 0)
Jump(OpCode.ENDTRY_L, _breakTargets.Peek());
else
Jump(OpCode.JMP_L, _breakTargets.Peek());
{
JumpTarget? breakTarget = null;
List<StatementContext> visitedTry = []; // from shallow to deep
foreach (StatementContext sc in _generalStatementStack)
{// start from the deepest context
// find the final break target
if (sc.BreakTarget != null)
{
breakTarget = sc.BreakTarget;
break;
}
// stage the try stacks on the way
if (sc.StatementSyntax is TryStatementSyntax)
visitedTry.Add(sc);
}
if (breakTarget == null)
// break is not handled
throw new CompilationException(syntax, DiagnosticId.SyntaxNotSupported, $"Cannot find what to break. " +
$"If not syntax error, this is probably a compiler bug. " +
$"Check whether the compiler is leaving out a push into {nameof(_generalStatementStack)}.");

foreach (StatementContext sc in visitedTry)
// start from the most external try
// internal try should ENDTRY, targeting the correct external break target
breakTarget = sc.AddEndTry(breakTarget);

Jump(OpCode.JMP_L, breakTarget);
// We could use ENDTRY if current statement calling `break` is a try statement,
// but this job can be done by the optimizer
// Note that, do not Jump(OpCode.ENDTRY_L, breakTarget) here,
// because the breakTarget here is already an ENDTRY_L for current try stack.
}
}
}
}
16 changes: 16 additions & 0 deletions src/Neo.Compiler.CSharp/MethodConvert/Statement/CommonForEachStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ private void ConvertIteratorForEachStatement(SemanticModel model, ForEachStateme
byte elementIndex = AddLocalVariable(elementSymbol);
PushContinueTarget(continueTarget);
PushBreakTarget(breakTarget);
StatementContext sc = new(syntax, breakTarget: breakTarget, continueTarget: continueTarget);
_generalStatementStack.Push(sc);
using (InsertSequencePoint(syntax.ForEachKeyword))
{
ConvertExpression(model, syntax.Expression);
Expand All @@ -97,6 +99,8 @@ private void ConvertIteratorForEachStatement(SemanticModel model, ForEachStateme
RemoveLocalVariable(elementSymbol);
PopContinueTarget();
PopBreakTarget();
if (_generalStatementStack.Pop() != sc)
throw new CompilationException(syntax, DiagnosticId.SyntaxNotSupported, $"Bad statement stack handling inside. This is a compiler bug.");
}

/// <summary>
Expand Down Expand Up @@ -151,6 +155,8 @@ private void ConvertIteratorForEachVariableStatement(SemanticModel model, ForEac
byte iteratorIndex = AddAnonymousVariable();
PushContinueTarget(continueTarget);
PushBreakTarget(breakTarget);
StatementContext sc = new(syntax, breakTarget: breakTarget, continueTarget: continueTarget);
_generalStatementStack.Push(sc);
using (InsertSequencePoint(syntax.ForEachKeyword))
{
ConvertExpression(model, syntax.Expression);
Expand Down Expand Up @@ -190,6 +196,8 @@ private void ConvertIteratorForEachVariableStatement(SemanticModel model, ForEac
RemoveLocalVariable(symbol);
PopContinueTarget();
PopBreakTarget();
if (_generalStatementStack.Pop() != sc)
throw new CompilationException(syntax, DiagnosticId.SyntaxNotSupported, $"Bad statement stack handling inside. This is a compiler bug.");
}

/// <summary>
Expand Down Expand Up @@ -220,6 +228,8 @@ private void ConvertArrayForEachStatement(SemanticModel model, ForEachStatementS
byte elementIndex = AddLocalVariable(elementSymbol);
PushContinueTarget(continueTarget);
PushBreakTarget(breakTarget);
StatementContext sc = new(syntax, breakTarget: breakTarget, continueTarget: continueTarget);
_generalStatementStack.Push(sc);
using (InsertSequencePoint(syntax.ForEachKeyword))
{
ConvertExpression(model, syntax.Expression);
Expand Down Expand Up @@ -255,6 +265,8 @@ private void ConvertArrayForEachStatement(SemanticModel model, ForEachStatementS
RemoveLocalVariable(elementSymbol);
PopContinueTarget();
PopBreakTarget();
if (_generalStatementStack.Pop() != sc)
throw new CompilationException(syntax, DiagnosticId.SyntaxNotSupported, $"Bad statement stack handling inside. This is a compiler bug.");
}

/// <summary>
Expand Down Expand Up @@ -284,6 +296,8 @@ private void ConvertArrayForEachVariableStatement(SemanticModel model, ForEachVa
byte iIndex = AddAnonymousVariable();
PushContinueTarget(continueTarget);
PushBreakTarget(breakTarget);
StatementContext sc = new(syntax, breakTarget: breakTarget, continueTarget: continueTarget);
_generalStatementStack.Push(sc);
using (InsertSequencePoint(syntax.ForEachKeyword))
{
ConvertExpression(model, syntax.Expression);
Expand Down Expand Up @@ -334,6 +348,8 @@ private void ConvertArrayForEachVariableStatement(SemanticModel model, ForEachVa
RemoveLocalVariable(symbol);
PopContinueTarget();
PopBreakTarget();
if (_generalStatementStack.Pop() != sc)
throw new CompilationException(syntax, DiagnosticId.SyntaxNotSupported, $"Bad statement stack handling inside. This is a compiler bug.");
}
}
}
4 changes: 4 additions & 0 deletions src/Neo.Compiler.CSharp/MethodConvert/Statement/DoStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ private void ConvertDoStatement(SemanticModel model, DoStatementSyntax syntax)
JumpTarget breakTarget = new();
PushContinueTarget(continueTarget);
PushBreakTarget(breakTarget);
StatementContext sc = new(syntax, breakTarget: breakTarget, continueTarget: continueTarget);
_generalStatementStack.Push(sc);
startTarget.Instruction = AddInstruction(OpCode.NOP);
ConvertStatement(model, syntax.Statement);
continueTarget.Instruction = AddInstruction(OpCode.NOP);
Expand All @@ -56,6 +58,8 @@ private void ConvertDoStatement(SemanticModel model, DoStatementSyntax syntax)
breakTarget.Instruction = AddInstruction(OpCode.NOP);
PopContinueTarget();
PopBreakTarget();
if (_generalStatementStack.Pop() != sc)
throw new CompilationException(syntax, DiagnosticId.SyntaxNotSupported, $"Bad statement stack handling inside. This is a compiler bug.");
}
}
}
4 changes: 4 additions & 0 deletions src/Neo.Compiler.CSharp/MethodConvert/Statement/ForStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ private void ConvertForStatement(SemanticModel model, ForStatementSyntax syntax)
JumpTarget breakTarget = new();
PushContinueTarget(continueTarget);
PushBreakTarget(breakTarget);
StatementContext sc = new(syntax, breakTarget: breakTarget, continueTarget: continueTarget);
_generalStatementStack.Push(sc);
foreach (var (variable, symbol) in variables)
{
byte variableIndex = AddLocalVariable(symbol);
Expand Down Expand Up @@ -99,6 +101,8 @@ private void ConvertForStatement(SemanticModel model, ForStatementSyntax syntax)
RemoveLocalVariable(symbol);
PopContinueTarget();
PopBreakTarget();
if (_generalStatementStack.Pop() != sc)
throw new CompilationException(syntax, DiagnosticId.SyntaxNotSupported, $"Bad statement stack handling inside. This is a compiler bug.");
}
}
}
59 changes: 59 additions & 0 deletions src/Neo.Compiler.CSharp/MethodConvert/Statement/Statement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,70 @@

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Neo.VM;
using System.Collections.Generic;

namespace Neo.Compiler
{
internal partial class MethodConvert
{
/// <summary>
/// Store the contexts of goto, break, continue targets
/// </summary>
internal class StatementContext(StatementSyntax statementSyntax,
JumpTarget? breakTarget = null, JumpTarget? continueTarget = null,
ExceptionHandlingState? tryState = null,
JumpTarget? catchTarget = null, JumpTarget? finallyTarget = null, JumpTarget? endFinallyTarget = null,
Dictionary<ILabelSymbol, JumpTarget>? gotoLabels = null,
Dictionary<SwitchLabelSyntax, JumpTarget>? switchLabels = null
)
{
public readonly StatementSyntax StatementSyntax = statementSyntax;
public readonly JumpTarget? BreakTarget = breakTarget;
public readonly JumpTarget? ContinueTarget = continueTarget;
public ExceptionHandlingState? TryState = tryState;
public readonly JumpTarget? CatchTarget = catchTarget;
public readonly JumpTarget? FinallyTarget = finallyTarget;
public readonly JumpTarget? EndFinallyTarget = endFinallyTarget;
public Dictionary<ILabelSymbol, JumpTarget>? GotoLabels = gotoLabels;
public Dictionary<SwitchLabelSyntax, JumpTarget>? SwitchLabels = switchLabels;
// handles `break`, `continue` and `goto` in multi-layered nested try with finally
// key: target of this ENDTRY
// value: this ENDTRY
public Dictionary<JumpTarget, JumpTarget>? AdditionalEndTryTargetToInstruction { get; protected set; } = null;

/// <param name="target">Jump target of this added ENDTRY</param>
/// <returns>The added ENDTRY</returns>
/// <exception cref="CompilationException"></exception>
public JumpTarget AddEndTry(JumpTarget target)
{
if (StatementSyntax is not TryStatementSyntax)
throw new CompilationException(StatementSyntax, DiagnosticId.SyntaxNotSupported, $"Can only append ENDTRY for TryStatement. Got {typeof(StatementSyntax)} {StatementSyntax}. This is a compiler bug.");
AdditionalEndTryTargetToInstruction ??= [];
if (AdditionalEndTryTargetToInstruction.TryGetValue(target, out JumpTarget? existingEndTry))
return existingEndTry;
Instruction i = new() { OpCode = OpCode.ENDTRY_L, Target = target };
existingEndTry = new JumpTarget() { Instruction = i };
AdditionalEndTryTargetToInstruction.Add(target, existingEndTry);
return existingEndTry;
}

public bool AddLabel(ILabelSymbol label, JumpTarget target)
{
GotoLabels ??= [];
return GotoLabels.TryAdd(label, target);
}
public bool AddLabel(SwitchLabelSyntax label, JumpTarget target)
{
SwitchLabels ??= [];
return SwitchLabels.TryAdd(label, target);
}
public bool ContainsLabel(ILabelSymbol label) => GotoLabels is not null && GotoLabels.ContainsKey(label);
public bool ContainsLabel(SwitchLabelSyntax label) => SwitchLabels is not null && SwitchLabels.ContainsKey(label);
}

private readonly Stack<StatementContext> _generalStatementStack = new();

private void ConvertStatement(SemanticModel model, StatementSyntax statement)
{
switch (statement)
Expand Down
4 changes: 4 additions & 0 deletions src/Neo.Compiler.CSharp/MethodConvert/Statement/SwitchStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ private void ConvertSwitchStatement(SemanticModel model, SwitchStatementSyntax s
JumpTarget breakTarget = new();
byte anonymousIndex = AddAnonymousVariable();
PushBreakTarget(breakTarget);
StatementContext sc = new(syntax, breakTarget: breakTarget, switchLabels: labels.ToDictionary());
_generalStatementStack.Push(sc);
using (InsertSequencePoint(syntax.Expression))
{
ConvertExpression(model, syntax.Expression);
Expand Down Expand Up @@ -111,6 +113,8 @@ private void ConvertSwitchStatement(SemanticModel model, SwitchStatementSyntax s
breakTarget.Instruction = AddInstruction(OpCode.NOP);
PopSwitchLabels();
PopBreakTarget();
if (_generalStatementStack.Pop() != sc)
throw new CompilationException(syntax, DiagnosticId.SyntaxNotSupported, $"Bad statement stack handling inside. This is a compiler bug.");
}
}
}
16 changes: 16 additions & 0 deletions src/Neo.Compiler.CSharp/MethodConvert/Statement/TryStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,17 @@ private void ConvertTryStatement(SemanticModel model, TryStatementSyntax syntax)
JumpTarget catchTarget = new();
JumpTarget finallyTarget = new();
JumpTarget endTarget = new();
StatementContext sc = new(syntax, catchTarget: catchTarget, finallyTarget: finallyTarget, endFinallyTarget: endTarget, tryState: ExceptionHandlingState.Try);
_generalStatementStack.Push(sc);
AddInstruction(new Instruction { OpCode = OpCode.TRY_L, Target = catchTarget, Target2 = finallyTarget });
_tryStack.Push(new ExceptionHandling { State = ExceptionHandlingState.Try });
ConvertStatement(model, syntax.Block);
Jump(OpCode.ENDTRY_L, endTarget);
if (syntax.Catches.Count == 0 && sc.AdditionalEndTryTargetToInstruction != null)
// handles `break`, `continue` and `goto` in multi-layered nested try with finally
foreach (JumpTarget i in sc.AdditionalEndTryTargetToInstruction.Values)
AddInstruction(i.Instruction!);

if (syntax.Catches.Count > 1)
throw new CompilationException(syntax.Catches[1], DiagnosticId.MultiplyCatches, "Only support one single catch.");
if (syntax.Catches.Count > 0)
Expand All @@ -65,6 +72,7 @@ private void ConvertTryStatement(SemanticModel model, TryStatementSyntax syntax)
if (catchClause.Filter is not null)
throw new CompilationException(catchClause.Filter, DiagnosticId.CatchFilter, $"Unsupported syntax: {catchClause.Filter}");
_tryStack.Peek().State = ExceptionHandlingState.Catch;
sc.TryState = ExceptionHandlingState.Catch;
ILocalSymbol? exceptionSymbol = null;
byte exceptionIndex;
if (catchClause.Declaration is null)
Expand All @@ -83,6 +91,11 @@ private void ConvertTryStatement(SemanticModel model, TryStatementSyntax syntax)
_exceptionStack.Push(exceptionIndex);
ConvertStatement(model, catchClause.Block);
Jump(OpCode.ENDTRY_L, endTarget);
if (sc.AdditionalEndTryTargetToInstruction != null)
// handles `break`, `continue` and `goto` in multi-layered nested try with finally
foreach (JumpTarget i in sc.AdditionalEndTryTargetToInstruction.Values)
AddInstruction(i.Instruction!);

if (exceptionSymbol is null)
RemoveAnonymousVariable(exceptionIndex);
else
Expand All @@ -92,12 +105,15 @@ private void ConvertTryStatement(SemanticModel model, TryStatementSyntax syntax)
if (syntax.Finally is not null)
{
_tryStack.Peek().State = ExceptionHandlingState.Finally;
sc.TryState = ExceptionHandlingState.Finally;
finallyTarget.Instruction = AddInstruction(OpCode.NOP);
ConvertStatement(model, syntax.Finally.Block);
AddInstruction(OpCode.ENDFINALLY);
}
endTarget.Instruction = AddInstruction(OpCode.NOP);
_tryStack.Pop();
if (_generalStatementStack.Pop() != sc)
throw new CompilationException(syntax, DiagnosticId.SyntaxNotSupported, $"Bad statement stack handling inside. This is a compiler bug.");
}
}
}
4 changes: 4 additions & 0 deletions src/Neo.Compiler.CSharp/MethodConvert/Statement/WhileStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ private void ConvertWhileStatement(SemanticModel model, WhileStatementSyntax syn
JumpTarget breakTarget = new();
PushContinueTarget(continueTarget);
PushBreakTarget(breakTarget);
StatementContext sc = new(syntax, breakTarget: breakTarget, continueTarget: continueTarget);
_generalStatementStack.Push(sc);
continueTarget.Instruction = AddInstruction(OpCode.NOP);
using (InsertSequencePoint(syntax.Condition))
{
Expand All @@ -56,6 +58,8 @@ private void ConvertWhileStatement(SemanticModel model, WhileStatementSyntax syn
breakTarget.Instruction = AddInstruction(OpCode.NOP);
PopContinueTarget();
PopBreakTarget();
if (_generalStatementStack.Pop() != sc)
throw new CompilationException(syntax, DiagnosticId.SyntaxNotSupported, $"Bad statement stack handling inside. This is a compiler bug.");
}
}
}
10 changes: 6 additions & 4 deletions src/Neo.Compiler.CSharp/Optimizer/Analysers/InstructionCoverage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public enum TryType
FINALLY = 1 << 3,
}

[DebuggerDisplay("{catchAddr}, {finallyAddr}, {tryStateType}, {continueAfterFinally}")]
[DebuggerDisplay("{catchAddr}, {finallyAddr}, {tryType}, {continueAfterFinally}")]
public struct TryState
{
public int catchAddr { get; init; }
Expand Down Expand Up @@ -248,7 +248,7 @@ public BranchType CoverInstruction(int addr, Stack<TryState>? tryStack = null,
if (continueAfterFinally)
return coveredMap[entranceAddr] = CoverInstruction(finallyAddr, tryStack, jumpFromBasicBlockEntranceAddr: entranceAddr);
// FINALLY is OK, but throwed in previous TRY (without catch) or CATCH
return BranchType.THROW; // Do not set coveredMap[entranceAddr] = BranchType.THROW;
return value; // Do not set coveredMap[entranceAddr] = BranchType.THROW;
}
//if (instruction.OpCode != OpCode.NOP)
{
Expand All @@ -274,7 +274,8 @@ public BranchType CoverInstruction(int addr, Stack<TryState>? tryStack = null,
returnedType = BranchType.ABORT;
foreach (int callaTarget in pushaTargets.Keys)
{
BranchType singleCallaResult = CoverInstruction(callaTarget, tryStack, jumpFromBasicBlockEntranceAddr: entranceAddr);
// Use `tryStack: null` to avoid using current try stack in a deeper call stack
BranchType singleCallaResult = CoverInstruction(callaTarget, tryStack: null, jumpFromBasicBlockEntranceAddr: entranceAddr);
if (singleCallaResult < returnedType)
returnedType = singleCallaResult;
// TODO: if a PUSHA cannot be covered, do not add it as a CALLA target
Expand All @@ -283,7 +284,8 @@ public BranchType CoverInstruction(int addr, Stack<TryState>? tryStack = null,
else
{
int callTarget = ComputeJumpTarget(addr, instruction);
returnedType = CoverInstruction(callTarget, tryStack, jumpFromBasicBlockEntranceAddr: entranceAddr);
// Use `tryStack: null` to avoid using current try stack in a deeper call stack
returnedType = CoverInstruction(callTarget, tryStack: null, jumpFromBasicBlockEntranceAddr: entranceAddr);
}
if (returnedType == BranchType.OK)
return coveredMap[entranceAddr] = CoverInstruction(addr + instruction.Size, tryStack, continueFromBasicBlockEntranceAddr: entranceAddr);
Expand Down
Loading
Loading