Only allow access to local variables and only if they have a stack size of one.

3 files changed, 53 insertions, 63 deletions

diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 2d6a782a..f26c0a39 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -25,6 +25,7 @@
 #include <boost/range/adaptor/reversed.hpp>
 #include <libsolidity/ast/AST.h>
 #include <libsolidity/inlineasm/AsmAnalysis.h>
+#include <libsolidity/inlineasm/AsmData.h>
 
 using namespace std;
 using namespace dev;
@@ -637,50 +638,59 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
 		auto ref = _inlineAssembly.annotation().externalReferences.find(&_identifier);
 		if (ref == _inlineAssembly.annotation().externalReferences.end())
 			return size_t(-1);
-		size_t valueSize = size_t(-1);
 		Declaration const* declaration = ref->second.declaration;
 		solAssert(!!declaration, "");
+		if (auto var = dynamic_cast<VariableDeclaration const*>(declaration))
+		{
+			if (!var->isLocalVariable())
+			{
+				typeError(_identifier.location, "Only local variables are supported.");
+				return size_t(-1);
+			}
+			if (var->type()->dataStoredIn(DataLocation::Storage))
+			{
+				typeError(_identifier.location, "Storage reference variables are not supported.");
+				return size_t(-1);
+			}
+			if (var->type()->sizeOnStack() != 1)
+			{
+				typeError(_identifier.location, "Only types that use one stack slot are supported.");
+				return size_t(-1);
+			}
+			if (var->isConstant())
+			{
+				typeError(_identifier.location, "Constant variables not supported by inline assembly.");
+				return size_t(-1);
+			}
+		}
+		else if (_context == assembly::IdentifierContext::LValue)
+		{
+			typeError(_identifier.location, "Only local variables can be assigned to in inline assembly.");
+			return size_t(-1);
+		}
+
 		if (_context == assembly::IdentifierContext::RValue)
 		{
 			solAssert(!!declaration->type(), "Type of declaration required but not yet determined.");
 			if (dynamic_cast<FunctionDefinition const*>(declaration))
-				valueSize = 1;
-			else if (auto var = dynamic_cast<VariableDeclaration const*>(declaration))
 			{
-				if (var->isConstant())
-					fatalTypeError(SourceLocation(), "Constant variables not yet implemented for inline assembly.");
-				if (var->isLocalVariable())
-					valueSize = var->type()->sizeOnStack();
-				else if (!var->type()->isValueType())
-					valueSize = 1;
-				else
-					// We cannot use `sizeOnStack()` here because we do not insert the value
-					// into inline assembly but rather the storage location.
-					valueSize = 2; // slot number, intra slot offset
+			}
+			else if (dynamic_cast<VariableDeclaration const*>(declaration))
+			{
 			}
 			else if (auto contract = dynamic_cast<ContractDefinition const*>(declaration))
 			{
 				if (!contract->isLibrary())
+				{
+					typeError(_identifier.location, "Expected a library.");
 					return size_t(-1);
-				valueSize = 1;
-			}
-			else
-				return size_t(-1);
-		}
-		else
-		{
-			// lvalue context
-			if (auto varDecl = dynamic_cast<VariableDeclaration const*>(declaration))
-			{
-				if (!varDecl->isLocalVariable())
-					return size_t(-1); // only local variables are inline-assembly lvalues
-				valueSize = size_t(declaration->type()->sizeOnStack());
+				}
 			}
 			else
 				return size_t(-1);
 		}
-		ref->second.valueSize = valueSize;
-		return valueSize;
+		ref->second.valueSize = 1;
+		return size_t(1);
 	};
 	assembly::AsmAnalyzer::Scopes scopes;
 	assembly::AsmAnalyzer analyzer(scopes, m_errors, identifierAccess);
diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp
index de53f182..9bef8a8f 100644
--- a/libsolidity/codegen/ContractCompiler.cpp
+++ b/libsolidity/codegen/ContractCompiler.cpp
@@ -557,33 +557,16 @@ bool ContractCompiler::visit(InlineAssembly const& _inlineAssembly)
 			else if (auto variable = dynamic_cast<VariableDeclaration const*>(decl))
 			{
 				solAssert(!variable->isConstant(), "");
-				if (m_context.isLocalVariable(variable))
-				{
-					int stackDiff = _assembly.deposit() - m_context.baseStackOffsetOfVariable(*variable);
-					if (stackDiff < 1 || stackDiff > 16)
-						BOOST_THROW_EXCEPTION(
-							CompilerError() <<
-							errinfo_sourceLocation(_inlineAssembly.location()) <<
-							errinfo_comment("Stack too deep, try removing local variables.")
-						);
-					for (unsigned i = 0; i < variable->type()->sizeOnStack(); ++i)
-						_assembly.append(dupInstruction(stackDiff));
-				}
-				else
-				{
-					solAssert(m_context.isStateVariable(variable), "Invalid variable type.");
-					auto const& location = m_context.storageLocationOfVariable(*variable);
-					if (!variable->type()->isValueType())
-					{
-						solAssert(location.second == 0, "Intra-slot offest assumed to be zero.");
-						_assembly.append(location.first);
-					}
-					else
-					{
-						_assembly.append(location.first);
-						_assembly.append(u256(location.second));
-					}
-				}
+				solAssert(m_context.isLocalVariable(variable), "");
+				int stackDiff = _assembly.deposit() - m_context.baseStackOffsetOfVariable(*variable);
+				if (stackDiff < 1 || stackDiff > 16)
+					BOOST_THROW_EXCEPTION(
+						CompilerError() <<
+						errinfo_sourceLocation(_inlineAssembly.location()) <<
+						errinfo_comment("Stack too deep, try removing local variables.")
+					);
+				solAssert(variable->type()->sizeOnStack() == 1, "");
+				_assembly.append(dupInstruction(stackDiff));
 			}
 			else if (auto contract = dynamic_cast<ContractDefinition const*>(decl))
 			{
@@ -602,19 +585,16 @@ bool ContractCompiler::visit(InlineAssembly const& _inlineAssembly)
 				!!variable && m_context.isLocalVariable(variable),
 				"Can only assign to stack variables in inline assembly."
 			);
-			unsigned size = variable->type()->sizeOnStack();
-			int stackDiff = _assembly.deposit() - m_context.baseStackOffsetOfVariable(*variable) - size;
+			solAssert(variable->type()->sizeOnStack() == 1, "");
+			int stackDiff = _assembly.deposit() - m_context.baseStackOffsetOfVariable(*variable) - 1;
 			if (stackDiff > 16 || stackDiff < 1)
 				BOOST_THROW_EXCEPTION(
 					CompilerError() <<
 					errinfo_sourceLocation(_inlineAssembly.location()) <<
 					errinfo_comment("Stack too deep, try removing local variables.")
 				);
-			for (unsigned i = 0; i < size; ++i)
-			{
-				_assembly.append(swapInstruction(stackDiff));
-				_assembly.append(Instruction::POP);
-			}
+			_assembly.append(swapInstruction(stackDiff));
+			_assembly.append(Instruction::POP);
 		}
 	};
 	codeGen.assemble(
diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp
index 71416a07..9ff15fd0 100644
--- a/libsolidity/inlineasm/AsmAnalysis.cpp
+++ b/libsolidity/inlineasm/AsmAnalysis.cpp
@@ -141,7 +141,7 @@ bool AsmAnalyzer::operator()(FunctionalInstruction const& _instr)
 			success = false;
 	}
 	// Parser already checks that the number of arguments is correct.
-	solAssert(instructionInfo(_instr.instruction).args == _instr.arguments.size(), "");
+	solAssert(instructionInfo(_instr.instruction.instruction).args == int(_instr.arguments.size()), "");
 	if (!(*this)(_instr.instruction))
 		success = false;
 	return success;