Merge pull request #4412 from ethereum/v050-reference-resolver-errorTypeForLoose

[BREAKING] permanently set errorTypeForLoose from Warning to SyntaxError
This commit is contained in:
Christian Parpart 2018-08-03 19:30:33 +02:00 committed by GitHub
commit a4ee1dfc83
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
22 changed files with 38 additions and 275 deletions

View File

@ -54,6 +54,7 @@ Breaking Changes:
* Type Checker: Fallback function must be external. This was already the case in the experimental 0.5.0 mode.
* Type Checker: Interface functions must be declared external. This was already the case in the experimental 0.5.0 mode.
* Type Checker: Address members are not included in contract types anymore. An explicit conversion is now required before invoking an ``address`` member from a contract.
* Type Checker: Disallow "loose assembly" syntax entirely. This means that jump labels, jumps and non-functional instructions cannot be used anymore.
* Remove obsolete ``std`` directory from the Solidity repository. This means accessing ``https://github.com/ethereum/soldity/blob/develop/std/*.sol`` (or ``https://github.com/ethereum/solidity/std/*.sol`` in Remix) will not be possible.
* References Resolver: Turn missing storage locations into an error. This was already the case in the experimental 0.5.0 mode.
* Syntax Checker: Disallow functions without implementation to use modifiers. This was already the case in the experimental 0.5.0 mode.

View File

@ -21,10 +21,9 @@ often hard to address the correct stack slot and provide arguments to opcodes at
point on the stack. Solidity's inline assembly tries to facilitate that and other issues
arising when writing manual assembly by the following features:
* functional-style opcodes: ``mul(1, add(2, 3))`` instead of ``push1 3 push1 2 add push1 1 mul``
* functional-style opcodes: ``mul(1, add(2, 3))``
* assembly-local variables: ``let x := add(2, 3) let y := mload(0x40) x := add(x, y)``
* access to external variables: ``function f(uint x) public { assembly { x := sub(x, 1) } }``
* labels: ``let x := 10 repeat: x := sub(x, 1) jumpi(repeat, eq(x, 0))``
* loops: ``for { let i := 0 } lt(i, x) { i := add(i, 1) } { y := mul(2, y) }``
* if statements: ``if slt(x, 0) { x := sub(0, x) }``
* switch statements: ``switch x case 0 { y := mul(x, 2) } default { y := 0 }``
@ -134,7 +133,6 @@ usual ``//`` and ``/* */`` comments. Inline assembly is marked by ``assembly { .
these curly braces, the following can be used (see the later sections for more details)
- literals, i.e. ``0x123``, ``42`` or ``"abc"`` (strings up to 32 characters)
- opcodes (in "instruction style"), e.g. ``mload sload dup1 sstore``, for a list see below
- opcodes in functional style, e.g. ``add(1, mlod(0))``
- labels, e.g. ``name:``
- variable declarations, e.g. ``let x := 7``, ``let x := add(y, 3)`` or ``let x`` (initial value of empty (0) is assigned)
@ -416,57 +414,8 @@ changes during the call, and thus references to local variables will be wrong.
Labels
------
.. note::
Labels are deprecated. Please use functions, loops, if or switch statements instead.
Another problem in EVM assembly is that ``jump`` and ``jumpi`` use absolute addresses
which can change easily. Solidity inline assembly provides labels to make the use of
jumps easier. Note that labels are a low-level feature and it is possible to write
efficient assembly without labels, just using assembly functions, loops, if and switch instructions
(see below). The following code computes an element in the Fibonacci series.
.. code::
{
let n := calldataload(4)
let a := 1
let b := a
loop:
jumpi(loopend, eq(n, 0))
a add swap1
n := sub(n, 1)
jump(loop)
loopend:
mstore(0, a)
return(0, 0x20)
}
Please note that automatically accessing stack variables can only work if the
assembler knows the current stack height. This fails to work if the jump source
and target have different stack heights. It is still fine to use such jumps, but
you should just not access any stack variables (even assembly variables) in that case.
Furthermore, the stack height analyser goes through the code opcode by opcode
(and not according to control flow), so in the following case, the assembler
will have a wrong impression about the stack height at label ``two``:
.. code::
{
let x := 8
jump(two)
one:
// Here the stack height is 2 (because we pushed x and 7),
// but the assembler thinks it is 1 because it reads
// from top to bottom.
// Accessing the stack variable x here will lead to errors.
x := 9
jump(three)
two:
7 // push something onto the stack
jump(one)
three:
}
Support for labels has been removed in version 0.5.0 of Solidity.
Please use functions, loops, if or switch statements instead.
Declaring Assembly-Local Variables
----------------------------------

View File

@ -277,7 +277,7 @@ bool ReferencesResolver::visit(InlineAssembly const& _inlineAssembly)
// Will be re-generated later with correct information
// We use the latest EVM version because we will re-run it anyway.
assembly::AsmAnalysisInfo analysisInfo;
boost::optional<Error::Type> errorTypeForLoose = m_experimental050Mode ? Error::Type::SyntaxError : Error::Type::Warning;
boost::optional<Error::Type> errorTypeForLoose = Error::Type::SyntaxError;
assembly::AsmAnalyzer(analysisInfo, errorsIgnored, EVMVersion(), errorTypeForLoose, assembly::AsmFlavour::Loose, resolver).analyze(_inlineAssembly.operations());
return false;
}

View File

@ -927,15 +927,11 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
};
solAssert(!_inlineAssembly.annotation().analysisInfo, "");
_inlineAssembly.annotation().analysisInfo = make_shared<assembly::AsmAnalysisInfo>();
boost::optional<Error::Type> errorTypeForLoose =
m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) ?
Error::Type::SyntaxError :
Error::Type::Warning;
assembly::AsmAnalyzer analyzer(
*_inlineAssembly.annotation().analysisInfo,
m_errorReporter,
m_evmVersion,
errorTypeForLoose,
Error::Type::SyntaxError,
assembly::AsmFlavour::Loose,
identifierAccess
);

View File

@ -57,7 +57,7 @@ bool AsmAnalyzer::operator()(Label const& _label)
solAssert(!_label.name.empty(), "");
checkLooseFeature(
_label.location,
"The use of labels is deprecated. Please use \"if\", \"switch\", \"for\" or function calls instead."
"The use of labels is disallowed. Please use \"if\", \"switch\", \"for\" or function calls instead."
);
m_info.stackHeightInfo[&_label] = m_stackHeight;
warnOnInstructions(solidity::Instruction::JUMPDEST, _label.location);
@ -68,7 +68,7 @@ bool AsmAnalyzer::operator()(assembly::Instruction const& _instruction)
{
checkLooseFeature(
_instruction.location,
"The use of non-functional instructions is deprecated. Please use functional notation instead."
"The use of non-functional instructions is disallowed. Please use functional notation instead."
);
auto const& info = instructionInfo(_instruction.instruction);
m_stackHeight += info.ret - info.args;
@ -201,7 +201,7 @@ bool AsmAnalyzer::operator()(assembly::StackAssignment const& _assignment)
{
checkLooseFeature(
_assignment.location,
"The use of stack assignment is deprecated. Please use assignment in functional notation instead."
"The use of stack assignment is disallowed. Please use assignment in functional notation instead."
);
bool success = checkAssignment(_assignment.variableName, size_t(-1));
m_info.stackHeightInfo[&_assignment] = m_stackHeight;

View File

@ -381,19 +381,16 @@ library RLP {
// we can write entire words, and just overwrite any excess.
assembly {
{
let i := 0 // Start at arr + 0x20
let words := div(add(btsLen, 31), 32)
let rOffset := btsPtr
let wOffset := add(tgt, 0x20)
tag_loop:
jumpi(end, eq(i, words))
// Start at arr + 0x20
for { let i := 0 } not(eq(i, words)) { i := add(i, 1) }
{
let offset := mul(i, 0x20)
mstore(add(wOffset, offset), mload(add(rOffset, offset)))
i := add(i, 1)
}
jump(tag_loop)
end:
mstore(add(tgt, add(0x20, mload(tgt))), 0)
}
}

View File

@ -8910,52 +8910,6 @@ BOOST_AUTO_TEST_CASE(inline_assembly_storage_access_via_pointer)
ABI_CHECK(callContractFunction("separator2()"), encodeArgs(u256(0)));
}
BOOST_AUTO_TEST_CASE(inline_assembly_jumps)
{
char const* sourceCode = R"(
contract C {
function f() public {
assembly {
let n := calldataload(4)
let a := 1
let b := a
loop:
jumpi(loopend, eq(n, 0))
a add swap1
n := sub(n, 1)
jump(loop)
loopend:
mstore(0, a)
return(0, 0x20)
}
}
}
)";
compileAndRun(sourceCode, 0, "C");
ABI_CHECK(callContractFunction("f()", u256(5)), encodeArgs(u256(13)));
ABI_CHECK(callContractFunction("f()", u256(7)), encodeArgs(u256(34)));
}
BOOST_AUTO_TEST_CASE(inline_assembly_function_access)
{
char const* sourceCode = R"(
contract C {
uint public x;
function g(uint y) public { x = 2 * y; assembly { stop } }
function f(uint _x) public {
assembly {
_x
jump(g)
pop
}
}
}
)";
compileAndRun(sourceCode, 0, "C");
ABI_CHECK(callContractFunction("f(uint256)", u256(5)), encodeArgs());
ABI_CHECK(callContractFunction("x()"), encodeArgs(u256(10)));
}
BOOST_AUTO_TEST_CASE(inline_assembly_function_call)
{
char const* sourceCode = R"(
@ -11261,7 +11215,7 @@ BOOST_AUTO_TEST_CASE(invalid_instruction)
contract C {
function f() public {
assembly {
invalid
invalid()
}
}
}
@ -11688,19 +11642,10 @@ BOOST_AUTO_TEST_CASE(keccak256_assembly)
ret := keccak256(0, 0)
}
}
function g() public pure returns (bytes32 ret) {
assembly {
0
0
keccak256
=: ret
}
}
}
)";
compileAndRun(sourceCode, 0, "C");
ABI_CHECK(callContractFunction("f()"), fromHex("0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"));
ABI_CHECK(callContractFunction("g()"), fromHex("0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"));
}
BOOST_AUTO_TEST_CASE(multi_modifiers)
@ -12547,50 +12492,6 @@ BOOST_AUTO_TEST_CASE(staticcall_for_view_and_pure)
}
}
BOOST_AUTO_TEST_CASE(swap_peephole_optimisation)
{
char const* sourceCode = R"(
contract C {
function lt(uint a, uint b) public returns (bool c) {
assembly {
a
b
swap1
lt
=: c
}
}
function add(uint a, uint b) public returns (uint c) {
assembly {
a
b
swap1
add
=: c
}
}
function div(uint a, uint b) public returns (uint c) {
assembly {
a
b
swap1
div
=: c
}
}
}
)";
compileAndRun(sourceCode);
BOOST_CHECK(callContractFunction("lt(uint256,uint256)", u256(1), u256(2)) == encodeArgs(u256(1)));
BOOST_CHECK(callContractFunction("lt(uint256,uint256)", u256(2), u256(1)) == encodeArgs(u256(0)));
BOOST_CHECK(callContractFunction("add(uint256,uint256)", u256(1), u256(2)) == encodeArgs(u256(3)));
BOOST_CHECK(callContractFunction("add(uint256,uint256)", u256(100), u256(200)) == encodeArgs(u256(300)));
BOOST_CHECK(callContractFunction("div(uint256,uint256)", u256(2), u256(1)) == encodeArgs(u256(2)));
BOOST_CHECK(callContractFunction("div(uint256,uint256)", u256(200), u256(10)) == encodeArgs(u256(20)));
BOOST_CHECK(callContractFunction("div(uint256,uint256)", u256(1), u256(0)) == encodeArgs(u256(0)));
BOOST_CHECK(callContractFunction("div(uint256,uint256)", u256(0), u256(1)) == encodeArgs(u256(0)));
}
BOOST_AUTO_TEST_CASE(bitwise_shifting_constantinople)
{
if (!dev::test::Options::get().evmVersion().hasBitwiseShifting())
@ -12599,26 +12500,17 @@ BOOST_AUTO_TEST_CASE(bitwise_shifting_constantinople)
contract C {
function shl(uint a, uint b) public returns (uint c) {
assembly {
a
b
shl
=: c
c := shl(b, a)
}
}
function shr(uint a, uint b) public returns (uint c) {
assembly {
a
b
shr
=: c
c := shr(b, a)
}
}
function sar(uint a, uint b) public returns (uint c) {
assembly {
a
b
sar
=: c
c := sar(b, a)
}
}
}
@ -12646,10 +12538,7 @@ BOOST_AUTO_TEST_CASE(bitwise_shifting_constants_constantinople)
function shl_1() public returns (bool) {
uint c;
assembly {
1
2
shl
=: c
c := shl(2, 1)
}
assert(c == 4);
return true;
@ -12657,10 +12546,7 @@ BOOST_AUTO_TEST_CASE(bitwise_shifting_constants_constantinople)
function shl_2() public returns (bool) {
uint c;
assembly {
0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
1
shl
=: c
c := shl(1, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
}
assert(c == 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe);
return true;
@ -12668,10 +12554,7 @@ BOOST_AUTO_TEST_CASE(bitwise_shifting_constants_constantinople)
function shl_3() public returns (bool) {
uint c;
assembly {
0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
256
shl
=: c
c := shl(256, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
}
assert(c == 0);
return true;
@ -12679,10 +12562,7 @@ BOOST_AUTO_TEST_CASE(bitwise_shifting_constants_constantinople)
function shr_1() public returns (bool) {
uint c;
assembly {
3
1
shr
=: c
c := shr(1, 3)
}
assert(c == 1);
return true;
@ -12690,10 +12570,7 @@ BOOST_AUTO_TEST_CASE(bitwise_shifting_constants_constantinople)
function shr_2() public returns (bool) {
uint c;
assembly {
0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
1
shr
=: c
c := shr(1, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
}
assert(c == 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff);
return true;
@ -12701,10 +12578,7 @@ BOOST_AUTO_TEST_CASE(bitwise_shifting_constants_constantinople)
function shr_3() public returns (bool) {
uint c;
assembly {
0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
256
shr
=: c
c := shr(256, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
}
assert(c == 0);
return true;

View File

@ -390,12 +390,10 @@ BOOST_AUTO_TEST_CASE(unsatisfied_version)
BOOST_AUTO_TEST_CASE(returndatasize_as_variable)
{
char const* text = R"(
contract c { function f() public { uint returndatasize; assembly { returndatasize }}}
contract C { function f() public pure { uint returndatasize; returndatasize; assembly { pop(returndatasize()) }}}
)";
vector<pair<Error::Type, std::string>> expectations(vector<pair<Error::Type, std::string>>{
{Error::Type::Warning, "Variable is shadowed in inline assembly by an instruction of the same name"},
{Error::Type::Warning, "The use of non-functional instructions is deprecated."},
{Error::Type::DeclarationError, "Unbalanced stack"}
{Error::Type::Warning, "Variable is shadowed in inline assembly by an instruction of the same name"}
});
if (!dev::test::Options::get().evmVersion().supportsReturndata())
expectations.emplace_back(make_pair(Error::Type::Warning, std::string("\"returndatasize\" instruction is only available for Byzantium-compatible")));
@ -405,15 +403,13 @@ BOOST_AUTO_TEST_CASE(returndatasize_as_variable)
BOOST_AUTO_TEST_CASE(create2_as_variable)
{
char const* text = R"(
contract c { function f() public { uint create2; assembly { create2(0, 0, 0, 0) } }}
contract c { function f() public { uint create2; create2; assembly { pop(create2(0, 0, 0, 0)) } }}
)";
// This needs special treatment, because the message mentions the EVM version,
// so cannot be run via isoltest.
CHECK_ALLOW_MULTI(text, (std::vector<std::pair<Error::Type, std::string>>{
{Error::Type::Warning, "Variable is shadowed in inline assembly by an instruction of the same name"},
{Error::Type::Warning, "The \"create2\" instruction is not supported by the VM version"},
{Error::Type::DeclarationError, "Unbalanced stack"},
{Error::Type::Warning, "not supposed to return values"}
}));
}

View File

@ -11,6 +11,6 @@ contract C {
}
// ----
// TypeError: (87-88): Expected 1 arguments but got 0.
// Warning: (87-90): Top-level expressions are not supposed to return values (this expression returns -1 values). Use ``pop()`` or assign them.
// SyntaxError: (87-90): Top-level expressions are not supposed to return values (this expression returns -1 values). Use ``pop()`` or assign them.
// TypeError: (108-109): Expected 1 arguments but got 2.
// Warning: (108-115): Top-level expressions are not supposed to return values (this expression returns 1 value). Use ``pop()`` or assign them.
// SyntaxError: (108-115): Top-level expressions are not supposed to return values (this expression returns 1 value). Use ``pop()`` or assign them.

View File

@ -8,6 +8,6 @@ contract C {
}
}
// ----
// Warning: (63-64): The use of labels is deprecated. Please use "if", "switch", "for" or function calls instead.
// Warning: (63-64): Jump instructions and labels are low-level EVM features that can lead to incorrect stack access. Because of that they are discouraged. Please consider using "switch", "if" or "for" statements instead.
// SyntaxError: (63-64): The use of labels is disallowed. Please use "if", "switch", "for" or function calls instead.
// SyntaxError: (63-64): Jump instructions and labels are low-level EVM features that can lead to incorrect stack access. Because of that they are discouraged. Please consider using "switch", "if" or "for" statements instead.
// TypeError: (73-74): Attempt to call label instead of function.

View File

@ -6,5 +6,5 @@ contract test {
}
}
// ----
// Warning: (73-74): Top-level expressions are not supposed to return values (this expression returns 1 value). Use ``pop()`` or assign them.
// SyntaxError: (73-74): Top-level expressions are not supposed to return values (this expression returns 1 value). Use ``pop()`` or assign them.
// DeclarationError: (59-84): Unbalanced stack at the end of a block: 1 surplus item(s).

View File

@ -6,5 +6,5 @@ contract test {
}
}
// ----
// Warning: (73-76): The use of non-functional instructions is deprecated. Please use functional notation instead.
// SyntaxError: (73-76): The use of non-functional instructions is disallowed. Please use functional notation instead.
// DeclarationError: (59-86): Unbalanced stack at the end of a block: 1 missing item(s).

View File

@ -1,11 +0,0 @@
pragma experimental "v0.5.0";
contract C {
function f() pure public {
assembly {
1
}
}
}
// ----
// SyntaxError: (105-106): Top-level expressions are not supposed to return values (this expression returns 1 value). Use ``pop()`` or assign them.
// DeclarationError: (91-116): Unbalanced stack at the end of a block: 1 surplus item(s).

View File

@ -6,5 +6,5 @@ contract C {
}
}
// ----
// Warning: (75-76): Top-level expressions are not supposed to return values (this expression returns 1 value). Use ``pop()`` or assign them.
// SyntaxError: (75-76): Top-level expressions are not supposed to return values (this expression returns 1 value). Use ``pop()`` or assign them.
// DeclarationError: (61-86): Unbalanced stack at the end of a block: 1 surplus item(s).

View File

@ -1,12 +0,0 @@
pragma experimental "v0.5.0";
contract C {
function f() view public {
assembly {
address
pop
}
}
}
// ----
// SyntaxError: (105-112): The use of non-functional instructions is deprecated. Please use functional notation instead.
// SyntaxError: (125-128): The use of non-functional instructions is deprecated. Please use functional notation instead.

View File

@ -7,5 +7,5 @@ contract C {
}
}
// ----
// Warning: (75-82): The use of non-functional instructions is deprecated. Please use functional notation instead.
// Warning: (95-98): The use of non-functional instructions is deprecated. Please use functional notation instead.
// SyntaxError: (75-82): The use of non-functional instructions is disallowed. Please use functional notation instead.
// SyntaxError: (95-98): The use of non-functional instructions is disallowed. Please use functional notation instead.

View File

@ -1,11 +0,0 @@
pragma experimental "v0.5.0";
contract C {
function f() pure public {
assembly {
label:
}
}
}
// ----
// SyntaxError: (105-110): The use of labels is deprecated. Please use "if", "switch", "for" or function calls instead.
// SyntaxError: (105-110): Jump instructions and labels are low-level EVM features that can lead to incorrect stack access. Because of that they are discouraged. Please consider using "switch", "if" or "for" statements instead.

View File

@ -6,5 +6,5 @@ contract C {
}
}
// ----
// Warning: (75-80): The use of labels is deprecated. Please use "if", "switch", "for" or function calls instead.
// Warning: (75-80): Jump instructions and labels are low-level EVM features that can lead to incorrect stack access. Because of that they are discouraged. Please consider using "switch", "if" or "for" statements instead.
// SyntaxError: (75-80): The use of labels is disallowed. Please use "if", "switch", "for" or function calls instead.
// SyntaxError: (75-80): Jump instructions and labels are low-level EVM features that can lead to incorrect stack access. Because of that they are discouraged. Please consider using "switch", "if" or "for" statements instead.

View File

@ -6,5 +6,4 @@ contract C {
}
}
// ----
// Warning: (75-82): Jump instructions and labels are low-level EVM features that can lead to incorrect stack access. Because of that they are discouraged. Please consider using "switch", "if" or "for" statements instead.
// TypeError: (75-82): Function declared as pure, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.
// SyntaxError: (75-82): Jump instructions and labels are low-level EVM features that can lead to incorrect stack access. Because of that they are discouraged. Please consider using "switch", "if" or "for" statements instead.

View File

@ -6,5 +6,5 @@ contract C {
}
}
// ----
// Warning: (75-83): Top-level expressions are not supposed to return values (this expression returns 1 value). Use ``pop()`` or assign them.
// SyntaxError: (75-83): Top-level expressions are not supposed to return values (this expression returns 1 value). Use ``pop()`` or assign them.
// DeclarationError: (61-93): Unbalanced stack at the end of a block: 1 surplus item(s).

View File

@ -1,7 +0,0 @@
contract C {
function k() public {
assembly { jump(2) }
}
}
// ----
// Warning: (58-65): Jump instructions and labels are low-level EVM features that can lead to incorrect stack access. Because of that they are discouraged. Please consider using "switch", "if" or "for" statements instead.

View File

@ -1,8 +0,0 @@
contract C {
function k() public view {
assembly { jump(2) }
}
}
// ----
// Warning: (63-70): Jump instructions and labels are low-level EVM features that can lead to incorrect stack access. Because of that they are discouraged. Please consider using "switch", "if" or "for" statements instead.
// TypeError: (63-70): Function declared as view, but this expression (potentially) modifies the state and thus requires non-payable (the default) or payable.