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

partially fix return in try; optimizer replaces JMP->ENDTRY with only ENDTRY #1283

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+246 −105
Open

partially fix return in try; optimizer replaces JMP->ENDTRY with only ENDTRY #1283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bb983f6
partially fix return in try
Hecate2 Jan 22, 2025
45a6c03
Merge branch 'master' of github.com:neo-project/neo-devpack-dotnet in…
Hecate2 Jan 23, 2025
be4c40e
optimizer replaces JMP-ENDTRY with only ENDTRY
Hecate2 Jan 23, 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
27 changes: 23 additions & 4 deletions src/Neo.Compiler.CSharp/MethodConvert/Statement/ReturnStatement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Neo.VM;
using System.Collections.Generic;

namespace Neo.Compiler
{
Expand Down Expand Up @@ -44,10 +45,28 @@ private void ConvertReturnStatement(SemanticModel model, ReturnStatementSyntax s
{
if (syntax.Expression is not null)
ConvertExpression(model, syntax.Expression);
if (_tryStack.Count > 0)
Jump(OpCode.ENDTRY_L, _returnTarget);
else
Jump(OpCode.JMP_L, _returnTarget);

// The following case is not considered:
// Method -> try (with finally) -> function in method -> return from function in method
JumpTarget? returnTarget = _returnTarget;
List<StatementContext> visitedTry = []; // from shallow to deep
foreach (StatementContext sc in _generalStatementStack)
{// start from the deepest context
// stage the try stacks on the way
if (sc.StatementSyntax is TryStatementSyntax)
visitedTry.Add(sc);
}

foreach (StatementContext sc in visitedTry)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearer if add { }

// start from the most external try
// internal try should ENDTRY, targeting the correct external return target
returnTarget = sc.AddEndTry(returnTarget);

Jump(OpCode.JMP_L, returnTarget);
// We could use ENDTRY if current statement calling `return` is a try statement,
// but this job can be done by the optimizer
// Note that, do not Jump(OpCode.ENDTRY_L, returnTarget) here,
// because the returnTarget here is already an ENDTRY_L for current try stack.
}
}
}
Expand Down
23 changes: 23 additions & 0 deletions src/Neo.Compiler.CSharp/Optimizer/Strategies/JumpCompresser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@ public static (NefFile, ContractManifest, JObject?) ReplaceJumpWithRet(NefFile n

/// <summary>
/// If an unconditional jump targets an unconditional jump, re-target the first unconditional jump to its final destination
/// If an unconditional jump targets an ENDTRY, replace the unconditional jump with ENDTRY
/// If a conditional jump targets an unconditional jump, re-target the conditional jump to its final destination
/// If an unconditional jump targets a conditional jump, DO NOT replace the unconditional jump with a conditional jump to its final destination. THIS IS WRONG.
/// This should be executed very early, before <see cref="Reachability.RemoveUncoveredInstructions"/>
Expand Down Expand Up @@ -415,6 +416,28 @@ public static (NefFile, ContractManifest, JObject?) FoldJump(NefFile nef, Contra
jumpTargetToSources[finalTarget].Add(i);
continue;
}
if ((target.OpCode == OpCode.ENDTRY || target.OpCode == OpCode.ENDTRY_L)
&& unconditionalJump.Contains(i.OpCode))
{// replace this JMP with ENDTRY
modified = true;
Instruction finalTarget = jumpSourceToTargets[target];
oldSequencePointAddressToNew.Add(a, newAddr);
// handle the reference of the deleted JMP
jumpSourceToTargets.Remove(i);
jumpTargetToSources[target].Remove(i);
// handle the reference of the added RET
Instruction newEndTry = new Script(new byte[] { (byte)OpCode.ENDTRY_L }.Concat(BitConverter.GetBytes(0)).ToArray()).GetInstruction(0);
// above is a workaround of new Instruction(OpCode.ENDTRY_L)
OptimizedScriptBuilder.RetargetJump(i, newEndTry,
jumpSourceToTargets, trySourceToTargets, jumpTargetToSources);
jumpSourceToTargets[newEndTry] = finalTarget;
jumpTargetToSources[finalTarget].Remove(i);
jumpTargetToSources[finalTarget].Add(newEndTry);

simplifiedInstructionsToAddress.Add(newEndTry, newAddr);
newAddr += newEndTry.Size;
continue;
}
}
simplifiedInstructionsToAddress.Add(i, newAddr);
newAddr += i.Size;
Expand Down
57 changes: 57 additions & 0 deletions tests/Neo.Compiler.CSharp.TestContracts/Contract_Returns.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
// modifications are permitted.

using Neo.SmartContract.Framework;
using Neo.SmartContract.Framework.Services;

namespace Neo.Compiler.CSharp.TestContracts
{
Expand Down Expand Up @@ -56,5 +57,61 @@ public static ByteString ByteStringAdd(ByteString a, ByteString b)
{
return a + b;
}

private static int TryReturnInternal(bool exception)
{
int a = 0;
Storage.Put("\x00", "\x00");
try
{
try
{
if (exception)
throw new System.Exception();
else
return ++a;
}
catch { return ++a; }
finally
{
ExecutionEngine.Assert(Storage.Get("\x00")! == "\x00");
Storage.Put("\x00", "\x01");
a++;
if (exception)
throw new System.Exception();
}
}
finally
{
ExecutionEngine.Assert(Storage.Get("\x00")! == "\x01");
Storage.Put("\x00", "\x02");
++a;
}
#pragma warning disable CS0162 // Unreachable code detected
Storage.Put("\x00", "\x03");
#pragma warning restore CS0162 // Unreachable code detected
}

public static int TryReturn()
{
int a = 0;
// The following is an extremely dangerous case.
// catch { return ++a; } pushes an `a` into the evaluation stack
// but there is exception in finally, and the pushed `a` is never popped
// and its value is kept into current evaluation stack in TryReturn.
// No idea about the fix.
//try { a = TryReturnInternal(true); }
//catch
//{
// ExecutionEngine.Assert(a == 0);
// a += 1;
//}
//finally { ExecutionEngine.Assert(Storage.Get("\x00")! == "\x02"); }
//ExecutionEngine.Assert(a == 1);
a = TryReturnInternal(false);
ExecutionEngine.Assert(a == 1);
ExecutionEngine.Assert(Storage.Get("\x00")! == "\x02");
return ++a;
}
}
}
56 changes: 27 additions & 29 deletions tests/Neo.Compiler.CSharp.UnitTests/TestingArtifacts/Contract_Break.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public abstract class Contract_Break(Neo.SmartContract.Testing.SmartContractInit
/// <summary>
/// Optimization: "All"
/// </summary>
public static Neo.SmartContract.NefFile Nef => Convert.FromBase64String(@"TkVGM1Rlc3RpbmdFbmdpbmUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAP2QAVcGAQwBAAwC/wA1VwEAABAMAf81XgEAAHAiG2hB81S/HXE7AAU9FwwBAQwC/wA1MwEAAD9oQZwI7Zwk4QwC/wA1QQEAAAwBAZc5EAwB/zUjAQAAcCJsaEHzVL8dcTsaMRByIg4MCWV4Y2VwdGlvbjpqE7Uk8T1LcjsAByIRPUsMAQAMAv8ANdsAAAA/PfEMAv8ANe8AAAAMAQCXORIREBPASnLKcxB0IhRqbM51DAECDAL/ADWtAAAAIgZsazDsP2hBnAjtnCSQDAL/ADW1AAAADAEClzkQDAH/NZcAAABwIm1oQfNUvx1xOxpPEHJqE7VFeCYODAlleGNlcHRpb246PUxyEHNrE7VFaxCXOTsOEwwBAwwC/wA0SWo6dCIZPTYMAv8ANFwMAQOXOQwBAgwC/wA0Lj896QwC/wA0RQwBApc5DAEDDAL/ADQXP2hBnAjtnCSPDAL/ADQoDAEDlzlAVwACeXhBm/ZnzkHmPxiEQFcAAnl4Qfa0a+JB3zC4mkBXAAF4Qfa0a+JBkl3oMUCP84XP").AsSerializable<Neo.SmartContract.NefFile>();
public static Neo.SmartContract.NefFile Nef => Convert.FromBase64String(@"TkVGM1Rlc3RpbmdFbmdpbmUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAP2MAVcGAQwBAAwC/wA1UwEAABAMAf81WgEAAHAiG2hB81S/HXE7AAU9FwwBAQwC/wA1LwEAAD9oQZwI7Zwk4QwC/wA1PQEAAAwBAZc5EAwB/zUfAQAAcCJqaEHzVL8dcTsaLxByIg4MCWV4Y2VwdGlvbjpqE7Uk8T1JcjsABz0CPUkMAQAMAv8ANdcAAAA/DAL/ADXtAAAADAEAlzkSERATwEpyynMQdCIUamzOdQwBAgwC/wA1qwAAACIGbGsw7D9oQZwI7ZwkkgwC/wA1swAAAAwBApc5EAwB/zWVAAAAcCJraEHzVL8dcTsaTRByahO1RXgmDgwJZXhjZXB0aW9uOj1KchBzaxO1RWsQlzk7DhMMAQMMAv8ANEdqOnQ9Aj00DAL/ADRaDAEDlzkMAQIMAv8ANCw/DAL/ADRFDAEClzkMAQMMAv8ANBc/aEGcCO2cJJEMAv8ANCgMAQOXOUBXAAJ5eEGb9mfOQeY/GIRAVwACeXhB9rRr4kHfMLiaQFcAAXhB9rRr4kGSXegxQI9uWGA=").AsSerializable<Neo.SmartContract.NefFile>();

#endregion

Expand All @@ -26,14 +26,14 @@ public abstract class Contract_Break(Neo.SmartContract.Testing.SmartContractInit
/// Unsafe method
/// </summary>
/// <remarks>
/// Script: VwYBDAEADAL/ADVXAQAAEAwB/zVeAQAAcCIbaEHzVL8dcTsABT0XDAEBDAL/ADUzAQAAP2hBnAjtnCThDAL/ADVBAQAADAEBlzkQDAH/NSMBAABwImxoQfNUvx1xOxoxEHIiDgwJZXhjZXB0aW9uOmoTtSTxPUtyOwAHIhE9SwwBAAwC/wA12wAAAD898QwC/wA17wAAAAwBAJc5EhEQE8BKcspzEHQiFGpsznUMAQIMAv8ANa0AAAAiBmxrMOw/aEGcCO2cJJAMAv8ANbUAAAAMAQKXORAMAf81lwAAAHAibWhB81S/HXE7Gk8QcmoTtUV4Jg4MCWV4Y2VwdGlvbjo9THIQc2sTtUVrEJc5Ow4TDAEDDAL/ADRJajp0Ihk9NgwC/wA0XAwBA5c5DAECDAL/ADQuPz3pDAL/ADRFDAEClzkMAQMMAv8ANBc/aEGcCO2cJI8MAv8ANCgMAQOXOUA=
/// Script: VwYBDAEADAL/ADVTAQAAEAwB/zVaAQAAcCIbaEHzVL8dcTsABT0XDAEBDAL/ADUvAQAAP2hBnAjtnCThDAL/ADU9AQAADAEBlzkQDAH/NR8BAABwImpoQfNUvx1xOxovEHIiDgwJZXhjZXB0aW9uOmoTtSTxPUlyOwAHPQI9SQwBAAwC/wA11wAAAD8MAv8ANe0AAAAMAQCXORIREBPASnLKcxB0IhRqbM51DAECDAL/ADWrAAAAIgZsazDsP2hBnAjtnCSSDAL/ADWzAAAADAEClzkQDAH/NZUAAABwImtoQfNUvx1xOxpNEHJqE7VFeCYODAlleGNlcHRpb246PUpyEHNrE7VFaxCXOTsOEwwBAwwC/wA0R2o6dD0CPTQMAv8ANFoMAQOXOQwBAgwC/wA0LD8MAv8ANEUMAQKXOQwBAwwC/wA0Fz9oQZwI7ZwkkQwC/wA0KAwBA5c5QA==
/// INITSLOT 0601 [64 datoshi]
/// PUSHDATA1 00 [8 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL_L 57010000 [512 datoshi]
/// CALL_L 53010000 [512 datoshi]
/// PUSH0 [1 datoshi]
/// PUSHDATA1 FF '?' [8 datoshi]
/// CALL_L 5E010000 [512 datoshi]
/// CALL_L 5A010000 [512 datoshi]
/// STLOC0 [2 datoshi]
/// JMP 1B [2 datoshi]
/// LDLOC0 [2 datoshi]
Expand All @@ -43,25 +43,25 @@ public abstract class Contract_Break(Neo.SmartContract.Testing.SmartContractInit
/// ENDTRY 17 [4 datoshi]
/// PUSHDATA1 01 [8 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL_L 33010000 [512 datoshi]
/// CALL_L 2F010000 [512 datoshi]
/// ENDFINALLY [4 datoshi]
/// LDLOC0 [2 datoshi]
/// SYSCALL 9C08ED9C 'System.Iterator.Next' [32768 datoshi]
/// JMPIF E1 [2 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL_L 41010000 [512 datoshi]
/// CALL_L 3D010000 [512 datoshi]
/// PUSHDATA1 01 [8 datoshi]
/// EQUAL [32 datoshi]
/// ASSERT [1 datoshi]
/// PUSH0 [1 datoshi]
/// PUSHDATA1 FF '?' [8 datoshi]
/// CALL_L 23010000 [512 datoshi]
/// CALL_L 1F010000 [512 datoshi]
/// STLOC0 [2 datoshi]
/// JMP 6C [2 datoshi]
/// JMP 6A [2 datoshi]
/// LDLOC0 [2 datoshi]
/// SYSCALL F354BF1D 'System.Iterator.Value' [16 datoshi]
/// STLOC1 [2 datoshi]
/// TRY 1A31 [4 datoshi]
/// TRY 1A2F [4 datoshi]
/// PUSH0 [1 datoshi]
/// STLOC2 [2 datoshi]
/// JMP 0E [2 datoshi]
Expand All @@ -71,18 +71,17 @@ public abstract class Contract_Break(Neo.SmartContract.Testing.SmartContractInit
/// PUSH3 [1 datoshi]
/// LT [8 datoshi]
/// JMPIF F1 [2 datoshi]
/// ENDTRY 4B [4 datoshi]
/// ENDTRY 49 [4 datoshi]
/// STLOC2 [2 datoshi]
/// TRY 0007 [4 datoshi]
/// JMP 11 [2 datoshi]
/// ENDTRY 4B [4 datoshi]
/// ENDTRY 02 [4 datoshi]
/// ENDTRY 49 [4 datoshi]
/// PUSHDATA1 00 [8 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL_L DB000000 [512 datoshi]
/// CALL_L D7000000 [512 datoshi]
/// ENDFINALLY [4 datoshi]
/// ENDTRY F1 [4 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL_L EF000000 [512 datoshi]
/// CALL_L ED000000 [512 datoshi]
/// PUSHDATA1 00 [8 datoshi]
/// EQUAL [32 datoshi]
/// ASSERT [1 datoshi]
Expand All @@ -104,29 +103,29 @@ public abstract class Contract_Break(Neo.SmartContract.Testing.SmartContractInit
/// STLOC5 [2 datoshi]
/// PUSHDATA1 02 [8 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL_L AD000000 [512 datoshi]
/// CALL_L AB000000 [512 datoshi]
/// JMP 06 [2 datoshi]
/// LDLOC4 [2 datoshi]
/// LDLOC3 [2 datoshi]
/// JMPLT EC [2 datoshi]
/// ENDFINALLY [4 datoshi]
/// LDLOC0 [2 datoshi]
/// SYSCALL 9C08ED9C 'System.Iterator.Next' [32768 datoshi]
/// JMPIF 90 [2 datoshi]
/// JMPIF 92 [2 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL_L B5000000 [512 datoshi]
/// CALL_L B3000000 [512 datoshi]
/// PUSHDATA1 02 [8 datoshi]
/// EQUAL [32 datoshi]
/// ASSERT [1 datoshi]
/// PUSH0 [1 datoshi]
/// PUSHDATA1 FF '?' [8 datoshi]
/// CALL_L 97000000 [512 datoshi]
/// CALL_L 95000000 [512 datoshi]
/// STLOC0 [2 datoshi]
/// JMP 6D [2 datoshi]
/// JMP 6B [2 datoshi]
/// LDLOC0 [2 datoshi]
/// SYSCALL F354BF1D 'System.Iterator.Value' [16 datoshi]
/// STLOC1 [2 datoshi]
/// TRY 1A4F [4 datoshi]
/// TRY 1A4D [4 datoshi]
/// PUSH0 [1 datoshi]
/// STLOC2 [2 datoshi]
/// LDLOC2 [2 datoshi]
Expand All @@ -137,7 +136,7 @@ public abstract class Contract_Break(Neo.SmartContract.Testing.SmartContractInit
/// JMPIFNOT 0E [2 datoshi]
/// PUSHDATA1 657863657074696F6E 'exception' [8 datoshi]
/// THROW [512 datoshi]
/// ENDTRY 4C [4 datoshi]
/// ENDTRY 4A [4 datoshi]
/// STLOC2 [2 datoshi]
/// PUSH0 [1 datoshi]
/// STLOC3 [2 datoshi]
Expand All @@ -152,22 +151,21 @@ public abstract class Contract_Break(Neo.SmartContract.Testing.SmartContractInit
/// TRY 0E13 [4 datoshi]
/// PUSHDATA1 03 [8 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL 49 [512 datoshi]
/// CALL 47 [512 datoshi]
/// LDLOC2 [2 datoshi]
/// THROW [512 datoshi]
/// STLOC4 [2 datoshi]
/// JMP 19 [2 datoshi]
/// ENDTRY 36 [4 datoshi]
/// ENDTRY 02 [4 datoshi]
/// ENDTRY 34 [4 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL 5C [512 datoshi]
/// CALL 5A [512 datoshi]
/// PUSHDATA1 03 [8 datoshi]
/// EQUAL [32 datoshi]
/// ASSERT [1 datoshi]
/// PUSHDATA1 02 [8 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL 2E [512 datoshi]
/// CALL 2C [512 datoshi]
/// ENDFINALLY [4 datoshi]
/// ENDTRY E9 [4 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL 45 [512 datoshi]
/// PUSHDATA1 02 [8 datoshi]
Expand All @@ -179,7 +177,7 @@ public abstract class Contract_Break(Neo.SmartContract.Testing.SmartContractInit
/// ENDFINALLY [4 datoshi]
/// LDLOC0 [2 datoshi]
/// SYSCALL 9C08ED9C 'System.Iterator.Next' [32768 datoshi]
/// JMPIF 8F [2 datoshi]
/// JMPIF 91 [2 datoshi]
/// PUSHDATA1 FF00 [8 datoshi]
/// CALL 28 [512 datoshi]
/// PUSHDATA1 03 [8 datoshi]
Expand Down
Loading
Loading