Skip to content

Commit

Permalink
fix: always check division overflow for int32 and int64 (#1287)
Browse files Browse the repository at this point in the history 
Co-authored-by: Jimmy <[email protected]>
  • Loading branch information
@nan01ab @Jim8y
nan01ab and Jim8y authored Feb 2, 2025
1 parent 4c939f9 commit 46cb9dd
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 9 deletions.
40 changes: 40 additions & 0 deletions src/Neo.Compiler.CSharp/MethodConvert/Expression/BinaryExpression.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

extern alias scfx;

using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand Down Expand Up @@ -91,6 +92,12 @@ private void ConvertBinaryExpression(SemanticModel model, BinaryExpressionSyntax
">=" => (OpCode.GE, false),
_ => throw new CompilationException(expression.OperatorToken, DiagnosticId.SyntaxNotSupported, $"Unsupported operator: {expression.OperatorToken}")
};

if (expression.OperatorToken.ValueText == "/")
{
CheckDivideOverflow(model.GetTypeInfo(expression).Type);
}

AddInstruction(opcode);
if (checkResult)
{
Expand All @@ -99,6 +106,39 @@ private void ConvertBinaryExpression(SemanticModel model, BinaryExpressionSyntax
}
}

private void CheckDivideOverflow(ITypeSymbol? type)
{
if (type is null) return;
while (type.NullableAnnotation == NullableAnnotation.Annotated)
{
// Supporting nullable integer like `byte?`
type = ((INamedTypeSymbol)type).TypeArguments.First();
}

// Check division overflow if checked statement.
// In C#, division overflow is checked or not in `unchecked` statement depends on the implementation.
if (!_checkedStack.Peek()) return;

// Only check overflow for int32 and int64
// NOTE: short / short -> int, ushort / ushort -> int, char / char -> int,
// sbyte / sbyte -> int, byte / byte -> int, so overflow check is not needed.
if (type.Name != "Int32" && type.Name != "Int64") return;

var minValue = type.Name == "Int64" ? long.MinValue : int.MinValue;
var endTarget = new JumpTarget();

AddInstruction(OpCode.DUP);
Push(-1);
Jump(OpCode.JMPNE_L, endTarget);

AddInstruction(OpCode.OVER);
Push(minValue);
Jump(OpCode.JMPNE_L, endTarget);

AddInstruction(OpCode.THROW);
endTarget.Instruction = AddInstruction(OpCode.NOP);
}

private void ConvertLogicalOrExpression(SemanticModel model, ExpressionSyntax left, ExpressionSyntax right)
{
JumpTarget rightTarget = new();
Expand Down
15 changes: 8 additions & 7 deletions src/Neo.Compiler.CSharp/MethodConvert/Expression/UnaryExpression.PrefixUnary.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,19 +279,20 @@ private void EmitIncrementOrDecrement(SyntaxToken operatorToken, ITypeSymbol? ty

private void EmitNegativeInteger(ITypeSymbol? typeSymbol)
{
if (typeSymbol is null || (typeSymbol.Name != "Int32" && typeSymbol.Name != "Int64"))
{
// -sbyte, -byte, -short, -ushort, -char -> int, -int, -uint -> long
AddInstruction(OpCode.NEGATE); // Emit NEGATE for other integer types
return;
}

if (typeSymbol is null) return;
while (typeSymbol.NullableAnnotation == NullableAnnotation.Annotated)
{
// Supporting nullable integer like `byte?`
typeSymbol = ((INamedTypeSymbol)typeSymbol).TypeArguments.First();
}

if (typeSymbol.Name != "Int32" && typeSymbol.Name != "Int64")
{
// -sbyte, -byte, -short, -ushort, -char -> int, -int, -uint -> long
AddInstruction(OpCode.NEGATE); // Emit NEGATE for other integer types
return;
}

var minValue = typeSymbol.Name == "Int64" ? long.MinValue : int.MinValue; // int32 or int64

JumpTarget negateTarget = new(), endTarget = new();
Expand Down
4 changes: 4 additions & 0 deletions tests/Neo.Compiler.CSharp.TestContracts/Contract_Overflow.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ public class Contract_Overflow : SmartContract.Framework.SmartContract
public static uint AddUInt(uint a, uint b) => a + b;
public static uint MulUInt(uint a, uint b) => a * b;

public static int DivInt(int a, int b) => checked(a / b);

public static int DivShort(short a, short b) => checked(a / b);

public static int NegateIntChecked(int a) => checked(-a);
public static int NegateInt(int a) => -a;

Expand Down
46 changes: 44 additions & 2 deletions tests/Neo.Compiler.CSharp.UnitTests/TestingArtifacts/Contract_Overflow.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ public abstract class Contract_Overflow(Neo.SmartContract.Testing.SmartContractI
{
#region Compiled data

public static Neo.SmartContract.Manifest.ContractManifest Manifest => Neo.SmartContract.Manifest.ContractManifest.Parse(@"{""name"":""Contract_Overflow"",""groups"":[],""features"":{},""supportedstandards"":[],""abi"":{""methods"":[{""name"":""addInt"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":0,""safe"":false},{""name"":""mulInt"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":53,""safe"":false},{""name"":""addUInt"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":106,""safe"":false},{""name"":""mulUInt"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":141,""safe"":false},{""name"":""negateIntChecked"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":176,""safe"":false},{""name"":""negateInt"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":191,""safe"":false},{""name"":""negateLongChecked"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":206,""safe"":false},{""name"":""negateLong"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":225,""safe"":false},{""name"":""negateShortChecked"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":244,""safe"":false},{""name"":""negateShort"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":250,""safe"":false},{""name"":""negateAddInt"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":256,""safe"":false},{""name"":""negateAddIntChecked"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":319,""safe"":false},{""name"":""negateAddLong"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":354,""safe"":false},{""name"":""negateAddLongChecked"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":449,""safe"":false}],""events"":[]},""permissions"":[],""trusts"":[],""extra"":{""nef"":{""optimization"":""All""}}}");
public static Neo.SmartContract.Manifest.ContractManifest Manifest => Neo.SmartContract.Manifest.ContractManifest.Parse(@"{""name"":""Contract_Overflow"",""groups"":[],""features"":{},""supportedstandards"":[],""abi"":{""methods"":[{""name"":""addInt"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":0,""safe"":false},{""name"":""mulInt"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":53,""safe"":false},{""name"":""addUInt"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":106,""safe"":false},{""name"":""mulUInt"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":141,""safe"":false},{""name"":""divInt"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":176,""safe"":false},{""name"":""divShort"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":196,""safe"":false},{""name"":""negateIntChecked"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":216,""safe"":false},{""name"":""negateInt"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":231,""safe"":false},{""name"":""negateLongChecked"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":246,""safe"":false},{""name"":""negateLong"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":265,""safe"":false},{""name"":""negateShortChecked"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":284,""safe"":false},{""name"":""negateShort"",""parameters"":[{""name"":""a"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":290,""safe"":false},{""name"":""negateAddInt"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":296,""safe"":false},{""name"":""negateAddIntChecked"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":359,""safe"":false},{""name"":""negateAddLong"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":394,""safe"":false},{""name"":""negateAddLongChecked"",""parameters"":[{""name"":""a"",""type"":""Integer""},{""name"":""b"",""type"":""Integer""}],""returntype"":""Integer"",""offset"":489,""safe"":false}],""events"":[]},""permissions"":[],""trusts"":[],""extra"":{""nef"":{""optimization"":""All""}}}");

/// <summary>
/// Optimization: "All"
/// </summary>
public static Neo.SmartContract.NefFile Nef => Convert.FromBase64String(@"TkVGM1Rlc3RpbmdFbmdpbmUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAP3wAVcAAnh5nkoCAAAAgC4EIgpKAv///38yHgP/////AAAAAJFKAv///38yDAMAAAAAAQAAAJ9AVwACeHmgSgIAAACALgQiCkoC////fzIeA/////8AAAAAkUoC////fzIMAwAAAAABAAAAn0BXAAJ4eZ5KEC4EIg5KA/////8AAAAAMgwD/////wAAAACRQFcAAnh5oEoQLgQiDkoD/////wAAAAAyDAP/////AAAAAJFAVwABeEoCAAAAgCoDOptAVwABeEoCAAAAgCoDQJtAVwABeEoDAAAAAAAAAIAqAzqbQFcAAXhKAwAAAAAAAACAKgNAm0BXAAF4m0BXAAF4m0BXAAJ4eZ5KAgAAAIAuBCIKSgL///9/Mh4D/////wAAAACRSgL///9/MgwDAAAAAAEAAACfSgIAAACAKgNAm0BXAAJ4eZ5KAgAAAIAuAzpKAv///38yAzpKAgAAAIAqAzqbQFcAAnh5nkoDAAAAAAAAAIAuBCIOSgP/////////fzIyBP//////////AAAAAAAAAACRSgP/////////fzIUBAAAAAAAAAAAAQAAAAAAAACfSgMAAAAAAAAAgCoDQJtAVwACeHmeSgMAAAAAAAAAgC4DOkoD/////////38yAzpKAwAAAAAAAACAKgM6m0D1Twbi").AsSerializable<Neo.SmartContract.NefFile>();
public static Neo.SmartContract.NefFile Nef => Convert.FromBase64String(@"TkVGM1Rlc3RpbmdFbmdpbmUAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAP0YAlcAAnh5nkoCAAAAgC4EIgpKAv///38yHgP/////AAAAAJFKAv///38yDAMAAAAAAQAAAJ9AVwACeHmgSgIAAACALgQiCkoC////fzIeA/////8AAAAAkUoC////fzIMAwAAAAABAAAAn0BXAAJ4eZ5KEC4EIg5KA/////8AAAAAMgwD/////wAAAACRQFcAAnh5oEoQLgQiDkoD/////wAAAAAyDAP/////AAAAAJFAVwACeHlKDyoLSwIAAACAKgM6oUBXAAJ4eUoPKgtLAgAAAIAqAzqhQFcAAXhKAgAAAIAqAzqbQFcAAXhKAgAAAIAqA0CbQFcAAXhKAwAAAAAAAACAKgM6m0BXAAF4SgMAAAAAAAAAgCoDQJtAVwABeJtAVwABeJtAVwACeHmeSgIAAACALgQiCkoC////fzIeA/////8AAAAAkUoC////fzIMAwAAAAABAAAAn0oCAAAAgCoDQJtAVwACeHmeSgIAAACALgM6SgL///9/MgM6SgIAAACAKgM6m0BXAAJ4eZ5KAwAAAAAAAACALgQiDkoD/////////38yMgT//////////wAAAAAAAAAAkUoD/////////38yFAQAAAAAAAAAAAEAAAAAAAAAn0oDAAAAAAAAAIAqA0CbQFcAAnh5nkoDAAAAAAAAAIAuAzpKA/////////9/MgM6SgMAAAAAAAAAgCoDOptA4RUGTQ==").AsSerializable<Neo.SmartContract.NefFile>();

#endregion

Expand Down Expand Up @@ -73,6 +73,48 @@ public abstract class Contract_Overflow(Neo.SmartContract.Testing.SmartContractI
[DisplayName("addUInt")]
public abstract BigInteger? AddUInt(BigInteger? a, BigInteger? b);

/// <summary>
/// Unsafe method
/// </summary>
/// <remarks>
/// Script: VwACeHlKDyoLSwIAAACAKgM6oUA=
/// INITSLOT 0002 [64 datoshi]
/// LDARG0 [2 datoshi]
/// LDARG1 [2 datoshi]
/// DUP [2 datoshi]
/// PUSHM1 [1 datoshi]
/// JMPNE 0B [2 datoshi]
/// OVER [2 datoshi]
/// PUSHINT32 00000080 [1 datoshi]
/// JMPNE 03 [2 datoshi]
/// THROW [512 datoshi]
/// DIV [8 datoshi]
/// RET [0 datoshi]
/// </remarks>
[DisplayName("divInt")]
public abstract BigInteger? DivInt(BigInteger? a, BigInteger? b);

/// <summary>
/// Unsafe method
/// </summary>
/// <remarks>
/// Script: VwACeHlKDyoLSwIAAACAKgM6oUA=
/// INITSLOT 0002 [64 datoshi]
/// LDARG0 [2 datoshi]
/// LDARG1 [2 datoshi]
/// DUP [2 datoshi]
/// PUSHM1 [1 datoshi]
/// JMPNE 0B [2 datoshi]
/// OVER [2 datoshi]
/// PUSHINT32 00000080 [1 datoshi]
/// JMPNE 03 [2 datoshi]
/// THROW [512 datoshi]
/// DIV [8 datoshi]
/// RET [0 datoshi]
/// </remarks>
[DisplayName("divShort")]
public abstract BigInteger? DivShort(BigInteger? a, BigInteger? b);

/// <summary>
/// Unsafe method
/// </summary>
Expand Down
13 changes: 13 additions & 0 deletions tests/Neo.Compiler.CSharp.UnitTests/UnitTest_Overflow.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,5 +102,18 @@ public void Test_NegateChecked()
Assert.AreEqual(-9223372036854775808, Contract.NegateAddLong(-9223372036854775807, -1));
Assert.ThrowsException<TestException>(() => Contract.NegateAddLongChecked(-9223372036854775807, -1));
}

[TestMethod]
public void Test_DivOverflow()
{
Assert.AreEqual(int.MaxValue, Contract.DivInt(int.MaxValue, 1));
Assert.AreEqual(short.MaxValue, Contract.DivShort(short.MaxValue, 1));

// VMUnhandledException int.MinValue / -1
Assert.ThrowsException<TestException>(() => Contract.DivInt(int.MinValue, -1));

// short / -1 -> int, so no overflow
Assert.AreEqual(32768, Contract.DivShort(short.MinValue, -1));
}
}
}

0 comments on commit 46cb9dd

Please sign in to comment.