Merge pull request #12620 from ethereum/assemblyAnnotation

Memory-safety annotation for inline assembly.

libsolidity/analysis/DocStringTagParser.cpp

@@ -30,6 +30,7 @@
 #include <liblangutil/Common.h>
 
 #include <range/v3/algorithm/any_of.hpp>
+#include <range/v3/view/filter.hpp>
 
 #include <boost/algorithm/string.hpp>
 
@@ -162,6 +163,71 @@ bool DocStringTagParser::visit(ErrorDefinition const& _error)
 	return true;
 }
 
+bool DocStringTagParser::visit(InlineAssembly const& _assembly)
+{
+	if (!_assembly.documentation())
+		return true;
+	StructuredDocumentation documentation{-1, _assembly.location(), _assembly.documentation()};
+	ErrorList errors;
+	ErrorReporter errorReporter{errors};
+	auto docTags = DocStringParser{documentation, errorReporter}.parse();
+
+	if (!errors.empty())
+	{
+		SecondarySourceLocation ssl;
+		for (auto const& error: errors)
+			if (error->comment())
+				ssl.append(
+					*error->comment(),
+					_assembly.location()
+				);
+		m_errorReporter.warning(
+			7828_error,
+			_assembly.location(),
+			"Inline assembly has invalid NatSpec documentation.",
+			ssl
+		);
+	}
+
+	for (auto const& [tagName, tagValue]: docTags)
+	{
+		if (tagName == "solidity")
+		{
+			vector<string> values;
+			boost::split(values, tagValue.content, isWhiteSpace);
+
+			set<string> valuesSeen;
+			set<string> duplicates;
+			for (auto const& value: values | ranges::views::filter(not_fn(&string::empty)))
+				if (valuesSeen.insert(value).second)
+				{
+					if (value == "memory-safe-assembly")
+						_assembly.annotation().markedMemorySafe = true;
+					else
+						m_errorReporter.warning(
+							8787_error,
+							_assembly.location(),
+							"Unexpected value for @solidity tag in inline assembly: " + value
+						);
+				}
+				else if (duplicates.insert(value).second)
+					m_errorReporter.warning(
+						4377_error,
+						_assembly.location(),
+						"Value for @solidity tag in inline assembly specified multiple times: " + value
+					);
+		}
+		else
+			m_errorReporter.warning(
+				6269_error,
+				_assembly.location(),
+				"Unexpected NatSpec tag \"" + tagName + "\" with value \"" + tagValue.content + "\" in inline assembly."
+			);
+	}
+
+	return true;
+}
+
 void DocStringTagParser::checkParameters(
 	CallableDeclaration const& _callable,
 	StructurallyDocumented const& _node,

libsolidity/analysis/DocStringTagParser.h

@@ -48,6 +48,7 @@ private:
 	bool visit(ModifierDefinition const& _modifier) override;
 	bool visit(EventDefinition const& _event) override;
 	bool visit(ErrorDefinition const& _error) override;
+	bool visit(InlineAssembly const& _assembly) override;
 
 	void checkParameters(
 		CallableDeclaration const& _callable,

libsolidity/analysis/TypeChecker.cpp

@@ -763,6 +763,7 @@ void TypeChecker::endVisit(FunctionTypeName const& _funType)
 
 bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
 {
+	bool lvalueAccessToMemoryVariable = false;
 	// External references have already been resolved in a prior stage and stored in the annotation.
 	// We run the resolve step again regardless.
 	yul::ExternalIdentifierAccess::Resolver identifierAccess = [&](
@@ -787,6 +788,8 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
 		if (auto var = dynamic_cast<VariableDeclaration const*>(declaration))
 		{
 			solAssert(var->type(), "Expected variable type!");
+			if (_context == yul::IdentifierContext::LValue && var->type()->dataStoredIn(DataLocation::Memory))
+				lvalueAccessToMemoryVariable = true;
 			if (var->immutable())
 			{
 				m_errorReporter.typeError(3773_error, nativeLocationOf(_identifier), "Assembly access to immutable variables is not supported.");
@@ -974,8 +977,11 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
 		identifierAccess
 	);
 	if (!analyzer.analyze(_inlineAssembly.operations()))
-		return false;
-	return true;
+		solAssert(m_errorReporter.hasErrors());
+	_inlineAssembly.annotation().hasMemoryEffects =
+		lvalueAccessToMemoryVariable ||
+		(analyzer.sideEffects().memory != yul::SideEffects::None);
+	return false;
 }
 
 bool TypeChecker::visit(IfStatement const& _ifStatement)

libsolidity/ast/ASTAnnotations.h

@@ -220,6 +220,10 @@ struct InlineAssemblyAnnotation: StatementAnnotation
 	std::map<yul::Identifier const*, ExternalIdentifierInfo> externalReferences;
 	/// Information generated during analysis phase.
 	std::shared_ptr<yul::AsmAnalysisInfo> analysisInfo;
+	/// True, if the assembly block was annotated to be memory-safe.
+	bool markedMemorySafe = false;
+	/// True, if the assembly block involves any memory opcode or assigns to variables in memory.
+	SetOnce<bool> hasMemoryEffects;
 };
 
 struct BlockAnnotation: StatementAnnotation, ScopableAnnotation

libsolidity/codegen/ir/IRGenerationContext.h

@@ -160,8 +160,8 @@ public:
 
 	std::set<ContractDefinition const*, ASTNode::CompareByID>& subObjectsCreated() { return m_subObjects; }
 
-	bool inlineAssemblySeen() const { return m_inlineAssemblySeen; }
-	void setInlineAssemblySeen() { m_inlineAssemblySeen = true; }
+	bool memoryUnsafeInlineAssemblySeen() const { return m_memoryUnsafeInlineAssemblySeen; }
+	void setMemoryUnsafeInlineAssemblySeen() { m_memoryUnsafeInlineAssemblySeen = true; }
 
 	/// @returns the runtime ID to be used for the function in the dispatch routine
 	/// and for internal function pointers.
@@ -202,8 +202,8 @@ private:
 	/// Whether to use checked or wrapping arithmetic.
 	Arithmetic m_arithmetic = Arithmetic::Checked;
 
-	/// Flag indicating whether any inline assembly block was seen.
-	bool m_inlineAssemblySeen = false;
+	/// Flag indicating whether any memory-unsafe inline assembly block was seen.
+	bool m_memoryUnsafeInlineAssemblySeen = false;
 
 	/// Function definitions queued for code generation. They're the Solidity functions whose calls
 	/// were discovered by the IR generator during AST traversal.

libsolidity/codegen/ir/IRGenerator.cpp

@@ -213,8 +213,8 @@ string IRGenerator::generate(
 	t("subObjects", subObjectSources(m_context.subObjectsCreated()));
 
 	// This has to be called only after all other code generation for the creation object is complete.
-	bool creationInvolvesAssembly = m_context.inlineAssemblySeen();
-	t("memoryInitCreation", memoryInit(!creationInvolvesAssembly));
+	bool creationInvolvesMemoryUnsafeAssembly = m_context.memoryUnsafeInlineAssemblySeen();
+	t("memoryInitCreation", memoryInit(!creationInvolvesMemoryUnsafeAssembly));
 	t("useSrcMapCreation", formatUseSrcMap(m_context));
 
 	resetContext(_contract, ExecutionContext::Deployed);
@@ -239,8 +239,8 @@ string IRGenerator::generate(
 	t("useSrcMapDeployed", formatUseSrcMap(m_context));
 
 	// This has to be called only after all other code generation for the deployed object is complete.
-	bool deployedInvolvesAssembly = m_context.inlineAssemblySeen();
-	t("memoryInitDeployed", memoryInit(!deployedInvolvesAssembly));
+	bool deployedInvolvesMemoryUnsafeAssembly = m_context.memoryUnsafeInlineAssemblySeen();
+	t("memoryInitDeployed", memoryInit(!deployedInvolvesMemoryUnsafeAssembly));
 
 	solAssert(_contract.annotation().creationCallGraph->get() != nullptr, "");
 	solAssert(_contract.annotation().deployedCallGraph->get() != nullptr, "");

libsolidity/codegen/ir/IRGeneratorForStatements.cpp

@@ -2138,7 +2138,8 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess)
 bool IRGeneratorForStatements::visit(InlineAssembly const& _inlineAsm)
 {
 	setLocation(_inlineAsm);
-	m_context.setInlineAssemblySeen();
+	if (*_inlineAsm.annotation().hasMemoryEffects && !_inlineAsm.annotation().markedMemorySafe)
+		m_context.setMemoryUnsafeInlineAssemblySeen();
 	CopyTranslate bodyCopier{_inlineAsm.dialect(), m_context, _inlineAsm.annotation().externalReferences};
 
 	yul::Statement modified = bodyCopier(_inlineAsm.operations());