ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
23.34k stars 5.77k forks source link

[Yul -> EVM code transform] ICE due to stack underflow #13037

Open bshastry opened 2 years ago

bshastry commented 2 years ago
contract C {
    function f0(int8 i0) external {
        assembly
        {
            {
                // Dummy function with 19 input parameters and 17 output parameters
                function af0(ai0, ai1, ai2, ai3, ai4, ai5, ai6, ai7, ai8, ai9, ai10, a1, ai12, ai13, ai14, ai15, ai16, ai17, ai18) -> ao0, ao1, ao2, ao3, ao4, ao5, ao6, ao7, ao8, ao9, ao10, ao11, ao12, ao13, ao14, ao15, ao16
                {
                }
            }
            i0 := 1
        }
    }
}

throws

https://github.com/ethereum/solidity/blob/80d49f37028b13e162951b6b67b0a42f477ba93c/libevmasm/Assembly.cpp#L53

Repro

$ solc --optimize --asm test.sol
ekpyron commented 2 years ago

Note: this does work with --via-ir, so the issue here is (probably) the legacy code transform on inline assembly messing up the stack heights on a stack-too-deep case. But we should definitely investigate why this passes through to this stack height assertion and whether anything might slip through entirely.

NunoFilipeSantos commented 1 year ago

Hey! 👋 What is the impact of this bug? (High, Medium, Low)

ekpyron commented 1 year ago

Unknown. Probably low in the end, but it's weird that it's caught at the wrong place, so it should be investigated why that is and whether anything else can bleed through. We don't actually really need to fix it, since it's only a corner case in legacy codegen, but we need to check if there is a more severe bug hidden in the vicinity of this - so as long as we haven't done that, I'd consider it medium.