From 2deb2b370fd32f58bae2c843a64b666c83b0fe1b Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 28 Jan 2021 20:19:13 +0100 Subject: [PATCH] Code generation for errors. --- libsolidity/codegen/CompilerUtils.cpp | 16 ++ libsolidity/codegen/CompilerUtils.h | 6 + libsolidity/codegen/ExpressionCompiler.cpp | 142 ++++++++++++------ .../codegen/ir/IRGeneratorForStatements.cpp | 128 +++++++++------- .../codegen/ir/IRGeneratorForStatements.h | 7 + .../semanticTests/errors/named_error_args.sol | 10 ++ .../semanticTests/errors/named_require.sol | 11 ++ .../errors/require_conversion.sol | 13 ++ .../errors/require_side_effects.sol | 19 +++ .../errors/revert_conversion.sol | 12 ++ .../semanticTests/errors/simple.sol | 10 ++ .../semanticTests/errors/simple_require.sol | 11 ++ 12 files changed, 281 insertions(+), 104 deletions(-) create mode 100644 test/libsolidity/semanticTests/errors/named_error_args.sol create mode 100644 test/libsolidity/semanticTests/errors/named_require.sol create mode 100644 test/libsolidity/semanticTests/errors/require_conversion.sol create mode 100644 test/libsolidity/semanticTests/errors/require_side_effects.sol create mode 100644 test/libsolidity/semanticTests/errors/revert_conversion.sol create mode 100644 test/libsolidity/semanticTests/errors/simple.sol create mode 100644 test/libsolidity/semanticTests/errors/simple_require.sol diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index ad461d2da..db2c48767 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -103,6 +103,22 @@ void CompilerUtils::revertWithStringData(Type const& _argumentType) m_context << Instruction::REVERT; } +void CompilerUtils::revertWithError( + string const& _signature, + vector const& _parameterTypes, + vector const& _argumentTypes +) +{ + fetchFreeMemoryPointer(); + m_context << util::selectorFromSignature(_signature); + m_context << Instruction::DUP2 << Instruction::MSTORE; + m_context << u256(4) << Instruction::ADD; + // Stack: + abiEncode(_argumentTypes, _parameterTypes); + toSizeAfterFreeMemoryPointer(); + m_context << Instruction::REVERT; +} + void CompilerUtils::returnDataToArray() { if (m_context.evmVersion().supportsReturndata()) diff --git a/libsolidity/codegen/CompilerUtils.h b/libsolidity/codegen/CompilerUtils.h index 7bc26d971..0930ff1c8 100644 --- a/libsolidity/codegen/CompilerUtils.h +++ b/libsolidity/codegen/CompilerUtils.h @@ -69,6 +69,12 @@ public: /// Stack post: void revertWithStringData(Type const& _argumentType); + void revertWithError( + std::string const& _errorName, + std::vector const& _parameterTypes, + std::vector const& _argumentTypes + ); + /// Allocates a new array and copies the return data to it. /// If the EVM does not support return data, creates an empty array. void returnDataToArray(); diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 383455d25..6862eba46 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -29,6 +29,7 @@ #include #include +#include #include #include @@ -786,27 +787,46 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) break; case FunctionType::Kind::Revert: { - if (arguments.empty()) + bool usesString = + !arguments.empty() && + arguments.front()->annotation().type->isImplicitlyConvertibleTo(*TypeProvider::stringMemory()); + if (arguments.empty() || (usesString && m_context.revertStrings() == RevertStrings::Strip)) + { + if (!arguments.empty() && !*arguments.front()->annotation().isPure) + { + arguments.front()->accept(*this); + utils().popStackElement(*arguments.front()->annotation().type); + } m_context.appendRevert(); + } else { - // function-sel(Error(string)) + encoding solAssert(arguments.size() == 1, ""); - solUnimplementedAssert(arguments.front()->annotation().type->isImplicitlyConvertibleTo(*TypeProvider::stringMemory()), ""); - if (m_context.revertStrings() == RevertStrings::Strip) + arguments.front()->accept(*this); + + string signature; + vector parameterTypes; + vector argumentTypes; + if (usesString) { - if (!*arguments.front()->annotation().isPure) - { - arguments.front()->accept(*this); - utils().popStackElement(*arguments.front()->annotation().type); - } - m_context.appendRevert(); + signature = "Error(string)"; + parameterTypes.push_back(TypeProvider::stringMemory()); + argumentTypes = vector{arguments.front()->annotation().type}; } else { - arguments.front()->accept(*this); - utils().revertWithStringData(*arguments.front()->annotation().type); + ErrorDefinition const* error = nullptr; + FunctionCall const* errorCall = dynamic_cast(_functionCall.arguments().back().get()); + solAssert(errorCall, ""); + solAssert(*errorCall->annotation().kind == FunctionCallKind::FunctionCall, ""); + error = dynamic_cast(referencedDeclaration(errorCall->expression())); + solAssert(error, ""); + signature = error->functionType(true)->externalSignature(); + parameterTypes = error->functionType(true)->parameterTypes(); + for (ASTPointer const& arg: errorCall->sortedArguments()) + argumentTypes.push_back(arg->annotation().type); } + utils().revertWithError(signature, parameterTypes, argumentTypes); } break; } @@ -910,7 +930,10 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) } case FunctionType::Kind::Error: { - solAssert(false, ""); + for (ASTPointer const& arg: arguments) + arg->accept(*this); + // will be consumed in the revert / require call. + break; } case FunctionType::Kind::BlockHash: { @@ -1082,49 +1105,72 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) { acceptAndConvert(*arguments.front(), *TypeProvider::boolean(), false); - bool haveReasonString = arguments.size() > 1 && m_context.revertStrings() != RevertStrings::Strip; + solAssert(arguments.size() == 1|| arguments.size() == 2, ""); - if (arguments.size() > 1) + bool usesString = + arguments.size() >= 2 && + arguments.at(1)->annotation().type->isImplicitlyConvertibleTo(*TypeProvider::stringMemory()); + + if (arguments.size() == 1 || (usesString && m_context.revertStrings() == RevertStrings::Strip)) { - // Users probably expect the second argument to be evaluated - // even if the condition is false, as would be the case for an actual - // function call. - solAssert(arguments.size() == 2, ""); - solAssert(function.kind() == FunctionType::Kind::Require, ""); - solUnimplementedAssert(arguments.back()->annotation().type->isImplicitlyConvertibleTo(*TypeProvider::stringMemory()), ""); - if (m_context.revertStrings() == RevertStrings::Strip) + // Shortcut without reason string. + if (arguments.size() == 2 && !*arguments.at(1)->annotation().isPure) { - if (!*arguments.at(1)->annotation().isPure) - { - arguments.at(1)->accept(*this); - utils().popStackElement(*arguments.at(1)->annotation().type); - } + arguments.at(1)->accept(*this); + utils().popStackElement(*arguments.at(1)->annotation().type); + } + m_context << Instruction::ISZERO << Instruction::ISZERO; + auto success = m_context.appendConditionalJump(); + if (function.kind() == FunctionType::Kind::Assert) + // condition was not met, flag an error + m_context.appendPanic(util::PanicCode::Assert); + else + m_context.appendRevert(); + m_context << success; + } + else + { + solAssert(function.kind() == FunctionType::Kind::Require, ""); + solAssert(arguments.size() == 2, ""); + + string signature; + vector parameterTypes; + vector argumentTypes; + if (usesString) + { + signature = "Error(string)"; + parameterTypes.push_back(TypeProvider::stringMemory()); + argumentTypes = vector{arguments.at(1)->annotation().type}; + arguments.at(1)->accept(*this); + utils().moveIntoStack(1, argumentTypes.front()->sizeOnStack()); } else { - arguments.at(1)->accept(*this); - utils().moveIntoStack(1, arguments.at(1)->annotation().type->sizeOnStack()); + // Cannot use the size of the argument itself because its type is wrong. + ErrorDefinition const* error = nullptr; + FunctionCall const* errorCall = dynamic_cast(arguments.at(1).get()); + solAssert(errorCall, ""); + solAssert(*errorCall->annotation().kind == FunctionCallKind::FunctionCall, ""); + error = dynamic_cast(referencedDeclaration(errorCall->expression())); + solAssert(error, ""); + signature = error->functionType(true)->externalSignature(); + parameterTypes = error->functionType(true)->parameterTypes(); + // This uses a different visiting order, but that's the case for all named function calls. + for (ASTPointer const& arg: errorCall->sortedArguments()) + { + arg->accept(*this); + argumentTypes.push_back(arg->annotation().type); + utils().moveIntoStack(1, arg->annotation().type->sizeOnStack()); + } } + m_context << Instruction::ISZERO << Instruction::ISZERO; + auto success = m_context.appendConditionalJump(); + unsigned argSize = TupleType(argumentTypes).sizeOnStack(); + utils().revertWithError(signature, parameterTypes, argumentTypes); + m_context.adjustStackOffset(static_cast(argSize)); + m_context << success; + utils().popStackSlots(argSize); } - // Stack: - // jump if condition was met - m_context << Instruction::ISZERO << Instruction::ISZERO; - auto success = m_context.appendConditionalJump(); - if (function.kind() == FunctionType::Kind::Assert) - // condition was not met, flag an error - m_context.appendPanic(util::PanicCode::Assert); - else if (haveReasonString) - { - utils().revertWithStringData(*arguments.at(1)->annotation().type); - // Here, the argument is consumed, but in the other branch, it is still there. - m_context.adjustStackOffset(static_cast(arguments.at(1)->annotation().type->sizeOnStack())); - } - else - m_context.appendRevert(); - // the success branch - m_context << success; - if (haveReasonString) - utils().popStackElement(*arguments.at(1)->annotation().type); break; } case FunctionType::Kind::ABIEncode: diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp index ea8f82aeb..72a73f11c 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp +++ b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp @@ -1053,33 +1053,24 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall) } case FunctionType::Kind::Error: { - solAssert(false, ""); + // no-op + break; } case FunctionType::Kind::Assert: + { + solAssert(arguments.size() == 1, ""); + m_code << m_utils.requireOrAssertFunction(true) << "(" << IRVariable(*arguments[0]).name() << ")"; + break; + } case FunctionType::Kind::Require: { - solAssert(arguments.size() > 0, "Expected at least one parameter for require/assert"); - solAssert(arguments.size() <= 2, "Expected no more than two parameters for require/assert"); - if (arguments.size() == 2) - solAssert(functionType->kind() == FunctionType::Kind::Require, ""); - - if (arguments.size() == 2) - solUnimplementedAssert(arguments.back()->annotation().type->isImplicitlyConvertibleTo(*TypeProvider::stringMemory()), ""); - - Type const* messageArgumentType = - arguments.size() > 1 && m_context.revertStrings() != RevertStrings::Strip ? - arguments[1]->annotation().type : - nullptr; - string requireOrAssertFunction = m_utils.requireOrAssertFunction( - functionType->kind() == FunctionType::Kind::Assert, - messageArgumentType - ); - - m_code << move(requireOrAssertFunction) << "(" << IRVariable(*arguments[0]).name(); - if (messageArgumentType && messageArgumentType->sizeOnStack() > 0) - m_code << ", " << IRVariable(*arguments[1]).commaSeparatedList(); - m_code << ")\n"; - + solAssert(arguments.size() <= 2, ""); + m_code << "if iszero(" << IRVariable(*arguments[0]).name() << ") {\n"; + if (arguments.size() == 1) + m_code << "revert(0, 0)\n"; + else + revertWithError(arguments.at(1)); + m_code << "}\n"; break; } case FunctionType::Kind::ABIEncode: @@ -1211,42 +1202,11 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall) } case FunctionType::Kind::Revert: { + solAssert(arguments.size() <= 1, ""); if (arguments.empty()) m_code << "revert(0, 0)\n"; else - { - solAssert(arguments.size() == 1, ""); - solUnimplementedAssert(arguments.front()->annotation().type->isImplicitlyConvertibleTo(*TypeProvider::stringMemory()), ""); - - if (m_context.revertStrings() == RevertStrings::Strip) - m_code << "revert(0, 0)\n"; - else - { - solAssert(type(*arguments.front()).isImplicitlyConvertibleTo(*TypeProvider::stringMemory()),""); - - Whiskers templ(R"({ - let := () - mstore(, ) - let := (add(, 4) ) - revert(, sub(, )) - })"); - templ("pos", m_context.newYulVariable()); - templ("end", m_context.newYulVariable()); - templ("hash", util::selectorFromSignature("Error(string)").str()); - templ("allocateUnbounded", m_utils.allocateUnboundedFunction()); - templ( - "argumentVars", - joinHumanReadablePrefixed(IRVariable{*arguments.front()}.stackSlots()) - ); - templ("encode", m_context.abiFunctions().tupleEncoder( - {&type(*arguments.front())}, - {TypeProvider::stringMemory()} - )); - - m_code << templ.render(); - } - } - + revertWithError(arguments.front()); break; } // Array creation using new @@ -3118,6 +3078,62 @@ void IRGeneratorForStatements::rethrow() m_code << "revert(0, 0) // rethrow\n"s; } +void IRGeneratorForStatements::revertWithError(ASTPointer const& _error) +{ + solAssert(_error, ""); + + bool usesString = _error->annotation().type->isImplicitlyConvertibleTo(*TypeProvider::stringMemory()); + if (usesString && m_context.revertStrings() == RevertStrings::Strip) + { + m_code << "revert(0, 0)\n"; + return; + } + + string signature; + vector> errorArguments; + vector parameterTypes; + if (usesString) + { + signature = "Error(string)"; + errorArguments.push_back(_error); + parameterTypes.push_back(TypeProvider::stringMemory()); + } + else + { + FunctionCall const* errorCall = dynamic_cast(_error.get()); + solAssert(errorCall, ""); + solAssert(*errorCall->annotation().kind == FunctionCallKind::FunctionCall, ""); + ErrorDefinition const* error = dynamic_cast(referencedDeclaration(errorCall->expression())); + solAssert(error, ""); + signature = error->functionType(true)->externalSignature(); + parameterTypes = error->functionType(true)->parameterTypes(); + errorArguments = errorCall->sortedArguments(); + } + + Whiskers templ(R"({ + let := () + mstore(, ) + let := (add(, 4) ) + revert(, sub(, )) + })"); + templ("pos", m_context.newYulVariable()); + templ("end", m_context.newYulVariable()); + templ("hash", util::selectorFromSignature(signature).str()); + templ("allocateUnbounded", m_utils.allocateUnboundedFunction()); + + vector errorArgumentVars; + vector errorArgumentTypes; + for (ASTPointer const& arg: errorArguments) + { + errorArgumentVars += IRVariable(*arg).stackSlots(); + errorArgumentTypes.push_back(arg->annotation().type); + } + templ("argumentVars", joinHumanReadablePrefixed(errorArgumentVars)); + templ("encode", m_context.abiFunctions().tupleEncoder(errorArgumentTypes, parameterTypes)); + + m_code << templ.render(); +} + bool IRGeneratorForStatements::visit(TryCatchClause const& _clause) { _clause.block().accept(*this); diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.h b/libsolidity/codegen/ir/IRGeneratorForStatements.h index e212398b8..7cf3217d8 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.h +++ b/libsolidity/codegen/ir/IRGeneratorForStatements.h @@ -106,6 +106,13 @@ private: /// Generates code to rethrow an exception. void rethrow(); + /// Generates code to revert with an error, which might be a plain string + /// or an error instance. The error arguments and the strings are assumed to + /// be already evaluated and available in local IRVariables, but not yet + /// converted. + /// Honors the revert strings setting. + void revertWithError(ASTPointer const& _error); + void handleVariableReference( VariableDeclaration const& _variable, Expression const& _referencingExpression diff --git a/test/libsolidity/semanticTests/errors/named_error_args.sol b/test/libsolidity/semanticTests/errors/named_error_args.sol new file mode 100644 index 000000000..1789b3349 --- /dev/null +++ b/test/libsolidity/semanticTests/errors/named_error_args.sol @@ -0,0 +1,10 @@ +error E(uint a, uint b); +contract C { + function f() public pure { + revert(E({b: 7, a: 2})); + } +} +// ==== +// compileViaYul: also +// ---- +// f() -> FAILURE, hex"85208890", hex"0000000000000000000000000000000000000000000000000000000000000002", hex"0000000000000000000000000000000000000000000000000000000000000007" diff --git a/test/libsolidity/semanticTests/errors/named_require.sol b/test/libsolidity/semanticTests/errors/named_require.sol new file mode 100644 index 000000000..fdd3ecc4d --- /dev/null +++ b/test/libsolidity/semanticTests/errors/named_require.sol @@ -0,0 +1,11 @@ +error E(uint a, uint b); +contract C { + function f(bool c) public pure { + require(c, E({b: 7, a: 2})); + } +} +// ==== +// compileViaYul: also +// ---- +// f(bool): true -> +// f(bool): false -> FAILURE, hex"85208890", hex"0000000000000000000000000000000000000000000000000000000000000002", hex"0000000000000000000000000000000000000000000000000000000000000007" diff --git a/test/libsolidity/semanticTests/errors/require_conversion.sol b/test/libsolidity/semanticTests/errors/require_conversion.sol new file mode 100644 index 000000000..27498e68c --- /dev/null +++ b/test/libsolidity/semanticTests/errors/require_conversion.sol @@ -0,0 +1,13 @@ +error E(string a, uint[] b); +contract C { + uint[] x; + function f(bool c) public { + x.push(7); + require(c, E("abc", x)); + } +} +// ==== +// compileViaYul: also +// ---- +// f(bool): false -> FAILURE, hex"59e4d4df", 0x40, 0x80, 3, "abc", 1, 7 +// f(bool): true -> diff --git a/test/libsolidity/semanticTests/errors/require_side_effects.sol b/test/libsolidity/semanticTests/errors/require_side_effects.sol new file mode 100644 index 000000000..c505bae24 --- /dev/null +++ b/test/libsolidity/semanticTests/errors/require_side_effects.sol @@ -0,0 +1,19 @@ +error E(uint a, uint b); +contract C { + uint public x; + function f(bool c) public { + require(c, E(g(), 7)); + } + function g() public returns (uint) { + x++; + return 2; + } +} +// ==== +// compileViaYul: also +// ---- +// x() -> 0 +// f(bool): true -> +// x() -> 1 +// f(bool): false -> FAILURE, hex"85208890", hex"0000000000000000000000000000000000000000000000000000000000000002", hex"0000000000000000000000000000000000000000000000000000000000000007" +// x() -> 1 diff --git a/test/libsolidity/semanticTests/errors/revert_conversion.sol b/test/libsolidity/semanticTests/errors/revert_conversion.sol new file mode 100644 index 000000000..53e332933 --- /dev/null +++ b/test/libsolidity/semanticTests/errors/revert_conversion.sol @@ -0,0 +1,12 @@ +error E(string a, uint[] b); +contract C { + uint[] x; + function f() public { + x.push(7); + revert(E("abc", x)); + } +} +// ==== +// compileViaYul: also +// ---- +// f() -> FAILURE, hex"59e4d4df", 0x40, 0x80, 3, "abc", 1, 7 diff --git a/test/libsolidity/semanticTests/errors/simple.sol b/test/libsolidity/semanticTests/errors/simple.sol new file mode 100644 index 000000000..f70aea077 --- /dev/null +++ b/test/libsolidity/semanticTests/errors/simple.sol @@ -0,0 +1,10 @@ +error E(uint a, uint b); +contract C { + function f() public pure { + revert(E(2, 7)); + } +} +// ==== +// compileViaYul: also +// ---- +// f() -> FAILURE, hex"85208890", 2, 7 diff --git a/test/libsolidity/semanticTests/errors/simple_require.sol b/test/libsolidity/semanticTests/errors/simple_require.sol new file mode 100644 index 000000000..8b3f7c970 --- /dev/null +++ b/test/libsolidity/semanticTests/errors/simple_require.sol @@ -0,0 +1,11 @@ +error E(uint a, uint b); +contract C { + function f(bool c) public pure { + require(c, E(2, 7)); + } +} +// ==== +// compileViaYul: also +// ---- +// f(bool): true -> +// f(bool): false -> FAILURE, hex"85208890", hex"0000000000000000000000000000000000000000000000000000000000000002", hex"0000000000000000000000000000000000000000000000000000000000000007"