cerc-io/solidity
16
0
Fork 0
mirror of https://github.com/ethereum/solidity synced 2023-10-03 13:03:40 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #13479 from ethereum/unusedStoreStorageBug

Browse Source 
Fix of unused store remover storage bug.
This commit is contained in:
Daniel Kirchner 2022-09-08 14:04:04 +02:00 committed by GitHub
commit 727591b984
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 210 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Changelog.md
View File

@ -1,6 +1,7 @@
### 0.8.17 (unreleased)
Important Bugfixes:
* Yul Optimizer: Prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call.
Language Features:

13
docs/bugs.json
View File

@ -1,4 +1,17 @@
[
{
"uid": "SOL-2022-7",
"name": "StorageWriteRemovalBeforeConditionalTermination",
"summary": "Calling functions that conditionally terminate the external EVM call using the assembly statements ``return(...)`` or ``stop()`` may result in incorrect removals of prior storage writes.",
"description": "A call to a Yul function that conditionally terminates the external EVM call could result in prior storage writes being incorrectly removed by the Yul optimizer. This used to happen in cases in which it would have been valid to remove the store, if the Yul function in question never actually terminated the external call, and the control flow always returned back to the caller instead. Conditional termination within the same Yul block instead of within a called function was not affected. In Solidity with optimized via-IR code generation, any storage write before a function conditionally calling ``return(...)`` or ``stop()`` in inline assembly, may have been incorrectly removed, whenever it would have been valid to remove the write without the ``return(...)`` or ``stop()``. In optimized legacy code generation, only inline assembly that did not refer to any Solidity variables and that involved conditionally-terminating user-defined assembly functions could be affected.",
"link": "https://blog.soliditylang.org/2022/09/08/storage-write-removal-before-conditional-termination/",
"introduced": "0.8.13",
"fixed": "0.8.17",
"severity": "medium/high",
"conditions": {
"yulOptimizer": true
}
},
{
"uid": "SOL-2022-6",
"name": "AbiReencodingHeadOverflowWithStaticArrayCleanup",

7
docs/bugs_by_version.json
View File

@ -1737,6 +1737,7 @@
},
"0.8.13": {
"bugs": [
"StorageWriteRemovalBeforeConditionalTermination",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"InlineAssemblyMemorySideEffects",
@ -1747,6 +1748,7 @@
},
"0.8.14": {
"bugs": [
"StorageWriteRemovalBeforeConditionalTermination",
"AbiReencodingHeadOverflowWithStaticArrayCleanup",
"DirtyBytesArrayToStorage",
"InlineAssemblyMemorySideEffects"
@ -1755,12 +1757,15 @@
},
"0.8.15": {
"bugs": [
"StorageWriteRemovalBeforeConditionalTermination",
"AbiReencodingHeadOverflowWithStaticArrayCleanup"
],
"released": "2022-06-15"
},
"0.8.16": {
"bugs": [],
"bugs": [
"StorageWriteRemovalBeforeConditionalTermination"
],
"released": "2022-08-08"
},
"0.8.2": {

5
libyul/optimiser/UnusedStoreEliminator.cpp
View File

@ -105,10 +105,13 @@ void UnusedStoreEliminator::operator()(FunctionCall const& _functionCall)
else
sideEffects = m_controlFlowSideEffects.at(_functionCall.functionName.name);
if (sideEffects.canTerminate)
changeUndecidedTo(State::Used, Location::Storage);
if (!sideEffects.canContinue)
{
changeUndecidedTo(State::Unused, Location::Memory);
changeUndecidedTo(sideEffects.canTerminate ? State::Used : State::Unused, Location::Storage);
if (!sideEffects.canTerminate)
changeUndecidedTo(State::Unused, Location::Storage);
}
}

15
test/libsolidity/semanticTests/unused_store_storage_removal_bug.sol Normal file
View File

@ -0,0 +1,15 @@
contract C {
uint public x;
function f() public {
x = 1; // This write used to be removed by the Yul optimizer due to the StorageWriteRemovalBeforeConditionalTermination bug.
g();
x = 2;
}
function g() internal {
if (msg.data.length > 4) return;
assembly { return(0, 0) }
}
}
// ----
// f() ->
// x() -> 1

28
test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call.yul Normal file
View File

@ -0,0 +1,28 @@
{
function conditionallyStop() {
if calldataload(0) { leave }
return(0, 0)
}
let x := 0
let y := 1
sstore(x, y) // used to be removed due to the StorageWriteRemovalBeforeConditionalTermination
conditionallyStop()
sstore(x, y)
}
// ----
// step: unusedStoreEliminator
//
// {
// {
// let x := 0
// let y := 1
// sstore(x, y)
// conditionallyStop()
// sstore(x, y)
// }
// function conditionallyStop()
// {
// if calldataload(0) { leave }
// return(0, 0)
// }
// }

60
test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call_complex_control_flow.yul Normal file
View File

@ -0,0 +1,60 @@
{
function conditionallyStop() {
if calldataload(0) { leave }
return(0, 0)
}
function g() {
let a := 0
let b := 1
sstore(a, b)
}
let x := 0
let y := 1
let z := 2
switch calldataload(64)
case 0 {
sstore(z, x)
g()
}
default {
sstore(x, z) // used to be removed due to the StorageWriteRemovalBeforeConditionalTermination
}
conditionallyStop()
switch calldataload(32)
case 0 {
revert(0, 0)
}
default {
sstore(x, z)
}
}
// ----
// step: unusedStoreEliminator
//
// {
// {
// let x := 0
// let y := 1
// let z := 2
// switch calldataload(64)
// case 0 {
// sstore(z, x)
// g()
// }
// default { sstore(x, z) }
// conditionallyStop()
// switch calldataload(32)
// case 0 { revert(0, 0) }
// default { sstore(x, z) }
// }
// function conditionallyStop()
// {
// if calldataload(0) { leave }
// return(0, 0)
// }
// function g()
// {
// let a := 0
// sstore(a, 1)
// }
// }

27
test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_conditionally_terminating_function_call_revert.yul Normal file
View File

@ -0,0 +1,27 @@
{
function conditionallyStop() {
if calldataload(0) { leave }
return(0, 0)
}
let x := 0
let y := 1
sstore(x, y) // used to be removed due to the to the StorageWriteRemovalBeforeConditionalTermination bug
conditionallyStop()
revert(0,0)
}
// ----
// step: unusedStoreEliminator
//
// {
// {
// let x := 0
// sstore(x, 1)
// conditionallyStop()
// revert(0, 0)
// }
// function conditionallyStop()
// {
// if calldataload(0) { leave }
// return(0, 0)
// }
// }

33
test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_indirectly_conditionally_terminating_function_call.yul Normal file
View File

@ -0,0 +1,33 @@
{
function conditionallyStop() {
if calldataload(0) { leave }
returnEmpty()
}
function returnEmpty() {
return(0, 0)
}
let x := 0
let y := 1
sstore(x, y) // used to be removed due to a bug
conditionallyStop()
sstore(x, y)
}
// ----
// step: unusedStoreEliminator
//
// {
// {
// let x := 0
// let y := 1
// sstore(x, y)
// conditionallyStop()
// sstore(x, y)
// }
// function conditionallyStop()
// {
// if calldataload(0) { leave }
// returnEmpty()
// }
// function returnEmpty()
// { return(0, 0) }
// }

23
test/libyul/yulOptimizerTests/unusedStoreEliminator/store_before_unconditionally_terminating_function_call.yul Normal file
View File

@ -0,0 +1,23 @@
{
function neverStop() {
if calldataload(0) { leave } // prevent inlining
}
let x := 0
let y := 1
sstore(x, y) // should be removed
neverStop()
sstore(x, y)
}
// ----
// step: unusedStoreEliminator
//
// {
// {
// let x := 0
// let y := 1
// neverStop()
// sstore(x, y)
// }
// function neverStop()
// { if calldataload(0) { leave } }
// }