Merge remote-tracking branch 'origin/develop' into HEAD

Changelog.md

@@ -27,6 +27,9 @@ Compiler Features:
 
 
 Bugfixes:
+ * Type Checker: Disallow constructor of the same class to be used as modifier
+ * Code Generator: Fixed a faulty assert that would wrongly trigger for array sizes exceeding unsigned integer
+ * Type Checker: Treat magic variables as unknown identifiers in inline assembly
 
 
 ### 0.5.13 (unreleased)

docs/050-breaking-changes.rst

@@ -456,7 +456,7 @@ New version:
             // Throw is now disallowed.
             require(x > 100);
             int y = -3 >> 1;
-         // y == -2 (correct)
+            require(y == -2);
             do {
                 x += 1;
                 if (x > 10) continue;
@@ -473,7 +473,7 @@ New version:
 
         using address_make_payable for address;
         // Data location for 'arr' must be specified
-      function g(uint[] memory arr, bytes8 x, OtherContract otherContract, address unknownContract) public payable {
+        function g(uint[] memory /* arr */, bytes8 x, OtherContract otherContract, address unknownContract) public payable {
             // 'otherContract.transfer' is not provided.
             // Since the code of 'OtherContract' is known and has the fallback
             // function, address(otherContract) has type 'address payable'.

docs/contracts/visibility-and-getters.rst

@@ -158,13 +158,13 @@ to write a function, for example:
 
         // Getter function generated by the compiler
         /*
-    function myArray(uint i) returns (uint) {
+        function myArray(uint i) public view returns (uint) {
             return myArray[i];
         }
         */
 
         // function that returns entire array
-    function getArray() returns (uint[] memory) {
+        function getArray() public view returns (uint[] memory) {
             return myArray;
         }
     }

docs/security-considerations.rst

@@ -554,7 +554,7 @@ types.
 
     pragma solidity >=0.5.0;
     pragma experimental SMTChecker;
-   // This will not compile
+    // This will report a warning
     contract Aliasing
     {
         uint[] array;

libsolidity/analysis/TypeChecker.cpp

@@ -388,11 +388,13 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
 	set<Declaration const*> modifiers;
 	for (ASTPointer<ModifierInvocation> const& modifier: _function.modifiers())
 	{
+		auto baseContracts = dynamic_cast<ContractDefinition const&>(*_function.scope()).annotation().linearizedBaseContracts;
+		// Delete first base which is just the main contract itself
+		baseContracts.erase(baseContracts.begin());
+
 		visitManually(
 			*modifier,
-			_function.isConstructor() ?
+			_function.isConstructor() ? baseContracts : vector<ContractDefinition const*>()
-			dynamic_cast<ContractDefinition const&>(*_function.scope()).annotation().linearizedBaseContracts :
-			vector<ContractDefinition const*>()
 		);
 		Declaration const* decl = &dereference(*modifier->name());
 		if (modifiers.count(decl))
@@ -520,7 +522,12 @@ void TypeChecker::visitManually(
 		_modifier.arguments() ? *_modifier.arguments() : std::vector<ASTPointer<Expression>>();
 	for (ASTPointer<Expression> const& argument: arguments)
 		argument->accept(*this);
+
+	{
+		m_insideModifierInvocation = true;
+		ScopeGuard resetFlag{[&] () { m_insideModifierInvocation = false; }};
 		_modifier.name()->accept(*this);
+	}
 
 	auto const* declaration = &dereference(*_modifier.name());
 	vector<ASTPointer<VariableDeclaration>> emptyParameterList;
@@ -693,6 +700,9 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
 		}
 		else if (_context == yul::IdentifierContext::LValue)
 		{
+			if (dynamic_cast<MagicVariableDeclaration const*>(declaration))
+				return size_t(-1);
+
 			m_errorReporter.typeError(_identifier.location, "Only local variables can be assigned to in inline assembly.");
 			return size_t(-1);
 		}
@@ -2595,12 +2605,21 @@ bool TypeChecker::visit(Identifier const& _identifier)
 		if (_identifier.name() == "sha3" && fType->kind() == FunctionType::Kind::KECCAK256)
 			m_errorReporter.typeError(
 				_identifier.location(),
-				"\"sha3\" has been deprecated in favour of \"keccak256\""
+				"\"sha3\" has been deprecated in favour of \"keccak256\"."
 			);
 		else if (_identifier.name() == "suicide" && fType->kind() == FunctionType::Kind::Selfdestruct)
 			m_errorReporter.typeError(
 				_identifier.location(),
-				"\"suicide\" has been deprecated in favour of \"selfdestruct\""
+				"\"suicide\" has been deprecated in favour of \"selfdestruct\"."
+			);
+	}
+
+	if (!m_insideModifierInvocation)
+		if (ModifierType const* type = dynamic_cast<decltype(type)>(_identifier.annotation().type))
+		{
+			m_errorReporter.typeError(
+				_identifier.location(),
+				"Modifier can only be referenced in function headers."
 			);
 		}
 

libsolidity/analysis/TypeChecker.h

@@ -168,6 +168,9 @@ private:
 	/// Flag indicating whether we are currently inside a StructDefinition.
 	bool m_insideStruct = false;
 
+	/// Flag indicating whether we are currently inside the invocation of a modifier
+	bool m_insideModifierInvocation = false;
+
 	langutil::ErrorReporter& m_errorReporter;
 };
 

libsolidity/ast/Types.cpp

@@ -1861,7 +1861,7 @@ u256 ArrayType::memoryDataSize() const
 	solAssert(m_location == DataLocation::Memory, "");
 	solAssert(!isByteArray(), "");
 	bigint size = bigint(m_length) * m_baseType->memoryHeadSize();
-	solAssert(size <= numeric_limits<unsigned>::max(), "Array size does not fit u256.");
+	solAssert(size <= numeric_limits<u256>::max(), "Array size does not fit u256.");
 	return u256(size);
 }
 

libyul/backends/wasm/EWasmAST.h

@@ -33,7 +33,6 @@ struct Literal;
 struct StringLiteral;
 struct LocalVariable;
 struct GlobalVariable;
-struct Label;
 struct FunctionCall;
 struct BuiltinCall;
 struct LocalAssignment;
@@ -42,11 +41,11 @@ struct Block;
 struct If;
 struct Loop;
 struct Break;
-struct Continue;
+struct BreakIf;
 using Expression = boost::variant<
-	Literal, StringLiteral, LocalVariable, GlobalVariable, Label,
+	Literal, StringLiteral, LocalVariable, GlobalVariable,
 	FunctionCall, BuiltinCall, LocalAssignment, GlobalAssignment,
-	Block, If, Loop, Break, Continue
+	Block, If, Loop, Break, BreakIf
 >;
 
 struct Literal { uint64_t value; };
@@ -66,7 +65,7 @@ struct If {
 };
 struct Loop { std::string labelName; std::vector<Expression> statements; };
 struct Break { Label label; };
-struct Continue { Label label; };
+struct BreakIf { Label label; std::unique_ptr<Expression> condition; };
 
 struct VariableDeclaration { std::string variableName; };
 struct GlobalVariableDeclaration { std::string variableName; };

libyul/backends/wasm/EWasmCodeTransform.cpp

@@ -248,8 +248,7 @@ wasm::Expression EWasmCodeTransform::operator()(ForLoop const& _for)
 
 	wasm::Loop loop;
 	loop.statements = visit(_for.pre.statements);
-	loop.statements.emplace_back(wasm::BuiltinCall{"br_if", make_vector<wasm::Expression>(
+	loop.statements.emplace_back(wasm::BreakIf{wasm::Label{breakLabel}, make_unique<wasm::Expression>(
-		wasm::Label{breakLabel},
 		wasm::BuiltinCall{"i64.eqz", make_vector<wasm::Expression>(
 			visitReturnByValue(*_for.condition)
 		)}
@@ -267,7 +266,7 @@ wasm::Expression EWasmCodeTransform::operator()(Break const&)
 
 wasm::Expression EWasmCodeTransform::operator()(Continue const&)
 {
-	return wasm::Continue{wasm::Label{m_breakContinueLabelNames.top().second}};
+	return wasm::Break{wasm::Label{m_breakContinueLabelNames.top().second}};
 }
 
 wasm::Expression EWasmCodeTransform::operator()(Block const& _block)

libyul/backends/wasm/EWasmToText.cpp

@@ -82,11 +82,6 @@ string EWasmToText::operator()(wasm::GlobalVariable const& _identifier)
 	return "(get_global $" + _identifier.name + ")";
 }
 
-string EWasmToText::operator()(wasm::Label const& _label)
-{
-	return "$" + _label.name;
-}
-
 string EWasmToText::operator()(wasm::BuiltinCall const& _builtinCall)
 {
 	string args = joinTransformed(_builtinCall.arguments);
@@ -128,9 +123,9 @@ string EWasmToText::operator()(wasm::Break const& _break)
 	return "(break $" + _break.label.name + ")\n";
 }
 
-string EWasmToText::operator()(wasm::Continue const& _continue)
+string EWasmToText::operator()(wasm::BreakIf const& _break)
 {
-	return "(continue $" + _continue.label.name + ")\n";
+	return "(br_if $" + _break.label.name + " " + visit(*_break.condition) + ")\n";
 }
 
 string EWasmToText::operator()(wasm::Block const& _block)

libyul/backends/wasm/EWasmToText.h

@@ -42,7 +42,6 @@ public:
 	std::string operator()(wasm::StringLiteral const& _literal);
 	std::string operator()(wasm::LocalVariable const& _identifier);
 	std::string operator()(wasm::GlobalVariable const& _identifier);
-	std::string operator()(wasm::Label const& _label);
 	std::string operator()(wasm::BuiltinCall const& _builinCall);
 	std::string operator()(wasm::FunctionCall const& _functionCall);
 	std::string operator()(wasm::LocalAssignment const& _assignment);
@@ -50,7 +49,7 @@ public:
 	std::string operator()(wasm::If const& _if);
 	std::string operator()(wasm::Loop const& _loop);
 	std::string operator()(wasm::Break const& _break);
-	std::string operator()(wasm::Continue const& _continue);
+	std::string operator()(wasm::BreakIf const& _break);
 	std::string operator()(wasm::Block const& _block);
 
 private:

scripts/isolate_tests.py

@@ -39,6 +39,7 @@ def extract_test_cases(path):
 # and abort a line not indented properly.
 def extract_docs_cases(path):
     inside = False
+    extractedLines = []
     tests = []
 
     # Collect all snippets of indented blocks
@@ -46,15 +47,23 @@ def extract_docs_cases(path):
         if l != '':
             if not inside and l.startswith(' '):
                 # start new test
-                tests += ['']
+                extractedLines += ['']
             inside = l.startswith(' ')
         if inside:
-            tests[-1] += l + '\n'
+            extractedLines[-1] += l + '\n'
-    # Filter all tests that do not contain Solidity
+
-    return [
+    codeStart = "(pragma solidity|contract.*{|library.*{|interface.*{)"
-        test for test in tests
+
-        if re.search(r'^    [ ]*(pragma solidity|contract |library |interface )', test, re.MULTILINE)
+    # Filter all tests that do not contain Solidity or are intended incorrectly.
-    ]
+    for lines in extractedLines:
+        if re.search(r'^\s{0,3}' + codeStart, lines, re.MULTILINE):
+            print("Intendation error in " + path + ":")
+            print(lines)
+            exit(1)
+        if re.search(r'^\s{4}' + codeStart, lines, re.MULTILINE):
+            tests.append(lines)
+
+    return tests
 
 def write_cases(f, tests):
     cleaned_filename = f.replace(".","_").replace("-","_").replace(" ","_").lower()

test/externalTests/common.sh

@@ -90,15 +90,14 @@ function truffle_setup
 
     setup_solcjs "$DIR" "$SOLJSON" "v0.5.0" "solc"
     download_project "$repo" "$branch" "$DIR"
-
-    replace_version_pragmas
 }
 
 function replace_version_pragmas
 {
-    # Replace fixed-version pragmas in Gnosis (part of Consensys best practice)
+    # Replace fixed-version pragmas (part of Consensys best practice).
+    # Include all directories to also cover node dependencies.
     printLog "Replacing fixed-version pragmas..."
-    find contracts test -name '*.sol' -type f -print0 | xargs -0 sed -i -e 's/pragma solidity [\^0-9\.]*/pragma solidity >=0.0/'
+    find . test -name '*.sol' -type f -print0 | xargs -0 sed -i -e 's/pragma solidity [\^0-9\.]*/pragma solidity >=0.0/'
 }
 
 function replace_libsolc_call
@@ -211,6 +210,8 @@ function run_test
     local compile_fn="$1"
     local test_fn="$2"
 
+    replace_version_pragmas
+
     printLog "Running compile function..."
     $compile_fn
 

test/libsolidity/syntaxTests/array/length/not_too_large.sol

@@ -0,0 +1,10 @@
+// Used to cause ICE because of a too strict assert
+pragma experimental ABIEncoderV2;
+contract C {
+    struct S { uint a; T[222222222222222222222222222] sub; }
+    struct T { uint[] x; }
+    function f() public returns (uint, S memory) {
+    }
+}
+// ----
+// Warning: (52-85): Experimental features are turned on. Do not use experimental features on live deployments.

test/libsolidity/syntaxTests/deprecated_functions.sol

@@ -8,5 +8,5 @@ contract test {
 	}
 }
 // ----
-// TypeError: (58-62): "sha3" has been deprecated in favour of "keccak256"
+// TypeError: (58-62): "sha3" has been deprecated in favour of "keccak256".
-// TypeError: (101-108): "suicide" has been deprecated in favour of "selfdestruct"
+// TypeError: (101-108): "suicide" has been deprecated in favour of "selfdestruct".

test/libsolidity/syntaxTests/globalFunctions/sha3_no_call.sol

@@ -5,4 +5,4 @@ contract C
 	}
 }
 // ----
-// TypeError: (60-64): "sha3" has been deprecated in favour of "keccak256"
+// TypeError: (60-64): "sha3" has been deprecated in favour of "keccak256".

test/libsolidity/syntaxTests/globalFunctions/suicide_no_call.sol

@@ -5,4 +5,4 @@ contract C
 	}
 }
 // ----
-// TypeError: (60-67): "suicide" has been deprecated in favour of "selfdestruct"
+// TypeError: (60-67): "suicide" has been deprecated in favour of "selfdestruct".

test/libsolidity/syntaxTests/inlineAssembly/assignment_to_special.sol

@@ -2,12 +2,18 @@ contract C {
   function f() public {
     assembly {
       super := 1
+      this := 1
+      msg := 1
+      block := 1
       f := 1
       C := 1
     }
   }
 }
 // ----
-// TypeError: (58-63): Only local variables can be assigned to in inline assembly.
+// DeclarationError: (58-63): Variable not found or variable not lvalue.
-// TypeError: (75-76): Only local variables can be assigned to in inline assembly.
+// DeclarationError: (75-79): Variable not found or variable not lvalue.
-// TypeError: (88-89): Only local variables can be assigned to in inline assembly.
+// DeclarationError: (91-94): Variable not found or variable not lvalue.
+// DeclarationError: (106-111): Variable not found or variable not lvalue.
+// TypeError: (123-124): Only local variables can be assigned to in inline assembly.
+// TypeError: (136-137): Only local variables can be assigned to in inline assembly.

test/libsolidity/syntaxTests/modifiers/constructor_as_modifier.sol

@@ -0,0 +1,5 @@
+contract C {
+    constructor() C() public {}
+}
+// ----
+// TypeError: (31-34): Referenced declaration is neither modifier nor base class.

test/libsolidity/syntaxTests/modifiers/use_in_invalid_context.sol

@@ -0,0 +1,9 @@
+contract test {
+    modifier mod() { _; }
+
+    function f() public {
+        mod  ;
+    }
+}
+// ----
+// TypeError: (77-80): Modifier can only be referenced in function headers.