Skip to content

Commit 2bbc9a1

Browse files
authored
Merge pull request #13040 from ethereum/returndatacopyOptimizer
Do not remove potentially reverting returndatacopy cases.
2 parents be85d28 + 9fa907a commit 2bbc9a1

File tree

6 files changed

+73
-1
lines changed

6 files changed

+73
-1
lines changed

Changelog.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Compiler Features:
99

1010
Bugfixes:
1111
* ABI Encoder: When encoding an empty string coming from storage do not add a superfluous empty slot for data.
12+
* Yul Optimizer: Do not remove ``returndatacopy`` in cases in which it might perform out-of-bounds reads that unconditionally revert as out-of-gas. Previously, any ``returndatacopy`` that wrote to memory that was never read from was removed without accounting for the out-of-bounds condition.
1213

1314

1415
### 0.8.14 (2022-05-17)

libyul/optimiser/UnusedStoreEliminator.cpp

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,25 @@ void UnusedStoreEliminator::visit(Statement const& _statement)
157157
yulAssert(isCandidateForRemoval == (isStorageWrite || (!m_ignoreMemory && isMemoryWrite)));
158158
if (isCandidateForRemoval)
159159
{
160-
m_stores[YulString{}].insert({&_statement, State::Undecided});
160+
State initialState = State::Undecided;
161+
if (*instruction == Instruction::RETURNDATACOPY)
162+
{
163+
initialState = State::Used;
164+
auto startOffset = identifierNameIfSSA(funCall->arguments.at(1));
165+
auto length = identifierNameIfSSA(funCall->arguments.at(2));
166+
KnowledgeBase knowledge(m_dialect, [this](YulString _var) { return util::valueOrNullptr(m_ssaValues, _var); });
167+
if (length && startOffset)
168+
{
169+
FunctionCall const* lengthCall = get_if<FunctionCall>(m_ssaValues.at(*length).value);
170+
if (
171+
knowledge.knownToBeZero(*startOffset) &&
172+
lengthCall &&
173+
toEVMInstruction(m_dialect, lengthCall->functionName.name) == Instruction::RETURNDATASIZE
174+
)
175+
initialState = State::Undecided;
176+
}
177+
}
178+
m_stores[YulString{}].insert({&_statement, initialState});
161179
vector<Operation> operations = operationsFromFunctionCall(*funCall);
162180
yulAssert(operations.size() == 1, "");
163181
m_storeOperations[&_statement] = move(operations.front());
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
returndatacopy(0,0,32)
3+
}
4+
// ====
5+
// EVMVersion: >homestead
6+
// ----
7+
// step: unusedStoreEliminator
8+
//
9+
// { { returndatacopy(0, 0, 32) } }
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
returndatacopy(0,0,returndatasize())
3+
}
4+
// ====
5+
// EVMVersion: >homestead
6+
// ----
7+
// step: unusedStoreEliminator
8+
//
9+
// {
10+
// {
11+
// let _1 := returndatasize()
12+
// let _2 := 0
13+
// let _3 := 0
14+
// }
15+
// }
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
{
2+
returndatacopy(0,1,returndatasize())
3+
}
4+
// ====
5+
// EVMVersion: >homestead
6+
// ----
7+
// step: unusedStoreEliminator
8+
//
9+
// {
10+
// {
11+
// returndatacopy(0, 1, returndatasize())
12+
// }
13+
// }
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
let s := returndatasize()
3+
returndatacopy(0,0,s)
4+
}
5+
// ====
6+
// EVMVersion: >homestead
7+
// ----
8+
// step: unusedStoreEliminator
9+
//
10+
// {
11+
// {
12+
// let s := returndatasize()
13+
// let _1 := 0
14+
// let _2 := 0
15+
// }
16+
// }

0 commit comments

Comments
 (0)