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

solidity-stringutils dependency can cause problems with compilation #89

Open
StackOverflowExcept1on opened this issue Jan 12, 2025 · 1 comment · May be fixed by #91
Open

solidity-stringutils dependency can cause problems with compilation #89

StackOverflowExcept1on opened this issue Jan 12, 2025 · 1 comment · May be fixed by #91

Comments

@StackOverflowExcept1on
Copy link

StackOverflowExcept1on commented Jan 12, 2025

We use quite complex code in our tests and most likely run out of space on the stack, but Solidity knows how to solve this problem with StackLimitEvader (by moving the stack into memory). However, solidity-stringutils prevents this because it uses assembly instead of assembly ("memory-safe"). I think openzeppelin-foundry-upgrades should switch from the deprecated solidity-stringutils dependency to vm.split and other funcs: https://github.com/foundry-rs/forge-std/blob/726a6ee5fc8427a0013d6f624e486c9130c0e336/src/Vm.sol#L1132.

$ forge --version
forge 0.3.0 (6f81e76 2025-01-12T00:24:43.972872225Z)

$ make ethexe-contracts-pre-commit
 > Cleaning contracts
 > Formatting contracts
 > Building contracts
[⠊] Compiling...
[⠒] Compiling 128 files with Solc 0.8.28
[⠒] Solc 0.8.28 finished in 13.04s
Error: Compiler run failed:
Error: Yul exception:Cannot swap Variable expr_17 with Variable expr_70667_mpos: too deep in the stack by 1 slots in [ RET expr_17 _16 _15 expr_38 expr_37 expr_36 expr_35 expr_34 expr_33 expr_32 expr_31 expr_30 expr_29 expr_26 expr_14 expr_23 expr_20 expr_70667_mpos ]
No memoryguard was present. Consider using memory-safe assembly only and annotating it via 'assembly ("memory-safe") { ... }'.
YulException: Cannot swap Variable expr_17 with Variable expr_70667_mpos: too deep in the stack by 1 slots in [ RET expr_17 _16 _15 expr_38 expr_37 expr_36 expr_35 expr_34 expr_33 expr_32 expr_31 expr_30 expr_29 expr_26 expr_14 expr_23 expr_20 expr_70667_mpos ]
No memoryguard was present. Consider using memory-safe assembly only and annotating it via 'assembly ("memory-safe") { ... }'.
  --> test/Base.t.sol:71:40:
   |
71 |         Middleware.Config memory cfg = Middleware.Config({
   |                                        ^ (Relevant source part starts here and spans across multiple lines).
@StackOverflowExcept1on
Copy link
Author

After I replaced assembly with assembly ("memory-safe") in lib/openzeppelin-foundry-upgrades/src/internal/Core.sol and lib/openzeppelin-foundry-upgrades/lib/solidity-stringutils/src/strings.sol this problem was solved. However, I don't want to get into researching how safe memory-safe is in solidity-stringutils. Instead, it would be worth just removing this deprecated dependency.

@ericglau ericglau linked a pull request Jan 14, 2025 that will close this issue
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

Successfully merging a pull request may close this issue.

1 participant