Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

viaIR: true, but still got YulException: Variable expr_25 is 1 too deep in the stack #13906

Open
greenlucid opened this issue Jan 31, 2023 · 4 comments
Labels
bug 🐛 medium effort Default level of effort medium impact Default level of impact must have Something we consider an essential part of Solidity 1.0.

Comments

@greenlucid
Copy link

greenlucid commented Jan 31, 2023

Description

I would have expected my code to compile, but I got "too deep in the stack" error. This is because I'm using viaIR: true (as I handle many variables)
I looked around and found this function which I assume handled the error
https://github.com/ethereum/solidity/blob/develop/libyul/backends/evm/OptimizedEVMCodeTransform.cpp#L273

This only happened after my last commit

Environment

  • Compiler version: v0.8.17
  • Target EVM version (as per compiler settings): ? default
  • Framework/IDE (e.g. Truffle or Remix): Hardhat
  • EVM execution environment / backend / blockchain client: Hardhat ?
  • Operating system: Ubuntu 22.10

Steps to Reproduce

setup environment

git clone https://github.com/kleros/stake-curate
git checkout 777dfd5f1bdd810491bcfce2794a8ba5c3cc49b3
cd contracts
yarn

compile

npx hardhat size-contracts

@greenlucid
Copy link
Author

Any suggestions to work around this issue are welcome (should I split the offending code into more functions?)
although in this case, the compiler error is not telling me which function is producing the error, so I can only guess.

I suspect it's this line https://github.com/kleros/stake-curate/blob/777dfd5f1bdd810491bcfce2794a8ba5c3cc49b3/contracts/contracts/StakeCurate.sol#L816

@cameel
Copy link
Member

cameel commented Feb 5, 2023

I reduced this to a minimal example:

solc test.sol --optimize --via-ir --bin
contract C {
    struct S1 {
        uint a;
        uint b;
    }

    struct S2 {
        uint a;
        uint b;
        uint c;
        uint d;
        uint e;
        uint f;
        uint g;
        uint h;
        uint i;
    }

    struct S3 {
        uint a;
        uint b;
        uint c;
        uint d;
        uint e;
        uint f;
        uint g;
        uint h;
        uint i;
        uint j;
        uint k;
    }

    struct S4 {
        uint56 a56;
        uint b;
        uint c;
        uint d;
        uint e;
        uint32 f32;
        uint g;
        uint h;
        uint i;
        uint j;
        uint k;
    }

    struct S5 {
        uint a;
    }

    mapping(uint => S4) s4map;

    function ext1() external pure returns (uint) {}
    function ext2(uint) external returns (bool) {}

    function to32(uint x) internal pure returns (uint32) {
        if (x == 0)
            return 0;
    }

    function main1() external {
        S3(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
    }

    function main2(bytes calldata) external {
        S1 memory s1;
        S2 memory s2;
        S3 memory s3;
        S5 memory s5;

        uint u1 = this.ext1();
        uint u2 = uint32(s1.a);
        uint u3 = s2.a * u2 / 10000;

        this.ext2(s1.a);
        if (u1 + u2 > 0)
            this.ext2(u1);

        payable(address(uint160(s5.a))).send(u1);

        s4map[0] = S4({
            a56: 0,
            b: uint56(s1.b),
            c: 0,
            d: to32(u2 / 10000),
            e: to32(u1 + u1),
            f32: uint32(s3.a),
            g: s2.a,
            h: 0,
            i: to32(u3),
            j: 0,
            k: 0
        });
    }
}

Reproducible down to 0.8.15.

@ekpyron Hopefully this is small enough to debug the EVM transform.

@greenlucid As for a workaround, there seem to be quite a few details that need to be combined to cause this - changing pretty much anything in the above repro makes the error go away and it's not even that small. main2() is based on revealChallenge(). Try moving things around in that function or adding/removing locals. For example simply swapping send() and transfer() lines makes the error go away for me.

@cameel cameel added bug 🐛 medium effort Default level of effort medium impact Default level of impact must have Something we consider an essential part of Solidity 1.0. labels Feb 5, 2023
@ekpyron ekpyron self-assigned this Feb 6, 2023
@greenlucid
Copy link
Author

I find that repro fascinating, and highly specific. Thanks for looking into it. Swapping two lines and having it compile blew my mind

@cameel
Copy link
Member

cameel commented Feb 7, 2023

Well, the annoying thing about the Yul->EVM transform is that having 16 stack slots to play with gives it plenty of headroom in simple examples and the ones to trip it must reach a certain level of complexity. That's hard to test manually. This repro is what you get when you take real code and cut out absolutely everything that's not relevant to the problem - you're still left with quite a lot :) It's not any single feature causing the problem, just this particular arrangement of features. This is also why the error goes away when you keep the same features but just move them around a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 medium effort Default level of effort medium impact Default level of impact must have Something we consider an essential part of Solidity 1.0.
Projects
None yet
Development

No branches or pull requests

3 participants