Fix NY codegen wth folded guard + inlined for-loop (#561)

Fixes a ICE wth folded guard in inlined for-loop in NY.

Closes #560

---------

Signed-off-by: xermicus <bigcyrill@hotmail.com>

CHANGELOG.md

@@ -7,6 +7,7 @@ Supported `polkadot-sdk` rev: `2604.2.0`
 ### Fixed
 
 - Restored the top-level `revive_version` field in `--standard-json` output and the `resolc_version` field in `--combined-json` output, both of which stopped being populated in `v0.4.0`. [#557](https://github.com/paritytech/revive/pull/557)
+- ICE triggered by `--disable-solc-optimizer --newyork where inlined loops could emit an unterminated basic block in the inlined entrypoint. [#561](https://github.com/paritytech/revive/pull/561)
 
 ## v1.3.0
 

crates/integration/contracts/FoldedGuardZext.yul

@@ -0,0 +1,173 @@
+/// ICE PoC (paritytech/revive#560): the one-armed `if` lowering in `to_llvm`
+/// built the conditional branch first and materialized the fall-through
+/// `inputs` afterwards, with the builder still positioned at the end of the
+/// now-terminated entry block. That is invisible while every input is already
+/// word-typed (no instruction is emitted), but a narrow input — here the raw
+/// `gt` comparison result flowing out of the inlined `fun_h0` — needs a `zext`
+/// to reach the join phi, and that `zext` landed after the terminator:
+/// "Basic Block in function 'fun_run_runtime' does not have terminator!".
+///
+/// The shape needs all of:
+///   - `fun_h0`'s `if 0x01 { ... leave }`: the constant-true guard folds, so
+///     the `leave`-carried return value copy-propagates to the bare `gt`
+///     (an i1 value that is neither a constant nor a word-typed if-output);
+///   - `fun_h1(acc, acc)`: equal arguments fold `iszero(lt(a, b))` to
+///     constant true, collapsing the leave-`if` so the `leave`-elimination
+///     guard around the remainder keeps the raw `gt` as its fall-through
+///     input with a constant-false condition;
+///   - the rotated `for` loop (condition `1`, `break` in the body — solc's
+///     unoptimized lowering) before that guard, which keeps the simplifier
+///     from folding the constant-condition `if` away;
+///   - `fun_run` staying above the never-inline size threshold (the verbose
+///     copy chains are load-bearing): fully inlined into the entry block,
+///     the guard folds and the shape disappears.
+///
+/// This is solc's `--no-optimize-yul` output for issue #560's contract with
+/// the selector dispatch and revert-helper indirections trimmed. `run(s)`
+/// never assigns its unnamed return value, so every call must return 0.
+object "FoldedGuardZext" {
+    code {
+        let size := datasize("FoldedGuardZext_deployed")
+        codecopy(0, dataoffset("FoldedGuardZext_deployed"), size)
+        return(0, size)
+    }
+    object "FoldedGuardZext_deployed" {
+        code {
+            {
+                mstore(64, memoryguard(0x80))
+                external_fun_run()
+                revert(0, 0)
+            }
+            function allocate_unbounded() -> memPtr
+            { memPtr := mload(64) }
+            function cleanup_uint256(value) -> cleaned
+            { cleaned := value }
+            function validator_revert_uint256(value)
+            {
+                if iszero(eq(value, cleanup_uint256(value))) { revert(0, 0) }
+            }
+            function abi_decode_uint256(offset, end) -> value
+            {
+                value := calldataload(offset)
+                validator_revert_uint256(value)
+            }
+            function abi_decode_tuple_uint256(headStart, dataEnd) -> value0
+            {
+                if slt(sub(dataEnd, headStart), 32)
+                {
+                    revert(0, 0)
+                }
+                let offset := 0
+                value0 := abi_decode_uint256(add(headStart, offset), dataEnd)
+            }
+            function abi_encode_uint256_to_uint256(value, pos)
+            {
+                mstore(pos, cleanup_uint256(value))
+            }
+            function abi_encode_uint256(headStart, value0) -> tail
+            {
+                tail := add(headStart, 32)
+                abi_encode_uint256_to_uint256(value0, add(headStart, 0))
+            }
+            function external_fun_run()
+            {
+                if callvalue()
+                {
+                    revert(0, 0)
+                }
+                let param := abi_decode_tuple_uint256(4, calldatasize())
+                let ret := fun_run(param)
+                let memPos := allocate_unbounded()
+                let memEnd := abi_encode_uint256(memPos, ret)
+                return(memPos, sub(memEnd, memPos))
+            }
+            function zero_value_for_split_uint256() -> ret
+            { ret := 0 }
+            function fun_run(var_s) -> var
+            {
+                let zero_uint256 := zero_value_for_split_uint256()
+                var := zero_uint256
+                let _1 := var_s
+                let expr := _1
+                let var_acc := expr
+                let _2 := var_acc
+                let expr_1 := _2
+                let _3 := var_acc
+                let expr_2 := _3
+                let expr_3 := fun_h0(expr_1, expr_2)
+                var_acc := expr_3
+                let _4 := var_acc
+                let expr_4 := _4
+                let _5 := var_acc
+                let expr_5 := _5
+                let expr_6 := fun_h1(expr_4, expr_5)
+                var_acc := expr_6
+            }
+            function fun_h0(var_a, var_b) -> var_r
+            {
+                let zero_t_uint256 := zero_value_for_split_uint256()
+                var_r := zero_t_uint256
+                let expr := 0x01
+                if expr
+                {
+                    var_r := gt(var_r, var_b)
+                    let _1 := var_r
+                    let expr_1 := _1
+                    var_r := expr_1
+                    leave
+                }
+            }
+            function cleanup_t_rational_by(value) -> cleaned
+            { cleaned := value }
+            function identity(value) -> ret
+            { ret := value }
+            function convert_t_rational_by_to_t_uint256(value) -> converted
+            {
+                converted := cleanup_uint256(identity(cleanup_t_rational_by(value)))
+            }
+            function increment_wrapping_uint256(value) -> ret
+            {
+                ret := cleanup_uint256(add(value, 1))
+            }
+            function cleanup_rational_by(value) -> cleaned
+            { cleaned := value }
+            function convert_rational_by_to_uint256(value) -> converted
+            {
+                converted := cleanup_uint256(identity(cleanup_rational_by(value)))
+            }
+            function fun_h1(var_a, var_b) -> var_r
+            {
+                let zero_uint256 := zero_value_for_split_uint256()
+                var_r := zero_uint256
+                let expr := 0x00
+                let var_i := convert_t_rational_by_to_t_uint256(expr)
+                for { }
+                1
+                {
+                    let _1 := var_i
+                    let _2 := increment_wrapping_uint256(_1)
+                    var_i := _2
+                }
+                {
+                    let _3 := var_i
+                    let expr_1 := _3
+                    let expr_2 := 0x01
+                    let expr_3 := lt(cleanup_uint256(expr_1), convert_rational_by_to_uint256(expr_2))
+                    if iszero(expr_3) { break }
+                }
+                let _4 := var_a
+                let expr_4 := _4
+                let _5 := var_b
+                let expr_5 := _5
+                let expr_6 := iszero(lt(cleanup_uint256(expr_4), cleanup_uint256(expr_5)))
+                if expr_6
+                {
+                    let _6 := var_a
+                    let expr_7 := _6
+                    var_r := expr_7
+                    leave
+                }
+            }
+        }
+    }
+}

crates/integration/src/tests.rs

@@ -2844,6 +2844,30 @@ fn switch_callvalue_cse_dangling() {
     run_differential(actions);
 }
 
+/// The one-armed `if` lowering must materialize its fall-through `inputs`
+/// before the conditional branch terminates the entry block: a narrow (i1) input needs a `zext` to
+/// reach the join phi, and emitting it after the branch leaves the block without a trailing
+/// terminator ("Basic Block ... does not have terminator! label %for_join"). Compiling at all
+/// exercises the fix; `run(s)` never assigns its return value, so every call returns 0. See
+/// FoldedGuardZext.yul (paritytech/revive#560).
+#[test]
+fn folded_guard_narrow_if_input_zext() {
+    let mut actions = instantiate_yul("contracts/FoldedGuardZext.yul", "FoldedGuardZext");
+    for s in [U256::ZERO, U256::from(7u64), U256::MAX] {
+        let mut data = vec![0xa4, 0x44, 0xf5, 0xe9];
+        data.extend_from_slice(&s.to_be_bytes::<32>());
+        actions.push(Call {
+            origin: TestAddress::Alice,
+            dest: TestAddress::Instantiated(0),
+            value: 0,
+            gas_limit: None,
+            storage_deposit_limit: None,
+            data,
+        });
+    }
+    run_differential(actions);
+}
+
 /// The panic-pattern outliner must not collapse a window that contains a
 /// side-effecting call. `inner()` reverts with 0xdeadbeef before the panic revert; dropping it and
 /// emitting PanicRevert would revert with the wrong payload. EVM reverts with 0xdeadbeef; the bug

crates/newyork/src/to_llvm.rs

@@ -4714,8 +4714,6 @@ impl<'ctx> LlvmCodegen<'ctx> {
                     }
                 } else {
                     let entry_block = context.basic_block();
-                    context.build_conditional_branch(condition_bool, then_block, join_block)?;
-
                     let mut else_yields = Vec::new();
                     for (index, input_value) in inputs.iter().enumerate() {
                         else_yields.push(self.translate_value_as_word(
@@ -4724,6 +4722,7 @@ impl<'ctx> LlvmCodegen<'ctx> {
                             &format!("input_yield_{}", index),
                         )?);
                     }
+                    context.build_conditional_branch(condition_bool, then_block, join_block)?;
                     phi_incoming.push((else_yields, entry_block));
 
                     context.set_basic_block(then_block);

crates/resolc/src/cli_utils.rs

@@ -15,6 +15,10 @@ pub const SOLIDITY_DEPENDENCY_CONTRACT_PATH: &str = "src/tests/data/solidity/dep
 /// The simple Solidity contract containing i256 divisions and remains
 /// that should be compiled correctly.
 pub const SOLIDITY_LARGE_DIV_REM_CONTRACT_PATH: &str = "src/tests/data/solidity/large_div_rem.sol";
+/// The verbatim reproducer from paritytech/revive#560: a folded guard plus an
+/// inlined for-loop that crashed the newyork pipeline under `--disable-solc-optimizer`.
+pub const SOLIDITY_FOLDED_GUARD_INLINED_LOOP_PATH: &str =
+    "src/tests/data/solidity/folded_guard_inlined_loop.sol";
 
 /// The simple YUL contract test fixture path.
 pub const YUL_CONTRACT_PATH: &str = "src/tests/data/yul/contract.yul";

crates/resolc/src/tests/cli/bin.rs

@@ -2,8 +2,9 @@
 
 use crate::{
     cli_utils::{
-        absolute_path, assert_command_success, execute_command, CommandResult, ResolcOptSettings,
-        SolcOptSettings, SOLIDITY_CONTRACT_PATH, STANDARD_JSON_CONTRACTS_PATH,
+        absolute_path, assert_command_success, execute_command, execute_resolc, CommandResult,
+        ResolcOptSettings, SolcOptSettings, SOLIDITY_CONTRACT_PATH,
+        SOLIDITY_FOLDED_GUARD_INLINED_LOOP_PATH, STANDARD_JSON_CONTRACTS_PATH,
         YUL_MEMSET_CONTRACT_PATH,
     },
     SolcCompiler,
@@ -165,6 +166,20 @@ fn compiles_yul_to_binary_blob() {
     assert_evm_blob_from_yul(&solc_result);
 }
 
+/// Compile regression for [paritytech/revive#560](https://github.com/paritytech/revive/issues/560):
+/// the verbatim Solidity from the issue crashed the newyork pipeline under
+/// `--disable-solc-optimizer` with "Basic Block in function 'fun_run_runtime' does
+/// not have terminator! label %for_join" (the one-armed `if` lowering emitted the
+/// fall-through input `zext` after the branch terminator).
+#[test]
+fn compiles_newyork_folded_guard_inlined_loop() {
+    let path = absolute_path(SOLIDITY_FOLDED_GUARD_INLINED_LOOP_PATH);
+    let arguments = &[&path, "--newyork", "--disable-solc-optimizer", "--bin"];
+
+    let result = execute_resolc(arguments);
+    assert_pvm_blob(&result);
+}
+
 /// This test mimics the command used in the `resolc` benchmarks when compiling Solidity via standard JSON input.
 #[test]
 fn compiles_json_to_binary_blob() {

crates/resolc/src/tests/data/solidity/folded_guard_inlined_loop.sol

@@ -0,0 +1,15 @@
+pragma solidity ^0.8.0;
+contract C {
+  function h0(uint256 a, uint256 b) internal returns (uint256 r) {
+    if (true) { assembly { r := gt(r, b) } return r; }
+  }
+  function h1(uint256 a, uint256 b) internal returns (uint256 r) {
+    for (uint256 i = 0; i < 1; i++) {}
+    if (a >= b) { return a; }
+  }
+  function run(uint256 s) external returns (uint256) {
+    uint256 acc = s;
+    acc = h0(acc, acc);
+    acc = h1(acc, acc);
+  }
+}