From 9abea9d11714e2925e4adf5f7cb89dc4d6f6ea19 Mon Sep 17 00:00:00 2001 From: Abinav Puthan Purayil Date: Sat, 1 Jun 2024 22:21:38 +0200 Subject: [PATCH] Fix the try-catch lowering hack This patch moves the success tag tracking of the try-call ast to its annotation instead of CompilerContext. This fixes the lowering of nested try-catch statements. This fixes CPR-1705. --- 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 b932742dd4..740d59030a 100644 --- a/libsolidity/ast/ASTAnnotations.h +++ b/libsolidity/ast/ASTAnnotations.h @@ -317,6 +317,13 @@ struct FunctionCallAnnotation: ExpressionAnnotation FunctionCallKind kind = FunctionCallKind::Unset; /// 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 9464ea64c1..9217445d2c 100644 --- a/libsolidity/codegen/CompilerContext.h +++ b/libsolidity/codegen/CompilerContext.h @@ -349,11 +349,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 e9746f3cf8..9df7e79a7f 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -932,7 +932,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] @@ -945,8 +948,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 4941c04676..eb922f522e 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -706,7 +706,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."); @@ -766,7 +767,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; } @@ -2238,7 +2240,8 @@ void ExpressionCompiler::appendShiftOperatorCode(Token _operator, Type const& _v void ExpressionCompiler::appendExternalFunctionCall( FunctionType const& _functionType, vector> const& _arguments, - bool _tryCall + bool _tryCall, + FunctionCallAnnotation* _annotation ) { solAssert( @@ -2516,8 +2519,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 414cbd1158..193a6978d9 100644 --- a/libsolidity/codegen/ExpressionCompiler.h +++ b/libsolidity/codegen/ExpressionCompiler.h @@ -108,7 +108,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.