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

chore: warn optimization is disabled when forge coverage is called #8840

Open
2 tasks done
akshatmittal opened this issue Sep 10, 2024 · 13 comments · May be fixed by #9366
Open
2 tasks done

chore: warn optimization is disabled when forge coverage is called #8840

akshatmittal opened this issue Sep 10, 2024 · 13 comments · May be fixed by #9366
Assignees
Labels
A-compiler Area: compiler C-forge Command: forge Cmd-forge-build Command: forge build Cmd-forge-coverage Command: forge coverage P-normal Priority: normal T-bug Type: bug
Milestone

Comments

@akshatmittal
Copy link

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (ea3ba89 2024-09-05T00:28:03.529093300Z)

What command(s) is the bug in?

No response

Operating System

Windows

Describe the bug

The codebase does not have viaIR enabled.

  • forge compile -> works
  • forge test -> works - all tests pass
  • forge coverage -> fails (error below)
  • forge coverage --ir-minimum -> works but tests fail

Error: (forge coverage)

[⠊] Compiling...
[⠆] Compiling 141 files with Solc 0.8.25
[⠒] Solc 0.8.25 finished in 8.17s
Error: 
Compiler run failed:
Error: Compiler error (C:\Users\circleci\project\libyul\backends\evm\AsmCodeGen.cpp:67):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 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.
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 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.

This is super confusing given that just running the tests is fine but running with coverage doesn't compile and when you enable ir the tests themselves break.

@akshatmittal akshatmittal added T-bug Type: bug T-needs-triage Type: this issue needs to be labelled labels Sep 10, 2024
@grandizzy
Copy link
Collaborator

Please check discussions in #8695 and limitations when using such

@akshatmittal
Copy link
Author

@grandizzy That issue only talks about the via-ir part, but doesn't answer why forge coverage isn't working without ir when forge test works fine.

@grandizzy
Copy link
Collaborator

The difference is that forge coverage disables any optimization, ir-minimum is an option but is not reliable

@akshatmittal
Copy link
Author

Yeah that's also ok, @grandizzy! The codebase compiles and runs through the tests without any optimizations as well. Coverage should also just work but it doesn't.

@grandizzy
Copy link
Collaborator

Interesting, can you share code to reproduce and debug locally?

@grandizzy grandizzy reopened this Sep 10, 2024
@akshatmittal
Copy link
Author

Yeah that's what I want to do, but the codebase is currently private (let me see if I can just make it public) and since the error doesn't tell me which file it is it's hard to create a small reproduction.

@grandizzy
Copy link
Collaborator

ah, too bad. Don't think would help, but at least the foundry.toml to check settings then?

@akshatmittal
Copy link
Author

Here's the foundry.toml, extremely barebones.

[profile.default]
  # Directory Setup
  src = "contracts"
  script = "script"
  test = "test"

  # Compiler Options
  solc_version = "0.8.25"
  evm_version = "shanghai"

  # Permissions
  fs_permissions = [{ access = "read-write", path = "./" }]
  • forge compile -> works
  • forge test -> works - all tests pass
  • forge coverage -> fails

Also, related question, would you know why with forge test all tests pass but with forge coverage --ir-minimum the tests don't pass? (Don't care about source maps here, but ir seems to change test outcome?)

@grandizzy
Copy link
Collaborator

re settings, just a try, if you set optimizer = false dies compile still work?

Re tests not passing, there are several things that could go wrong, one of them being related to block.timestamp / skip / warp, e.g. #8756 (comment)

@akshatmittal
Copy link
Author

Oof, explicitly setting optimizer = false does not work (1 slot too deep, doesn't tell me which file), which explains a lot actually.

Thanks for the insights on ir stuff, so it does seem like ir changes the test outcome if there's any time related action (and possibly more). Well, that sucks. Although I see that most of the tickets linking to solc directly have been closed, I wonder if anything has changed.

This would really suck if this is a compiler issue in the end.

@grandizzy
Copy link
Collaborator

yeah, not sure there's anything we could do at the moment but exploring other paths

@zerosnacks zerosnacks added C-forge Command: forge Cmd-forge-coverage Command: forge coverage Cmd-forge-build Command: forge build A-compiler Area: compiler and removed T-needs-triage Type: this issue needs to be labelled labels Sep 11, 2024
@zerosnacks zerosnacks changed the title Forge test works without via-ir but coverage does not. bug: forge test works without --via-ir but coverage does not Sep 11, 2024
@zerosnacks zerosnacks added this to the v1.0.0 milestone Sep 11, 2024
@jenpaff
Copy link
Collaborator

jenpaff commented Sep 24, 2024

context:

  • forge build has optimizer set to true by default
  • forge coverage runs forge build with optimizer disabled, hence the inconsistency

our proposal would be to add a WARN when forge coverage is called to make this explicit, though as of now we will not be able to provide a fix for that

@jenpaff jenpaff changed the title bug: forge test works without --via-ir but coverage does not chore: warn optimization is disabled when forge coverage is called Sep 24, 2024
@jenpaff jenpaff self-assigned this Sep 24, 2024
@jenpaff jenpaff added this to Foundry Sep 30, 2024
@github-project-automation github-project-automation bot moved this to Todo in Foundry Sep 30, 2024
@zerosnacks zerosnacks self-assigned this Sep 30, 2024
@hochi-ho-partior
Copy link

hochi-ho-partior commented Oct 3, 2024

same happened for me, forge coverage changes the out folder and without noticing it, we tried to deploy the unoptimized contract and failed because of contract size.
That took us quite a bit of time to trace the issue.

@jenpaff
my proposal is that, forge coverage shall either save the artifacts in system's temp folder, or leave them in memory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiler Area: compiler C-forge Command: forge Cmd-forge-build Command: forge build Cmd-forge-coverage Command: forge coverage P-normal Priority: normal T-bug Type: bug
Projects
Status: Ready For Review
6 participants