Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Panic with error codes. #10013

Merged
merged 7 commits into from
Oct 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Breaking Changes:
* Type System: Unary negation can only be used on signed integers, not on unsigned integers.
* Type System: Disallow explicit conversions from negative literals and literals larger than ``type(uint160).max`` to ``address`` type.
* Parser: Exponentiation is right associative. ``a**b**c`` is parsed as ``a**(b**c)``.
* Code Generator: Use ``revert`` with error signature ``Panic(uint256)`` and error codes instead of invalid opcode on failing assertions.


Language Features:
Expand Down
9 changes: 9 additions & 0 deletions docs/080-breaking-changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ and it does something else afterwards.
* Exponentiation is right associative, i.e., the expression ``a**b**c`` is parsed as ``a**(b**c)``.
Before 0.8.0, it was parsed as ``(a**b)**c``.

Semantic only Changes
=====================

* Failing assertions (and other internal checks like division by zero) do not use the invalid opcode anymore but instead revert
with error data equal to a function call to ``Panic(uint256)`` with an error code specific to the circumstances.

This will save gas on errors while it still allows static analysis tools to detect these situations.


Syntactic Only Changes
======================

Expand Down
82 changes: 51 additions & 31 deletions docs/control-structures.rst
Original file line number Diff line number Diff line change
Expand Up @@ -564,35 +564,53 @@ of an exception instead of "bubbling up".

Exceptions can be caught with the ``try``/``catch`` statement.

``assert`` and ``require``
--------------------------
Exceptions can contain data that is passed back to the caller.
This data consists of a 4-byte selector and subsequent :ref:`ABI-encoded<abi>` data.
The selector is computed in the same way as a function selector, i.e.,
the first four bytes of the keccak256-hash of a function
signature - in this case an error signature.

Currently, Solidity supports two error signatures: ``Error(string)``
and ``Panic(uint256)``. The first ("error") is used for "regular" error conditions
while the second ("panic") is used for errors that should not be present in bug-free code.

Panic via ``assert`` and Error via ``require``
----------------------------------------------

The convenience functions ``assert`` and ``require`` can be used to check for conditions and throw an exception
if the condition is not met.

The ``assert`` function should only be used to test for internal
The ``assert`` function creates an error of type ``Panic(uint256)``.
The same error is created by the compiler in certain situations as listed below.

Assert should only be used to test for internal
errors, and to check invariants. Properly functioning code should
never reach a failing ``assert`` statement; if this happens there
never create a Panic, not even on invalid external input.
If this happens, then there
is a bug in your contract which you should fix. Language analysis
tools can evaluate your contract to identify the conditions and
function calls which will reach a failing ``assert``.

An ``assert``-style exception is generated in the following situations:

#. If you access an array or an array slice at a too large or negative index (i.e. ``x[i]`` where ``i >= x.length`` or ``i < 0``).
#. If you access a fixed-length ``bytesN`` at a too large or negative index.
#. If you divide or modulo by zero (e.g. ``5 / 0`` or ``23 % 0``).
#. If an arithmetic operation results in under- or overflow outside of an ``unchecked { ... }`` block.
#. If you convert a value too big or negative into an enum type.
#. If you call a zero-initialized variable of internal function type.
#. If you call ``assert`` with an argument that evaluates to false.

The ``require`` function should be used to ensure valid conditions
function calls which will cause a Panic.

A Panic exception is generated in the following situations.
The error code supplied with the error data indicates the kind of panic.

#. 0x01: If you call ``assert`` with an argument that evaluates to false.
#. 0x11: If an arithmetic operation results in underflow or overflow outside of an ``unchecked { ... }`` block.
leonardoalt marked this conversation as resolved.
Show resolved Hide resolved
#. 0x12; If you divide or modulo by zero (e.g. ``5 / 0`` or ``23 % 0``).
#. 0x21: If you convert a value that is too big or negative into an enum type.
#. 0x31: If you call ``.pop()`` on an empty array.
#. 0x32: If you access an array, ``bytesN`` or an array slice at an out-of-bounds or negative index (i.e. ``x[i]`` where ``i >= x.length`` or ``i < 0``).
#. 0x41: If you allocate too much memory or create an array that is too large.
#. 0x51: If you call a zero-initialized variable of internal function type.

The ``require`` function either creates an error of type ``Error(string)``
or an error without any error data and it
should be used to ensure valid conditions
that cannot be detected until execution time.
This includes conditions on inputs
or return values from calls to external contracts.

A ``require``-style exception is generated in the following situations:
A ``Error(string)`` exception is generated in the following situations:

#. Calling ``require`` with an argument that evaluates to ``false``.
#. If you call a function via a message call but it does not finish
Expand All @@ -611,6 +629,11 @@ A ``require``-style exception is generated in the following situations:

You can optionally provide a message string for ``require``, but not for ``assert``.

.. note::
If you do not provide a string argument to ``require``, it will revert
with empty error data, not even including the error selector.


The following example shows how you can use ``require`` to check conditions on inputs
and ``assert`` for internal error checking.

Expand All @@ -633,29 +656,29 @@ and ``assert`` for internal error checking.
}

Internally, Solidity performs a revert operation (instruction
``0xfd``) for a ``require``-style exception and executes an invalid operation
(instruction ``0xfe``) to throw an ``assert``-style exception. In both cases, this causes
``0xfd``). This causes
the EVM to revert all changes made to the state. The reason for reverting
is that there is no safe way to continue execution, because an expected effect
did not occur. Because we want to keep the atomicity of transactions, the
safest action is to revert all changes and make the whole transaction
(or at least call) without effect.

In both cases, the caller can react on such failures using ``try``/``catch``
(in the failing ``assert``-style exception only if enough gas is left), but
In both cases, the caller can react on such failures using ``try``/``catch``, but
the changes in the caller will always be reverted.

.. note::

``assert``-style exceptions consume all gas available to the call,
while ``require``-style exceptions do not consume any gas starting from the Metropolis release.
Panic exceptions used to use the ``invalid`` opcode before Solidity 0.8.0,
which consumed all gas available to the call.
Exceptions that use ``require`` used to consume all gas until before the Metropolis release.

``revert``
----------

The ``revert`` function is another way to trigger exceptions from within other code blocks to flag an error and
revert the current call. The function takes an optional string
message containing details about the error that is passed back to the caller.
message containing details about the error that is passed back to the caller
and it will create an ``Error(string)`` exception.

The following example shows how to use an error string together with ``revert`` and the equivalent ``require``:

Expand Down Expand Up @@ -726,9 +749,7 @@ A failure in an external call can be caught using a try/catch statement, as foll
errorCount++;
return (0, false);
} catch (bytes memory /*lowLevelData*/) {
// This is executed in case revert() was used
// or there was a failing assertion, division
// by zero, etc. inside getData.
// This is executed in case revert() was used.
errorCount++;
return (0, false);
}
Expand All @@ -754,9 +775,8 @@ It is planned to support other types of error data in the future.
The string ``Error`` is currently parsed as is and is not treated as an identifier.

The clause ``catch (bytes memory lowLevelData)`` is executed if the error signature
does not match any other clause, there was an error during decoding of the error
message, if there was a failing assertion in the external
call (for example due to a division by zero or a failing ``assert()``) or
does not match any other clause, if there was an error while decoding the error
message, or
if no error data was provided with the exception.
The declared variable provides access to the low-level error data in that case.

Expand Down
9 changes: 5 additions & 4 deletions docs/types/value-types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ Note that in contrast, division on :ref:`literals<rational_literals>` results in
of arbitrary precision.

.. note::
Division by zero causes a failing assert. This check can **not** be disabled through ``unchecked { ... }``.
Division by zero causes a :ref:`Panic error<assert-and-require>`. This check can **not** be disabled through ``unchecked { ... }``.

.. note::
The expression ``type(int).min / (-1)`` is the only case where division causes an overflow.
Expand All @@ -127,7 +127,7 @@ results in the same sign as its left operand (or zero) and ``a % n == -(-a % n)`
* ``int256(-5) % int256(-2) == int256(-1)``

.. note::
Modulo with zero causes a failing assert. This check can **not** be disabled through ``unchecked { ... }``.
Modulo with zero causes a :ref:`Panic error<assert-and-require>`. This check can **not** be disabled through ``unchecked { ... }``.

Exponentiation
^^^^^^^^^^^^^^
Expand Down Expand Up @@ -562,7 +562,8 @@ Enums

Enums are one way to create a user-defined type in Solidity. They are explicitly convertible
to and from all integer types but implicit conversion is not allowed. The explicit conversion
from integer checks at runtime that the value lies inside the range of the enum and causes a failing assert otherwise.
from integer checks at runtime that the value lies inside the range of the enum and causes a
:ref:`Panic error<assert-and-require>` otherwise.
Enums require at least one member, and its default value when declared is the first member.

The data representation is the same as for enums in C: The options are represented by
Expand Down Expand Up @@ -656,7 +657,7 @@ On the other hand, a ``non-payable`` function will reject Ether sent to it,
so ``non-payable`` functions cannot be converted to ``payable`` functions.

If a function type variable is not initialised, calling it results
in a failed assertion. The same happens if you call a function after using ``delete``
in a :ref:`Panic error<assert-and-require>`. The same happens if you call a function after using ``delete``
on it.

If external function types are used outside of the context of Solidity,
Expand Down
2 changes: 1 addition & 1 deletion docs/units-and-global-variables.rst
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ See the dedicated section on :ref:`assert and require<assert-and-require>` for
more details on error handling and when to use which function.

``assert(bool condition)``
causes an invalid opcode and thus state change reversion if the condition is not met - to be used for internal errors.
causes a Panic error and thus state change reversion if the condition is not met - to be used for internal errors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe link the panic section here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we would have to link it everywehre, but it is already linked in the paragraph above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok


``require(bool condition)``
reverts if the condition is not met - to be used for errors in inputs or external components.
Expand Down
20 changes: 15 additions & 5 deletions libsolidity/codegen/ArrayUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
#include <libsolidity/codegen/CompilerUtils.h>
#include <libsolidity/codegen/LValue.h>

#include <libsolutil/FunctionSelector.h>
#include <libsolutil/Whiskers.h>

#include <libevmasm/Instruction.h>
#include <liblangutil/Exceptions.h>

Expand Down Expand Up @@ -857,13 +860,17 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const

if (_type.isByteArray())
{
m_context.appendInlineAssembly(R"({
util::Whiskers code(R"({
let slot_value := sload(ref)
switch and(slot_value, 1)
case 0 {
// short byte array
let length := and(div(slot_value, 2), 0x1f)
if iszero(length) { invalid() }
if iszero(length) {
mstore(0, <panicSelector>)
mstore(4, <emptyArrayPop>)
revert(0, 0x24)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to extract these 3 lines similar to CompilerContext::appendPanic?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah YulUtilFunctions::panicFunction has that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you suggest to do it? I don't see a nice way to extract this without causing a function call.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make a call to YulUtilFunctions::panicFunction(util::PanicCode::EmptyArrayPop)

}

// Zero-out the suffix including the least significant byte.
let mask := sub(exp(0x100, sub(33, length)), 1)
Expand Down Expand Up @@ -901,7 +908,10 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const
sstore(ref, slot_value)
}
}
})", {"ref"});
})");
code("panicSelector", util::selectorFromSignature("Panic(uint256)").str());
code("emptyArrayPop", to_string(unsigned(util::PanicCode::EmptyArrayPop)));
m_context.appendInlineAssembly(code.render(), {"ref"});
m_context << Instruction::POP;
}
else
Expand All @@ -912,7 +922,7 @@ void ArrayUtils::popStorageArrayElement(ArrayType const& _type) const
m_context << Instruction::DUP1;
// stack: ArrayReference oldLength oldLength
m_context << Instruction::ISZERO;
m_context.appendConditionalInvalid();
m_context.appendConditionalPanic(util::PanicCode::EmptyArrayPop);

// Stack: ArrayReference oldLength
m_context << u256(1) << Instruction::SWAP1 << Instruction::SUB;
Expand Down Expand Up @@ -1058,7 +1068,7 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType, bool _doBoundsCheck, b
// check out-of-bounds access
m_context << Instruction::DUP2 << Instruction::LT << Instruction::ISZERO;
// out-of-bounds access throws exception
m_context.appendConditionalInvalid();
m_context.appendConditionalPanic(util::PanicCode::ArrayOutOfBounds);
}
if (location == DataLocation::CallData && _arrayType.isDynamicallySized())
// remove length if present
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/codegen/ArrayUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class ArrayUtils
/// Stack pre: reference (excludes byte offset)
/// Stack post: new_length
void incrementDynamicArraySize(ArrayType const& _type) const;
/// Decrements the size of a dynamic array by one if length is nonzero. Causes an invalid instruction otherwise.
/// Decrements the size of a dynamic array by one if length is nonzero. Causes a Panic otherwise.
/// Clears the removed data element. In case of a byte array, this might move the data.
/// Stack pre: reference
/// Stack post:
Expand Down
17 changes: 13 additions & 4 deletions libsolidity/codegen/CompilerContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include <libyul/Utilities.h>

#include <libsolutil/Whiskers.h>
#include <libsolutil/FunctionSelector.h>

#include <liblangutil/ErrorReporter.h>
#include <liblangutil/Scanner.h>
Expand Down Expand Up @@ -330,16 +331,24 @@ CompilerContext& CompilerContext::appendJump(evmasm::AssemblyItem::JumpType _jum
return *this << item;
}

CompilerContext& CompilerContext::appendInvalid()
CompilerContext& CompilerContext::appendPanic(util::PanicCode _code)
{
return *this << Instruction::INVALID;
Whiskers templ(R"({
mstore(0, <selector>)
mstore(4, <code>)
revert(0, 0x24)
})");
templ("selector", util::selectorFromSignature("Panic(uint256)").str());
templ("code", u256(_code).str());
appendInlineAssembly(templ.render());
return *this;
}

CompilerContext& CompilerContext::appendConditionalInvalid()
CompilerContext& CompilerContext::appendConditionalPanic(util::PanicCode _code)
{
*this << Instruction::ISZERO;
evmasm::AssemblyItem afterTag = appendConditionalJump();
*this << Instruction::INVALID;
appendPanic(_code);
*this << afterTag;
return *this;
}
Expand Down
9 changes: 5 additions & 4 deletions libsolidity/codegen/CompilerContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <liblangutil/ErrorReporter.h>
#include <liblangutil/EVMVersion.h>
#include <libsolutil/Common.h>
#include <libsolutil/ErrorCodes.h>

#include <libyul/AsmAnalysisInfo.h>
#include <libyul/backends/evm/EVMDialect.h>
Expand Down Expand Up @@ -195,10 +196,10 @@ class CompilerContext
evmasm::AssemblyItem appendJumpToNew() { return m_asm->appendJump().tag(); }
/// Appends a JUMP to a tag already on the stack
CompilerContext& appendJump(evmasm::AssemblyItem::JumpType _jumpType = evmasm::AssemblyItem::JumpType::Ordinary);
/// Appends an INVALID instruction
CompilerContext& appendInvalid();
/// Appends a conditional INVALID instruction
CompilerContext& appendConditionalInvalid();
/// Appends code to revert with a Panic(uint256) error.
CompilerContext& appendPanic(util::PanicCode _code);
/// Appends code to revert with a Panic(uint256) error if the topmost stack element is nonzero.
CompilerContext& appendConditionalPanic(util::PanicCode _code);
/// Appends a REVERT(0, 0) call
/// @param _message is an optional revert message used in debug mode
CompilerContext& appendRevert(std::string const& _message = "");
Expand Down
8 changes: 4 additions & 4 deletions libsolidity/codegen/CompilerUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ void CompilerUtils::convertType(
if (_asPartOfArgumentDecoding)
m_context.appendConditionalRevert(false, "Enum out of range");
else
m_context.appendConditionalInvalid();
m_context.appendConditionalPanic(util::PanicCode::EnumConversionError);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the line above, for "Enum out of range", shouldn't this also be a panic?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is this left for the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is a revert ("Error()") and not a panic.

enumOverflowCheckPending = false;
}
break;
Expand Down Expand Up @@ -849,7 +849,7 @@ void CompilerUtils::convertType(
EnumType const& enumType = dynamic_cast<decltype(enumType)>(_targetType);
solAssert(enumType.numberOfMembers() > 0, "empty enum should have caused a parser error.");
m_context << u256(enumType.numberOfMembers() - 1) << Instruction::DUP2 << Instruction::GT;
m_context.appendConditionalInvalid();
m_context.appendConditionalPanic(util::PanicCode::EnumConversionError);
enumOverflowCheckPending = false;
}
else if (targetTypeCategory == Type::Category::FixedPoint)
Expand Down Expand Up @@ -1213,13 +1213,13 @@ void CompilerUtils::pushZeroValue(Type const& _type)
if (funType->kind() == FunctionType::Kind::Internal)
{
m_context << m_context.lowLevelFunctionTag("$invalidFunction", 0, 0, [](CompilerContext& _context) {
_context.appendInvalid();
_context.appendPanic(util::PanicCode::InvalidInternalFunction);
});
if (CompilerContext* runCon = m_context.runtimeContext())
{
leftShiftNumberOnStack(32);
m_context << runCon->lowLevelFunctionTag("$invalidFunction", 0, 0, [](CompilerContext& _context) {
_context.appendInvalid();
_context.appendPanic(util::PanicCode::InvalidInternalFunction);
}).toSubAssemblyTag(m_context.runtimeSub());
m_context << Instruction::OR;
}
Expand Down
Loading