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

"Stack too deep" in hardhat coverage: YulException: Variable var_amount_1673 is 1 slot(s) too deep inside the stack #13938

Closed
k06a opened this issue Feb 6, 2023 · 14 comments

Comments

@k06a
Copy link

k06a commented Feb 6, 2023

Description

How to understand in which file and line the following error occurred?
https://github.com/1inch/limit-order-protocol/actions/runs/4105345200/jobs/7082068963

YulException: Variable var_amount_1673 is 1 slot(s) too deep inside the stack. 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.

Error in plugin solidity-coverage: HardhatError: HH600: Compilation failed

HardhatPluginError: HardhatError: HH600: Compilation failed
    at SimpleTaskDefinition.action (/home/runner/work/limit-order-protocol/limit-order-protocol/node_modules/solidity-coverage/plugins/hardhat.plugin.js:274:35)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at Environment._runTaskDefinition (/home/runner/work/limit-order-protocol/limit-order-protocol/node_modules/hardhat/src/internal/core/runtime-environment.ts:311:14)
    at Environment.run (/home/runner/work/limit-order-protocol/limit-order-protocol/node_modules/hardhat/src/internal/core/runtime-environment.ts:159:14)
    at main (/home/runner/work/limit-order-protocol/limit-order-protocol/node_modules/hardhat/src/internal/cli/cli.ts:276:7)

Environment

  • Compiler version: 0.8.17
  • Framework/IDE (e.g. Truffle or Remix): Hardhat

Steps to Reproduce

yarn && yarn coverage
@k06a k06a changed the title How to re YulException: Variable var_amount_1673 is 1 slot(s) too deep inside the stack Feb 6, 2023
@cameel
Copy link
Member

cameel commented Feb 6, 2023

Unfortunately, there's no straightforward way. In the codegen, where the error is detected it is unfortunately a bit too late for the compiler to give a good suggestion (#12449).

The go to way to do this is to revert back to the last working version of your code and readd changes one by one to see what triggered it. Then, if you're still getting it with via-IR and optimizer enabled, create a repro like #13906 (comment) and we'll try to fix it.

@k06a
Copy link
Author

k06a commented Feb 6, 2023

@cameel is there any chance to fix compiler to provide additional information?

@cameel
Copy link
Member

cameel commented Feb 6, 2023

For the legacy codegen, probably not. We might try some ideas as a part of #12449 but ultimately the problem boils down to the fact that in the codegen we're no longer dealing with the original source and very often it's hard to tell what in that source triggered it. For example the problem often occurs in the ABI encoding code generated in Yul by the compiler and not directly in your own code.

Our plan (and it's actually something we're committed to solving in Q2: #13721) is to address this in the new codegen to the point that having to debug this is very rare and basically a non-issue.

@k06a
Copy link
Author

k06a commented Feb 6, 2023

@cameel I thought we're using new codegen, since we are using via-ir flag.

@cameel
Copy link
Member

cameel commented Feb 6, 2023

Is this the config file used by that build? I don't see the flag there. But, honestly, I assumed that you're not using because the error message tells you to. I'm actually not sure if that message has it hard-coded or if it actually checks :) If you are using it, then disregard that.

But anyway, giving you the right location is hard in either case. Maybe with viaIR it's slightly easier because if it happens in your code maybe we'd be able to give you the Yul function name that you might be able to map to the Solidity function it was compiled from. Still, the main objective for now is to make this error rare in the first place.

@k06a
Copy link
Author

k06a commented Feb 6, 2023

@cameel
Copy link
Member

cameel commented Feb 7, 2023

@k06a I tried to reproduce the "stack too deep" on that branch and I couldn't, even after trying various earlier commits or commenting out the blocks marked as "stack too deep". Then I noticed that in your CI it only failed on yarn coverage (and that's the command you used in issue description). That does reproduce the problem for me.

Looks like hardhat coverage runs without optimization by default. It has an experimental option called configureYulOptimizer. When I enable it in .solcover.js, it manages to finish without "stack too deep" (though 2 tests fail, likely due to NomicFoundation/hardhat#3365).

@cameel cameel changed the title YulException: Variable var_amount_1673 is 1 slot(s) too deep inside the stack "Stack too deep" in hardhat coverage: YulException: Variable var_amount_1673 is 1 slot(s) too deep inside the stack Feb 7, 2023
@k06a
Copy link
Author

k06a commented Feb 7, 2023

@cameel thank you, seems like you found and resolved an issue we were struggling last months!

@cameel
Copy link
Member

cameel commented Feb 7, 2023

Great!

In that case I'm closing the issue, since this was was actually caused by running with optimizer disabled, even if indirectly :) Please do report it if you run into it with optimizer enabled.

@cameel cameel closed this as not planned Won't fix, can't repro, duplicate, stale Feb 7, 2023
@k06a
Copy link
Author

k06a commented Feb 8, 2023

@cameel I faced this issue again: https://github.com/1inch/limit-order-protocol/actions/runs/4123970772/jobs/7122647334

In this branch: 1inch/limit-order-protocol#204

YulException: Variable var_actualMakingAmount is 1 too deep in the stack [ var_actualMakingAmount var_interaction_7226_length var_order_offset var_orderHash RET var_interaction_7226_offset var_input var_target var_remainingMakingAmount var_threshold RET[checked_mul_uint256] var_order_offset RET[checked_mul_uint256] 0x1fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff RET[fun_getTakingAmount] var_orderHash var_remainingMakingAmount ]

@ekpyron
Copy link
Member

ekpyron commented Feb 8, 2023

The error message indicates that stack-to-memory wasn't possible due to non-memory-safe assembly somewhere (potentially in some imported library only): Consider using memory-safe assembly only and annotating it via 'assembly ("memory-safe") { ... }'.
See https://docs.soliditylang.org/en/develop/assembly.html#memory-safety

@k06a
Copy link
Author

k06a commented Feb 8, 2023

@ekpyron I am almost sure all out assembly is covered with /// @solidity memory-safe-assembly pre-comments

@k06a
Copy link
Author

k06a commented Feb 8, 2023

I can't understand why coverage and test works differently with the same flags. Extra event should not affect this potentially.

@ekpyron
Copy link
Member

ekpyron commented Feb 8, 2023

Hm, ok, yeah, that's true... but yeah - I don't know myself precisely what those coverage builds really do, I can only explain what the compiler says there :-).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants