Make ConstantEvaluator truncate the result of shift operations

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.
 

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<IntegerType const*>(&_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<rational>(value.value))
+	if (!value.isRational())
 		return;
 
-	if (std::optional<rational> result = evaluateUnaryOperator(_operation.getOperator(), std::get<rational>(value.value)))
+	if (std::optional<rational> 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,15 +400,15 @@ 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"
 	// that is non-bool, but the result has to be bool.
 	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,29 +417,33 @@ 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;
 	}
 
 	left = convertType(left, *resultType);
 	right = convertType(right, *resultType);
 	if (
-		!std::holds_alternative<rational>(left.value) ||
-		!std::holds_alternative<rational>(right.value)
+		!left.isRational() ||
+		!right.isRational()
 	)
 		return;
 
 	if (std::optional<rational> value = evaluateBinaryOperator(
 		_operation.getOperator(),
-		std::get<rational>(left.value),
-		std::get<rational>(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<std::string>(stringArg.value))
+			if (!stringArg.isString())
 				return;
 
-			h256 innerKeccak = keccak256(std::get<std::string>(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<StringLiteralType const*>(_type) || dynamic_cast<ArrayType const*>(_type));
+		},
+		[&](rational const&) {
+			solAssert(dynamic_cast<RationalNumberType const*>(_type) || dynamic_cast<IntegerType const*>(_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<std::string>(&m_value);
+	solAssert(stringValue);
+	return *stringValue;
+}
+
+rational const& TypedValue::asRational() const
+{
+	auto const* rationalValue = std::get_if<rational>(&m_value);
+	solAssert(rationalValue);
+	return *rationalValue;
+}

libsolidity/analysis/ConstantEvaluator.h

@@ -38,19 +38,32 @@ 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.
  */
 class ConstantEvaluator: private ASTConstVisitor
 {
 public:
-	struct TypedValue
+	class TypedValue
 	{
+	public:
+		using Value = std::variant<std::monostate, rational, std::string>;
+
+		TypedValue(): m_type(nullptr), m_value(std::monostate()) {}
+		TypedValue(Type const* _type, Value const _value);
+		bool empty() const { return std::holds_alternative<std::monostate>(m_value); }
+		bool isString() const { return std::holds_alternative<std::string>(m_value); }
+		bool isRational() const { return std::holds_alternative<rational>(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<std::monostate, rational, std::string> value;
+		Type const* m_type;
+		Value m_value;
 	};
 
 	static TypedValue evaluate(

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<RationalNumberType const&>(*length->annotation().type).value();
 		else if (ConstantEvaluator::TypedValue value = ConstantEvaluator::evaluate(m_errorReporter, *length);
-			std::holds_alternative<rational>(value.value)
+			value.isRational()
 		)
-			lengthValue = std::get<rational>(value.value);
+			lengthValue = value.asRational();
 
 		if (!lengthValue)
 			m_errorReporter.typeError(

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<IntegerType const*>(typedRational.type));
-		if (!typedRational.type)
+		solAssert(!typedRational.type() || dynamic_cast<IntegerType const*>(typedRational.type()));
+		if (!typedRational.type())
 		{
 			m_errorReporter.typeError(
 				1505_error,
@@ -148,8 +148,8 @@ void PostTypeContractLevelChecker::checkStorageLayoutSpecifier(ContractDefinitio
 			);
 			return;
 		}
-		solAssert(std::holds_alternative<rational>(typedRational.value));
-		baseSlotRationalValue = std::get<rational>(typedRational.value);
+		solAssert(typedRational.isRational());
+		baseSlotRationalValue = typedRational.asRational();
 	}
 	else
 	{

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<rational>(rhs.value) &&
-			std::get<rational>(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<rational>(lastArg.value) &&
-					std::get<rational>(lastArg.value) == 0
+					lastArg.isRational() &&
+					lastArg.asRational() == 0
 				)
 					m_errorReporter.typeError(
 						4195_error,

libsolidity/ast/ASTUtils.cpp

@@ -146,11 +146,11 @@ std::optional<u256> erc7201CompileTimeValue(FunctionCall const& _erc7201Call)
 
 	auto evaluatedResult = ConstantEvaluator::tryEvaluate(_erc7201Call);
 
-	if (std::holds_alternative<std::monostate>(evaluatedResult.value))
+	if (evaluatedResult.empty())
 		return std::nullopt;
 
-	solAssert(std::holds_alternative<rational>(evaluatedResult.value));
-	auto rationalValue = std::get<rational>(evaluatedResult.value);
+	solAssert(evaluatedResult.isRational());
+	auto rationalValue = evaluatedResult.asRational();
 	solAssert(rationalValue.denominator() == 1);
 	bigint baseSlot = rationalValue.numerator();
 	solAssert(baseSlot <= std::numeric_limits<u256>::max());

libsolidity/formal/SMTEncoder.cpp

@@ -3124,9 +3124,9 @@ RationalNumberType const* SMTEncoder::isConstant(Expression const& _expr)
 
 	if (
 		auto typedValue = ConstantEvaluator::tryEvaluate(_expr);
-		std::holds_alternative<rational>(typedValue.value)
+		typedValue.isRational()
 	)
-		return TypeProvider::rationalNumber(std::get<rational>(typedValue.value));
+		return TypeProvider::rationalNumber(typedValue.asRational());
 
 	return nullptr;
 }

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

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