From 1013419597c0f4db4cb35dc8d0f668a64bbf5f45 Mon Sep 17 00:00:00 2001
From: chriseth <chris@ethereum.org>
Date: Mon, 28 Nov 2022 15:27:24 +0100
Subject: [PATCH] returndatacopy and bugfix.

---
 libyul/optimiser/UnusedStoreEliminator.cpp    | 58 +++++++++----------
 libyul/optimiser/UnusedStoreEliminator.h      |  7 ++-
 .../covering_calldatacopy_fixed.yul           |  3 +-
 3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/libyul/optimiser/UnusedStoreEliminator.cpp b/libyul/optimiser/UnusedStoreEliminator.cpp
index e2bbe7645..7e7d15cf7 100644
--- a/libyul/optimiser/UnusedStoreEliminator.cpp
+++ b/libyul/optimiser/UnusedStoreEliminator.cpp
@@ -78,12 +78,10 @@ void UnusedStoreEliminator::run(OptimiserStepContext& _context, Block& _ast)
 		ignoreMemory
 	};
 	rse(_ast);
-	if (
-		auto evmDialect = dynamic_cast<EVMDialect const*>(&_context.dialect);
-		evmDialect && evmDialect->providesObjectAccess()
-	)
-	{
-	}
+
+	auto evmDialect = dynamic_cast<EVMDialect const*>(&_context.dialect);
+	if (evmDialect && evmDialect->providesObjectAccess())
+		rse.clearActive(Location::Memory);
 	else
 		rse.markActiveAsUsed(Location::Memory);
 	rse.markActiveAsUsed(Location::Storage);
@@ -168,25 +166,30 @@ void UnusedStoreEliminator::visit(Statement const& _statement)
 	yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite)));
 	if (isCandidateForRemoval)
 	{
-		// TODO what is this special case?
-		//State initialState = State::Undecided;
-//		if (*instruction == Instruction::RETURNDATACOPY)
-//		{
-//			initialState = State::Used;
-//			auto startOffset = identifierNameIfSSA(funCall->arguments.at(1));
-//			auto length = identifierNameIfSSA(funCall->arguments.at(2));
-//			KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); });
-//			if (length && startOffset)
-//			{
-//				FunctionCall const* lengthCall = get_if<FunctionCall>(m_ssaValues.at(*length).value);
-//				if (
-//					knowledge.knownToBeZero(*startOffset) &&
-//					lengthCall &&
-//					toEVMInstruction(m_dialect, lengthCall->functionName.name) == Instruction::RETURNDATASIZE
-//				)
-//					initialState = State::Undecided;
-//			}
-//		}
+		if (*instruction == Instruction::RETURNDATACOPY)
+		{
+			// Out-of-bounds access to the returndata buffer results in a revert,
+			// so we are careful not to remove a potentially reverting call to a builtin.
+			// The only way the Solidity compiler uses `returndatacopy` is
+			// `returndatacopy(X, 0, returndatasize())`, so we only allow to remove this pattern
+			// (which is guaranteed to never cause an out-of-bounds revert).
+			bool allowReturndatacopyToBeRemoved = false;
+			auto startOffset = identifierNameIfSSA(funCall->arguments.at(1));
+			auto length = identifierNameIfSSA(funCall->arguments.at(2));
+			KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); });
+			if (length && startOffset)
+			{
+				FunctionCall const* lengthCall = get_if<FunctionCall>(m_ssaValues.at(*length).value);
+				if (
+					knowledge.knownToBeZero(*startOffset) &&
+					lengthCall &&
+					toEVMInstruction(m_dialect, lengthCall->functionName.name) == Instruction::RETURNDATASIZE
+				)
+					allowReturndatacopyToBeRemoved = true;
+			}
+			if (!allowReturndatacopyToBeRemoved)
+				return;
+		}
 		m_allStores.insert(&_statement);
 		vector<Operation> operations = operationsFromFunctionCall(*funCall);
 		yulAssert(operations.size() == 1, "");
@@ -198,11 +201,6 @@ void UnusedStoreEliminator::visit(Statement const& _statement)
 	}
 }
 
-void UnusedStoreEliminator::finalizeFunctionDefinition(FunctionDefinition const&)
-{
-	markActiveAsUsed();
-}
-
 vector<UnusedStoreEliminator::Operation> UnusedStoreEliminator::operationsFromFunctionCall(
 	FunctionCall const& _functionCall
 ) const
diff --git a/libyul/optimiser/UnusedStoreEliminator.h b/libyul/optimiser/UnusedStoreEliminator.h
index 94620ab7b..f8227bd10 100644
--- a/libyul/optimiser/UnusedStoreEliminator.h
+++ b/libyul/optimiser/UnusedStoreEliminator.h
@@ -98,14 +98,17 @@ public:
 
 private:
 	std::set<Statement const*>& activeMemoryStores() { return m_activeStores["m"_yulstring]; }
-	std::set<Statement const*>& activeStorageStores() { return m_activeStores["m"_yulstring]; }
+	std::set<Statement const*>& activeStorageStores() { return m_activeStores["s"_yulstring]; }
 
 	void shortcutNestedLoop(ActiveStores const&) override
 	{
 		// We might only need to do this for newly introduced stores in the loop.
 		markActiveAsUsed();
 	}
-	void finalizeFunctionDefinition(FunctionDefinition const&) override;
+	void finalizeFunctionDefinition(FunctionDefinition const&) override
+	{
+		markActiveAsUsed();
+	}
 
 	std::vector<Operation> operationsFromFunctionCall(FunctionCall const& _functionCall) const;
 	void applyOperation(Operation const& _operation);
diff --git a/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul b/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul
index 47e0b95fa..bcafffcec 100644
--- a/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul
+++ b/test/libyul/yulOptimizerTests/unusedStoreEliminator/covering_calldatacopy_fixed.yul
@@ -39,8 +39,7 @@
 //         }
 //         if calldataload(2)
 //         {
-//             let _17 := 7
-//             let _18 := 2
+//             mstore8(2, 7)
 //             calldatacopy(0, 0, 3)
 //         }
 //         if calldataload(3)