Bubble up error messages.

This commit is contained in:
chriseth 2017-12-30 20:13:41 +01:00
parent ae1d040285
commit 7a9ee69e98
6 changed files with 62 additions and 10 deletions

View File

@ -262,12 +262,20 @@ CompilerContext& CompilerContext::appendRevert()
return *this << u256(0) << u256(0) << Instruction::REVERT; return *this << u256(0) << u256(0) << Instruction::REVERT;
} }
CompilerContext& CompilerContext::appendConditionalRevert() CompilerContext& CompilerContext::appendConditionalRevert(bool _forwardReturnData)
{ {
*this << Instruction::ISZERO; if (_forwardReturnData)
eth::AssemblyItem afterTag = appendConditionalJump(); appendInlineAssembly(R"({
appendRevert(); if condition {
*this << afterTag; returndatacopy(0, 0, returndatasize())
revert(0, returndatasize())
}
})", {"condition"});
else
appendInlineAssembly(R"({
if condition { revert(0, 0) }
})", {"condition"});
*this << Instruction::POP;
return *this; return *this;
} }

View File

@ -156,8 +156,9 @@ public:
CompilerContext& appendConditionalInvalid(); CompilerContext& appendConditionalInvalid();
/// Appends a REVERT(0, 0) call /// Appends a REVERT(0, 0) call
CompilerContext& appendRevert(); CompilerContext& appendRevert();
/// Appends a conditional REVERT(0, 0) call /// Appends a conditional REVERT-call, either forwarding the RETURNDATA or providing the
CompilerContext& appendConditionalRevert(); /// empty string. Consumes the condition.
CompilerContext& appendConditionalRevert(bool _forwardReturnData = false);
/// Appends a JUMP to a specific tag /// Appends a JUMP to a specific tag
CompilerContext& appendJumpTo(eth::AssemblyItem const& _tag) { m_asm->appendJump(_tag); return *this; } CompilerContext& appendJumpTo(eth::AssemblyItem const& _tag) { m_asm->appendJump(_tag); return *this; }
/// Appends pushing of a new tag and @returns the new tag. /// Appends pushing of a new tag and @returns the new tag.

View File

@ -691,6 +691,7 @@ void CompilerUtils::convertType(
solAssert(enumType.numberOfMembers() > 0, "empty enum should have caused a parser error."); solAssert(enumType.numberOfMembers() > 0, "empty enum should have caused a parser error.");
m_context << u256(enumType.numberOfMembers() - 1) << Instruction::DUP2 << Instruction::GT; m_context << u256(enumType.numberOfMembers() - 1) << Instruction::DUP2 << Instruction::GT;
if (_asPartOfArgumentDecoding) if (_asPartOfArgumentDecoding)
// TODO: error message?
m_context.appendConditionalRevert(); m_context.appendConditionalRevert();
else else
m_context.appendConditionalInvalid(); m_context.appendConditionalInvalid();

View File

@ -128,6 +128,7 @@ void ContractCompiler::appendCallValueCheck()
{ {
// Throw if function is not payable but call contained ether. // Throw if function is not payable but call contained ether.
m_context << Instruction::CALLVALUE; m_context << Instruction::CALLVALUE;
// TODO: error message?
m_context.appendConditionalRevert(); m_context.appendConditionalRevert();
} }
@ -327,6 +328,7 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
m_context << Instruction::STOP; m_context << Instruction::STOP;
} }
else else
// TODO: error message here?
m_context.appendRevert(); m_context.appendRevert();
for (auto const& it: interfaceFunctions) for (auto const& it: interfaceFunctions)

View File

@ -608,7 +608,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
m_context << Instruction::CREATE; m_context << Instruction::CREATE;
// Check if zero (out of stack or not enough balance). // Check if zero (out of stack or not enough balance).
m_context << Instruction::DUP1 << Instruction::ISZERO; m_context << Instruction::DUP1 << Instruction::ISZERO;
m_context.appendConditionalRevert(); // TODO: Can we bubble up here? There might be different reasons for failure, I think.
m_context.appendConditionalRevert(true);
if (function.valueSet()) if (function.valueSet())
m_context << swapInstruction(1) << Instruction::POP; m_context << swapInstruction(1) << Instruction::POP;
break; break;
@ -670,8 +671,9 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
if (function.kind() == FunctionType::Kind::Transfer) if (function.kind() == FunctionType::Kind::Transfer)
{ {
// Check if zero (out of stack or not enough balance). // Check if zero (out of stack or not enough balance).
// TODO: bubble up here, but might also be different error.
m_context << Instruction::ISZERO; m_context << Instruction::ISZERO;
m_context.appendConditionalRevert(); m_context.appendConditionalRevert(true);
} }
break; break;
case FunctionType::Kind::Selfdestruct: case FunctionType::Kind::Selfdestruct:
@ -1823,6 +1825,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
if (funKind == FunctionType::Kind::External || funKind == FunctionType::Kind::CallCode || funKind == FunctionType::Kind::DelegateCall) if (funKind == FunctionType::Kind::External || funKind == FunctionType::Kind::CallCode || funKind == FunctionType::Kind::DelegateCall)
{ {
m_context << Instruction::DUP1 << Instruction::EXTCODESIZE << Instruction::ISZERO; m_context << Instruction::DUP1 << Instruction::EXTCODESIZE << Instruction::ISZERO;
// TODO: error message?
m_context.appendConditionalRevert(); m_context.appendConditionalRevert();
existenceChecked = true; existenceChecked = true;
} }
@ -1865,7 +1868,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
{ {
//Propagate error condition (if CALL pushes 0 on stack). //Propagate error condition (if CALL pushes 0 on stack).
m_context << Instruction::ISZERO; m_context << Instruction::ISZERO;
m_context.appendConditionalRevert(); m_context.appendConditionalRevert(true);
} }
utils().popStackSlots(remainsSize); utils().popStackSlots(remainsSize);

View File

@ -10512,6 +10512,43 @@ BOOST_AUTO_TEST_CASE(require_with_message)
ABI_CHECK(callContractFunction("h()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "abc")); ABI_CHECK(callContractFunction("h()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "abc"));
} }
BOOST_AUTO_TEST_CASE(bubble_up_error_messages)
{
char const* sourceCode = R"(
contract D {
function f() public {
revert("message");
}
function g() public {
this.f();
}
}
contract C {
D d = new D();
function forward(address target, bytes data) internal returns (bool success, bytes retval) {
uint retsize;
assembly {
success := call(not(0), target, 0, add(data, 0x20), mload(data), 0, 0)
retsize := returndatasize()
}
retval = new bytes(retsize);
assembly {
returndatacopy(add(retval, 0x20), 0, returndatasize())
}
}
function f() public returns (bool, bytes) {
return forward(address(d), msg.data);
}
function g() public returns (bool, bytes) {
return forward(address(d), msg.data);
}
}
)";
compileAndRun(sourceCode, 0, "C");
ABI_CHECK(callContractFunction("f()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "message"));
ABI_CHECK(callContractFunction("g()"), encodeArgs(0, 0x40, 0x80, 0, 0x40, 3, "message"));
}
BOOST_AUTO_TEST_CASE(negative_stack_height) BOOST_AUTO_TEST_CASE(negative_stack_height)
{ {
// This code was causing negative stack height during code generation // This code was causing negative stack height during code generation