abi.decode failing with "Stack too deep" for a list of structs when optimizations are enabled

thedavidmeister commented 2 years ago

Description

Abi decoding SomeStruct[] fails with

CompilerError: Stack too deep. Try compiling with `--via-ir` (cli) or the equivalent `viaIR: true` (standard JSON) while enabling the optimizer. Otherwise, try removing local variables. When compiling inline assembly: Variable headStart is 3 slot(s) too deep inside the stack.K

Strangely, wrapping it in a struct with a SomeStruct field on it "fixes" the issue

https://github.com/beehive-innovation/rain-protocol/pull/452/files#diff-e5cf60434f03c13e62d93df0932957c2208a64a15065489486b9aea0d158e723R9

SomeStruct[] memory foo_ = abi.decode(data_, (SomeStruct[]));

fails, as does

struct Foo {
  SomeStruct[] bar;
}

but this passes

struct Foo {
  SomeStruct bar;
  SomeStruct[] baz;
}

The issue only appears with compiler optimizations enabled and appears for both IR and non-IR

Environment

Compiler version: 0.8.17
Target EVM version (as per compiler settings):
Framework/IDE (e.g. Truffle or Remix): hardhat
EVM execution environment / backend / blockchain client:
Operating system:

Steps to Reproduce

https://github.com/beehive-innovation/rain-protocol/pull/452/files#diff-e5cf60434f03c13e62d93df0932957c2208a64a15065489486b9aea0d158e723R9

thedavidmeister commented 2 years ago

cc @cameel

nikola-matic commented 1 year ago

Hi @thedavidmeister, and sorry for the late reply. If you have a minimal reproduction scenario, feel free to open another issue; if not, unfortunately, we don't have the time at the moment to sift through a 15k line PR to do so ourselves, especially seeing as we already have a set of such repros specifically for via-ir.

cameel commented 1 year ago

To add more context, it's not that we don't need more repros - please do report bugs when you run into them - just in this particular case we already have #13906, which could end up being the same thing (it's not certain though and a new repro would still be nice to have). At the same time this has been sitting here for a while untouched because, reducing a 15k LOC project into a small example is quite tedious. It's not hard, just needs some effort, and we'd be happy to reopen this if you could get us something more along the lines of https://github.com/ethereum/solidity/issues/13906#issuecomment-1417335621. You can do that by starting with what you have and gradually removing stuff. Removing those imports, contracts, functions, function bodies, comments that can be removed while still triggering the bug. Otherwise it's difficult to even look into the problem because a repro of this size is hellish to debug.

cameel commented 1 year ago

Actually, let's keep this one open, even if the repro is subpar. It's clearly a bug, and we have no clear evidence it's a duplicate so we should at least retry this once we fix #13906.

thedavidmeister commented 1 year ago

@cameel the PR is mostly irrelevant, i can reproduce the bug in that repo just by removing the dummy struct

struct FlowConfig {
    // https://github.com/ethereum/solidity/issues/13597
    EvaluableConfig dummyConfig;
    EvaluableConfig[] config;
}

where evaluableConfig is

struct EvaluableConfig {
    IExpressionDeployerV1 deployer;
    bytes[] sources;
    uint256[] constants;
}

i haven't stood up a repo for it, but maybe this is small enough to repro, and if it doesn't repro with this alone then that's a clue too probably

cameel commented 1 year ago

We need a complete self-contained example that we can compile and reproduce the error. This snippet alone is not enough unless it reproduces it on its own. The thing with "Stack too deep" errors is that the thing that seems to trigger it is just just a random thing that tips it over, not the cause in itself. The actual cause is the particular complex arrangement you have in your code (and that's also the reason why the compiler cannot really tell you where exactly it happens, there's no single place). We need to look at the code as a whole and that's only feasible when it's a relatively small example. If it's too large, we have to reduce it ourselves by removing stuff until there's nothing more we can remove.

thedavidmeister commented 1 year ago

@cameel https://github.com/rainprotocol/rain.flow/pull/7/files#diff-ecc19ce33f6459121b466a0713880a45c20c265623a39cdc7753699d3450bd01L9 weirdly i moved the code to a separate repo and removed the wrapper struct that was previously needed to compile, and now it compiles

main differences other than the repo that i can see

using foundry instead of hh
using solc 0.8.19 instead of 0.8.17
i made some other seemingly minor/unrelated changes to the code like documentation and updating some dependencies

ethereum / solidity