From 7bc3105115dbdd8114b38a753b1a8dd6d8e85dcb Mon Sep 17 00:00:00 2001 From: Abinav Puthan Purayil Date: Sat, 1 Jun 2024 22:18:36 +0200 Subject: [PATCH] [0.8.7] Fix the try-catch lowering hack (#648) --- libsolidity/ast/ASTAnnotations.h | 7 +++++++ libsolidity/codegen/CompilerContext.h | 5 ----- libsolidity/codegen/ContractCompiler.cpp | 12 +++++++++--- libsolidity/codegen/ExpressionCompiler.cpp | 12 ++++++++---- libsolidity/codegen/ExpressionCompiler.h | 3 ++- 5 files changed, 26 insertions(+), 13 deletions(-) diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h index 6b4b14e7c7..4e1b823f9d 100644 --- a/libsolidity/ast/ASTAnnotations.h +++ b/libsolidity/ast/ASTAnnotations.h @@ -331,6 +331,13 @@ struct FunctionCallAnnotation: ExpressionAnnotation util::SetOnce kind; /// If true, this is the external call of a try statement. bool tryCall = false; + + // HACK! + // We track the success tag here for the `TryStatement` lowering. This is to avoid the redundant status check and + // the conditional jump. Such patterns can confuse the zksolc translator. + // + // uint32_t since Assembly::new[Push]Tag() asserts that the tag is 32 bits. + std::optional tryCallSuccessTag; }; } diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h index 0bb020df1c..f2bb0be899 100644 --- a/libsolidity/codegen/CompilerContext.h +++ b/libsolidity/codegen/CompilerContext.h @@ -348,11 +348,6 @@ class CompilerContext RevertStrings revertStrings() const { return m_revertStrings; } - // HACK! - // We track the success tag here for the `TryStatement` lowering. This is to avoid the redundant status check and - // the conditional jump. Such patterns can confuse the zksolc translator. - evmasm::AssemblyItem currTryCallSuccessTag{evmasm::AssemblyItemType::UndefinedItem}; - private: /// Updates source location set in the assembly. void updateSourceLocation(); diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 888cef0c4d..bdfb33937c 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -979,7 +979,10 @@ bool ContractCompiler::visit(TryStatement const& _tryStatement) StackHeightChecker checker(m_context); CompilerContext::LocationSetter locationSetter(m_context, _tryStatement); - compileExpression(_tryStatement.externalCall()); + auto* externalCall = dynamic_cast(&_tryStatement.externalCall()); + solAssert(externalCall && externalCall->annotation().tryCall, ""); + compileExpression(*externalCall); + int const returnSize = static_cast(_tryStatement.externalCall().annotation().type->sizeOnStack()); // Stack: [ return values] @@ -992,8 +995,11 @@ bool ContractCompiler::visit(TryStatement const& _tryStatement) evmasm::AssemblyItem endTag = m_context.appendJumpToNew(); - solAssert(m_context.currTryCallSuccessTag.type() == AssemblyItemType::Tag, ""); - m_context << m_context.currTryCallSuccessTag; + auto& successTag = externalCall->annotation().tryCallSuccessTag; + solAssert(successTag, ""); + m_context << AssemblyItem(AssemblyItemType::Tag, *successTag); + successTag.reset(); + m_context.adjustStackOffset(returnSize); { // Success case. diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index 720b8317c5..2a8a895d0b 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -761,7 +761,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) case FunctionType::Kind::External: case FunctionType::Kind::DelegateCall: _functionCall.expression().accept(*this); - appendExternalFunctionCall(function, arguments, _functionCall.annotation().tryCall); + appendExternalFunctionCall( + function, arguments, _functionCall.annotation().tryCall, &_functionCall.annotation()); break; case FunctionType::Kind::BareCallCode: solAssert(false, "Callcode has been removed."); @@ -821,7 +822,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) // If this is a try call, return "
1" in the success case and // "0" in the error case. AssemblyItem errorCase = m_context.appendConditionalJump(); - m_context.currTryCallSuccessTag = m_context.appendJumpToNew(); + _functionCall.annotation().tryCallSuccessTag + = m_context.appendJumpToNew().data().convert_to(); m_context.adjustStackOffset(1); m_context << errorCase; } @@ -2514,7 +2516,8 @@ void ExpressionCompiler::appendExpOperatorCode(Type const& _valueType, Type cons void ExpressionCompiler::appendExternalFunctionCall( FunctionType const& _functionType, vector> const& _arguments, - bool _tryCall + bool _tryCall, + FunctionCallAnnotation* _annotation ) { solAssert( @@ -2792,8 +2795,9 @@ void ExpressionCompiler::appendExternalFunctionCall( if (_tryCall) { + solAssert(_annotation, ""); // Success branch will reach this, failure branch will directly jump to endTag. - m_context.currTryCallSuccessTag = m_context.appendJumpToNew(); + _annotation->tryCallSuccessTag = m_context.appendJumpToNew().data().convert_to(); m_context.adjustStackOffset(1); m_context << endTag; } diff --git a/libsolidity/codegen/ExpressionCompiler.h b/libsolidity/codegen/ExpressionCompiler.h index cfa36de4bc..0ba13f8258 100644 --- a/libsolidity/codegen/ExpressionCompiler.h +++ b/libsolidity/codegen/ExpressionCompiler.h @@ -110,7 +110,8 @@ class ExpressionCompiler: private ASTConstVisitor void appendExternalFunctionCall( FunctionType const& _functionType, std::vector> const& _arguments, - bool _tryCall + bool _tryCall, + FunctionCallAnnotation* _annotation = nullptr ); /// Appends code that evaluates a single expression and moves the result to memory. The memory offset is /// expected to be on the stack and is updated by this call.