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

Checked arithmetic by default. #9465

Merged
merged 5 commits into from
Oct 19, 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
4 changes: 4 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

Breaking Changes:
* Assembler: The artificial ASSIGNIMMUTABLE opcode and the corresponding builtin in the "EVM with object access" dialect of Yul take the base offset of the code to modify as additional argument.
* Code Generator: All arithmetic is checked by default. These checks can be disabled using ``unchecked { ... }``.
* 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)``.
Expand All @@ -15,6 +16,9 @@ Language Features:
* New AST Node ``IdentifierPath`` replacing in many places the ``UserDefinedTypeName``


AST Changes:
* New node type: unchecked block - used for ``unchecked { ... }``.

### 0.7.4 (unreleased)

Important Bugfixes:
Expand Down
64 changes: 64 additions & 0 deletions docs/control-structures.rst
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,69 @@ In any case, you will get a warning about the outer variable being shadowed.
}
}


.. _unchecked:

Checked or Unchecked Arithmetic
===============================

An overflow or underflow is the situation where the resulting value of an arithmetic operation,
when executed on an unrestricted integer, falls outside the range of the result type.

Prior to Solidity 0.8.0, arithmetic operations would always wrap in case of
under- or overflow leading to widespread use of libraries that introduce
additional checks.

Since Solidity 0.8.0, all arithmetic operations revert on over- and underflow by default,
thus making the use of these libraries unnecessary.

To obtain the previous behaviour, an ``unchecked`` block can be used:

::

// SPDX-License-Identifier: GPL-3.0
pragma solidity >0.7.99;
contract C {
function f(uint a, uint b) pure public returns (uint) {
// This addition will wrap on underflow.
unchecked { return a - b; }
}
function g(uint a, uint b) pure public returns (uint) {
// This addition will revert on underflow.
return a - b;
}
}

The call to ``f(2, 3)`` will return ``2**256-1``, while ``g(2, 3)`` will cause
a failing assertion.

The ``unchecked`` block can be used everywhere inside a block, but not as a replacement
for a block. It also cannot be nested.

The setting only affects the statements that are syntactically inside the block.
Functions called from within an ``unchecked`` block do not inherit the property.

.. note::
To avoid ambiguity, you cannot use ``_;`` inside an ``unchecked`` block.

The following operators will cause a failing assertion on overflow or underflow
and will wrap without an error if used inside an unchecked block:

``++``, ``--``, ``+``, binary ``-``, unary ``-``, ``*``, ``/``, ``%``, ``**``

``+=``, ``-=``, ``*=``, ``/=``, ``%=``

.. warning::
It is not possible to disable the check for division by zero
or modulo by zero using the ``unchecked`` block.

.. note::
The second statement in ``int x = type(int).min; -x;`` will result in an overflow
because the negative range can hold one more value than the positive range.

Explicit type conversions will always truncate and never cause a failing assertion
with the exception of a conversion from an integer to an enum type.

.. index:: ! exception, ! throw, ! assert, ! require, ! revert, ! errors

.. _assert-and-require:
Expand Down Expand Up @@ -519,6 +582,7 @@ 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.
Expand Down
5 changes: 4 additions & 1 deletion docs/grammar/Solidity.g4
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,10 @@ numberLiteral: (DecimalNumber | HexNumber) NumberUnit?;
/**
* A curly-braced block of statements. Opens its own scope.
*/
block: LBrace statement* RBrace;
block:
LBrace ( statement | uncheckedBlock )* RBrace;

uncheckedBlock: Unchecked block;
Copy link
Member

Choose a reason for hiding this comment

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

This allows for nested unchecked blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is not a parser but a syntax checker restriction.

Copy link
Member

Choose a reason for hiding this comment

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

Is if (<sth>) unchecked { x; } invalid? (I'll probably know when I'm done reviewing...)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, apparently it isn't - not sure if it couldn't be valid, but fine!


statement:
block
Expand Down
3 changes: 2 additions & 1 deletion docs/grammar/SolidityLexer.g4
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ ReservedKeywords:
'after' | 'alias' | 'apply' | 'auto' | 'case' | 'copyof' | 'default' | 'define' | 'final'
| 'implements' | 'in' | 'inline' | 'let' | 'macro' | 'match' | 'mutable' | 'null' | 'of'
| 'partial' | 'promise' | 'reference' | 'relocatable' | 'sealed' | 'sizeof' | 'static'
| 'supports' | 'switch' | 'typedef' | 'typeof' | 'unchecked' | 'var';
| 'supports' | 'switch' | 'typedef' | 'typeof' | 'var';

Pragma: 'pragma' -> pushMode(PragmaMode);
Abstract: 'abstract';
Expand Down Expand Up @@ -87,6 +87,7 @@ True: 'true';
Try: 'try';
Type: 'type';
Ufixed: 'ufixed' | ('ufixed' [0-9]+ 'x' [0-9]+);
Unchecked: 'unchecked';
/**
* Sized unsigned integer types.
* uint is an alias of uint256.
Expand Down
31 changes: 21 additions & 10 deletions docs/security-considerations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -246,21 +246,32 @@ Two's Complement / Underflows / Overflows
=========================================

As in many programming languages, Solidity's integer types are not actually integers.
They resemble integers when the values are small, but behave differently if the numbers are larger.
For example, the following is true: ``uint8(255) + uint8(1) == 0``. This situation is called
an *overflow*. It occurs when an operation is performed that requires a fixed size variable
to store a number (or piece of data) that is outside the range of the variable's data type.
An *underflow* is the converse situation: ``uint8(0) - uint8(1) == 255``.
They resemble integers when the values are small, but cannot represent arbitrarily large numbers.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
They resemble integers when the values are small, but cannot represent arbitrarily large numbers.
They resemble integers when the values are small, but do not represent arbitrary precision integers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't precision something else?


The following code causes an overflow because the result of the addition is too large
to be stored in the type ``uint8``:

::

uint8 x = 255;
uint8 y = 1;
return x + y;

Solidity has two modes in which it deals with these overflows: Checked and Unchecked or "wrapping" mode.

The default checked mode will detect overflows and cause a failing assertion. You can disable this check
using ``unchecked { ... }``, causing the overflow to be silently ignored. The above code would return
``0`` if wrapped in ``unchecked { ... }``.

Even in checked mode, do not assume you are protected from overflow bugs.
In this mode, overflows will always revert. If it is not possible to avoid the
overflow, this can lead to a smart contract being stuck in a certain state.

In general, read about the limits of two's complement representation, which even has some
more special edge cases for signed numbers.

Try to use ``require`` to limit the size of inputs to a reasonable range and use the
:ref:`SMT checker<smt_checker>` to find potential overflows, or use a library like
`SafeMath <https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/math/SafeMath.sol>`_
if you want all overflows to cause a revert.

Code such as ``require((balanceOf[_to] + _value) >= balanceOf[_to])`` can also help you check if values are what you expect.
:ref:`SMT checker<smt_checker>` to find potential overflows.

.. _clearing-mappings:

Expand Down
43 changes: 27 additions & 16 deletions docs/types/value-types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ access the minimum and maximum value representable by the type.
.. warning::

Integers in Solidity are restricted to a certain range. For example, with ``uint32``, this is ``0`` up to ``2**32 - 1``.
If the result of some operation on those numbers does not fit inside this range, it is truncated. These truncations can have
serious consequences that you should :ref:`be aware of and mitigate against<underflow-overflow>`.
There are two modes in which arithmetic is performed on these types: The "wrapping" or "unchecked" mode and the "checked" mode.
By default, arithmetic is always "checked", which mean that if the result of an operation falls outside the value range
of the type, the call is reverted through a :ref:`failing assertion<assert-and-require>`. You can switch to "unchecked" mode
using ``unchecked { ... }``. More details can be found in the section about :ref:`unchecked <unchecked>`.

Comparisons
^^^^^^^^^^^
Expand Down Expand Up @@ -77,23 +79,22 @@ Right operand must be unsigned type. Trying to shift by signed type will produce
Addition, Subtraction and Multiplication
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Addition, subtraction and multiplication have the usual semantics.
They wrap in two's complement representation, meaning that
for example ``uint256(0) - uint256(1) == 2**256 - 1``. You have to take these overflows
into account when designing safe smart contracts.
Addition, subtraction and multiplication have the usual semantics, with two different
modes in regard to over- and underflow:

By default, all arithmetic is checked for under- or overflow, but this can be disabled
using the :ref:`unchecked block<unchecked>`, resulting in wrapping arithmetic. More details
can be found in that section.

The expression ``-x`` is equivalent to ``(T(0) - x)`` where
``T`` is the type of ``x``. It can only be applied to signed types.
The value of ``-x`` can be
positive if ``x`` is negative. There is another caveat also resulting
from two's complement representation::

int x = -2**255;
assert(-x == x);

This means that even if a number is negative, you cannot assume that
its negation will be positive.
from two's complement representation:

If you have ``int x = type(int).min;``, then ``-x`` does not fit the positive range.
This means that ``unchecked { assert(-x == x); }`` works, and the expression ``-x``
when used in checked mode will result in a failing assertion.

Division
^^^^^^^^
Expand All @@ -106,7 +107,12 @@ Note that in contrast, division on :ref:`literals<rational_literals>` results in
of arbitrary precision.

.. note::
Division by zero causes a failing assert.
Division by zero causes a failing assert. This check can **not** be disabled through ``unchecked { ... }``.

.. note::
The expression ``type(int).min / (-1)`` is the only case where division causes an overflow.
In checked arithmetic mode, this will cause a failing assertion, while in wrapping
mode, the value will be ``type(int).min``.

Modulo
^^^^^^
Expand All @@ -121,14 +127,19 @@ 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.
Modulo with zero causes a failing assert. This check can **not** be disabled through ``unchecked { ... }``.

Exponentiation
^^^^^^^^^^^^^^

Exponentiation is only available for unsigned types in the exponent. The resulting type
of an exponentiation is always equal to the type of the base. Please take care that it is
large enough to hold the result and prepare for potential wrapping behaviour.
large enough to hold the result and prepare for potential assertion failures or wrapping behaviour.

.. note::
In checked mode, exponentiation only uses the comparatively cheap ``exp`` opcode for small bases.
For the cases of ``x**3``, the expression ``x*x*x`` might be cheaper.
In any case, gas cost tests and the use of the optimizer are advisable.

.. note::
Note that ``0**0`` is defined by the EVM as ``1``.
Expand Down
2 changes: 1 addition & 1 deletion liblangutil/Token.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ namespace solidity::langutil
K(Throw, "throw", 0) \
K(Try, "try", 0) \
K(Type, "type", 0) \
K(Unchecked, "unchecked", 0) \
K(Unicode, "unicode", 0) \
K(Using, "using", 0) \
K(View, "view", 0) \
Expand Down Expand Up @@ -266,7 +267,6 @@ namespace solidity::langutil
K(Switch, "switch", 0) \
K(Typedef, "typedef", 0) \
K(TypeOf, "typeof", 0) \
K(Unchecked, "unchecked", 0) \
K(Var, "var", 0) \
\
/* Yul-specific tokens, but not keywords. */ \
Expand Down
31 changes: 30 additions & 1 deletion libsolidity/analysis/SyntaxChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,28 @@ void SyntaxChecker::endVisit(ForStatement const&)
m_inLoopDepth--;
}

bool SyntaxChecker::visit(Block const& _block)
{
if (_block.unchecked())
{
if (m_uncheckedArithmetic)
m_errorReporter.syntaxError(
1941_error,
_block.location(),
"\"unchecked\" blocks cannot be nested."
);

m_uncheckedArithmetic = true;
}
return true;
}

void SyntaxChecker::endVisit(Block const& _block)
{
if (_block.unchecked())
m_uncheckedArithmetic = false;
}

bool SyntaxChecker::visit(Continue const& _continueStatement)
{
if (m_inLoopDepth <= 0)
Expand Down Expand Up @@ -288,8 +310,15 @@ bool SyntaxChecker::visit(InlineAssembly const& _inlineAssembly)
return false;
}

bool SyntaxChecker::visit(PlaceholderStatement const&)
bool SyntaxChecker::visit(PlaceholderStatement const& _placeholder)
{
if (m_uncheckedArithmetic)
m_errorReporter.syntaxError(
2573_error,
_placeholder.location(),
"The placeholder statement \"_\" cannot be used inside an \"unchecked\" block."
);

m_placeholderFound = true;
return true;
}
Expand Down
6 changes: 6 additions & 0 deletions libsolidity/analysis/SyntaxChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ class SyntaxChecker: private ASTConstVisitor
bool visit(ForStatement const& _forStatement) override;
void endVisit(ForStatement const& _forStatement) override;

bool visit(Block const& _block) override;
void endVisit(Block const& _block) override;

bool visit(Continue const& _continueStatement) override;
bool visit(Break const& _breakStatement) override;

Expand Down Expand Up @@ -100,6 +103,9 @@ class SyntaxChecker: private ASTConstVisitor
/// Flag that indicates whether some version pragma was present.
bool m_versionPragmaFound = false;

/// Flag that indicates whether we are inside an unchecked block.
bool m_uncheckedArithmetic = false;

int m_inLoopDepth = 0;
std::optional<ContractKind> m_currentContractKind;

Expand Down
8 changes: 7 additions & 1 deletion libsolidity/ast/AST.h
Original file line number Diff line number Diff line change
Expand Up @@ -1384,18 +1384,24 @@ class Block: public Statement, public Scopable, public ScopeOpener
int64_t _id,
SourceLocation const& _location,
ASTPointer<ASTString> const& _docString,
bool _unchecked,
std::vector<ASTPointer<Statement>> _statements
):
Statement(_id, _location, _docString), m_statements(std::move(_statements)) {}
Statement(_id, _location, _docString),
m_statements(std::move(_statements)),
m_unchecked(_unchecked)
{}
void accept(ASTVisitor& _visitor) override;
void accept(ASTConstVisitor& _visitor) const override;

std::vector<ASTPointer<Statement>> const& statements() const { return m_statements; }
bool unchecked() const { return m_unchecked; }

BlockAnnotation& annotation() const override;

private:
std::vector<ASTPointer<Statement>> m_statements;
bool m_unchecked;
};

/**
Expand Down
2 changes: 2 additions & 0 deletions libsolidity/ast/ASTEnums.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ enum class StateMutability { Pure, View, NonPayable, Payable };
/// Visibility ordered from restricted to unrestricted.
enum class Visibility { Default, Private, Internal, Public, External };

enum class Arithmetic { Checked, Wrapping };

inline std::string stateMutabilityToString(StateMutability const& _stateMutability)
{
switch (_stateMutability)
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/ast/ASTJsonConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ bool ASTJsonConverter::visit(InlineAssembly const& _node)

bool ASTJsonConverter::visit(Block const& _node)
{
setJsonNode(_node, "Block", {
setJsonNode(_node, _node.unchecked() ? "UncheckedBlock" : "Block", {
make_pair("statements", toJson(_node.statements()))
});
return false;
Expand Down
Loading