From 5b4b4011c7c122820603cec7306cd2f4774ecbce Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 4 Sep 2019 13:20:56 +0200 Subject: [PATCH 1/3] Extend memory beyond 32 byte addresses in interpreter. --- .../access_large_memory_offsets.yul | 14 ++++++++++++++ .../yulInterpreter/EVMInstructionInterpreter.cpp | 13 ++++++------- 2 files changed, 20 insertions(+), 7 deletions(-) create mode 100644 test/libyul/yulInterpreterTests/access_large_memory_offsets.yul diff --git a/test/libyul/yulInterpreterTests/access_large_memory_offsets.yul b/test/libyul/yulInterpreterTests/access_large_memory_offsets.yul new file mode 100644 index 000000000..483220bc1 --- /dev/null +++ b/test/libyul/yulInterpreterTests/access_large_memory_offsets.yul @@ -0,0 +1,14 @@ +{ + mstore(0, 7) + sstore(0, mload(0)) + mstore(sub(0, 1), sub(0, 1)) + sstore(1, mload(sub(0, 1))) +} +// ---- +// Trace: +// Memory dump: +// 0: ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff07 +// FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE0: 00000000000000000000000000000000000000000000000000000000000000ff +// Storage dump: +// 0000000000000000000000000000000000000000000000000000000000000000: 0000000000000000000000000000000000000000000000000000000000000007 +// 0000000000000000000000000000000000000000000000000000000000000001: ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff diff --git a/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp b/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp index 70e163bf4..45584a19f 100644 --- a/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp +++ b/test/tools/yulInterpreter/EVMInstructionInterpreter.cpp @@ -248,16 +248,15 @@ u256 EVMInstructionInterpreter::eval( return m_state.gaslimit; // --------------- memory / storage / logs --------------- case Instruction::MLOAD: - if (accessMemory(arg[0], 0x20)) - return readMemoryWord(arg[0]); - return 0; + accessMemory(arg[0], 0x20); + return readMemoryWord(arg[0]); case Instruction::MSTORE: - if (accessMemory(arg[0], 0x20)) - writeMemoryWord(arg[0], arg[1]); + accessMemory(arg[0], 0x20); + writeMemoryWord(arg[0], arg[1]); return 0; case Instruction::MSTORE8: - if (accessMemory(arg[0], 1)) - m_state.memory[arg[0]] = uint8_t(arg[1] & 0xff); + accessMemory(arg[0], 1); + m_state.memory[arg[0]] = uint8_t(arg[1] & 0xff); return 0; case Instruction::SLOAD: return m_state.storage[h256(arg[0])]; From 137a898eca70a1ac07ccb9767e6a6f2b389c0d2c Mon Sep 17 00:00:00 2001 From: Bhargava Shastry Date: Wed, 4 Sep 2019 21:14:26 +0200 Subject: [PATCH 2/3] Yul proto fuzzer: Fix typo in low level call --- test/tools/ossfuzz/protoToYul.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tools/ossfuzz/protoToYul.cpp b/test/tools/ossfuzz/protoToYul.cpp index 3f0f26f4c..70f0e3ec5 100644 --- a/test/tools/ossfuzz/protoToYul.cpp +++ b/test/tools/ossfuzz/protoToYul.cpp @@ -741,7 +741,7 @@ void ProtoConverter::visit(LowLevelCall const& _x) m_output << ", "; visit(_x.addr()); m_output << ", "; - if (type == LowLevelCall::CALL || LowLevelCall::CALLCODE) + if (type == LowLevelCall::CALL || type == LowLevelCall::CALLCODE) { visit(_x.wei()); m_output << ", "; From 7148792b8abb99d078ce60e129305e787627e58b Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Thu, 5 Sep 2019 13:00:07 +0200 Subject: [PATCH 3/3] Refactor ExpressionCompiler with acceptAndConvert. --- libsolidity/codegen/ExpressionCompiler.cpp | 97 +++++++--------------- libsolidity/codegen/ExpressionCompiler.h | 2 + 2 files changed, 33 insertions(+), 66 deletions(-) diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index c34684840..2f8bf9455 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -78,8 +78,7 @@ void ExpressionCompiler::appendStateVariableInitialization(VariableDeclaration c void ExpressionCompiler::appendConstStateVariableAccessor(VariableDeclaration const& _varDecl) { solAssert(_varDecl.isConstant(), ""); - _varDecl.value()->accept(*this); - utils().convertType(*_varDecl.value()->annotation().type, *_varDecl.annotation().type); + acceptAndConvert(*_varDecl.value(), *_varDecl.annotation().type); // append return m_context << dupInstruction(_varDecl.annotation().type->sizeOnStack() + 1); @@ -225,14 +224,12 @@ bool ExpressionCompiler::visit(Conditional const& _condition) CompilerContext::LocationSetter locationSetter(m_context, _condition); _condition.condition().accept(*this); eth::AssemblyItem trueTag = m_context.appendConditionalJump(); - _condition.falseExpression().accept(*this); - utils().convertType(*_condition.falseExpression().annotation().type, *_condition.annotation().type); + acceptAndConvert(_condition.falseExpression(), *_condition.annotation().type); eth::AssemblyItem endTag = m_context.appendJumpToNew(); m_context << trueTag; int offset = _condition.annotation().type->sizeOnStack(); m_context.adjustStackOffset(-offset); - _condition.trueExpression().accept(*this); - utils().convertType(*_condition.trueExpression().annotation().type, *_condition.annotation().type); + acceptAndConvert(_condition.trueExpression(), *_condition.annotation().type); m_context << endTag; return false; } @@ -322,8 +319,7 @@ bool ExpressionCompiler::visit(TupleExpression const& _tuple) for (auto const& component: _tuple.components()) { - component->accept(*this); - utils().convertType(*component->annotation().type, *arrayType.baseType(), true); + acceptAndConvert(*component, *arrayType.baseType(), true); utils().storeInMemoryDynamic(*arrayType.baseType(), true); } @@ -451,17 +447,13 @@ bool ExpressionCompiler::visit(BinaryOperation const& _binaryOperation) bool swap = m_optimiseOrderLiterals && TokenTraits::isCommutativeOp(c_op) && isLiteral(rightExpression) && !isLiteral(leftExpression); if (swap) { - leftExpression.accept(*this); - utils().convertType(*leftExpression.annotation().type, *leftTargetType, cleanupNeeded); - rightExpression.accept(*this); - utils().convertType(*rightExpression.annotation().type, *rightTargetType, cleanupNeeded); + acceptAndConvert(leftExpression, *leftTargetType, cleanupNeeded); + acceptAndConvert(rightExpression, *rightTargetType, cleanupNeeded); } else { - rightExpression.accept(*this); - utils().convertType(*rightExpression.annotation().type, *rightTargetType, cleanupNeeded); - leftExpression.accept(*this); - utils().convertType(*leftExpression.annotation().type, *leftTargetType, cleanupNeeded); + acceptAndConvert(rightExpression, *rightTargetType, cleanupNeeded); + acceptAndConvert(leftExpression, *leftTargetType, cleanupNeeded); } if (TokenTraits::isShiftOp(c_op)) // shift only cares about the signedness of both sides @@ -483,9 +475,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) { solAssert(_functionCall.arguments().size() == 1, ""); solAssert(_functionCall.names().empty(), ""); - Expression const& firstArgument = *_functionCall.arguments().front(); - firstArgument.accept(*this); - utils().convertType(*firstArgument.annotation().type, *_functionCall.annotation().type); + acceptAndConvert(*_functionCall.arguments().front(), *_functionCall.annotation().type); return false; } @@ -531,8 +521,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) for (unsigned i = 0; i < arguments.size(); ++i) { - arguments[i]->accept(*this); - utils().convertType(*arguments[i]->annotation().type, *functionType->parameterTypes()[i]); + acceptAndConvert(*arguments[i], *functionType->parameterTypes()[i]); utils().storeInMemoryDynamic(*functionType->parameterTypes()[i]); } m_context << Instruction::POP; @@ -552,10 +541,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) eth::AssemblyItem returnLabel = m_context.pushNewTag(); for (unsigned i = 0; i < arguments.size(); ++i) - { - arguments[i]->accept(*this); - utils().convertType(*arguments[i]->annotation().type, *function.parameterTypes()[i]); - } + acceptAndConvert(*arguments[i], *function.parameterTypes()[i]); { bool shortcutTaken = false; @@ -647,8 +633,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) // stack layout: contract_address function_id [gas] [value] _functionCall.expression().accept(*this); - arguments.front()->accept(*this); - utils().convertType(*arguments.front()->annotation().type, *TypeProvider::uint256(), true); + acceptAndConvert(*arguments.front(), *TypeProvider::uint256(), true); // Note that function is not the original function, but the ".gas" function. // Its values of gasSet and valueSet is equal to the original function's though. unsigned stackDepth = (function.gasSet() ? 1 : 0) + (function.valueSet() ? 1 : 0); @@ -673,11 +658,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) // Provide the gas stipend manually at first because we may send zero ether. // Will be zeroed if we send more than zero ether. m_context << u256(eth::GasCosts::callStipend); - arguments.front()->accept(*this); - utils().convertType( - *arguments.front()->annotation().type, - *function.parameterTypes().front(), true - ); + acceptAndConvert(*arguments.front(), *function.parameterTypes().front(), true); // gas <- gas * !value m_context << Instruction::SWAP1 << Instruction::DUP2; m_context << Instruction::ISZERO << Instruction::MUL << Instruction::SWAP1; @@ -705,8 +686,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) } break; case FunctionType::Kind::Selfdestruct: - arguments.front()->accept(*this); - utils().convertType(*arguments.front()->annotation().type, *function.parameterTypes().front(), true); + acceptAndConvert(*arguments.front(), *function.parameterTypes().front(), true); m_context << Instruction::SELFDESTRUCT; break; case FunctionType::Kind::Revert: @@ -754,10 +734,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) { unsigned logNumber = int(function.kind()) - int(FunctionType::Kind::Log0); for (unsigned arg = logNumber; arg > 0; --arg) - { - arguments[arg]->accept(*this); - utils().convertType(*arguments[arg]->annotation().type, *function.parameterTypes()[arg], true); - } + acceptAndConvert(*arguments[arg], *function.parameterTypes()[arg], true); arguments.front()->accept(*this); utils().fetchFreeMemoryPointer(); solAssert(function.parameterTypes().front()->isValueType(), ""); @@ -827,23 +804,18 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) } case FunctionType::Kind::BlockHash: { - arguments[0]->accept(*this); - utils().convertType(*arguments[0]->annotation().type, *function.parameterTypes()[0], true); + acceptAndConvert(*arguments[0], *function.parameterTypes()[0], true); m_context << Instruction::BLOCKHASH; break; } case FunctionType::Kind::AddMod: case FunctionType::Kind::MulMod: { - arguments[2]->accept(*this); - utils().convertType(*arguments[2]->annotation().type, *TypeProvider::uint256()); + acceptAndConvert(*arguments[2], *TypeProvider::uint256()); m_context << Instruction::DUP1 << Instruction::ISZERO; m_context.appendConditionalInvalid(); for (unsigned i = 1; i < 3; i ++) - { - arguments[2 - i]->accept(*this); - utils().convertType(*arguments[2 - i]->annotation().type, *TypeProvider::uint256()); - } + acceptAndConvert(*arguments[2 - i], *TypeProvider::uint256()); if (function.kind() == FunctionType::Kind::AddMod) m_context << Instruction::ADDMOD; else @@ -927,8 +899,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) solAssert(arguments.size() == 1, ""); // Fetch requested length. - arguments[0]->accept(*this); - utils().convertType(*arguments[0]->annotation().type, *TypeProvider::uint256()); + acceptAndConvert(*arguments[0], *TypeProvider::uint256()); // Stack: requested_length utils().fetchFreeMemoryPointer(); @@ -967,8 +938,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) case FunctionType::Kind::Assert: case FunctionType::Kind::Require: { - arguments.front()->accept(*this); - utils().convertType(*arguments.front()->annotation().type, *function.parameterTypes().front(), false); + acceptAndConvert(*arguments.front(), *function.parameterTypes().front(), false); if (arguments.size() > 1) { // Users probably expect the second argument to be evaluated @@ -1149,12 +1119,7 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess) if (auto funType = dynamic_cast(_memberAccess.annotation().type)) if (funType->bound()) { - _memberAccess.expression().accept(*this); - utils().convertType( - *_memberAccess.expression().annotation().type, - *funType->selfType(), - true - ); + acceptAndConvert(_memberAccess.expression(), *funType->selfType(), true); if (funType->kind() == FunctionType::Kind::Internal) { FunctionDefinition const& funDef = dynamic_cast(funType->declaration()); @@ -1568,8 +1533,7 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) ArrayType const& arrayType = dynamic_cast(baseType); solAssert(_indexAccess.indexExpression(), "Index expression expected."); - _indexAccess.indexExpression()->accept(*this); - utils().convertType(*_indexAccess.indexExpression()->annotation().type, *TypeProvider::uint256(), true); + acceptAndConvert(*_indexAccess.indexExpression(), *TypeProvider::uint256(), true); // stack layout: [] switch (arrayType.location()) { @@ -1597,8 +1561,7 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess) FixedBytesType const& fixedBytesType = dynamic_cast(baseType); solAssert(_indexAccess.indexExpression(), "Index expression expected."); - _indexAccess.indexExpression()->accept(*this); - utils().convertType(*_indexAccess.indexExpression()->annotation().type, *TypeProvider::uint256(), true); + acceptAndConvert(*_indexAccess.indexExpression(), *TypeProvider::uint256(), true); // stack layout: // check out-of-bounds access m_context << u256(fixedBytesType.numBytes()); @@ -2207,8 +2170,7 @@ void ExpressionCompiler::appendExternalFunctionCall( void ExpressionCompiler::appendExpressionCopyToMemory(Type const& _expectedType, Expression const& _expression) { solUnimplementedAssert(_expectedType.isValueType(), "Not implemented for non-value types."); - _expression.accept(*this); - utils().convertType(*_expression.annotation().type, _expectedType, true); + acceptAndConvert(_expression, _expectedType, true); utils().storeInMemoryDynamic(_expectedType); } @@ -2217,10 +2179,7 @@ void ExpressionCompiler::appendVariable(VariableDeclaration const& _variable, Ex if (!_variable.isConstant()) setLValueFromDeclaration(_variable, _expression); else - { - _variable.value()->accept(*this); - utils().convertType(*_variable.value()->annotation().type, *_variable.annotation().type); - } + acceptAndConvert(*_variable.value(), *_variable.annotation().type); } void ExpressionCompiler::setLValueFromDeclaration(Declaration const& _declaration, Expression const& _expression) @@ -2252,6 +2211,12 @@ bool ExpressionCompiler::cleanupNeededForOp(Type::Category _type, Token _op) return false; } +void ExpressionCompiler::acceptAndConvert(Expression const& _expression, Type const& _type, bool _cleanupNeeded) +{ + _expression.accept(*this); + utils().convertType(*_expression.annotation().type, _type, _cleanupNeeded); +} + CompilerUtils ExpressionCompiler::utils() { return CompilerUtils(m_context); diff --git a/libsolidity/codegen/ExpressionCompiler.h b/libsolidity/codegen/ExpressionCompiler.h index ca4e05b5f..645d67067 100644 --- a/libsolidity/codegen/ExpressionCompiler.h +++ b/libsolidity/codegen/ExpressionCompiler.h @@ -121,6 +121,8 @@ private: /// operation. static bool cleanupNeededForOp(Type::Category _type, Token _op); + void acceptAndConvert(Expression const& _expression, Type const& _type, bool _cleanupNeeded = false); + /// @returns the CompilerUtils object containing the current context. CompilerUtils utils();