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

[Bug][move-compiler-v2] stack overflow in call_function_with_many_acquires.move #13250

Closed
brmataptos opened this issue May 11, 2024 · 3 comments · Fixed by #13442 · May be fixed by #13436
Closed

[Bug][move-compiler-v2] stack overflow in call_function_with_many_acquires.move #13250

brmataptos opened this issue May 11, 2024 · 3 comments · Fixed by #13442 · May be fixed by #13436
Assignees
Labels
bug Something isn't working compiler-v2-stable compiler-v2 stale-exempt Prevents issues from being automatically marked and closed as stale

Comments

@brmataptos
Copy link
Contributor

🐛 Bug

We seem to run out of stack compiling this function. V1 succeeds at compiling (although the resulting bytecode is invalid and yields a load failure).

To reproduce

% MOVE_COMPILER_V2=1 UB=1 mycargo test -p bytecode-verifier-transactional-tests -- call_function_with_many_acquires 

yields

    Finished test [unoptimized + debuginfo] target(s) in 0.50s
     Running unittests src/lib.rs (target/debug/deps/bytecode_verifier_transactional_tests-99bb2fb4fd5f20a3)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running tests/tests.rs (target/debug/deps/tests-a56dba04cc5a38ce)

running 1 test

thread '<unknown>' has overflowed its stack
fatal runtime error: stack overflow
error: test failed, to rerun pass `-p bytecode-verifier-transactional-tests --test tests`

Caused by:
  process didn't exit successfully: `/Users/brm/code/aptos-core/target/debug/deps/tests-a56dba04cc5a38ce call_function_with_many_acquires` (signal: 6, SIGABRT: process abort signal)
@brmataptos brmataptos added bug Something isn't working compiler-v2 labels May 11, 2024
@rahxephon89
Copy link
Contributor

rahxephon89 commented May 14, 2024

It happens during rewrite of expression: https://github.com/aptos-labs/aptos-core/blob/main/third_party/move/move-model/src/builder/exp_builder.rs#L1858 and setting RUST_MIN_STACK to 335544320 will make the compilation succeed.

@brmataptos
Copy link
Contributor Author

Thanks. We can perhaps solve this by rewriting ExpData::rewrite_exp_and_pattern to use an expression stack instead of recursion. But that will either involve terrible macros (as V1 had) or some major changes to ExpRewriterFunctions.

@sausagee sausagee added the stale-exempt Prevents issues from being automatically marked and closed as stale label May 20, 2024
@wrwg
Copy link
Contributor

wrwg commented May 23, 2024

Need to be sure

  • it happens also in --release mode

If not its not urgent

brmataptos added a commit that referenced this issue May 27, 2024
…late_seq_recursively() to new translate_seq_iteratively().

Note that the old function created a stack frame for every expr in a sequence, which really shouldn't be necessary.

Fixes #13250 which overflowed stack compiling long sequences.

There are some slight variation in VMError details (exec_state) generated for a few tests.
brmataptos added a commit that referenced this issue May 27, 2024
…late_seq_recursively() to new translate_seq_iteratively().

Note that the old function created a stack frame for every expr in a sequence, which really shouldn't be necessary.

Fixes #13250 which overflowed stack compiling long sequences.

There are some slight variation in VMError details (exec_state) generated for a few tests.
@fEst1ck fEst1ck self-assigned this May 28, 2024
@rahxephon89 rahxephon89 assigned brmataptos and rahxephon89 and unassigned fEst1ck May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler-v2-stable compiler-v2 stale-exempt Prevents issues from being automatically marked and closed as stale
Projects
Status: Done
5 participants