Merge pull request #2460 from ethereum/disallowMultiModifier

Disallow invoking the same modifier multiple times.

5 files changed, 41 insertions, 0 deletions

diff --git a/Changelog.md b/Changelog.md
index 3f4d55cd..d2c9fefb 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -17,6 +17,7 @@ Features:
 Bugfixes:
  * Code generator: Use ``REVERT`` instead of ``INVALID`` for generated input validation routines.
  * Type Checker: Fix address literals not being treated as compile-time constants.
+ * Type Checker: Disallow invoking the same modifier multiple times.
  * Type Checker: Make UTF8-validation a bit more sloppy to include more valid sequences.
  * Fixed crash concerning non-callable types.
  * Unused variable warnings no longer issued for variables used inside inline assembly.
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 615bc402..40ff59f6 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -467,13 +467,26 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
 
 		var->accept(*this);
 	}
+	set<Declaration const*> modifiers;
 	for (ASTPointer<ModifierInvocation> const& modifier: _function.modifiers())
+	{
 		visitManually(
 			*modifier,
 			_function.isConstructor() ?
 			dynamic_cast<ContractDefinition const&>(*_function.scope()).annotation().linearizedBaseContracts :
 			vector<ContractDefinition const*>()
 		);
+		Declaration const* decl = &dereference(*modifier->name());
+		if (modifiers.count(decl))
+		{
+			if (dynamic_cast<ContractDefinition const*>(decl))
+				m_errorReporter.declarationError(modifier->location(), "Base constructor already provided.");
+			else
+				m_errorReporter.declarationError(modifier->location(), "Modifier already used for this function.");
+		}
+		else
+			modifiers.insert(decl);
+	}
 	if (m_scope->contractKind() == ContractDefinition::ContractKind::Interface)
 	{
 		if (_function.isImplemented())
diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp
index 1937b529..6875bda1 100644
--- a/libsolidity/codegen/CompilerContext.cpp
+++ b/libsolidity/codegen/CompilerContext.cpp
@@ -124,6 +124,7 @@ void CompilerContext::addVariable(VariableDeclaration const& _declaration,
 								  unsigned _offsetToCurrent)
 {
 	solAssert(m_asm->deposit() >= 0 && unsigned(m_asm->deposit()) >= _offsetToCurrent, "");
+	solAssert(m_localVariables.count(&_declaration) == 0, "Variable already present");
 	m_localVariables[&_declaration] = unsigned(m_asm->deposit()) - _offsetToCurrent;
 }
 
diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp
index ace82ad4..977a2c7c 100644
--- a/libsolidity/codegen/ContractCompiler.cpp
+++ b/libsolidity/codegen/ContractCompiler.cpp
@@ -878,6 +878,7 @@ void ContractCompiler::appendModifierOrFunctionCode()
 	solAssert(m_currentFunction, "");
 	unsigned stackSurplus = 0;
 	Block const* codeBlock = nullptr;
+	vector<VariableDeclaration const*> addedVariables;
 
 	m_modifierDepth++;
 
@@ -901,6 +902,7 @@ void ContractCompiler::appendModifierOrFunctionCode()
 			for (unsigned i = 0; i < modifier.parameters().size(); ++i)
 			{
 				m_context.addVariable(*modifier.parameters()[i]);
+				addedVariables.push_back(modifier.parameters()[i].get());
 				compileExpression(
 					*modifierInvocation->arguments()[i],
 					modifier.parameters()[i]->annotation().type
@@ -927,6 +929,8 @@ void ContractCompiler::appendModifierOrFunctionCode()
 		m_returnTags.pop_back();
 
 		CompilerUtils(m_context).popStackSlots(stackSurplus);
+		for (auto var: addedVariables)
+			m_context.removeVariable(*var);
 	}
 	m_modifierDepth--;
 }
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index e18303f5..1088b3d5 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -1049,6 +1049,28 @@ BOOST_AUTO_TEST_CASE(function_modifier_invocation_local_variables)
 	CHECK_SUCCESS(text);
 }
 
+BOOST_AUTO_TEST_CASE(function_modifier_double_invocation)
+{
+	char const* text = R"(
+		contract B {
+			function f(uint x) mod(x) mod(2) { }
+			modifier mod(uint a) { if (a > 0) _; }
+		}
+	)";
+	CHECK_ERROR(text, DeclarationError, "Modifier already used for this function");
+}
+
+BOOST_AUTO_TEST_CASE(base_constructor_double_invocation)
+{
+	char const* text = R"(
+		contract C { function C(uint a) {} }
+		contract B is C {
+			function B() C(2) C(2) {}
+		}
+	)";
+	CHECK_ERROR(text, DeclarationError, "Base constructor already provided");
+}
+
 BOOST_AUTO_TEST_CASE(legal_modifier_override)
 {
 	char const* text = R"(