From f0d256bfdbf569973f55a9e4a7739b639ffc25e2 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Fri, 3 Jul 2020 12:26:35 +0100 Subject: [PATCH 1/7] Add test case for jumpi in inline assembly --- .../inlineAssembly/invalid/jumpi_disallowed.sol | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 test/libsolidity/syntaxTests/inlineAssembly/invalid/jumpi_disallowed.sol diff --git a/test/libsolidity/syntaxTests/inlineAssembly/invalid/jumpi_disallowed.sol b/test/libsolidity/syntaxTests/inlineAssembly/invalid/jumpi_disallowed.sol new file mode 100644 index 000000000..1b4fc4a65 --- /dev/null +++ b/test/libsolidity/syntaxTests/inlineAssembly/invalid/jumpi_disallowed.sol @@ -0,0 +1,9 @@ +contract C { + function f() pure public { + assembly { + jumpi(2, 1) + } + } +} +// ---- +// DeclarationError 4619: (75-80): Function not found. From 8cdf14f1b37281e45080ef369cb06c0d7173f48d Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Mon, 6 Jul 2020 13:44:33 +0200 Subject: [PATCH 2/7] Replace "doesn't" with "does not" in error message --- libsolidity/analysis/TypeChecker.cpp | 2 +- .../282_invalid_different_types_for_conditional_expression.sol | 2 +- .../284_conditional_expression_with_different_struct.sol | 2 +- .../285_conditional_expression_with_different_function_type.sol | 2 +- .../286_conditional_expression_with_different_enum.sol | 2 +- .../287_conditional_expression_with_different_mapping.sol | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 8ac88d506..5c49bf1dc 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -1336,7 +1336,7 @@ bool TypeChecker::visit(Conditional const& _conditional) _conditional.location(), "True expression's type " + trueType->toString() + - " doesn't match false expression's type " + + " does not match false expression's type " + falseType->toString() + "." ); diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/282_invalid_different_types_for_conditional_expression.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/282_invalid_different_types_for_conditional_expression.sol index 78097742d..da2ae63ae 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/282_invalid_different_types_for_conditional_expression.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/282_invalid_different_types_for_conditional_expression.sol @@ -4,4 +4,4 @@ contract C { } } // ---- -// TypeError 1080: (47-62): True expression's type bool doesn't match false expression's type uint8. +// TypeError 1080: (47-62): True expression's type bool does not match false expression's type uint8. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/284_conditional_expression_with_different_struct.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/284_conditional_expression_with_different_struct.sol index 509e559d2..1141ff084 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/284_conditional_expression_with_different_struct.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/284_conditional_expression_with_different_struct.sol @@ -12,4 +12,4 @@ contract C { } } // ---- -// TypeError 1080: (165-177): True expression's type struct C.s1 memory doesn't match false expression's type struct C.s2 memory. +// TypeError 1080: (165-177): True expression's type struct C.s1 memory does not match false expression's type struct C.s2 memory. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/285_conditional_expression_with_different_function_type.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/285_conditional_expression_with_different_function_type.sol index 9846bd656..7d55d61ef 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/285_conditional_expression_with_different_function_type.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/285_conditional_expression_with_different_function_type.sol @@ -7,4 +7,4 @@ contract C { } } // ---- -// TypeError 1080: (106-118): True expression's type function (bool) doesn't match false expression's type function (). +// TypeError 1080: (106-118): True expression's type function (bool) does not match false expression's type function (). diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/286_conditional_expression_with_different_enum.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/286_conditional_expression_with_different_enum.sol index dd1303336..c0cb69db6 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/286_conditional_expression_with_different_enum.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/286_conditional_expression_with_different_enum.sol @@ -10,4 +10,4 @@ contract C { } } // ---- -// TypeError 1080: (139-151): True expression's type enum C.small doesn't match false expression's type enum C.big. +// TypeError 1080: (139-151): True expression's type enum C.small does not match false expression's type enum C.big. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/287_conditional_expression_with_different_mapping.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/287_conditional_expression_with_different_mapping.sol index b20c872d9..8bf1268c0 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/287_conditional_expression_with_different_mapping.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/287_conditional_expression_with_different_mapping.sol @@ -7,4 +7,4 @@ contract C { } } // ---- -// TypeError 1080: (121-143): True expression's type mapping(uint8 => uint8) doesn't match false expression's type mapping(uint32 => uint8). +// TypeError 1080: (121-143): True expression's type mapping(uint8 => uint8) does not match false expression's type mapping(uint32 => uint8). From 5e4aeaa4606d03664653ee9390afb539ee1903e5 Mon Sep 17 00:00:00 2001 From: a3d4 Date: Sat, 4 Jul 2020 00:04:09 +0200 Subject: [PATCH 3/7] Add variable name to the "Variable covers a large part of storage ...." message --- libsolidity/analysis/StaticAnalyzer.cpp | 6 ++- .../large_storage_array_mapping.sol | 2 +- .../largeTypes/large_storage_array_simple.sol | 2 +- .../large_storage_arrays_struct.sol | 2 +- .../largeTypes/large_storage_structs.sol | 46 ++++++++++--------- .../rational_number_array_index_limit.sol | 2 +- 6 files changed, 32 insertions(+), 28 deletions(-) diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index bee1197b3..b650f1fc1 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -163,7 +163,8 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable) m_errorReporter.warning( 3408_error, _variable.location(), - "Variable covers a large part of storage and thus makes collisions likely. " + "Variable " + util::escapeAndQuoteString(_variable.name()) + + " covers a large part of storage and thus makes collisions likely. " "Either use mappings or dynamic arrays and allow their size to be increased only " "in small quantities per transaction." ); @@ -171,7 +172,8 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable) m_errorReporter.warning( 7325_error, _variable.location(), - "Type " + type->canonicalName() + " has large size and thus makes collisions likely. " + "Type " + util::escapeAndQuoteString(type->canonicalName()) + + " has large size and thus makes collisions likely. " "Either use mappings or dynamic arrays and allow their size to be increased only " "in small quantities per transaction." ); diff --git a/test/libsolidity/syntaxTests/largeTypes/large_storage_array_mapping.sol b/test/libsolidity/syntaxTests/largeTypes/large_storage_array_mapping.sol index 0471307ef..01a68bcda 100644 --- a/test/libsolidity/syntaxTests/largeTypes/large_storage_array_mapping.sol +++ b/test/libsolidity/syntaxTests/largeTypes/large_storage_array_mapping.sol @@ -2,4 +2,4 @@ contract C { mapping(uint => uint[2**100]) x; } // ---- -// Warning 7325: (17-48): Type uint256[1267650600228229401496703205376] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (17-48): Type "uint256[1267650600228229401496703205376]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. diff --git a/test/libsolidity/syntaxTests/largeTypes/large_storage_array_simple.sol b/test/libsolidity/syntaxTests/largeTypes/large_storage_array_simple.sol index 5b8f306a9..3f7499ca2 100644 --- a/test/libsolidity/syntaxTests/largeTypes/large_storage_array_simple.sol +++ b/test/libsolidity/syntaxTests/largeTypes/large_storage_array_simple.sol @@ -2,4 +2,4 @@ contract C { uint[2**64] x; } // ---- -// Warning 3408: (17-30): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 3408: (17-30): Variable "x" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. diff --git a/test/libsolidity/syntaxTests/largeTypes/large_storage_arrays_struct.sol b/test/libsolidity/syntaxTests/largeTypes/large_storage_arrays_struct.sol index 02b7276d6..285b99deb 100644 --- a/test/libsolidity/syntaxTests/largeTypes/large_storage_arrays_struct.sol +++ b/test/libsolidity/syntaxTests/largeTypes/large_storage_arrays_struct.sol @@ -3,4 +3,4 @@ contract C { S[2**20] x; } // ---- -// Warning 3408: (64-74): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 3408: (64-74): Variable "x" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. diff --git a/test/libsolidity/syntaxTests/largeTypes/large_storage_structs.sol b/test/libsolidity/syntaxTests/largeTypes/large_storage_structs.sol index 17113fc7b..2956a6549 100644 --- a/test/libsolidity/syntaxTests/largeTypes/large_storage_structs.sol +++ b/test/libsolidity/syntaxTests/largeTypes/large_storage_structs.sol @@ -2,64 +2,66 @@ contract C { struct P { uint256[2**63] x; } struct S0 { - P[2**62] x; + P[101] x; P y; } S0 s0; struct S1 { P x; - P[2**62] y; + P[102] y; } S1 s1; struct S2 { - mapping(uint => P[2**62]) x; - mapping(uint => P[2**62]) y; - mapping(uint => S2) z; + mapping(uint => P[103]) x; + mapping(uint => P[103]) y; + mapping(uint => P[104]) z; + mapping(uint => S2) t; } S2 s2; struct Q0 { - uint[1][][2**65] x; - uint[2**65][][1] y; - uint[][2**65] z; - uint[2**65][] t; + uint[1][][10**20 + 4] x; + uint[10**20 + 4][][1] y; + uint[][10**20 + 4] z; + uint[10**20 + 4][] t; } Q0 q0; struct Q1 { - uint[1][][2**65] x; + uint[1][][10**20 + 5] x; } Q1 q1; struct Q2 { - uint[2**65][][1] y; + uint[10**20 + 6][][1] y; } Q2 q2; struct Q3 { - uint[][2**65] z; + uint[][10**20 + 7] x; } Q3 q3; struct Q4 { - uint[2**65][] t; + uint[10**20 + 8][] y; } Q4 q4; } // ---- -// Warning 3408: (108-113): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 3408: (175-180): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 7325: (314-319): Type C.P[4611686018427387904] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 3408: (458-463): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 7325: (458-463): Type uint256[36893488147419103232] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 3408: (524-529): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 7325: (590-595): Type uint256[36893488147419103232] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 3408: (653-658): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 7325: (716-721): Type uint256[36893488147419103232] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 3408: (106-111): Variable "s0" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 3408: (171-176): Variable "s1" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (341-346): Type "C.P[103]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (341-346): Type "C.P[104]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 3408: (505-510): Variable "q0" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (505-510): Type "uint256[100000000000000000004]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 3408: (576-581): Variable "q1" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (647-652): Type "uint256[100000000000000000006]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 3408: (715-720): Variable "q3" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (783-788): Type "uint256[100000000000000000008]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. diff --git a/test/libsolidity/syntaxTests/types/rational_number_array_index_limit.sol b/test/libsolidity/syntaxTests/types/rational_number_array_index_limit.sol index 912d8df14..9b4f05c42 100644 --- a/test/libsolidity/syntaxTests/types/rational_number_array_index_limit.sol +++ b/test/libsolidity/syntaxTests/types/rational_number_array_index_limit.sol @@ -2,4 +2,4 @@ contract c { uint[2**253] data; } // ---- -// Warning 3408: (17-34): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 3408: (17-34): Variable "data" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. From 55e41b602adbde79bc03c1ba9bea2e7f1c029201 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Fri, 3 Jul 2020 18:01:07 +0100 Subject: [PATCH 4/7] Explicitly check for jump instructions in EVMDialect --- libyul/backends/evm/EVMDialect.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libyul/backends/evm/EVMDialect.cpp b/libyul/backends/evm/EVMDialect.cpp index 602b6c8ff..bf0c63bfa 100644 --- a/libyul/backends/evm/EVMDialect.cpp +++ b/libyul/backends/evm/EVMDialect.cpp @@ -114,12 +114,15 @@ pair createFunction( map createBuiltins(langutil::EVMVersion _evmVersion, bool _objectAccess) { map builtins; + // NOTE: Parser::instructions() will filter JUMPDEST and PUSHnn too for (auto const& instr: Parser::instructions()) if ( !evmasm::isDupInstruction(instr.second) && !evmasm::isSwapInstruction(instr.second) && + !evmasm::isPushInstruction(instr.second) && instr.second != evmasm::Instruction::JUMP && instr.second != evmasm::Instruction::JUMPI && + instr.second != evmasm::Instruction::JUMPDEST && _evmVersion.hasOpcode(instr.second) ) builtins.emplace(createEVMFunction(instr.first, instr.second)); From 67ebb206eab4863d47f67ed219ee4dbaa985819f Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Mon, 6 Jul 2020 14:46:13 +0100 Subject: [PATCH 5/7] Turn Instruction::JUMP* into yulAssert --- libyul/AsmAnalysis.cpp | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/libyul/AsmAnalysis.cpp b/libyul/AsmAnalysis.cpp index c7e825759..ad697fb40 100644 --- a/libyul/AsmAnalysis.cpp +++ b/libyul/AsmAnalysis.cpp @@ -558,6 +558,13 @@ bool AsmAnalyzer::validateInstructions(evmasm::Instruction _instr, SourceLocatio // Similarly we assume bitwise shifting and create2 go together. yulAssert(m_evmVersion.hasBitwiseShifting() == m_evmVersion.hasCreate2(), ""); + // These instructions are disabled in the dialect. + yulAssert( + _instr != evmasm::Instruction::JUMP && + _instr != evmasm::Instruction::JUMPI && + _instr != evmasm::Instruction::JUMPDEST, + ""); + auto errorForVM = [&](ErrorId _errorId, string const& vmKindMessage) { m_errorReporter.typeError( _errorId, @@ -602,19 +609,6 @@ bool AsmAnalyzer::validateInstructions(evmasm::Instruction _instr, SourceLocatio ); else if (_instr == evmasm::Instruction::SELFBALANCE && !m_evmVersion.hasSelfBalance()) errorForVM(3672_error, "only available for Istanbul-compatible"); - else if ( - _instr == evmasm::Instruction::JUMP || - _instr == evmasm::Instruction::JUMPI || - _instr == evmasm::Instruction::JUMPDEST - ) - m_errorReporter.error( - 4316_error, - Error::Type::SyntaxError, - _location, - "Jump instructions and labels are low-level EVM features that can lead to " - "incorrect stack access. Because of that they are disallowed in strict assembly. " - "Use functions, \"switch\", \"if\" or \"for\" statements instead." - ); else return false; From 60d4b1e8ccb198cfab27b013df966d7ad51bcd2e Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Mon, 6 Jul 2020 15:31:25 +0100 Subject: [PATCH 6/7] Rename drop to i64.drop in WasmDialect Also properly implement support for both i32.drop and i64.drop in BinaryTransform, TextTransform, and YulInterpreter --- Changelog.md | 1 + libyul/backends/wasm/BinaryTransform.cpp | 2 +- libyul/backends/wasm/TextTransform.cpp | 5 ++++- libyul/backends/wasm/WasmDialect.cpp | 6 +++--- test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/Changelog.md b/Changelog.md index 837c0f1b9..6407cd8b1 100644 --- a/Changelog.md +++ b/Changelog.md @@ -20,6 +20,7 @@ Bugfixes: * Type Checker: Fix internal error related to ``using for`` applied to non-libraries. * Type Checker: Do not disallow assigning to calldata variables. * Wasm backend: Fix code generation for for-loops with pre statements. + * Wasm backend: Properly support both ``i32.drop`` and ``i64.drop``, and remove ``drop``. * Yul: Fix source location of variable multi-assignment. diff --git a/libyul/backends/wasm/BinaryTransform.cpp b/libyul/backends/wasm/BinaryTransform.cpp index 82de96b51..38438e12e 100644 --- a/libyul/backends/wasm/BinaryTransform.cpp +++ b/libyul/backends/wasm/BinaryTransform.cpp @@ -390,7 +390,7 @@ bytes BinaryTransform::operator()(BuiltinCall const& _call) return toBytes(Opcode::Unreachable); else if (_call.functionName == "nop") return toBytes(Opcode::Nop); - else if (_call.functionName == "drop") + else if (_call.functionName == "i32.drop" || _call.functionName == "i64.drop") return toBytes(Opcode::Drop); else { diff --git a/libyul/backends/wasm/TextTransform.cpp b/libyul/backends/wasm/TextTransform.cpp index b7f1e81ea..ee2aa86e4 100644 --- a/libyul/backends/wasm/TextTransform.cpp +++ b/libyul/backends/wasm/TextTransform.cpp @@ -94,7 +94,10 @@ string TextTransform::operator()(wasm::GlobalVariable const& _identifier) string TextTransform::operator()(wasm::BuiltinCall const& _builtinCall) { string args = joinTransformed(_builtinCall.arguments); - return "(" + _builtinCall.functionName + (args.empty() ? "" : " " + args) + ")"; + string funcName = _builtinCall.functionName; + if (funcName == "i32.drop" || funcName == "i64.drop") + funcName = "drop"; + return "(" + funcName + (args.empty() ? "" : " " + args) + ")"; } string TextTransform::operator()(wasm::FunctionCall const& _functionCall) diff --git a/libyul/backends/wasm/WasmDialect.cpp b/libyul/backends/wasm/WasmDialect.cpp index c9dfe05c7..39138dfb2 100644 --- a/libyul/backends/wasm/WasmDialect.cpp +++ b/libyul/backends/wasm/WasmDialect.cpp @@ -91,9 +91,9 @@ WasmDialect::WasmDialect() m_functions["i64.load"_yulstring].sideEffects.sideEffectFreeIfNoMSize = true; // Drop is actually overloaded for all types, but Yul does not support that. - // Because of that, we introduce "i32.drop". - addFunction("drop", {i64}, {}); + // Because of that, we introduce "i32.drop" and "i64.drop". addFunction("i32.drop", {i32}, {}); + addFunction("i64.drop", {i64}, {}); addFunction("nop", {}, {}); addFunction("unreachable", {}, {}, false); @@ -122,7 +122,7 @@ BuiltinFunction const* WasmDialect::discardFunction(YulString _type) const if (_type == "i32"_yulstring) return builtin("i32.drop"_yulstring); yulAssert(_type == "i64"_yulstring, ""); - return builtin("drop"_yulstring); + return builtin("i64.drop"_yulstring); } BuiltinFunction const* WasmDialect::equalityFunction(YulString _type) const diff --git a/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp b/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp index b523e7efe..b09fb5a87 100644 --- a/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp +++ b/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp @@ -93,7 +93,7 @@ u256 EwasmBuiltinInterpreter::evalBuiltin(YulString _fun, vector const& _a ); return 0; } - else if (_fun == "drop"_yulstring || _fun == "nop"_yulstring) + else if (_fun == "i32.drop"_yulstring || _fun == "i64.drop"_yulstring || _fun == "nop"_yulstring) return {}; else if (_fun == "i32.wrap_i64"_yulstring) return arg.at(0) & uint32_t(-1); From f372ba6feaf679012417697a32d79e17b282f9c3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 6 Jul 2020 16:52:51 +0200 Subject: [PATCH 7/7] Constructors cannot have calldata parameters. --- Changelog.md | 1 + libsolidity/ast/AST.cpp | 11 ++++++++++- libsolidity/ast/AST.h | 1 + .../constructor/calldata_constructor_args.sol | 5 +++++ 4 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 test/libsolidity/syntaxTests/constructor/calldata_constructor_args.sol diff --git a/Changelog.md b/Changelog.md index 837c0f1b9..fa4e46800 100644 --- a/Changelog.md +++ b/Changelog.md @@ -19,6 +19,7 @@ Bugfixes: * NatSpec: Do not consider ``////`` and ``/***`` as NatSpec comments. * Type Checker: Fix internal error related to ``using for`` applied to non-libraries. * Type Checker: Do not disallow assigning to calldata variables. + * Type Checker: Disallow constructor parameters with ``calldata`` data location. * Wasm backend: Fix code generation for for-loops with pre statements. * Yul: Fix source location of variable multi-assignment. diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index f3f79f451..cba5bccc3 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -588,6 +588,15 @@ bool VariableDeclaration::isInternalCallableParameter() const return false; } +bool VariableDeclaration::isConstructorParameter() const +{ + if (!isCallableOrCatchParameter()) + return false; + if (auto const* function = dynamic_cast(scope())) + return function->isConstructor(); + return false; +} + bool VariableDeclaration::isLibraryFunctionParameter() const { if (!isCallableOrCatchParameter()) @@ -622,7 +631,7 @@ set VariableDeclaration::allowedDataLocations() c set locations{ Location::Memory }; if (isInternalCallableParameter() || isLibraryFunctionParameter() || isTryCatchParameter()) locations.insert(Location::Storage); - if (!isTryCatchParameter()) + if (!isTryCatchParameter() && !isConstructorParameter()) locations.insert(Location::CallData); return locations; diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index 88c3e1beb..34fefeb0a 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -928,6 +928,7 @@ public: /// @returns true if this variable is a parameter or return parameter of an internal function /// or a function type of internal visibility. bool isInternalCallableParameter() const; + bool isConstructorParameter() const; /// @returns true iff this variable is a parameter(or return parameter of a library function bool isLibraryFunctionParameter() const; /// @returns true if the type of the variable does not need to be specified, i.e. it is declared diff --git a/test/libsolidity/syntaxTests/constructor/calldata_constructor_args.sol b/test/libsolidity/syntaxTests/constructor/calldata_constructor_args.sol new file mode 100644 index 000000000..c4ce52102 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/calldata_constructor_args.sol @@ -0,0 +1,5 @@ +contract C { + constructor(uint[] calldata) public {} +} +// ---- +// TypeError 6651: (29-44): Data location must be "memory" for parameter in function, but "calldata" was given.