Merge pull request #588 from chriseth/cleanup

Fix: Perform proper cleanup for bytesNN types.

7 files changed, 49 insertions, 26 deletions

diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index d989cb06..e964d683 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -923,19 +923,6 @@ bool FixedBytesType::operator==(Type const& _other) const
 	return other.m_bytes == m_bytes;
 }
 
-bool BoolType::isExplicitlyConvertibleTo(Type const& _convertTo) const
-{
-	// conversion to integer is fine, but not to address
-	// this is an example of explicit conversions being not transitive (though implicit should be)
-	if (_convertTo.category() == category())
-	{
-		IntegerType const& convertTo = dynamic_cast<IntegerType const&>(_convertTo);
-		if (!convertTo.isAddress())
-			return true;
-	}
-	return isImplicitlyConvertibleTo(_convertTo);
-}
-
 u256 BoolType::literalValue(Literal const* _literal) const
 {
 	solAssert(_literal, "");
diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h
index 0c437316..967e968c 100644
--- a/libsolidity/ast/Types.h
+++ b/libsolidity/ast/Types.h
@@ -470,7 +470,6 @@ class BoolType: public Type
 public:
 	BoolType() {}
 	virtual Category category() const override { return Category::Bool; }
-	virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const override;
 	virtual TypePointer unaryOperatorResult(Token::Value _operator) const override;
 	virtual TypePointer binaryOperatorResult(Token::Value _operator, TypePointer const& _other) const override;
 
diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp
index e35f3374..efb9b10a 100644
--- a/libsolidity/codegen/CompilerUtils.cpp
+++ b/libsolidity/codegen/CompilerUtils.cpp
@@ -323,18 +323,17 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 		}
 		else
 		{
-			// clear lower-order bytes for conversion to shorter bytes - we always clean
+			// clear for conversion to longer bytes
 			solAssert(targetTypeCategory == Type::Category::FixedBytes, "Invalid type conversion requested.");
 			FixedBytesType const& targetType = dynamic_cast<FixedBytesType const&>(_targetType);
-			if (targetType.numBytes() < typeOnStack.numBytes())
+			if (targetType.numBytes() > typeOnStack.numBytes() || _cleanupNeeded)
 			{
-				if (targetType.numBytes() == 0)
-					m_context << Instruction::DUP1 << Instruction::XOR;
+				if (typeOnStack.numBytes() == 0)
+					m_context << Instruction::POP << u256(0);
 				else
 				{
-					m_context << (u256(1) << (256 - targetType.numBytes() * 8));
-					m_context << Instruction::DUP1 << Instruction::SWAP2;
-					m_context << Instruction::DIV << Instruction::MUL;
+					m_context << ((u256(1) << (256 - typeOnStack.numBytes() * 8)) - 1);
+					m_context << Instruction::NOT << Instruction::AND;
 				}
 			}
 		}
@@ -637,6 +636,10 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 		}
 		break;
 	}
+	case Type::Category::Bool:
+		solAssert(_targetType == _typeOnStack, "Invalid conversion for bool.");
+		if (_cleanupNeeded)
+			m_context << Instruction::ISZERO << Instruction::ISZERO;
 	default:
 		// All other types should not be convertible to non-equal types.
 		solAssert(_typeOnStack == _targetType, "Invalid type conversion requested.");
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index a01e306e..16677093 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -360,8 +360,11 @@ bool ExpressionCompiler::visit(BinaryOperation const& _binaryOperation)
 		m_context << commonType.literalValue(nullptr);
 	else
 	{
-		bool cleanupNeeded = commonType.category() == Type::Category::Integer &&
-			(Token::isCompareOp(c_op) || c_op == Token::Div || c_op == Token::Mod);
+		bool cleanupNeeded = false;
+		if (Token::isCompareOp(c_op))
+			cleanupNeeded = true;
+		if (commonType.category() == Type::Category::Integer && (c_op == Token::Div || c_op == Token::Mod))
+			cleanupNeeded = true;
 
 		// for commutative operators, push the literal as late as possible to allow improved optimization
 		auto isLiteral = [](Expression const& _e)
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 8c537346..30ae5792 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -6775,6 +6775,24 @@ BOOST_AUTO_TEST_CASE(iszero_bnot_correct)
 	BOOST_CHECK(callContractFunction("f()") == encodeArgs(true));
 }
 
+BOOST_AUTO_TEST_CASE(cleanup_bytes_types)
+{
+	// Checks that bytesXX types are properly cleaned before they are compared.
+	char const* sourceCode = R"(
+		contract C {
+			function f(bytes2 a, uint16 x) returns (uint) {
+				if (a != "ab") return 1;
+				if (x != 0x0102) return 2;
+				if (bytes3(x) != 0x0102) return 3;
+				return 0;
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "C");
+	// We input longer data on purpose.
+	BOOST_CHECK(callContractFunction("f(bytes2,uint16)", string("abc"), u256(0x040102)) == encodeArgs(0));
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }
diff --git a/test/libsolidity/SolidityExpressionCompiler.cpp b/test/libsolidity/SolidityExpressionCompiler.cpp
index 398ac24f..967b2907 100644
--- a/test/libsolidity/SolidityExpressionCompiler.cpp
+++ b/test/libsolidity/SolidityExpressionCompiler.cpp
@@ -267,10 +267,10 @@ BOOST_AUTO_TEST_CASE(comparison)
 							 "}\n";
 	bytes code = compileFirstExpression(sourceCode);
 
-	bytes expectation({byte(Instruction::PUSH1), 0x1,
+	bytes expectation({byte(Instruction::PUSH1), 0x1, byte(Instruction::ISZERO), byte(Instruction::ISZERO),
 					   byte(Instruction::PUSH2), 0x11, 0xaa,
 					   byte(Instruction::PUSH2), 0x10, 0xaa,
-					   byte(Instruction::LT),
+					   byte(Instruction::LT), byte(Instruction::ISZERO), byte(Instruction::ISZERO),
 					   byte(Instruction::EQ),
 					   byte(Instruction::ISZERO)});
 	BOOST_CHECK_EQUAL_COLLECTIONS(code.begin(), code.end(), expectation.begin(), expectation.end());
@@ -296,7 +296,8 @@ BOOST_AUTO_TEST_CASE(short_circuiting)
 					   byte(Instruction::EQ),
 					   byte(Instruction::ISZERO), // after this we have 9 != 2
 					   byte(Instruction::JUMPDEST),
-					   byte(Instruction::PUSH1), 0x1,
+					   byte(Instruction::ISZERO), byte(Instruction::ISZERO),
+					   byte(Instruction::PUSH1), 0x1, byte(Instruction::ISZERO), byte(Instruction::ISZERO),
 					   byte(Instruction::EQ),
 					   byte(Instruction::ISZERO)});
 	BOOST_CHECK_EQUAL_COLLECTIONS(code.begin(), code.end(), expectation.begin(), expectation.end());
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index bbc77d34..52e0bf58 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -3683,6 +3683,18 @@ BOOST_AUTO_TEST_CASE(zero_handling)
 	BOOST_CHECK(success(text));
 }
 
+BOOST_AUTO_TEST_CASE(missing_bool_conversion)
+{
+	char const* text = R"(
+		contract test {
+			function b(uint a) {
+				bool(a == 1);
+			}
+		}
+	)";
+	BOOST_CHECK(success(text));
+}
+
 BOOST_AUTO_TEST_CASE(integer_and_fixed_interaction)
 {
 	char const* text = R"(