Skip to content

Commit 9bcfcc6

Browse files
committed
Inline assembly without memory effects is implicitly memory safe.
1 parent e6848ca commit 9bcfcc6

File tree

6 files changed

+18
-5
lines changed

6 files changed

+18
-5
lines changed

libsolidity/analysis/DocStringTagParser.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ bool DocStringTagParser::visit(InlineAssembly const& _assembly)
202202
if (valuesSeen.insert(value).second)
203203
{
204204
if (value == "memory-safe-assembly")
205-
_assembly.annotation().memorySafe = true;
205+
_assembly.annotation().markedMemorySafe = true;
206206
else
207207
m_errorReporter.warning(
208208
8787_error,

libsolidity/analysis/TypeChecker.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,7 @@ void TypeChecker::endVisit(FunctionTypeName const& _funType)
763763

764764
bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
765765
{
766+
bool lvalueAccessToMemoryVariable = false;
766767
// External references have already been resolved in a prior stage and stored in the annotation.
767768
// We run the resolve step again regardless.
768769
yul::ExternalIdentifierAccess::Resolver identifierAccess = [&](
@@ -787,6 +788,8 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly)
787788
if (auto var = dynamic_cast<VariableDeclaration const*>(declaration))
788789
{
789790
solAssert(var->type(), "Expected variable type!");
791+
if (_context == yul::IdentifierContext::LValue && var->type()->dataStoredIn(DataLocation::Memory))
792+
lvalueAccessToMemoryVariable = true;
790793
if (var->immutable())
791794
{
792795
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)
974977
identifierAccess
975978
);
976979
if (!analyzer.analyze(_inlineAssembly.operations()))
977-
return false;
978-
return true;
980+
solAssert(m_errorReporter.hasErrors());
981+
_inlineAssembly.annotation().hasMemoryEffects =
982+
lvalueAccessToMemoryVariable ||
983+
(analyzer.sideEffects().memory != yul::SideEffects::None);
984+
return false;
979985
}
980986

981987
bool TypeChecker::visit(IfStatement const& _ifStatement)

libsolidity/ast/ASTAnnotations.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,9 @@ struct InlineAssemblyAnnotation: StatementAnnotation
221221
/// Information generated during analysis phase.
222222
std::shared_ptr<yul::AsmAnalysisInfo> analysisInfo;
223223
/// True, if the assembly block was annotated to be memory-safe.
224-
bool memorySafe = false;
224+
bool markedMemorySafe = false;
225+
/// True, if the assembly block involves any memory opcode or assigns to variables in memory.
226+
SetOnce<bool> hasMemoryEffects;
225227
};
226228

227229
struct BlockAnnotation: StatementAnnotation, ScopableAnnotation

libsolidity/codegen/ir/IRGeneratorForStatements.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2138,7 +2138,7 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess)
21382138
bool IRGeneratorForStatements::visit(InlineAssembly const& _inlineAsm)
21392139
{
21402140
setLocation(_inlineAsm);
2141-
if (!_inlineAsm.annotation().memorySafe)
2141+
if (*_inlineAsm.annotation().hasMemoryEffects && !_inlineAsm.annotation().markedMemorySafe)
21422142
m_context.setMemoryUnsafeInlineAssemblySeen();
21432143
CopyTranslate bodyCopier{_inlineAsm.dialect(), m_context, _inlineAsm.annotation().externalReferences};
21442144

libyul/AsmAnalysis.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,7 @@ vector<YulString> AsmAnalyzer::operator()(FunctionCall const& _funCall)
316316
literalArguments = &f->literalArguments;
317317

318318
validateInstructions(_funCall);
319+
m_sideEffects += f->sideEffects;
319320
}
320321
else if (m_currentScope->lookup(_funCall.functionName.name, GenericVisitor{
321322
[&](Scope::Variable const&)

libyul/AsmAnalysis.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ class AsmAnalyzer
9494
void operator()(Leave const&) { }
9595
void operator()(Block const& _block);
9696

97+
/// @returns the worst side effects encountered during analysis (including within defined functions).
98+
SideEffects const& sideEffects() const { return m_sideEffects; }
9799
private:
98100
/// Visits the expression, expects that it evaluates to exactly one value and
99101
/// returns the type. Reports errors on errors and returns the default type.
@@ -128,6 +130,8 @@ class AsmAnalyzer
128130
/// Names of data objects to be referenced by builtin functions with literal arguments.
129131
std::set<YulString> m_dataNames;
130132
ForLoop const* m_currentForLoop = nullptr;
133+
/// Worst side effects encountered during analysis (including within defined functions).
134+
SideEffects m_sideEffects;
131135
};
132136

133137
}

0 commit comments

Comments
 (0)