From 3b03af14d2800e41b075b4a88aa28a5d175462a7 Mon Sep 17 00:00:00 2001 From: Sean Young Date: Thu, 6 Apr 2023 16:52:37 +0100 Subject: [PATCH] explicitly disallow rational comparisons (#1221) This is not permitted by solc either. Improve the diagnostic, and ensure the diagnostic gets surfaced even used within a overloaded function call. Signed-off-by: Sean Young --- src/sema/expression/arithmetic.rs | 8 +- src/sema/expression/resolve_expression.rs | 34 +++-- .../solana/rational_comparison.dot | 126 +++++------------- .../solana/rational_comparison2.dot | 48 +++++++ .../solana/rational_comparison2.sol | 30 +++++ 5 files changed, 136 insertions(+), 110 deletions(-) create mode 100644 tests/contract_testcases/solana/rational_comparison2.dot create mode 100644 tests/contract_testcases/solana/rational_comparison2.sol diff --git a/src/sema/expression/arithmetic.rs b/src/sema/expression/arithmetic.rs index 233409723..fa5e065e6 100644 --- a/src/sema/expression/arithmetic.rs +++ b/src/sema/expression/arithmetic.rs @@ -616,9 +616,11 @@ pub(super) fn equal( }; if ty.is_rational() { - if let Err(diag) = eval_const_rational(&expr, ns) { - diagnostics.push(diag); - } + diagnostics.push(Diagnostic::error( + *loc, + "cannot use rational numbers with '!=' or '==' operator".into(), + )); + return Err(()); } Ok(expr) diff --git a/src/sema/expression/resolve_expression.rs b/src/sema/expression/resolve_expression.rs index 52da54cd6..7851d929e 100644 --- a/src/sema/expression/resolve_expression.rs +++ b/src/sema/expression/resolve_expression.rs @@ -25,7 +25,7 @@ use crate::sema::{ { ast::{Builtin, Expression, Namespace, RetrieveType, Type}, diagnostics::Diagnostics, - eval::{check_term_for_constant_overflow, eval_const_rational}, + eval::check_term_for_constant_overflow, }, }; use num_bigint::BigInt; @@ -165,9 +165,11 @@ pub fn expression( }; if ty.is_rational() { - if let Err(diag) = eval_const_rational(&expr, ns) { - diagnostics.push(diag); - } + diagnostics.push(Diagnostic::error( + *loc, + "cannot use rational numbers with '>' operator".into(), + )); + return Err(()); } Ok(expr) @@ -206,9 +208,11 @@ pub fn expression( }; if ty.is_rational() { - if let Err(diag) = eval_const_rational(&expr, ns) { - diagnostics.push(diag); - } + diagnostics.push(Diagnostic::error( + *loc, + "cannot use rational numbers with '<' operator".into(), + )); + return Err(()); } Ok(expr) @@ -246,9 +250,11 @@ pub fn expression( }; if ty.is_rational() { - if let Err(diag) = eval_const_rational(&expr, ns) { - diagnostics.push(diag); - } + diagnostics.push(Diagnostic::error( + *loc, + "cannot use rational numbers with '>=' operator".into(), + )); + return Err(()); } Ok(expr) @@ -286,9 +292,11 @@ pub fn expression( }; if ty.is_rational() { - if let Err(diag) = eval_const_rational(&expr, ns) { - diagnostics.push(diag); - } + diagnostics.push(Diagnostic::error( + *loc, + "cannot use rational numbers with '<=' operator".into(), + )); + return Err(()); } Ok(expr) diff --git a/tests/contract_testcases/solana/rational_comparison.dot b/tests/contract_testcases/solana/rational_comparison.dot index 6cd7b5fb3..5918f0bd8 100644 --- a/tests/contract_testcases/solana/rational_comparison.dot +++ b/tests/contract_testcases/solana/rational_comparison.dot @@ -3,114 +3,52 @@ strict digraph "tests/contract_testcases/solana/rational_comparison.sol" { foo1 [label="function foo1\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:3:2-57\nsignature foo1(uint64,uint64)\nvisibility public\nmutability nonpayable"] parameters [label="parameters\nuint64 a\nuint64 b"] returns [label="returns\nbool "] - return [label="return\ntests/contract_testcases/solana/rational_comparison.sol:4:3-23"] - more_equal [label="more equal\ntests/contract_testcases/solana/rational_comparison.sol:4:10-23"] - cast [label="cast rational\ntests/contract_testcases/solana/rational_comparison.sol:4:11-14"] - divide [label="divide\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:4:11-14"] - variable [label="variable: a\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:4:11-12"] - variable_10 [label="variable: b\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:4:13-14"] - rational_literal [label="rational rational literal: 1/20\ntests/contract_testcases/solana/rational_comparison.sol:4:19-23"] foo2 [label="function foo2\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:6:2-57\nsignature foo2(uint64,uint64)\nvisibility public\nmutability nonpayable"] - parameters_13 [label="parameters\nuint64 a\nuint64 b"] - returns_14 [label="returns\nbool "] - return_15 [label="return\ntests/contract_testcases/solana/rational_comparison.sol:7:3-19"] - more [label="more\ntests/contract_testcases/solana/rational_comparison.sol:7:10-19"] - rational_literal_17 [label="rational rational literal: 11/5\ntests/contract_testcases/solana/rational_comparison.sol:7:10-15"] - cast_18 [label="cast rational\ntests/contract_testcases/solana/rational_comparison.sol:7:18-19"] - variable_19 [label="variable: a\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:7:18-19"] + parameters_6 [label="parameters\nuint64 a\nuint64 b"] + returns_7 [label="returns\nbool "] foo3 [label="function foo3\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:9:2-57\nsignature foo3(uint64,uint64)\nvisibility public\nmutability nonpayable"] - parameters_21 [label="parameters\nuint64 a\nuint64 b"] - returns_22 [label="returns\nbool "] - return_23 [label="return\ntests/contract_testcases/solana/rational_comparison.sol:10:3-19"] - equal [label="equal\ntests/contract_testcases/solana/rational_comparison.sol:10:10-19"] - rational_literal_25 [label="rational rational literal: 1\ntests/contract_testcases/solana/rational_comparison.sol:10:10-11"] - rational_literal_26 [label="rational rational literal: 1/20\ntests/contract_testcases/solana/rational_comparison.sol:10:15-19"] + parameters_9 [label="parameters\nuint64 a\nuint64 b"] + returns_10 [label="returns\nbool "] foo4 [label="function foo4\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:12:2-57\nsignature foo4(uint64,uint64)\nvisibility public\nmutability nonpayable"] - parameters_28 [label="parameters\nuint64 a\nuint64 b"] - returns_29 [label="returns\nbool "] + parameters_12 [label="parameters\nuint64 a\nuint64 b"] + returns_13 [label="returns\nbool "] foo5 [label="function foo5\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:15:2-57\nsignature foo5(uint64,uint64)\nvisibility public\nmutability nonpayable"] - parameters_31 [label="parameters\nuint64 a\nuint64 b"] - returns_32 [label="returns\nbool "] - return_33 [label="return\ntests/contract_testcases/solana/rational_comparison.sol:16:3-26"] - less_equal [label="less equal\ntests/contract_testcases/solana/rational_comparison.sol:16:10-26"] - cast_35 [label="cast rational\ntests/contract_testcases/solana/rational_comparison.sol:16:11-17"] - shift_left [label="shift left\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:16:11-17"] - variable_37 [label="variable: a\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:16:11-12"] - variable_38 [label="variable: b\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:16:16-17"] - rational_literal_39 [label="rational rational literal: 1/20\ntests/contract_testcases/solana/rational_comparison.sol:16:22-26"] + parameters_15 [label="parameters\nuint64 a\nuint64 b"] + returns_16 [label="returns\nbool "] foo6 [label="function foo6\ncontract: c\ntests/contract_testcases/solana/rational_comparison.sol:18:2-57\nsignature foo6(uint64,uint64)\nvisibility public\nmutability nonpayable"] - parameters_41 [label="parameters\nuint64 a\nuint64 b"] - returns_42 [label="returns\nbool "] - return_43 [label="return\ntests/contract_testcases/solana/rational_comparison.sol:19:3-24"] - not [label="not\ntests/contract_testcases/solana/rational_comparison.sol:19:10-24"] - equal_45 [label="equal\ntests/contract_testcases/solana/rational_comparison.sol:19:10-24"] - rational_literal_46 [label="rational rational literal: 6/5\ntests/contract_testcases/solana/rational_comparison.sol:19:10-13"] - cast_47 [label="cast rational\ntests/contract_testcases/solana/rational_comparison.sol:19:18-23"] - bitwise_xor [label="bitwise xor\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:19:18-23"] - variable_49 [label="variable: a\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:19:18-19"] - variable_50 [label="variable: b\nuint64\ntests/contract_testcases/solana/rational_comparison.sol:19:22-23"] + parameters_18 [label="parameters\nuint64 a\nuint64 b"] + returns_19 [label="returns\nbool "] diagnostic [label="found contract 'c'\nlevel Debug\ntests/contract_testcases/solana/rational_comparison.sol:2:1-21:2"] - diagnostic_53 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:4:10-23"] - diagnostic_54 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:7:10-19"] - diagnostic_55 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:10:10-19"] - diagnostic_56 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:13:10-11"] - diagnostic_57 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:16:10-26"] - diagnostic_58 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:19:10-24"] + diagnostic_22 [label="cannot use rational numbers with '>=' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:4:10-23"] + diagnostic_23 [label="cannot use rational numbers with '>' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:7:10-19"] + diagnostic_24 [label="cannot use rational numbers with '!=' or '==' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:10:10-19"] + diagnostic_25 [label="expression not allowed in constant rational number expression\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:13:10-11"] + diagnostic_26 [label="cannot use rational numbers with '<=' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:16:10-26"] + diagnostic_27 [label="cannot use rational numbers with '!=' or '==' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison.sol:19:10-24"] contracts -> contract contract -> foo1 [label="function"] foo1 -> parameters [label="parameters"] foo1 -> returns [label="returns"] - foo1 -> return [label="body"] - return -> more_equal [label="expr"] - more_equal -> cast [label="left"] - cast -> divide [label="expr"] - divide -> variable [label="left"] - divide -> variable_10 [label="right"] - more_equal -> rational_literal [label="right"] contract -> foo2 [label="function"] - foo2 -> parameters_13 [label="parameters"] - foo2 -> returns_14 [label="returns"] - foo2 -> return_15 [label="body"] - return_15 -> more [label="expr"] - more -> rational_literal_17 [label="left"] - more -> cast_18 [label="right"] - cast_18 -> variable_19 [label="expr"] + foo2 -> parameters_6 [label="parameters"] + foo2 -> returns_7 [label="returns"] contract -> foo3 [label="function"] - foo3 -> parameters_21 [label="parameters"] - foo3 -> returns_22 [label="returns"] - foo3 -> return_23 [label="body"] - return_23 -> equal [label="expr"] - equal -> rational_literal_25 [label="left"] - equal -> rational_literal_26 [label="right"] + foo3 -> parameters_9 [label="parameters"] + foo3 -> returns_10 [label="returns"] contract -> foo4 [label="function"] - foo4 -> parameters_28 [label="parameters"] - foo4 -> returns_29 [label="returns"] + foo4 -> parameters_12 [label="parameters"] + foo4 -> returns_13 [label="returns"] contract -> foo5 [label="function"] - foo5 -> parameters_31 [label="parameters"] - foo5 -> returns_32 [label="returns"] - foo5 -> return_33 [label="body"] - return_33 -> less_equal [label="expr"] - less_equal -> cast_35 [label="left"] - cast_35 -> shift_left [label="expr"] - shift_left -> variable_37 [label="left"] - shift_left -> variable_38 [label="right"] - less_equal -> rational_literal_39 [label="right"] + foo5 -> parameters_15 [label="parameters"] + foo5 -> returns_16 [label="returns"] contract -> foo6 [label="function"] - foo6 -> parameters_41 [label="parameters"] - foo6 -> returns_42 [label="returns"] - foo6 -> return_43 [label="body"] - return_43 -> not [label="expr"] - not -> equal_45 [label="expr"] - equal_45 -> rational_literal_46 [label="left"] - equal_45 -> cast_47 [label="right"] - cast_47 -> bitwise_xor [label="expr"] - bitwise_xor -> variable_49 [label="left"] - bitwise_xor -> variable_50 [label="right"] + foo6 -> parameters_18 [label="parameters"] + foo6 -> returns_19 [label="returns"] diagnostics -> diagnostic [label="Debug"] - diagnostics -> diagnostic_53 [label="Error"] - diagnostics -> diagnostic_54 [label="Error"] - diagnostics -> diagnostic_55 [label="Error"] - diagnostics -> diagnostic_56 [label="Error"] - diagnostics -> diagnostic_57 [label="Error"] - diagnostics -> diagnostic_58 [label="Error"] + diagnostics -> diagnostic_22 [label="Error"] + diagnostics -> diagnostic_23 [label="Error"] + diagnostics -> diagnostic_24 [label="Error"] + diagnostics -> diagnostic_25 [label="Error"] + diagnostics -> diagnostic_26 [label="Error"] + diagnostics -> diagnostic_27 [label="Error"] } diff --git a/tests/contract_testcases/solana/rational_comparison2.dot b/tests/contract_testcases/solana/rational_comparison2.dot new file mode 100644 index 000000000..724b3890f --- /dev/null +++ b/tests/contract_testcases/solana/rational_comparison2.dot @@ -0,0 +1,48 @@ +strict digraph "tests/contract_testcases/solana/rational_comparison2.sol" { + contract [label="contract c\ntests/contract_testcases/solana/rational_comparison2.sol:1:1-29:2"] + eq [label="function eq\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:2:2-37\nsignature eq()\nvisibility public\nmutability nonpayable"] + returns [label="returns\nbool "] + ne [label="function ne\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:6:2-37\nsignature ne()\nvisibility public\nmutability nonpayable"] + returns_5 [label="returns\nbool "] + lt [label="function lt\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:10:2-22\nsignature lt()\nvisibility public\nmutability nonpayable"] + le [label="function le\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:14:2-28\nsignature le(bool)\nvisibility public\nmutability nonpayable"] + parameters [label="parameters\nbool a"] + gt [label="function gt\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:18:2-28\nsignature gt(bool)\nvisibility public\nmutability nonpayable"] + parameters_10 [label="parameters\nbool a"] + gt_11 [label="function gt\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:22:2-42\nsignature gt(int256)\nvisibility public\nmutability nonpayable"] + parameters_12 [label="parameters\nint256 a"] + returns_13 [label="returns\nbool "] + ge [label="function ge\ncontract: c\ntests/contract_testcases/solana/rational_comparison2.sol:26:2-28\nsignature ge(bool)\nvisibility public\nmutability nonpayable"] + parameters_15 [label="parameters\nbool a"] + diagnostic [label="found contract 'c'\nlevel Debug\ntests/contract_testcases/solana/rational_comparison2.sol:1:1-29:2"] + diagnostic_18 [label="cannot use rational numbers with '!=' or '==' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:3:10-20"] + diagnostic_19 [label="cannot use rational numbers with '!=' or '==' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:7:10-20"] + diagnostic_20 [label="cannot use rational numbers with '<' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:11:11-20"] + diagnostic_21 [label="cannot use rational numbers with '<=' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:15:16-26"] + diagnostic_22 [label="cannot use rational numbers with '>' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:19:7-14"] + diagnostic_23 [label="cannot use rational numbers with '>' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:23:10-17"] + diagnostic_24 [label="cannot use rational numbers with '>=' operator\nlevel Error\ntests/contract_testcases/solana/rational_comparison2.sol:27:6-15"] + contracts -> contract + contract -> eq [label="function"] + eq -> returns [label="returns"] + contract -> ne [label="function"] + ne -> returns_5 [label="returns"] + contract -> lt [label="function"] + contract -> le [label="function"] + le -> parameters [label="parameters"] + contract -> gt [label="function"] + gt -> parameters_10 [label="parameters"] + contract -> gt_11 [label="function"] + gt_11 -> parameters_12 [label="parameters"] + gt_11 -> returns_13 [label="returns"] + contract -> ge [label="function"] + ge -> parameters_15 [label="parameters"] + diagnostics -> diagnostic [label="Debug"] + diagnostics -> diagnostic_18 [label="Error"] + diagnostics -> diagnostic_19 [label="Error"] + diagnostics -> diagnostic_20 [label="Error"] + diagnostics -> diagnostic_21 [label="Error"] + diagnostics -> diagnostic_22 [label="Error"] + diagnostics -> diagnostic_23 [label="Error"] + diagnostics -> diagnostic_24 [label="Error"] +} diff --git a/tests/contract_testcases/solana/rational_comparison2.sol b/tests/contract_testcases/solana/rational_comparison2.sol new file mode 100644 index 000000000..1374fc5c0 --- /dev/null +++ b/tests/contract_testcases/solana/rational_comparison2.sol @@ -0,0 +1,30 @@ +contract c { + function eq() public returns (bool) { + return 1.1 == 1.0; + } + + function ne() public returns (bool) { + return 1.1 != 1.0; + } + + function lt() public { + require(1.0 < 1.1); + } + + function le(bool a) public { + require(a && 0.1 <= 1.1); + } + + function gt(bool a) public { + if (1 > 0.5) { } + } + + function gt(int a) public returns (bool) { + return a > 1.1; + } + + function ge(bool a) public { + gt(1 >= 1.02); + } +} +