Skip to content

Commit 4b618e8

Browse files
nan01abnan01ab
authored
fix: always check division overflow for int32 and int64 (#1287)
Co-authored-by: Jimmy <[email protected]>
1 parent 8943a2f commit 4b618e8

File tree

5 files changed

+109
-9
lines changed

5 files changed

+109
-9
lines changed

‎src/Neo.Compiler.CSharp/MethodConvert/Expression/BinaryExpression.cs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
extern alias scfx;
1313

14+
using System.Linq;
1415
using Microsoft.CodeAnalysis;
1516
using Microsoft.CodeAnalysis.CSharp;
1617
using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -91,6 +92,12 @@ private void ConvertBinaryExpression(SemanticModel model, BinaryExpressionSyntax
9192
">=" => (OpCode.GE, false),
9293
_ => throw new CompilationException(expression.OperatorToken, DiagnosticId.SyntaxNotSupported, $"Unsupported operator: {expression.OperatorToken}")
9394
};
95+
96+
if (expression.OperatorToken.ValueText == "/")
97+
{
98+
CheckDivideOverflow(model.GetTypeInfo(expression).Type);
99+
}
100+
94101
AddInstruction(opcode);
95102
if (checkResult)
96103
{
@@ -99,6 +106,39 @@ private void ConvertBinaryExpression(SemanticModel model, BinaryExpressionSyntax
99106
}
100107
}
101108

109+
private void CheckDivideOverflow(ITypeSymbol? type)
110+
{
111+
if (type is null) return;
112+
while (type.NullableAnnotation == NullableAnnotation.Annotated)
113+
{
114+
// Supporting nullable integer like `byte?`
115+
type = ((INamedTypeSymbol)type).TypeArguments.First();
116+
}
117+
118+
// Check division overflow if checked statement.
119+
// In C#, division overflow is checked or not in `unchecked` statement depends on the implementation.
120+
if (!_checkedStack.Peek()) return;
121+
122+
// Only check overflow for int32 and int64
123+
// NOTE: short / short -> int, ushort / ushort -> int, char / char -> int,
124+
// sbyte / sbyte -> int, byte / byte -> int, so overflow check is not needed.
125+
if (type.Name != "Int32" && type.Name != "Int64") return;
126+
127+
var minValue = type.Name == "Int64" ? long.MinValue : int.MinValue;
128+
var endTarget = new JumpTarget();
129+
130+
AddInstruction(OpCode.DUP);
131+
Push(-1);
132+
Jump(OpCode.JMPNE_L, endTarget);
133+
134+
AddInstruction(OpCode.OVER);
135+
Push(minValue);
136+
Jump(OpCode.JMPNE_L, endTarget);
137+
138+
AddInstruction(OpCode.THROW);
139+
endTarget.Instruction = AddInstruction(OpCode.NOP);
140+
}
141+
102142
private void ConvertLogicalOrExpression(SemanticModel model, ExpressionSyntax left, ExpressionSyntax right)
103143
{
104144
JumpTarget rightTarget = new();

‎src/Neo.Compiler.CSharp/MethodConvert/Expression/UnaryExpression.PrefixUnary.cs‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -279,19 +279,20 @@ private void EmitIncrementOrDecrement(SyntaxToken operatorToken, ITypeSymbol? ty
279279

280280
private void EmitNegativeInteger(ITypeSymbol? typeSymbol)
281281
{
282-
if (typeSymbol is null || (typeSymbol.Name != "Int32" && typeSymbol.Name != "Int64"))
283-
{
284-
// -sbyte, -byte, -short, -ushort, -char -> int, -int, -uint -> long
285-
AddInstruction(OpCode.NEGATE); // Emit NEGATE for other integer types
286-
return;
287-
}
288-
282+
if (typeSymbol is null) return;
289283
while (typeSymbol.NullableAnnotation == NullableAnnotation.Annotated)
290284
{
291285
// Supporting nullable integer like `byte?`
292286
typeSymbol = ((INamedTypeSymbol)typeSymbol).TypeArguments.First();
293287
}
294288

289+
if (typeSymbol.Name != "Int32" && typeSymbol.Name != "Int64")
290+
{
291+
// -sbyte, -byte, -short, -ushort, -char -> int, -int, -uint -> long
292+
AddInstruction(OpCode.NEGATE); // Emit NEGATE for other integer types
293+
return;
294+
}
295+
295296
var minValue = typeSymbol.Name == "Int64" ? long.MinValue : int.MinValue; // int32 or int64
296297

297298
JumpTarget negateTarget = new(), endTarget = new();

‎tests/Neo.Compiler.CSharp.TestContracts/Contract_Overflow.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ public class Contract_Overflow : SmartContract.Framework.SmartContract
1818
public static uint AddUInt(uint a, uint b) => a + b;
1919
public static uint MulUInt(uint a, uint b) => a * b;
2020

21+
public static int DivInt(int a, int b) => checked(a / b);
22+
23+
public static int DivShort(short a, short b) => checked(a / b);
24+
2125
public static int NegateIntChecked(int a) => checked(-a);
2226
public static int NegateInt(int a) => -a;
2327

‎tests/Neo.Compiler.CSharp.UnitTests/TestingArtifacts/Contract_Overflow.cs‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@ public abstract class Contract_Overflow(Neo.SmartContract.Testing.SmartContractI
1111
{
1212
#region Compiled data
1313

14-
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""}}}");
14+
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""}}}");
1515

1616
/// <summary>
1717
/// Optimization: "All"
1818
/// </summary>
19-
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>();
19+
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>();
2020

2121
#endregion
2222

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

76+
/// <summary>
77+
/// Unsafe method
78+
/// </summary>
79+
/// <remarks>
80+
/// Script: VwACeHlKDyoLSwIAAACAKgM6oUA=
81+
/// INITSLOT 0002 [64 datoshi]
82+
/// LDARG0 [2 datoshi]
83+
/// LDARG1 [2 datoshi]
84+
/// DUP [2 datoshi]
85+
/// PUSHM1 [1 datoshi]
86+
/// JMPNE 0B [2 datoshi]
87+
/// OVER [2 datoshi]
88+
/// PUSHINT32 00000080 [1 datoshi]
89+
/// JMPNE 03 [2 datoshi]
90+
/// THROW [512 datoshi]
91+
/// DIV [8 datoshi]
92+
/// RET [0 datoshi]
93+
/// </remarks>
94+
[DisplayName("divInt")]
95+
public abstract BigInteger? DivInt(BigInteger? a, BigInteger? b);
96+
97+
/// <summary>
98+
/// Unsafe method
99+
/// </summary>
100+
/// <remarks>
101+
/// Script: VwACeHlKDyoLSwIAAACAKgM6oUA=
102+
/// INITSLOT 0002 [64 datoshi]
103+
/// LDARG0 [2 datoshi]
104+
/// LDARG1 [2 datoshi]
105+
/// DUP [2 datoshi]
106+
/// PUSHM1 [1 datoshi]
107+
/// JMPNE 0B [2 datoshi]
108+
/// OVER [2 datoshi]
109+
/// PUSHINT32 00000080 [1 datoshi]
110+
/// JMPNE 03 [2 datoshi]
111+
/// THROW [512 datoshi]
112+
/// DIV [8 datoshi]
113+
/// RET [0 datoshi]
114+
/// </remarks>
115+
[DisplayName("divShort")]
116+
public abstract BigInteger? DivShort(BigInteger? a, BigInteger? b);
117+
76118
/// <summary>
77119
/// Unsafe method
78120
/// </summary>

‎tests/Neo.Compiler.CSharp.UnitTests/UnitTest_Overflow.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,5 +102,18 @@ public void Test_NegateChecked()
102102
Assert.AreEqual(-9223372036854775808, Contract.NegateAddLong(-9223372036854775807, -1));
103103
Assert.ThrowsException<TestException>(() => Contract.NegateAddLongChecked(-9223372036854775807, -1));
104104
}
105+
106+
[TestMethod]
107+
public void Test_DivOverflow()
108+
{
109+
Assert.AreEqual(int.MaxValue, Contract.DivInt(int.MaxValue, 1));
110+
Assert.AreEqual(short.MaxValue, Contract.DivShort(short.MaxValue, 1));
111+
112+
// VMUnhandledException int.MinValue / -1
113+
Assert.ThrowsException<TestException>(() => Contract.DivInt(int.MinValue, -1));
114+
115+
// short / -1 -> int, so no overflow
116+
Assert.AreEqual(32768, Contract.DivShort(short.MinValue, -1));
117+
}
105118
}
106119
}

0 commit comments

Comments
 (0)