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

Optimizer = false, raises Stack too deep when compiling inline assembly #10354

Closed
Ferparishuertas opened this issue Nov 19, 2020 · 27 comments
Closed
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. stale The issue/PR was marked as stale because it has been open for too long.

Comments

@Ferparishuertas
Copy link

As mentioned here sc-forks/solidity-coverage#417 (comment), we are trying to execute solidity coverage for a Smart contract set with solidity 7.1, abiencoderv2 and the usage of structs.

We had the contract without structs, and as soon as we put pragma experimental ABIEncoderV2 on the contract, which is over 24kb, the following error appears. (using structs doesnt matter)

CompileError: CompilerError: Stack too deep when compiling inline assembly: Variable headStart is 1 slot(s) too deep inside the stack.

With optimizer it works, but due solidity coverage works without optimizer we cannot have coverage.

Can you help us?

@chriseth
Copy link
Contributor

This cannot easily be fixed because the EVM just does not provide access to enough stack slots. Which problems do you have doing coverage analysis on optimized code?

@Ferparishuertas
Copy link
Author

https://github.com/sc-forks/solidity-coverage works with optimizer=false to inject traces etc.
This is why the problem raised

@chriseth
Copy link
Contributor

i know, but why does it not work when the optimizer is activated? @cgewecke ?

@cgewecke
Copy link

@chriseth

The coverage tool injects instrumentation that pushes a hash onto to the stack by passing it as an argument to a tracing function.

contract A {
  function _trace(bytes32 hash) public pure {} /* Injected trace method def. */

  function someMethod() public {
    _trace(0xabc....) /* tracer called w/ hash for line 5 */ 

Then it hooks into ethereumjs-vm at runtime, watching the stack at each step to record hits in a hash map of lines, branches etc.

I think the optimizer strips the tracing functions out because they're no-ops.

@Ferparishuertas It might be helpful if you gave an example of the code triggers this error - you're the only person to report a stack too deep fault at solidity-coverage for over a year. There could be something unusual about your case.

@chriseth
Copy link
Contributor

Interesting, @cgewecke ! Why don't you use source maps? They are retained during optimization (as far as possible).

@cgewecke
Copy link

cgewecke commented Nov 23, 2020

@chriseth

Yes, agree that's a better design. As a practical matter, it seems like it's been challenging to build a source-map based tracing engine that's able to map the execution path to source files for all cases.

For example, there are some gaps in Buidler/HardhatEVM tracing reported here. (Haven't experimented with truffle debugger's implementation recently - it's possible they've resolved things like this on their end.)

The instrumentation strategy solidity-coverage uses is reasonably reliable for all JS development platforms. But it corrupts the user's code - a passive trace would be the best way of doing this.

@Ferparishuertas
Copy link
Author

We cannot share the Sol file, due License aspects and client requirements :(.
Having said that, due the contract where the error raises is to big 32kb, an quite complex, we will refactor it and keep you posted

@chriseth
Copy link
Contributor

chriseth commented Dec 1, 2020

@cgewecke I would be delighted to know more about the problems! Source maps have been working for remix for years.

@cgewecke
Copy link

cgewecke commented Dec 16, 2020

@chriseth Apologies for the delay circling back here.

Just ran into this issue again while trying to compile aave/protocol-v2 which uses solc 0.6.12.

There are 93 files in the project.

Modifying this compiler config so that optimization is turned off and executing npm run compile results in a single solc error:

[
 {
  "component": "general",
  "formattedMessage": "CompilerError: Stack too deep when compiling inline assembly: Variable headStart is 1 slot(s) too deep inside the stack.\n",
  "message": "Compiler error (/Users/distiller/project/libyul/backends/evm/AsmCodeGen.cpp:253):Stack too deep when compiling inline assembly: Variable headStart is 1 slot(s) too deep inside the stack.",
  "severity": "error",
  "type": "CompilerError"
 }
]

Do you have any advice about which Solidity statements might trigger this? If those can be isolated it might be possible on my end to selectively compile files which require optimization with the appropriate settings.

RE the tracing question. Is there any trick that could be used to reliably map on-chain library code or contract-which-uses-immutable code to a corresponding evm.deployedBytecode compiler output? The use case I'm thinking of is where execution jumps from one contract to another and it's necessary to resolve the source identity of the destination.

@chriseth
Copy link
Contributor

chriseth commented Jan 4, 2021

@cgewecke my guess is that the relevant code is inside the ABI coder, and it might actually be a problem for all files. The more values are decoded at the same time the bigger the stack pressure.

I don't see how determining the deployed bytecode relates to source maps. If you have immutables, you can only simulate the creation - there is no other way that works in all cases. Is the problem that you need to re-compile the contracts because you do not know which compiler artefact corresponds to which contract address?

@andy-uphold
Copy link

Just ran into this issue again while trying to compile aave/protocol-v2 which uses solc 0.6.12.

Same here. Trying to mock the LendingPool but can't compile it.

@dileepfrog
Copy link

dileepfrog commented May 19, 2021

I am hitting this issue as well using Solidity 0.8.4, in which AbiEncoderV2 is enabled by default. Given that struct arguments are now an accepted pattern and that there is really only one coverage tool in the ecosystem AFAIK (thanks @cgewecke!) perhaps this can be solved by adding a flag to the optimizer that selectively ignores particular no-ops for instrumentation purposes?

Or perhaps only the stack optimization flag should be left on by solidity-coverage when using the Yul optimizer, disabling the rest https://docs.soliditylang.org/en/latest/internals/optimizer.html#stackcompressor

@dileepfrog
Copy link

After playing with this for a while disabling the "peephole" optimizer switch while leaving the optimizer on no longer stripped the trace function calls from contracts, although hit maps were not populated for library functions invoked via DELEGATECALL - is the latter a known issue already?

@cgewecke
Copy link

@dileepfrog

hit maps were not populated for library functions invoked via DELEGATECALL - is the latter a known issue already?

No, not as far as I'm aware. Could you give an example of that usage? Is it typical of library calls or something unusual you're doing?

If it's possible to mostly resolve this by setting the config correctly as you suggest that would be great.

@dileepfrog
Copy link

@cgewecke I tried creating an isolated example repo to demo this, however disabling the peephole optimization no longer preserved the trace statements leading to 0 hits across the board - so perhaps simple config modifications are not a complete solution here (I also disabled all of the Yul steps leaving only stackAllocation enabled).

I think it would be a great thing for the community to have 100% test coverage before deploying to mainnet a standard practice (as of writing this very few projects publish any coverage stats at all). @chriseth In pursuit of this what do you think of a bespoke optimizer flag that defeats removing trace no-ops to allow instrumentation?

@cgewecke
Copy link

@chriseth

RE: the meaning of the ComplerError you said above that...

my guess is that the relevant code is inside the ABI coder, and it might actually be a problem for all files. The more values are decoded at the same time the bigger the stack pressure.

Could you explain this in a little more depth?

Does it mean either of the following?

  • If the total size of the abi for a given contract were smaller the error could be avoided?
  • If the total size of the compilation set (e.g. all contracts) were smaller the error could be avoided?

If so that can (likely) be managed on solidity-coverage's side.

@dileepfrog
Copy link

dileepfrog commented May 20, 2021

My understanding of this issue is that the error actually emanates from compilation of the Yul intermediate representation (IR) of a single function rather than from compilation of its Solidity code, making it difficult to track down where exactly it's coming from in the source. Setting settings.optimizer.details.yulDetails.stackAllocation to true fixes the error, but this cannot be enabled without also stripping trace statements (unclear if it's the Yul optimizer or "old" optimizer doing this stripping).

I suspect the inliner optimizer flag (which is currently not possible to disable per #11418) may be the culprit though this is only conjecture.

@dileepfrog
Copy link

I put together a repo that demonstrates the issues at hand:

  1. Running coverage will throw the Yul error mentioned by the OP here.
  2. Modifying this line to config.compilers.solc.settings.optimizer.details = { peephole: false }; will fix the Yul error, but result in 0 coverage hits even though the tx receipt shows the contract function was indeed called.

Hope this helps and looking forward to your findings!

@dileepfrog
Copy link

@cgewecke I just confirmed the following changes to the aforementioned line will in fact allow the Yul stack optimizer to run as well as disable removing the trace no-ops. I motion this issue be resolved in light of this and a corresponding one opened for solidity-coverage to add:

config.compilers.solc.settings.optimizer.enabled = false;
config.compilers.solc.settings.optimizer.details = {
  yul: true,
  yulDetails: {
    stackAllocation: true,
  },
};

@cgewecke
Copy link

cgewecke commented May 24, 2021

@dileepfrog That's so great!! Wow. Thank you for looking into this :)

Closing Continuing at solidity-coverage 636

(Ooops, not my issue)

@Mubashir-ali-baig
Copy link

Mubashir-ali-baig commented Oct 11, 2021

I also encountered the same error and followed the solution @dileepfrog suggested to enable optimizer in truffle-plugin, but now the contract size exceeds from 23 kb to 59.31kb, also is there any way to set the runs value for coverage tool. Btw i'm using hardhat environment so i also enabled the optimizer in hardhat.plugin.js. Would be great if someone suggests a solution.

@chriseth
Copy link
Contributor

If you decrease the "runs" parameter, the size should also go down.

@Mubashir-ali-baig
Copy link

@chriseth compiling my contracts normally works fine, but when i run yarn coverage, the size goes bananas. Also how can i set runs value for coverage tool?. As clearly coverage tool is not taking any effect when i set runs value in hardhat config to 5.

@chriseth
Copy link
Contributor

I guess this question can better be answered in a hardhat channel.

@cgewecke
Copy link

@Mubashir-ali-baig The size goes bananas in the coverage tool because it injects lots of instrumentation into the solidity code. Hardhat's coverage plugin is maintained at sc-forks/solidity-coverage, feel free to open an issue about this there.

@Mubashir-ali-baig
Copy link

@cgwecke Thanks i opened the issue. Is there a way i can set runs manually in the hardhat-plugin.js for coverage tool?

andy-at-mask added a commit to DimensionDev/MysteryBox that referenced this issue Feb 15, 2022
    CompilerError: Stack too deep when compiling inline assembly: Variable headStart is 1 slot(s) too deep inside the stack.
    issue description: ethereum/solidity#10354
    workaround: sc-forks/solidity-coverage#636
andy-at-mask added a commit to DimensionDev/MysteryBox that referenced this issue Feb 18, 2022
    CompilerError: Stack too deep when compiling inline assembly: Variable headStart is 1 slot(s) too deep inside the stack.
    issue description: ethereum/solidity#10354
    workaround: sc-forks/solidity-coverage#636
@NunoFilipeSantos NunoFilipeSantos added the stale The issue/PR was marked as stale because it has been open for too long. label Nov 25, 2022
@github-actions
Copy link

github-actions bot commented Feb 5, 2023

Hi everyone! This issue has been closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Feb 5, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. stale The issue/PR was marked as stale because it has been open for too long.
Projects
None yet
Development

No branches or pull requests

7 participants