From e6848caac1246650ac5108f9abd08ac877b0f592 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Thu, 3 Feb 2022 11:49:16 +0100 Subject: [PATCH] Allow annotating inline assembly as memory-safe. --- Changelog.md | 1 + libsolidity/analysis/DocStringTagParser.cpp | 66 +++++++++++++++++++ libsolidity/analysis/DocStringTagParser.h | 1 + libsolidity/ast/ASTAnnotations.h | 2 + libsolidity/codegen/ir/IRGenerationContext.h | 8 +-- libsolidity/codegen/ir/IRGenerator.cpp | 8 +-- .../codegen/ir/IRGeneratorForStatements.cpp | 3 +- .../inlineAssembly/invalid_natspec.sol | 14 ++++ .../inlineAssembly/natspec_memory_safe.sol | 6 ++ 9 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 test/libsolidity/syntaxTests/inlineAssembly/invalid_natspec.sol create mode 100644 test/libsolidity/syntaxTests/inlineAssembly/natspec_memory_safe.sol diff --git a/Changelog.md b/Changelog.md index fd7dda584..181c386aa 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,7 @@ ### 0.8.13 (unreleased) Language Features: + * General: Allow annotating inline assembly as memory-safe to allow optimizations and stack limit evasion that rely on respecting Solidity's memory model. Compiler Features: diff --git a/libsolidity/analysis/DocStringTagParser.cpp b/libsolidity/analysis/DocStringTagParser.cpp index 137cdb409..5dc68d97f 100644 --- a/libsolidity/analysis/DocStringTagParser.cpp +++ b/libsolidity/analysis/DocStringTagParser.cpp @@ -30,6 +30,7 @@ #include #include +#include #include @@ -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 values; + boost::split(values, tagValue.content, isWhiteSpace); + + set valuesSeen; + set 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().memorySafe = 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, diff --git a/libsolidity/analysis/DocStringTagParser.h b/libsolidity/analysis/DocStringTagParser.h index 84fde6c6c..248befcb8 100644 --- a/libsolidity/analysis/DocStringTagParser.h +++ b/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, diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h index 53931e426..4489dce8d 100644 --- a/libsolidity/ast/ASTAnnotations.h +++ b/libsolidity/ast/ASTAnnotations.h @@ -220,6 +220,8 @@ struct InlineAssemblyAnnotation: StatementAnnotation std::map externalReferences; /// Information generated during analysis phase. std::shared_ptr analysisInfo; + /// True, if the assembly block was annotated to be memory-safe. + bool memorySafe = false; }; struct BlockAnnotation: StatementAnnotation, ScopableAnnotation diff --git a/libsolidity/codegen/ir/IRGenerationContext.h b/libsolidity/codegen/ir/IRGenerationContext.h index cf01dc4f4..ea960bb70 100644 --- a/libsolidity/codegen/ir/IRGenerationContext.h +++ b/libsolidity/codegen/ir/IRGenerationContext.h @@ -160,8 +160,8 @@ public: std::set& 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. diff --git a/libsolidity/codegen/ir/IRGenerator.cpp b/libsolidity/codegen/ir/IRGenerator.cpp index 6b52010d9..cb187b424 100644 --- a/libsolidity/codegen/ir/IRGenerator.cpp +++ b/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, ""); diff --git a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp b/libsolidity/codegen/ir/IRGeneratorForStatements.cpp index 9fb73f021..cf0977c6e 100644 --- a/libsolidity/codegen/ir/IRGeneratorForStatements.cpp +++ b/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().memorySafe) + m_context.setMemoryUnsafeInlineAssemblySeen(); CopyTranslate bodyCopier{_inlineAsm.dialect(), m_context, _inlineAsm.annotation().externalReferences}; yul::Statement modified = bodyCopier(_inlineAsm.operations()); diff --git a/test/libsolidity/syntaxTests/inlineAssembly/invalid_natspec.sol b/test/libsolidity/syntaxTests/inlineAssembly/invalid_natspec.sol new file mode 100644 index 000000000..3a6ccefb3 --- /dev/null +++ b/test/libsolidity/syntaxTests/inlineAssembly/invalid_natspec.sol @@ -0,0 +1,14 @@ +contract C { + function f() public pure { + /// @test test + assembly {} + /// @solidity test + assembly {} + /// @param + assembly {} + } +} +// ---- +// Warning 6269: (60-71): Unexpected natspec tag in inline assembly: test +// Warning 8787: (95-106): Unexpected value for @solidity tag in inline assembly: test +// Warning 7828: (122-133): Inline assembly has invalid natspec documentation. diff --git a/test/libsolidity/syntaxTests/inlineAssembly/natspec_memory_safe.sol b/test/libsolidity/syntaxTests/inlineAssembly/natspec_memory_safe.sol new file mode 100644 index 000000000..b93b20318 --- /dev/null +++ b/test/libsolidity/syntaxTests/inlineAssembly/natspec_memory_safe.sol @@ -0,0 +1,6 @@ +contract C { + function f() public pure { + // @solidity memory-safe-assembly + assembly {} + } +} \ No newline at end of file