Merge pull request #12663 from ethereum/assemblyAnnotationDialectString

Assembly annotation as memory-safe using assembly flags.

docs/assembly.rst

@@ -297,8 +297,7 @@ model as follows:
 
 .. code-block:: solidity
 
-    /// @solidity memory-safe-assembly
-    assembly {
+    assembly ("memory-safe") {
         ...
     }
 
@@ -327,8 +326,7 @@ But the following is:
 
 .. code-block:: solidity
 
-    /// @solidity memory-safe-assembly
-    assembly {
+    assembly ("memory-safe") {
       let p := mload(0x40)
       returndatacopy(p, 0, returndatasize())
       revert(p, returndatasize())
@@ -341,8 +339,7 @@ If the memory operations use a length of zero, it is also fine to just use any o
 
 .. code-block:: solidity
 
-    /// @solidity memory-safe-assembly
-    assembly {
+    assembly ("memory-safe") {
       revert(0, 0)
     }
 
@@ -364,3 +361,16 @@ in memory is automatically considered memory-safe and does not need to be annota
     It is your responsibility to make sure that the assembly actually satisfies the memory model. If you annotate
     an assembly block as memory-safe, but violate one of the memory assumptions, this **will** lead to incorrect and
     undefined behaviour that cannot easily be discovered by testing.
+
+In case you are developing a library that is meant to be compatible across multiple versions
+of solidity, you can use a special comment to annotate an assembly block as memory-safe:
+
+.. code-block:: solidity
+
+    /// @solidity memory-safe-assembly
+    assembly {
+        ...
+    }
+
+Note that we will disallow the annotation via comment in a future breaking release, so if you are not concerned with
+backwards-compatibility with older compiler versions, prefer using the dialect string.

docs/grammar/SolidityLexer.g4

@@ -251,6 +251,12 @@ mode AssemblyBlockMode;
 AssemblyDialect: '"evmasm"';
 AssemblyLBrace: '{' -> popMode, pushMode(YulMode);
 
+AssemblyFlagString: '"' DoubleQuotedStringCharacter+ '"';
+
+AssemblyBlockLParen: '(';
+AssemblyBlockRParen: ')';
+AssemblyBlockComma: ',';
+
 AssemblyBlockWS: [ \t\r\n\u000C]+ -> skip ;
 AssemblyBlockCOMMENT: '/*' .*? '*/' -> channel(HIDDEN) ;
 AssemblyBlockLINE_COMMENT: '//' ~[\r\n]* -> channel(HIDDEN) ;

docs/grammar/SolidityParser.g4

@@ -476,7 +476,13 @@ revertStatement: Revert expression callArgumentList Semicolon;
  * The contents of an inline assembly block use a separate scanner/lexer, i.e. the set of keywords and
  * allowed identifiers is different inside an inline assembly block.
  */
-assemblyStatement: Assembly AssemblyDialect? AssemblyLBrace yulStatement* YulRBrace;
+assemblyStatement: Assembly AssemblyDialect? assemblyFlags? AssemblyLBrace yulStatement* YulRBrace;
+
+/**
+ * Assembly flags.
+ * Comma-separated list of double-quoted strings as flags.
+ */
+assemblyFlags: AssemblyBlockLParen AssemblyFlagString (AssemblyBlockComma AssemblyFlagString)* AssemblyBlockRParen;
 
 //@doc:inline
 variableDeclarationList: variableDeclarations+=variableDeclaration (Comma variableDeclarations+=variableDeclaration)*;

libsolidity/analysis/DocStringTagParser.cpp

@@ -202,7 +202,17 @@ bool DocStringTagParser::visit(InlineAssembly const& _assembly)
 				if (valuesSeen.insert(value).second)
 				{
 					if (value == "memory-safe-assembly")
+					{
+						if (_assembly.annotation().markedMemorySafe)
+							m_errorReporter.warning(
+								8544_error,
+								_assembly.location(),
+								"Inline assembly marked as memory safe using both a NatSpec tag and an assembly flag. "
+								"If you are not concerned with backwards compatibility, only use the assembly flag, "
+								"otherwise only use the NatSpec tag."
+							);
 						_assembly.annotation().markedMemorySafe = true;
+					}
 					else
 						m_errorReporter.warning(
 							8787_error,

libsolidity/analysis/SyntaxChecker.cpp

@@ -334,6 +334,27 @@ bool SyntaxChecker::visit(UnaryOperation const& _operation)
 
 bool SyntaxChecker::visit(InlineAssembly const& _inlineAssembly)
 {
+	if (_inlineAssembly.flags())
+		for (auto flag: *_inlineAssembly.flags())
+		{
+			if (*flag == "memory-safe")
+			{
+				if (_inlineAssembly.annotation().markedMemorySafe)
+					m_errorReporter.syntaxError(
+						7026_error,
+						_inlineAssembly.location(),
+						"Inline assembly marked memory-safe multiple times."
+					);
+				_inlineAssembly.annotation().markedMemorySafe = true;
+			}
+			else
+				m_errorReporter.warning(
+					4430_error,
+					_inlineAssembly.location(),
+					"Unknown inline assembly flag: \"" + *flag + "\""
+				);
+		}
+
 	if (!m_useYulOptimizer)
 		return false;
 

libsolidity/ast/AST.h

@@ -1463,19 +1463,26 @@ public:
 		SourceLocation const& _location,
 		ASTPointer<ASTString> const& _docString,
 		yul::Dialect const& _dialect,
+		ASTPointer<std::vector<ASTPointer<ASTString>>> _flags,
 		std::shared_ptr<yul::Block> _operations
 	):
-		Statement(_id, _location, _docString), m_dialect(_dialect), m_operations(std::move(_operations)) {}
+		Statement(_id, _location, _docString),
+		m_dialect(_dialect),
+		m_flags(move(_flags)),
+		m_operations(std::move(_operations))
+	{}
 	void accept(ASTVisitor& _visitor) override;
 	void accept(ASTConstVisitor& _visitor) const override;
 
 	yul::Dialect const& dialect() const { return m_dialect; }
 	yul::Block const& operations() const { return *m_operations; }
+	ASTPointer<std::vector<ASTPointer<ASTString>>> const& flags() const { return m_flags; }
 
 	InlineAssemblyAnnotation& annotation() const override;
 
 private:
 	yul::Dialect const& m_dialect;
+	ASTPointer<std::vector<ASTPointer<ASTString>>> m_flags;
 	std::shared_ptr<yul::Block> m_operations;
 };
 

libsolidity/ast/ASTJsonConverter.cpp

@@ -600,11 +600,23 @@ bool ASTJsonConverter::visit(InlineAssembly const& _node)
 	for (Json::Value& it: externalReferences | ranges::views::values)
 		externalReferencesJson.append(std::move(it));
 
-	setJsonNode(_node, "InlineAssembly", {
+	std::vector<pair<string, Json::Value>> attributes = {
 		make_pair("AST", Json::Value(yul::AsmJsonConverter(sourceIndexFromLocation(_node.location()))(_node.operations()))),
 		make_pair("externalReferences", std::move(externalReferencesJson)),
 		make_pair("evmVersion", dynamic_cast<solidity::yul::EVMDialect const&>(_node.dialect()).evmVersion().name())
-	});
+	};
+
+	if (_node.flags())
+	{
+		Json::Value flags(Json::arrayValue);
+		for (auto const& flag: *_node.flags())
+			if (flag)
+				flags.append(*flag);
+			else
+				flags.append(Json::nullValue);
+		attributes.emplace_back(make_pair("flags", move(flags)));
+	}
+	setJsonNode(_node, "InlineAssembly", move(attributes));
 
 	return false;
 }

libsolidity/ast/ASTJsonImporter.cpp

@@ -626,11 +626,24 @@ ASTPointer<InlineAssembly> ASTJsonImporter::createInlineAssembly(Json::Value con
 	astAssert(m_evmVersion == evmVersion, "Imported tree evm version differs from configured evm version!");
 
 	yul::Dialect const& dialect = yul::EVMDialect::strictAssemblyForEVM(evmVersion.value());
+	ASTPointer<vector<ASTPointer<ASTString>>> flags;
+	if (_node.isMember("flags"))
+	{
+		flags = make_shared<vector<ASTPointer<ASTString>>>();
+		Json::Value const& flagsNode = _node["flags"];
+		astAssert(flagsNode.isArray(), "Assembly flags must be an array.");
+		for (Json::ArrayIndex i = 0; i < flagsNode.size(); ++i)
+		{
+			astAssert(flagsNode[i].isString(), "Assembly flag must be a string.");
+			flags->emplace_back(make_shared<ASTString>(flagsNode[i].asString()));
+		}
+	}
 	shared_ptr<yul::Block> operations = make_shared<yul::Block>(yul::AsmJsonImporter(m_sourceNames).createBlock(member(_node, "AST")));
 	return createASTNode<InlineAssembly>(
 		_node,
 		nullOrASTString(_node, "documentation"),
 		dialect,
+		move(flags),
 		operations
 	);
 }

libsolidity/parsing/Parser.cpp

@@ -1321,13 +1321,28 @@ ASTPointer<InlineAssembly> Parser::parseInlineAssembly(ASTPointer<ASTString> con
 		advance();
 	}
 
+	ASTPointer<vector<ASTPointer<ASTString>>> flags;
+	if (m_scanner->currentToken() == Token::LParen)
+	{
+		flags = make_shared<vector<ASTPointer<ASTString>>>();
+		do
+		{
+			advance();
+			expectToken(Token::StringLiteral, false);
+			flags->emplace_back(make_shared<ASTString>(m_scanner->currentLiteral()));
+			advance();
+		}
+		while (m_scanner->currentToken() == Token::Comma);
+		expectToken(Token::RParen);
+	}
+
 	yul::Parser asmParser(m_errorReporter, dialect);
 	shared_ptr<yul::Block> block = asmParser.parseInline(m_scanner);
 	if (block == nullptr)
 		BOOST_THROW_EXCEPTION(FatalError());
 
 	location.end = nativeLocationOf(*block).end;
-	return make_shared<InlineAssembly>(nextID(), location, _docString, dialect, block);
+	return make_shared<InlineAssembly>(nextID(), location, _docString, dialect, move(flags), block);
 }
 
 ASTPointer<IfStatement> Parser::parseIfStatement(ASTPointer<ASTString> const& _docString)

test/libsolidity/memoryGuardTests/dialectString/constructor_safe_deploy_unsafe.sol

@@ -0,0 +1,17 @@
+contract C {
+    constructor() {
+        uint256 x;
+        assembly { x := 0 }
+        f();
+    }
+    function f() internal pure {
+        assembly "evmasm" ("memory-safe") { mstore(0, 0) }
+        assembly ("memory-safe") { mstore(0, 0) }
+    }
+    function g() public pure {
+        assembly { mstore(0, 0) }
+    }
+}
+// ----
+// :C(creation) true
+// :C(runtime) false

test/libsolidity/memoryGuardTests/dialectString/constructor_unsafe_deploy_safe.sol

@@ -0,0 +1,16 @@
+contract C {
+    constructor() {
+        uint256 x;
+        assembly { x := 0 }
+        f();
+    }
+    function f() internal pure {
+        assembly { mstore(0, 0) }
+    }
+    function g() public pure {
+        assembly "evmasm" ("memory-safe") { mstore(0, 0) }
+    }
+}
+// ----
+// :C(creation) false
+// :C(runtime) true

test/libsolidity/memoryGuardTests/dialectString/free_function.sol

@@ -0,0 +1,28 @@
+function safe() pure returns (uint256 x) {
+    assembly { x := 42 }
+    assembly "evmasm" ("memory-safe") { mstore(0, 0) }
+}
+function unsafe() pure returns (uint256 x) {
+    assembly { pop(mload(0)) }
+}
+contract C {
+    constructor() {
+        unsafe();
+    }
+    function f() public pure {
+        safe();
+    }
+}
+contract D {
+    constructor() {
+        safe();
+    }
+    function f() public pure {
+        unsafe();
+    }
+}
+// ----
+// :C(creation) false
+// :C(runtime) true
+// :D(creation) true
+// :D(runtime) false

test/libsolidity/memoryGuardTests/dialectString/multiple_contracts.sol

@@ -0,0 +1,23 @@
+contract C {
+    constructor(uint256 x) {
+        assembly { x := 4 }
+        assembly "evmasm" ("memory-safe") { mstore(0, 0) }
+    }
+    function f() public pure {
+        assembly { mstore(0,0) }
+    }
+}
+contract D {
+    constructor() {
+        assembly { mstore(0,0) }
+    }
+    function f(uint256 x) public pure {
+        assembly { x := 4 }
+        assembly ("memory-safe") { mstore(0, 0) }
+    }
+}
+// ----
+// :C(creation) true
+// :C(runtime) false
+// :D(creation) false
+// :D(runtime) true

test/libsolidity/memoryGuardTests/dialectString/safe_and_unmarked_empty.sol

@@ -0,0 +1,9 @@
+contract C {
+    function f() external pure {
+        assembly "evmasm" ("memory-safe") {}
+        assembly {}
+    }
+}
+// ----
+// :C(creation) true
+// :C(runtime) true

test/libsolidity/memoryGuardTests/dialectString/safe_and_unmarked_unsafe.sol

@@ -0,0 +1,9 @@
+contract C {
+    function f() external pure {
+        assembly "evmasm" ("memory-safe") {}
+        assembly { mstore(0,0) }
+    }
+}
+// ----
+// :C(creation) true
+// :C(runtime) false

test/libsolidity/memoryGuardTests/dialectString/stub.sol

@@ -0,0 +1,4 @@
+contract C {}
+// ----
+// :C(creation) true
+// :C(runtime) true

test/libsolidity/syntaxTests/inlineAssembly/assembly_dialect_duplicate_option.sol

@@ -0,0 +1,5 @@
+function f() pure {
+    assembly "evmasm" ("memory-safe", "memory-safe") {}
+}
+// ----
+// SyntaxError 7026: (24-75): Inline assembly marked memory-safe multiple times.

test/libsolidity/syntaxTests/inlineAssembly/assembly_dialect_invalid_options.sol

@@ -0,0 +1,8 @@
+function f() pure {
+    assembly "evmasm" ("a", "b", "c", "c") {}
+}
+// ----
+// Warning 4430: (24-65): Unknown inline assembly flag: "a"
+// Warning 4430: (24-65): Unknown inline assembly flag: "b"
+// Warning 4430: (24-65): Unknown inline assembly flag: "c"
+// Warning 4430: (24-65): Unknown inline assembly flag: "c"

test/libsolidity/syntaxTests/inlineAssembly/assembly_dialect_leading_space.sol

@@ -0,0 +1,5 @@
+function f() pure {
+    assembly " evmasm" {}
+}
+// ----
+// ParserError 4531: (33-42): Only "evmasm" supported.

test/libsolidity/syntaxTests/inlineAssembly/assembly_duplicate_option.sol

@@ -0,0 +1,5 @@
+function f() pure {
+    assembly ("memory-safe", "memory-safe") {}
+}
+// ----
+// SyntaxError 7026: (24-66): Inline assembly marked memory-safe multiple times.

test/libsolidity/syntaxTests/inlineAssembly/assembly_empty_option_list.sol

@@ -0,0 +1,5 @@
+function f() pure {
+    assembly () {}
+}
+// ----
+// ParserError 2314: (34-35): Expected 'StringLiteral' but got ')'

test/libsolidity/syntaxTests/inlineAssembly/assembly_empty_option_list_dialect.sol

@@ -0,0 +1,5 @@
+function f() pure {
+    assembly "evmasm" () {}
+}
+// ----
+// ParserError 2314: (43-44): Expected 'StringLiteral' but got ')'

test/libsolidity/syntaxTests/inlineAssembly/assembly_flags_delimiter.sol

@@ -0,0 +1,5 @@
+function f() pure {
+	assembly ("a" "b") {}
+}
+// ----
+// ParserError 2314: (35-38): Expected ')' but got 'StringLiteral'

test/libsolidity/syntaxTests/inlineAssembly/assembly_invalid_options.sol

@@ -0,0 +1,8 @@
+function f() pure {
+    assembly ("a", "b", "c", "c") {}
+}
+// ----
+// Warning 4430: (24-56): Unknown inline assembly flag: "a"
+// Warning 4430: (24-56): Unknown inline assembly flag: "b"
+// Warning 4430: (24-56): Unknown inline assembly flag: "c"
+// Warning 4430: (24-56): Unknown inline assembly flag: "c"

test/libsolidity/syntaxTests/inlineAssembly/memory_safe_dialect_string_and_comment.sol

@@ -0,0 +1,7 @@
+function f() pure {
+    /// @solidity memory-safe-assembly
+    assembly "evmasm" ("memory-safe") {
+    }
+}
+// ----
+// Warning 8544: (63-104): Inline assembly marked as memory safe using both a NatSpec tag and an assembly flag. If you are not concerned with backwards compatibility, only use the assembly flag, otherwise only use the NatSpec tag.

test/libsolidity/syntaxTests/inlineAssembly/memory_safe_in_dialect_string.sol

@@ -0,0 +1,3 @@
+function f() pure {
+    assembly "evmasm" ("memory-safe") {}
+}