Merge pull request #1606 from ethereum/inlineasm-instr-shadowing

Avoid shadowing of inline assembly instructions with variables

5 files changed, 33 insertions, 4 deletions

diff --git a/Changelog.md b/Changelog.md
index 7dd607d8..e50cb3bd 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -10,6 +10,7 @@ Features:
 
 Bugfixes:
  * Code generator: Allow recursive structs.
+ * Inline assembly: Disallow variables named like opcodes.
  * Type checker: Allow multiple events of the same name (but with different arities or argument types)
 
 ### 0.4.8 (2017-01-13)
diff --git a/libsolidity/inlineasm/AsmParser.cpp b/libsolidity/inlineasm/AsmParser.cpp
index ef3da255..fcc92dbb 100644
--- a/libsolidity/inlineasm/AsmParser.cpp
+++ b/libsolidity/inlineasm/AsmParser.cpp
@@ -71,6 +71,8 @@ assembly::Statement Parser::parseStatement()
 		expectToken(Token::Colon);
 		assignment.variableName.location = location();
 		assignment.variableName.name = m_scanner->currentLiteral();
+		if (instructions().count(assignment.variableName.name))
+			fatalParserError("Identifier expected, got instruction name.");
 		assignment.location.end = endPosition();
 		expectToken(Token::Identifier);
 		return assignment;
@@ -101,6 +103,8 @@ assembly::Statement Parser::parseStatement()
 		{
 			// functional assignment
 			FunctionalAssignment funAss = createWithLocation<FunctionalAssignment>(identifier.location);
+			if (instructions().count(identifier.name))
+				fatalParserError("Cannot use instruction names for identifier names.");
 			m_scanner->next();
 			funAss.variableName = identifier;
 			funAss.value.reset(new Statement(parseExpression()));
@@ -130,7 +134,7 @@ assembly::Statement Parser::parseExpression()
 		return operation;
 }
 
-assembly::Statement Parser::parseElementaryOperation(bool _onlySinglePusher)
+std::map<string, dev::solidity::Instruction> const& Parser::instructions()
 {
 	// Allowed instructions, lowercase names.
 	static map<string, dev::solidity::Instruction> s_instructions;
@@ -151,7 +155,11 @@ assembly::Statement Parser::parseElementaryOperation(bool _onlySinglePusher)
 		// add alias for selfdestruct
 		s_instructions["selfdestruct"] = solidity::Instruction::SUICIDE;
 	}
+	return s_instructions;
+}
 
+assembly::Statement Parser::parseElementaryOperation(bool _onlySinglePusher)
+{
 	Statement ret;
 	switch (m_scanner->currentToken())
 	{
@@ -170,9 +178,9 @@ assembly::Statement Parser::parseElementaryOperation(bool _onlySinglePusher)
 		else
 			literal = m_scanner->currentLiteral();
 		// first search the set of instructions.
-		if (s_instructions.count(literal))
+		if (instructions().count(literal))
 		{
-			dev::solidity::Instruction const& instr = s_instructions[literal];
+			dev::solidity::Instruction const& instr = instructions().at(literal);
 			if (_onlySinglePusher)
 			{
 				InstructionInfo info = dev::solidity::instructionInfo(instr);
@@ -207,6 +215,8 @@ assembly::VariableDeclaration Parser::parseVariableDeclaration()
 	VariableDeclaration varDecl = createWithLocation<VariableDeclaration>();
 	expectToken(Token::Let);
 	varDecl.name = m_scanner->currentLiteral();
+	if (instructions().count(varDecl.name))
+		fatalParserError("Cannot use instruction names for identifier names.");
 	expectToken(Token::Identifier);
 	expectToken(Token::Colon);
 	expectToken(Token::Assign);
diff --git a/libsolidity/inlineasm/AsmParser.h b/libsolidity/inlineasm/AsmParser.h
index 8b56ab90..643548dd 100644
--- a/libsolidity/inlineasm/AsmParser.h
+++ b/libsolidity/inlineasm/AsmParser.h
@@ -64,6 +64,7 @@ protected:
 	Statement parseStatement();
 	/// Parses a functional expression that has to push exactly one stack element
 	Statement parseExpression();
+	std::map<std::string, dev::solidity::Instruction> const& instructions();
 	Statement parseElementaryOperation(bool _onlySinglePusher = false);
 	VariableDeclaration parseVariableDeclaration();
 	FunctionalInstruction parseFunctionalInstruction(Statement&& _instruction);
diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp
index 64073edc..c051a982 100644
--- a/test/libsolidity/InlineAssembly.cpp
+++ b/test/libsolidity/InlineAssembly.cpp
@@ -182,6 +182,24 @@ BOOST_AUTO_TEST_CASE(error_tag)
 	BOOST_CHECK(successAssemble("{ invalidJumpLabel }"));
 }
 
+BOOST_AUTO_TEST_CASE(inline_assembly_shadowed_instruction_declaration)
+{
+	// Error message: "Cannot use instruction names for identifier names."
+	BOOST_CHECK(!successAssemble("{ let gas := 1 }"));
+}
+
+BOOST_AUTO_TEST_CASE(inline_assembly_shadowed_instruction_assignment)
+{
+	// Error message: "Identifier expected, got instruction name."
+	BOOST_CHECK(!successAssemble("{ 2 =: gas }"));
+}
+
+BOOST_AUTO_TEST_CASE(inline_assembly_shadowed_instruction_functional_assignment)
+{
+	// Error message: "Cannot use instruction names for identifier names."
+	BOOST_CHECK(!successAssemble("{ gas := 2 }"));
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }
diff --git a/test/libsolidity/SolidityParser.cpp b/test/libsolidity/SolidityParser.cpp
index a3bfab75..e5362e78 100644
--- a/test/libsolidity/SolidityParser.cpp
+++ b/test/libsolidity/SolidityParser.cpp
@@ -1479,7 +1479,6 @@ BOOST_AUTO_TEST_CASE(function_type_state_variable)
 	BOOST_CHECK(successParse(text));
 }
 
-
 BOOST_AUTO_TEST_SUITE_END()
 
 }