cerc-io/solidity
16
0
Fork 0
mirror of https://github.com/ethereum/solidity synced 2023-10-03 13:03:40 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Cleaner shift handling and type conversion for binary operations.

Browse Source
This commit is contained in:
chriseth 2016-12-06 23:45:17 +01:00
parent 2df60bec92
commit 2738045030
4 changed files with 122 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
libsolidity/ast/Types.cpp
View File

@ -251,6 +251,19 @@ MemberList::MemberMap Type::boundFunctions(Type const& _type, ContractDefinition
return members;
}
bool isValidShiftAndAmountType(Token::Value _operator, Type const& _shiftAmountType)
{
// Disable >>> here.
if (_operator == Token::SHR)
return false;
else if (IntegerType const* otherInt = dynamic_cast<decltype(otherInt)>(&_shiftAmountType))
return !otherInt->isAddress();
else if (RationalNumberType const* otherRat = dynamic_cast<decltype(otherRat)>(&_shiftAmountType))
return otherRat->integerType() && !otherRat->integerType()->isSigned();
else
return false;
}
IntegerType::IntegerType(int _bits, IntegerType::Modifier _modifier):
m_bits(_bits), m_modifier(_modifier)
{
@ -342,24 +355,13 @@ TypePointer IntegerType::binaryOperatorResult(Token::Value _operator, TypePointe
return TypePointer();
if (Token::isShiftOp(_operator))
{
// Disable >>> here.
if (_operator == Token::SHR)
return TypePointer();
// Shifts are not symmetric with respect to the type
if (isAddress())
return TypePointer();
if (IntegerType const* otherInt = dynamic_cast<decltype(otherInt)>(_other.get()))
{
if (!otherInt->isAddress())
return shared_from_this();
}
else if (RationalNumberType const* otherRat = dynamic_cast<decltype(otherRat)>(_other.get()))
{
if (!otherRat->isFractional())
return shared_from_this();
}
return TypePointer();
if (isValidShiftAndAmountType(_operator, *_other))
return shared_from_this();
else
return TypePointer();
}
auto commonType = Type::commonType(shared_from_this(), _other); //might be a integer or fixed point
@ -978,22 +980,10 @@ TypePointer FixedBytesType::binaryOperatorResult(Token::Value _operator, TypePoi
{
if (Token::isShiftOp(_operator))
{
// Disable >>> here.
if (_operator == Token::SHR)
if (isValidShiftAndAmountType(_operator, *_other))
return shared_from_this();
else
return TypePointer();
// Shifts are not symmetric with respect to the type
if (IntegerType const* otherInt = dynamic_cast<decltype(otherInt)>(_other.get()))
{
if (!otherInt->isAddress())
return shared_from_this();
}
else if (RationalNumberType const* otherRat = dynamic_cast<decltype(otherRat)>(_other.get()))
{
if (!otherRat->isFractional())
return shared_from_this();
}
return TypePointer();
}
auto commonType = dynamic_pointer_cast<FixedBytesType const>(Type::commonType(shared_from_this(), _other));

119
libsolidity/codegen/ExpressionCompiler.cpp
View File

@ -197,22 +197,37 @@ bool ExpressionCompiler::visit(Conditional const& _condition)
bool ExpressionCompiler::visit(Assignment const& _assignment)
{
CompilerContext::LocationSetter locationSetter(m_context, _assignment);
Token::Value op = _assignment.assignmentOperator();
Token::Value binOp = op == Token::Assign ? op : Token::AssignmentToBinaryOp(op);
Type const& leftType = *_assignment.leftHandSide().annotation().type;
if (leftType.category() == Type::Category::Tuple)
{
solAssert(*_assignment.annotation().type == TupleType(), "");
solAssert(op == Token::Assign, "");
}
else
solAssert(*_assignment.annotation().type == leftType, "");
bool cleanupNeeded = false;
if (op != Token::Assign)
cleanupNeeded = cleanupNeededForOp(leftType.category(), binOp);
_assignment.rightHandSide().accept(*this);
// Perform some conversion already. This will convert storage types to memory and literals
// to their actual type, but will not convert e.g. memory to storage.
TypePointer type = _assignment.rightHandSide().annotation().type->closestTemporaryType(
_assignment.leftHandSide().annotation().type
);
utils().convertType(*_assignment.rightHandSide().annotation().type, *type);
TypePointer rightIntermediateType;
if (op != Token::Assign && Token::isShiftOp(binOp))
rightIntermediateType = _assignment.rightHandSide().annotation().type->mobileType();
else
rightIntermediateType = _assignment.rightHandSide().annotation().type->closestTemporaryType(
_assignment.leftHandSide().annotation().type
);
utils().convertType(*_assignment.rightHandSide().annotation().type, *rightIntermediateType, cleanupNeeded);
_assignment.leftHandSide().accept(*this);
solAssert(!!m_currentLValue, "LValue not retrieved.");
Token::Value op = _assignment.assignmentOperator();
if (op != Token::Assign) // compound assignment
{
Token::Value target_op = Token::AssignmentToBinaryOp(op);
solUnimplementedAssert(_assignment.annotation().type->isValueType(), "Compound operators not implemented for non-value types.");
solAssert(leftType.isValueType(), "Compound operators only available for value types.");
unsigned lvalueSize = m_currentLValue->sizeOnStack();
unsigned itemSize = _assignment.annotation().type->sizeOnStack();
if (lvalueSize > 0)
@ -222,11 +237,17 @@ bool ExpressionCompiler::visit(Assignment const& _assignment)
// value lvalue_ref value lvalue_ref
}
m_currentLValue->retrieveValue(_assignment.location(), true);
if (Token::isShiftOp(target_op))
// shift only cares about the signedness of both sides
appendShiftOperatorCode(target_op, *_assignment.leftHandSide().annotation().type, *_assignment.rightHandSide().annotation().type);
utils().convertType(leftType, leftType, cleanupNeeded);
Token::Value targetOp = Token::AssignmentToBinaryOp(op);
if (Token::isShiftOp(targetOp))
appendShiftOperatorCode(targetOp, leftType, *rightIntermediateType);
else
appendOrdinaryBinaryOperatorCode(target_op, *_assignment.annotation().type);
{
solAssert(leftType == *rightIntermediateType, "");
appendOrdinaryBinaryOperatorCode(targetOp, leftType);
}
if (lvalueSize > 0)
{
solAssert(itemSize + lvalueSize <= 16, "Stack too deep, try removing local variables.");
@ -235,7 +256,7 @@ bool ExpressionCompiler::visit(Assignment const& _assignment)
m_context << swapInstruction(itemSize + lvalueSize) << Instruction::POP;
}
}
m_currentLValue->storeValue(*type, _assignment.location());
m_currentLValue->storeValue(*rightIntermediateType, _assignment.location());
m_currentLValue.reset();
return false;
}
@ -356,20 +377,19 @@ bool ExpressionCompiler::visit(BinaryOperation const& _binaryOperation)
Expression const& leftExpression = _binaryOperation.leftExpression();
Expression const& rightExpression = _binaryOperation.rightExpression();
solAssert(!!_binaryOperation.annotation().commonType, "");
Type const& commonType = *_binaryOperation.annotation().commonType;
TypePointer const& commonType = _binaryOperation.annotation().commonType;
Token::Value const c_op = _binaryOperation.getOperator();
if (c_op == Token::And || c_op == Token::Or) // special case: short-circuiting
appendAndOrOperatorCode(_binaryOperation);
else if (commonType.category() == Type::Category::RationalNumber)
m_context << commonType.literalValue(nullptr);
else if (commonType->category() == Type::Category::RationalNumber)
m_context << commonType->literalValue(nullptr);
else
{
bool cleanupNeeded = false;
if (Token::isCompareOp(c_op) || Token::isShiftOp(c_op))
cleanupNeeded = true;
if (commonType.category() == Type::Category::Integer && (c_op == Token::Div || c_op == Token::Mod))
cleanupNeeded = true;
bool cleanupNeeded = cleanupNeededForOp(commonType->category(), c_op);
TypePointer leftTargetType = commonType;
TypePointer rightTargetType = Token::isShiftOp(c_op) ? rightExpression.annotation().type->mobileType() : commonType;
// for commutative operators, push the literal as late as possible to allow improved optimization
auto isLiteral = [](Expression const& _e)
@ -380,24 +400,24 @@ bool ExpressionCompiler::visit(BinaryOperation const& _binaryOperation)
if (swap)
{
leftExpression.accept(*this);
utils().convertType(*leftExpression.annotation().type, commonType, cleanupNeeded);
utils().convertType(*leftExpression.annotation().type, *leftTargetType, cleanupNeeded);
rightExpression.accept(*this);
utils().convertType(*rightExpression.annotation().type, commonType, cleanupNeeded);
utils().convertType(*rightExpression.annotation().type, *rightTargetType, cleanupNeeded);
}
else
{
rightExpression.accept(*this);
utils().convertType(*rightExpression.annotation().type, commonType, cleanupNeeded);
utils().convertType(*rightExpression.annotation().type, *rightTargetType, cleanupNeeded);
leftExpression.accept(*this);
utils().convertType(*leftExpression.annotation().type, commonType, cleanupNeeded);
utils().convertType(*leftExpression.annotation().type, *leftTargetType, cleanupNeeded);
}
if (Token::isShiftOp(c_op))
// shift only cares about the signedness of both sides
appendShiftOperatorCode(c_op, *leftExpression.annotation().type, *rightExpression.annotation().type);
appendShiftOperatorCode(c_op, *leftTargetType, *rightTargetType);
else if (Token::isCompareOp(c_op))
appendCompareOperatorCode(c_op, commonType);
appendCompareOperatorCode(c_op, *commonType);
else
appendOrdinaryBinaryOperatorCode(c_op, commonType);
appendOrdinaryBinaryOperatorCode(c_op, *commonType);
}
// do not visit the child nodes, we already did that explicitly
@ -1396,30 +1416,31 @@ void ExpressionCompiler::appendBitOperatorCode(Token::Value _operator)
}
}
void ExpressionCompiler::appendShiftOperatorCode(Token::Value _operator, Type const& _leftType, Type const& _rightType)
void ExpressionCompiler::appendShiftOperatorCode(Token::Value _operator, Type const& _valueType, Type const& _shiftAmountType)
{
// stack: rvalue lvalue
// stack: shift_amount value_to_shift
bool c_leftSigned = false;
if (auto leftType = dynamic_cast<IntegerType const*>(&_leftType))
c_leftSigned = leftType->isSigned();
bool c_valueSigned = false;
if (auto valueType = dynamic_cast<IntegerType const*>(&_valueType))
c_valueSigned = valueType->isSigned();
else
solUnimplemented("Only IntegerType can be shifted.");
solAssert(dynamic_cast<FixedBytesType const*>(&_valueType), "Only integer and fixed bytes type supported for shifts.");
// The RValue can be a RationalNumberType too.
bool c_rightSigned = false;
if (auto rightType = dynamic_cast<RationalNumberType const*>(&_rightType))
// The amount can be a RationalNumberType too.
bool c_amountSigned = false;
if (auto amountType = dynamic_cast<RationalNumberType const*>(&_shiftAmountType))
{
solAssert(rightType->integerType(), "integerType() called for fractional number.");
c_rightSigned = rightType->integerType()->isSigned();
// This should be handled by the type checker.
solAssert(amountType->integerType(), "");
solAssert(!amountType->integerType()->isSigned(), "");
}
else if (auto rightType = dynamic_cast<IntegerType const*>(&_rightType))
c_rightSigned = rightType->isSigned();
else if (auto amountType = dynamic_cast<IntegerType const*>(&_shiftAmountType))
c_amountSigned = amountType->isSigned();
else
solUnimplemented("Not implemented yet - FixedPointType.");
solAssert(false, "Invalid shift amount type.");
// shift with negative rvalue throws exception
if (c_rightSigned)
// shift by negative amount throws exception
if (c_amountSigned)
{
m_context << u256(0) << Instruction::DUP3 << Instruction::SLT;
m_context.appendConditionalJumpTo(m_context.errorTag());
@ -1431,7 +1452,7 @@ void ExpressionCompiler::appendShiftOperatorCode(Token::Value _operator, Type co
m_context << Instruction::SWAP1 << u256(2) << Instruction::EXP << Instruction::MUL;
break;
case Token::SAR:
m_context << Instruction::SWAP1 << u256(2) << Instruction::EXP << Instruction::SWAP1 << (c_leftSigned ? Instruction::SDIV : Instruction::DIV);
m_context << Instruction::SWAP1 << u256(2) << Instruction::EXP << Instruction::SWAP1 << (c_valueSigned ? Instruction::SDIV : Instruction::DIV);
break;
case Token::SHR:
default:
@ -1721,6 +1742,16 @@ void ExpressionCompiler::setLValueToStorageItem(Expression const& _expression)
setLValue<StorageItem>(_expression, *_expression.annotation().type);
}
bool ExpressionCompiler::cleanupNeededForOp(Type::Category _type, Token::Value _op)
{
if (Token::isCompareOp(_op) || Token::isShiftOp(_op))
return true;
else if (_type == Type::Category::Integer && (_op == Token::Div || _op == Token::Mod))
return true;
else
return false;
}
CompilerUtils ExpressionCompiler::utils()
{
return CompilerUtils(m_context);

6
libsolidity/codegen/ExpressionCompiler.h
View File

@ -91,7 +91,7 @@ private:
void appendArithmeticOperatorCode(Token::Value _operator, Type const& _type);
void appendBitOperatorCode(Token::Value _operator);
void appendShiftOperatorCode(Token::Value _operator, Type const& _leftType, Type const& _rightType);
void appendShiftOperatorCode(Token::Value _operator, Type const& _valueType, Type const& _shiftAmountType);
/// @}
/// Appends code to call a function of the given type with the given arguments.
@ -117,6 +117,10 @@ private:
template <class _LValueType, class... _Arguments>
void setLValue(Expression const& _expression, _Arguments const&... _arguments);
/// @returns true if the operator applied to the given type requires a cleanup prior to the
/// operation.
bool cleanupNeededForOp(Type::Category _type, Token::Value _op);
/// @returns the CompilerUtils object containing the current context.
CompilerUtils utils();

25
test/libsolidity/SolidityEndToEndTest.cpp
View File

@ -8507,17 +8507,18 @@ BOOST_AUTO_TEST_CASE(shift_left_uint8)
BOOST_AUTO_TEST_CASE(shift_left_larger_type)
{
// This basically tests proper cleanup and conversion. It should not convert x to int8.
char const* sourceCode = R"(
contract C {
function f() returns (int8) {
uint8 x = 255;
uint8 x = 254;
int8 y = 1;
return a << b;
return y << x;
}
}
)";
compileAndRun(sourceCode, 0, "C");
BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(1) << 255));
BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(0)));
}
BOOST_AUTO_TEST_CASE(shift_left_assignment)
@ -8570,6 +8571,7 @@ BOOST_AUTO_TEST_CASE(shift_right_garbled)
)";
compileAndRun(sourceCode, 0, "C");
BOOST_CHECK(callContractFunction("f(uint256,uint256)", u256(0x0), u256(4)) == encodeArgs(u256(0xf)));
BOOST_CHECK(callContractFunction("f(uint256,uint256)", u256(0x0), u256(0x1004)) == encodeArgs(u256(0xf)));
}
BOOST_AUTO_TEST_CASE(shift_right_uint32)
@ -8769,6 +8771,23 @@ BOOST_AUTO_TEST_CASE(shift_overflow)
BOOST_CHECK(callContractFunction("leftS(int8,int8)", 1, 6) == encodeArgs(u256(64)));
}
BOOST_AUTO_TEST_CASE(cleanup_in_compound_assign)
{
char const* sourceCode = R"(
contract C {
function test() returns (uint, uint) {
uint32 a = 0xffffffff;
uint16 x = uint16(a);
uint16 y = x;
x /= 0x100;
return (x, y);
}
}
)";
compileAndRun(sourceCode, 0, "C");
BOOST_CHECK(callContractFunction("test()") == encodeArgs(u256(0xff), u256(0xff)));
}
BOOST_AUTO_TEST_CASE(inline_assembly_in_modifiers)
{
char const* sourceCode = R"(