Skip to content

Commit 116735f

Browse files
committed
Yul expression simplifier: Don't substitute out of scope variables
In some circumstances (in particular if not the AST is not in SSA form), it could happen that the ExpressionSimplifier would substitute out-of-scope variables.
1 parent c79f2cc commit 116735f

File tree

6 files changed

+155
-1
lines changed

6 files changed

+155
-1
lines changed

Changelog.md

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

88
Bugfixes:
99
* Assembler: Fix not using a fixed-width type for IDs being assigned to subassemblies nested more than one level away, resulting in inconsistent `--asm-json` output between target architectures.
10+
* Yul Optimizer: Fix edge case in which invalid Yul code is produced by ExpressionSimplifier due to expressions being substituted that contain out-of-scope variables.
1011

1112
### 0.8.30 (2025-05-07)
1213

libyul/optimiser/ExpressionSimplifier.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <libyul/optimiser/SimplificationRules.h>
2626
#include <libyul/optimiser/OptimiserStep.h>
2727
#include <libyul/optimiser/OptimizerUtilities.h>
28+
#include <libyul/optimiser/Semantics.h>
2829
#include <libyul/AST.h>
2930
#include <libyul/Utilities.h>
3031

@@ -45,7 +46,19 @@ void ExpressionSimplifier::visit(Expression& _expression)
4546
while (auto const* match = SimplificationRules::findFirstMatch(
4647
_expression,
4748
m_dialect,
48-
[this](YulName _var) { return variableValue(_var); }
49+
[this](YulName const& _var) -> AssignedValue const* {
50+
AssignedValue const* value = variableValue(_var);
51+
if (!value || !value->value)
52+
return nullptr;
53+
54+
// check that all variables in the value expression are in current scope
55+
MovableChecker const checker(m_dialect, *value->value);
56+
for (YulName const& referencedVar: checker.referencedVariables())
57+
if (!inScope(referencedVar))
58+
return nullptr; // don't substitute if any referenced var is out of scope
59+
60+
return value;
61+
}
4962
))
5063
{
5164
auto const* evmDialect = dynamic_cast<EVMDialect const*>(&m_dialect);
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
--optimize --yul-optimizations Md[F]vtDI[jxVpjCDpTnvon]TFSI[v]:Etcns --via-ir --bin
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// SPDX-License-Identifier: GPL-3.0
2+
// reproducing https://github.com/ethereum/solidity/issues/16155
3+
4+
pragma solidity ^0.8.19;
5+
6+
contract PlaceholderContract {
7+
function tZhBDeXU4NnUR6(bool assert_in1) internal virtual returns (int128) {
8+
return ((
9+
(assert_in1 && false)
10+
? (int128(638) - int128(932))
11+
: (assert_in1 ? int128(923) : int128(392))
12+
) / ((int128(517) * int128(573)) / (int128(273) % int128(605))));
13+
}
14+
15+
function QtdPeAwoi7LD(
16+
bool assert_in1,
17+
bool assert_in2
18+
) internal pure returns (int128) {
19+
return ((
20+
(assert_in2 || assert_in1)
21+
? (int128(478) % int128(983))
22+
: (int128(298) + int128(316))
23+
) +
24+
(
25+
(assert_in1 && assert_in1)
26+
? (int128(71) / int128(885))
27+
: (-int128(596))
28+
));
29+
}
30+
31+
function fmMG$apfQ14W86hu_3M(
32+
bool assert_out2
33+
) internal virtual returns (bool) {
34+
return assert_out2;
35+
}
36+
37+
function check_entrypoint(
38+
bool /*assert_in0*/,
39+
bool assert_in1,
40+
bool assert_in2,
41+
bool /*assert_in3*/
42+
) public {
43+
unchecked {
44+
bool assert_out1 = (tZhBDeXU4NnUR6(assert_in1) <
45+
QtdPeAwoi7LD(assert_in1, assert_in2));
46+
bool assert_out2 = ((((
47+
false
48+
? (int128(638) - int128(932))
49+
: ((((
50+
(!(!assert_in1))
51+
? int128(923)
52+
: ((int128(392) + (int128(0) & int128(82))) &
53+
((int128(0) + int128(392)) & int128(392)))
54+
) + int128(0)) - (int128(77) & int128(0))) /
55+
(int128(1) | int128(0)))
56+
) /
57+
((int128(573) * int128(517)) /
58+
(-(-(-(int128(0) + (-(int128(273) % int128(605))))))))) <
59+
((
60+
(true && (assert_in2 || assert_in1))
61+
? (((int128(478) % int128(983)) - int128(76)) +
62+
int128(76))
63+
: (int128(298) + int128(316))
64+
) +
65+
(
66+
assert_in1
67+
? (int128(73) +
68+
((((int128(71) - int128(94)) + int128(94)) /
69+
int128(885)) - int128(73)))
70+
: (-int128(596))
71+
))) ||
72+
((((int128(0) |
73+
(
74+
false
75+
? (int128(638) - int128(932))
76+
: ((
77+
(!(!(!(!assert_in1))))
78+
? int128(923)
79+
: (int128(392) &
80+
((int128(82) & int128(0)) +
81+
int128(392)))
82+
) | int128(0))
83+
)) /
84+
((int128(573) * int128(517)) /
85+
((-(-(-(int128(0) - (int128(273) % int128(605)))))) +
86+
int128(0)))) | int128(0)) <
87+
((
88+
(((assert_in2 || assert_in1) && true) && true)
89+
? (int128(478) % int128(983))
90+
: (int128(298) + int128(316))
91+
) +
92+
(
93+
((true && assert_in1) && assert_in1)
94+
? (((int128(71) - int128(94)) + int128(94)) /
95+
int128(885))
96+
: (-int128(596))
97+
))));
98+
assert((assert_out1 == fmMG$apfQ14W86hu_3M(assert_out2)));
99+
}
100+
}
101+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
======= input.sol:PlaceholderContract =======
3+
Binary:
4+
<BYTECODE REMOVED>
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// regression test for https://github.com/ethereum/solidity/issues/16155
2+
{
3+
function identity(value) -> ret { ret := value }
4+
5+
function f() -> ret
6+
{
7+
let id1 := identity(0x01)
8+
let x := 0x05
9+
{
10+
let const_six := 0x06
11+
let id2 := identity(0x01)
12+
x := or(0x06, id2)
13+
}
14+
// check that we don't substitute `or(const_six, id2)` for `x`
15+
ret := or(id1, x)
16+
}
17+
18+
mstore(42, f())
19+
}
20+
// ----
21+
// step: expressionSimplifier
22+
//
23+
// {
24+
// { mstore(42, f()) }
25+
// function identity(value) -> ret
26+
// { ret := value }
27+
// function f() -> ret_1
28+
// {
29+
// let id1 := identity(0x01)
30+
// let x := 0x05
31+
// { x := or(0x06, id1) }
32+
// ret_1 := or(id1, x)
33+
// }
34+
// }

0 commit comments

Comments
 (0)