diff --git a/Changelog.md b/Changelog.md index 5c1420ca346c..c39c300d9149 100644 --- a/Changelog.md +++ b/Changelog.md @@ -17,6 +17,7 @@ Compiler Features: * Yul Optimizer: Improve performance of control flow side effects collector and function references resolver. Bugfixes: +* Constant Evaluator: Fix incorrect result of bit operations which were not consistent with codegen. * Yul: Fix incorrect serialization of Yul object names containing double quotes and escape sequences, producing output that could not be parsed as valid Yul. * Yul EVM Code Transform: Improve stack shuffler performance by fixing a BFS deduplication issue. diff --git a/libsolidity/analysis/ConstantEvaluator.cpp b/libsolidity/analysis/ConstantEvaluator.cpp index f094988d9bd6..60d5a5fcdd0a 100644 --- a/libsolidity/analysis/ConstantEvaluator.cpp +++ b/libsolidity/analysis/ConstantEvaluator.cpp @@ -272,7 +272,30 @@ TypedValue convertType(TypedValue const& _value, Type const& _type) [&](std::monostate const&) { return TypedValue{}; } - }, _value.value); + }, _value.value()); +} + +rational truncateToType(rational const& _value, Type const& _type) +{ + if (_type.category() != Type::Category::Integer) + return _value; + + auto const* integerType = dynamic_cast(&_type); + solAssert(integerType); + + solAssert(_value.denominator() == 1); + bigint integerValue = _value.numerator(); + + unsigned int numBits = integerType->numBits(); + bigint mask = (bigint(1) << numBits) - 1; + // clean bits out of range + integerValue = integerValue & mask; + + // extend sign if needed + if (integerType->isSigned() && boost::multiprecision::bit_test(integerValue, numBits - 1)) + integerValue = integerValue | ~mask; + + return rational(integerValue); } TypedValue constantToTypedValue(Type const& _type) @@ -346,20 +369,24 @@ TypedValue ConstantEvaluator::evaluate(ASTNode const& _node) void ConstantEvaluator::endVisit(UnaryOperation const& _operation) { TypedValue value = evaluate(_operation.subExpression()); - if (!value.type) + if (value.empty()) return; - Type const* resultType = value.type->unaryOperatorResult(_operation.getOperator()); + Type const* resultType = value.type()->unaryOperatorResult(_operation.getOperator()); if (!resultType) return; value = convertType(value, *resultType); - if (!std::holds_alternative(value.value)) + if (!value.isRational()) return; - if (std::optional result = evaluateUnaryOperator(_operation.getOperator(), std::get(value.value))) + if (std::optional result = evaluateUnaryOperator(_operation.getOperator(), value.asRational())) { - TypedValue convertedValue = convertType(*result, *resultType); - if (!convertedValue.type) + rational resultValue = *result; + if (TokenTraits::isBitOp(_operation.getOperator())) + resultValue = truncateToType(resultValue, *resultType); + + TypedValue convertedValue = convertType(resultValue, *resultType); + if (!convertedValue.type()) m_errorReporter.fatalTypeError( 3667_error, _operation.location(), @@ -373,7 +400,7 @@ void ConstantEvaluator::endVisit(BinaryOperation const& _operation) { TypedValue left = evaluate(_operation.leftExpression()); TypedValue right = evaluate(_operation.rightExpression()); - if (!left.type || !right.type) + if (!left.type() || !right.type()) return; // If this is implemented in the future: Comparison operators have a "binaryOperatorResult" @@ -381,7 +408,7 @@ void ConstantEvaluator::endVisit(BinaryOperation const& _operation) if (TokenTraits::isCompareOp(_operation.getOperator())) return; - Type const* resultType = left.type->binaryOperatorResult(_operation.getOperator(), right.type); + Type const* resultType = left.type()->binaryOperatorResult(_operation.getOperator(), right.type()); if (!resultType) { m_errorReporter.fatalTypeError( @@ -390,9 +417,9 @@ void ConstantEvaluator::endVisit(BinaryOperation const& _operation) "Operator " + std::string(TokenTraits::toString(_operation.getOperator())) + " not compatible with types " + - left.type->toString() + + left.type()->toString() + " and " + - right.type->toString() + right.type()->toString() ); return; } @@ -400,19 +427,23 @@ void ConstantEvaluator::endVisit(BinaryOperation const& _operation) left = convertType(left, *resultType); right = convertType(right, *resultType); if ( - !std::holds_alternative(left.value) || - !std::holds_alternative(right.value) + !left.isRational() || + !right.isRational() ) return; if (std::optional value = evaluateBinaryOperator( _operation.getOperator(), - std::get(left.value), - std::get(right.value) + left.asRational(), + right.asRational() )) { - TypedValue convertedValue = convertType(*value, *resultType); - if (!convertedValue.type) + rational resultValue = *value; + if (TokenTraits::isShiftOp(_operation.getOperator())) + resultValue = truncateToType(*value, *resultType); + + TypedValue convertedValue = convertType(resultValue, *resultType); + if (!convertedValue.type()) m_errorReporter.fatalTypeError( 2643_error, _operation.location(), @@ -455,10 +486,10 @@ void ConstantEvaluator::endVisit(FunctionCall const& _functionCall) { solAssert(_functionCall.arguments().size() == 1); auto stringArg = evaluate(*(_functionCall.arguments()[0].get())); - if (!std::holds_alternative(stringArg.value)) + if (!stringArg.isString()) return; - h256 innerKeccak = keccak256(std::get(stringArg.value)); + h256 innerKeccak = keccak256(stringArg.asString()); h256 outerKeccak = keccak256(h256(u256(innerKeccak) - 1)); outerKeccak.data()[31] = 0; u256 slot = outerKeccak; @@ -470,3 +501,35 @@ void ConstantEvaluator::endVisit(FunctionCall const& _functionCall) break; } } + +TypedValue::TypedValue(Type const* _type, TypedValue::Value const _value) +{ + std::visit(util::GenericVisitor{ + [&](std::string const&) { + solAssert(dynamic_cast(_type) || dynamic_cast(_type)); + }, + [&](rational const&) { + solAssert(dynamic_cast(_type) || dynamic_cast(_type)); + }, + [&](std::monostate const&) { + solAssert(!_type); + } + }, _value); + + m_type = _type; + m_value = _value; +} + +std::string const& TypedValue::asString() const +{ + auto const* stringValue = std::get_if(&m_value); + solAssert(stringValue); + return *stringValue; +} + +rational const& TypedValue::asRational() const +{ + auto const* rationalValue = std::get_if(&m_value); + solAssert(rationalValue); + return *rationalValue; +} diff --git a/libsolidity/analysis/ConstantEvaluator.h b/libsolidity/analysis/ConstantEvaluator.h index 67adc0be1a64..75e06a1fbc32 100644 --- a/libsolidity/analysis/ConstantEvaluator.h +++ b/libsolidity/analysis/ConstantEvaluator.h @@ -38,7 +38,7 @@ namespace solidity::frontend class TypeChecker; /** - * Small drop-in replacement for TypeChecker to evaluate simple expressions of integer constants. + * Small drop-in replacement for TypeChecker to evaluate simple expressions of integer and string constants. * * Note: This always use "checked arithmetic" in the sense that any over- or underflow * results in "unknown" value. @@ -46,11 +46,24 @@ class TypeChecker; class ConstantEvaluator: private ASTConstVisitor { public: - struct TypedValue + class TypedValue { + public: + using Value = std::variant; + + TypedValue(): m_type(nullptr), m_value(std::monostate()) {} + TypedValue(Type const* _type, Value const _value); + bool empty() const { return std::holds_alternative(m_value); } + bool isString() const { return std::holds_alternative(m_value); } + bool isRational() const { return std::holds_alternative(m_value); } + Type const* type() const { return m_type; } + Value const& value() const { return m_value; } + std::string const& asString() const; + rational const& asRational() const; + private: // Type may be RationalType or IntegerType for value rational - Type const* type; - std::variant value; + Type const* m_type; + Value m_value; }; static TypedValue evaluate( diff --git a/libsolidity/analysis/DeclarationTypeChecker.cpp b/libsolidity/analysis/DeclarationTypeChecker.cpp index e2f0cf6e8b00..5d9de97680fd 100644 --- a/libsolidity/analysis/DeclarationTypeChecker.cpp +++ b/libsolidity/analysis/DeclarationTypeChecker.cpp @@ -339,9 +339,9 @@ void DeclarationTypeChecker::endVisit(ArrayTypeName const& _typeName) if (length->annotation().type && length->annotation().type->category() == Type::Category::RationalNumber) lengthValue = dynamic_cast(*length->annotation().type).value(); else if (ConstantEvaluator::TypedValue value = ConstantEvaluator::evaluate(m_errorReporter, *length); - std::holds_alternative(value.value) + value.isRational() ) - lengthValue = std::get(value.value); + lengthValue = value.asRational(); if (!lengthValue) m_errorReporter.typeError( diff --git a/libsolidity/analysis/PostTypeContractLevelChecker.cpp b/libsolidity/analysis/PostTypeContractLevelChecker.cpp index be5d008a6161..7acf5d1774e8 100644 --- a/libsolidity/analysis/PostTypeContractLevelChecker.cpp +++ b/libsolidity/analysis/PostTypeContractLevelChecker.cpp @@ -137,8 +137,8 @@ void PostTypeContractLevelChecker::checkStorageLayoutSpecifier(ContractDefinitio if (integerType) { ConstantEvaluator::TypedValue typedRational = ConstantEvaluator::evaluate(m_errorReporter, baseSlotExpression); - solAssert(!typedRational.type || dynamic_cast(typedRational.type)); - if (!typedRational.type) + solAssert(!typedRational.type() || dynamic_cast(typedRational.type())); + if (!typedRational.type()) { m_errorReporter.typeError( 1505_error, @@ -148,8 +148,8 @@ void PostTypeContractLevelChecker::checkStorageLayoutSpecifier(ContractDefinitio ); return; } - solAssert(std::holds_alternative(typedRational.value)); - baseSlotRationalValue = std::get(typedRational.value); + solAssert(typedRational.isRational()); + baseSlotRationalValue = typedRational.asRational(); } else { diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index 1ad3e77441db..a0c6e5d55655 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -314,12 +314,12 @@ bool StaticAnalyzer::visit(BinaryOperation const& _operation) if ( *_operation.rightExpression().annotation().isPure && (_operation.getOperator() == Token::Div || _operation.getOperator() == Token::Mod) && - ConstantEvaluator::evaluate(m_errorReporter, _operation.leftExpression()).type + ConstantEvaluator::evaluate(m_errorReporter, _operation.leftExpression()).type() ) if ( auto rhs = ConstantEvaluator::evaluate(m_errorReporter, _operation.rightExpression()); - std::holds_alternative(rhs.value) && - std::get(rhs.value) == 0 + rhs.isRational() && + rhs.asRational() == 0 ) m_errorReporter.typeError( 1211_error, @@ -342,8 +342,8 @@ bool StaticAnalyzer::visit(FunctionCall const& _functionCall) if (*_functionCall.arguments()[2]->annotation().isPure) if ( auto lastArg = ConstantEvaluator::evaluate(m_errorReporter, *(_functionCall.arguments())[2]); - std::holds_alternative(lastArg.value) && - std::get(lastArg.value) == 0 + lastArg.isRational() && + lastArg.asRational() == 0 ) m_errorReporter.typeError( 4195_error, diff --git a/libsolidity/ast/ASTUtils.cpp b/libsolidity/ast/ASTUtils.cpp index 532b0bc7af35..a5a380d25704 100644 --- a/libsolidity/ast/ASTUtils.cpp +++ b/libsolidity/ast/ASTUtils.cpp @@ -146,11 +146,11 @@ std::optional erc7201CompileTimeValue(FunctionCall const& _erc7201Call) auto evaluatedResult = ConstantEvaluator::tryEvaluate(_erc7201Call); - if (std::holds_alternative(evaluatedResult.value)) + if (evaluatedResult.empty()) return std::nullopt; - solAssert(std::holds_alternative(evaluatedResult.value)); - auto rationalValue = std::get(evaluatedResult.value); + solAssert(evaluatedResult.isRational()); + auto rationalValue = evaluatedResult.asRational(); solAssert(rationalValue.denominator() == 1); bigint baseSlot = rationalValue.numerator(); solAssert(baseSlot <= std::numeric_limits::max()); diff --git a/libsolidity/formal/SMTEncoder.cpp b/libsolidity/formal/SMTEncoder.cpp index df902fd8627a..4141dd7f9b5e 100644 --- a/libsolidity/formal/SMTEncoder.cpp +++ b/libsolidity/formal/SMTEncoder.cpp @@ -3124,9 +3124,9 @@ RationalNumberType const* SMTEncoder::isConstant(Expression const& _expr) if ( auto typedValue = ConstantEvaluator::tryEvaluate(_expr); - std::holds_alternative(typedValue.value) + typedValue.isRational() ) - return TypeProvider::rationalNumber(std::get(typedValue.value)); + return TypeProvider::rationalNumber(typedValue.asRational()); return nullptr; } diff --git a/test/libsolidity/semanticTests/constantEvaluator/bit_not_operation_runtime_equivalence.sol b/test/libsolidity/semanticTests/constantEvaluator/bit_not_operation_runtime_equivalence.sol new file mode 100644 index 000000000000..984c919b332c --- /dev/null +++ b/test/libsolidity/semanticTests/constantEvaluator/bit_not_operation_runtime_equivalence.sol @@ -0,0 +1,36 @@ +uint8 constant U253 = 253; // 1111 1101 +int8 constant I128 = -128; // 1000 0000 +int8 constant IONE = 1; // 0000 0001 +contract C { + uint8 constant UNSIGNED = ~U253; // = 2 (0000 0010) + uint[UNSIGNED] a; + int8 constant NEGATIVE_SIGNED = ~I128; // = 127 (0111 1111) + uint[NEGATIVE_SIGNED] b; + int8 constant POSITIVE_SIGNED = ~IONE; // = -2 (1111 1110) + uint[POSITIVE_SIGNED * -1] c; + function testUnsignedEquivalence() public view returns (bool) { + uint8 runTimeResult = ~U253; + + return + UNSIGNED == runTimeResult && + a.length == runTimeResult; + } + function testNegativeSignedEquivalence() public view returns (bool) { + int8 runTimeResult = ~I128; + + return + NEGATIVE_SIGNED == runTimeResult && + b.length == 127; + } + function testPositiveSignedEquivalence() public view returns (bool) { + int8 runTimeResult = ~IONE; + + return + POSITIVE_SIGNED == runTimeResult && + c.length == 2; + } +} +// ---- +// testUnsignedEquivalence() -> true +// testNegativeSignedEquivalence() -> true +// testPositiveSignedEquivalence() -> true diff --git a/test/libsolidity/semanticTests/constantEvaluator/shift_signed_left_operand_runtime_equivalence.sol b/test/libsolidity/semanticTests/constantEvaluator/shift_signed_left_operand_runtime_equivalence.sol new file mode 100644 index 000000000000..47b698396704 --- /dev/null +++ b/test/libsolidity/semanticTests/constantEvaluator/shift_signed_left_operand_runtime_equivalence.sol @@ -0,0 +1,53 @@ +uint256 constant ONE = 1; +int8 constant I8_NEGATIVE_63 = -63; +int8 constant I8_POSITIVE_127 = 127; +int16 constant I16_POSITIVE_127 = 127; + +contract C { + // right side cannot be signed + int256 constant LITERAL_WRAP = -2**255 << ONE; // = 0 + uint[LITERAL_WRAP + 1] a; + int8 constant CONST_NO_WRAP = I8_NEGATIVE_63 << 1; + uint[CONST_NO_WRAP * -1] b; + int8 constant CONST_WRAP = I8_POSITIVE_127 << 1; // = -2 (1111 1110) + uint[CONST_WRAP * -1] c; + int16 constant CONST_SIGN_CHANGED = I16_POSITIVE_127 << 9; // = -512 (1111 1110 0000 0000) + uint[CONST_SIGN_CHANGED * -1] d; + + function testLiteralWrapEquivalence() public view returns (bool) { + int256 runTimeResult = -2**255 << ONE; + + return + LITERAL_WRAP == runTimeResult && + a.length == 1; + } + + function testConstNoWrapEquivalence() public view returns (bool) { + int8 runTimeResult = I8_NEGATIVE_63 << 1; + + return + CONST_NO_WRAP == runTimeResult && + b.length == 126; + } + + function testConstWrapEquivalence() public view returns (bool) { + int8 runTimeResult = I8_POSITIVE_127 << 1; + + return + CONST_WRAP == runTimeResult && + c.length == 2; + } + + function testConstSignChanged() public view returns (bool) { + int16 runTimeResult = I16_POSITIVE_127 << 9; + + return + CONST_SIGN_CHANGED == runTimeResult && + d.length == 512; + } +} +// ---- +// testLiteralWrapEquivalence() -> true +// testConstNoWrapEquivalence() -> true +// testConstWrapEquivalence() -> true +// testConstSignChanged() -> true diff --git a/test/libsolidity/semanticTests/constantEvaluator/shift_unsigned_left_operand_runtime_equivalence.sol b/test/libsolidity/semanticTests/constantEvaluator/shift_unsigned_left_operand_runtime_equivalence.sol new file mode 100644 index 000000000000..ca054425d331 --- /dev/null +++ b/test/libsolidity/semanticTests/constantEvaluator/shift_unsigned_left_operand_runtime_equivalence.sol @@ -0,0 +1,43 @@ +uint256 constant ONE = 1; +uint8 constant U8_64 = 64; +uint8 constant U8_255 = 255; + +contract C { + // Expression with only literals have rational type with unlimited precision, + // so we use a integer constant to force the literal have uint256 (mobileType). + // The whole expression then has type uint256. + uint256 constant LITERAL_WRAP = 2**255 << ONE; // = 0 + uint[LITERAL_WRAP + 1] a; + uint8 constant CONST_NO_WRAP = U8_64 << 1; + uint[CONST_NO_WRAP] b; + uint8 constant CONST_WRAP = U8_255 << 4; // = 240 (1111 0000) + uint[CONST_WRAP] c; + + function testLiteralWrapEquivalence() public view returns (bool) { + uint256 runTimeResult = 2**255 << ONE; + + return + LITERAL_WRAP == runTimeResult && + a.length == runTimeResult + 1; + } + + function testConstNoWrapEquivalence() public view returns (bool) { + uint8 runTimeResult = U8_64 << 1; + + return + CONST_NO_WRAP == runTimeResult && + b.length == runTimeResult; + } + + function testConstWrapEquivalence() public view returns (bool) { + uint8 runTimeResult = U8_255 << 4; + + return + CONST_WRAP == runTimeResult && + c.length == runTimeResult; + } +} +// ---- +// testLiteralWrapEquivalence() -> true +// testConstNoWrapEquivalence() -> true +// testConstWrapEquivalence() -> true diff --git a/test/libsolidity/smtCheckerTests/operators/constant_evaluation_bitwise_not.sol b/test/libsolidity/smtCheckerTests/operators/constant_evaluation_bitwise_not.sol index 54159b18074d..b9418c1f043c 100644 --- a/test/libsolidity/smtCheckerTests/operators/constant_evaluation_bitwise_not.sol +++ b/test/libsolidity/smtCheckerTests/operators/constant_evaluation_bitwise_not.sol @@ -7,3 +7,4 @@ contract C { // ==== // SMTEngine: chc // ---- +// Warning 6031: (186-199): Internal error: Expression undefined for SMT solver. diff --git a/test/libsolidity/syntaxTests/storageLayoutSpecifier/constant_divided_by_its_negation.sol b/test/libsolidity/syntaxTests/storageLayoutSpecifier/constant_divided_by_its_negation.sol index 9f8acbeb6a36..9db4eb809866 100644 --- a/test/libsolidity/syntaxTests/storageLayoutSpecifier/constant_divided_by_its_negation.sol +++ b/test/libsolidity/syntaxTests/storageLayoutSpecifier/constant_divided_by_its_negation.sol @@ -1,4 +1,3 @@ uint constant N = 100; contract C layout at N / ~N {} // ---- -// TypeError 3667: (48-50): Arithmetic error when computing constant value.