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][compiler-v2] No warning for unreachable code after abort #14320

Closed
wrwg opened this issue Aug 19, 2024 · 1 comment
Closed

[Bug][compiler-v2] No warning for unreachable code after abort #14320

wrwg opened this issue Aug 19, 2024 · 1 comment
Assignees
Labels
bug Something isn't working compiler-v2 stale-exempt Prevents issues from being automatically marked and closed as stale

Comments

@wrwg
Copy link
Contributor

wrwg commented Aug 19, 2024

We do not detected unreachable code as in:

//# publish
module 0xCAFE::m {
    struct S {}

    public fun f() {
        let _x = S {};
        abort 1;
        let S {} = _x;
    }
}

//# run 0xCAFE::m::f

v1 will produce a warning:

comparison between v1 and v2 failed:
- processed 2 tasks
+ processed 2 tasks
= 
- task 0 'publish'. lines 1-10:
- warning[W09005]: dead or unreachable code
-   ┌─ TEMPFILE:7:9
-   │
- 7 │         let S {} = _x;
-   │         ^^^^^^^^^^^^^ Unreachable code. This statement (and any following statements) will not be executed.
- 
- 
- 
= task 1 'run'. lines 12-12:
= Error: Function execution failed with VMError: {
=     major_status: ABORTED,
=     sub_status: Some(1),
=     location: 0xcafe::m,
=     indices: redacted,
=     offsets: redacted,
= }
= 

Test case is at third_party/move/move-compiler-v2/transactional-tests/tests/optimization/bug_14233_unused_non_droppable.move.

@wrwg wrwg added bug Something isn't working compiler-v2 labels Aug 19, 2024
wrwg added a commit that referenced this issue Aug 19, 2024
Fixes #14233

On stack flush we must not pop values which cannot be dropped. Rather they should be stored in temps. This faciltates scenarios where an `abort` happens before the end of a block. Without this, we pop the value before the abort, leading to a bytecode verification error.

There is still an issue with a missing warning, added #14320 for that.
wrwg added a commit that referenced this issue Aug 19, 2024
Fixes #14233

On stack flush we must not pop values which cannot be dropped. Rather they should be stored in temps. This faciltates scenarios where an `abort` happens before the end of a block. Without this, we pop the value before the abort, leading to a bytecode verification error.

There is still an issue with a missing warning, added #14320 for that.
wrwg added a commit that referenced this issue Aug 19, 2024
Fixes #14233

On stack flush we must not pop values which cannot be dropped. Rather they should be stored in temps. This faciltates scenarios where an `abort` happens before the end of a block. Without this, we pop the value before the abort, leading to a bytecode verification error.

There is still an issue with a missing warning, added #14320 for that.
wrwg added a commit that referenced this issue Aug 19, 2024
Fixes #14233

On stack flush we must not pop values which cannot be dropped. Rather they should be stored in temps. This faciltates scenarios where an `abort` happens before the end of a block. Without this, we pop the value before the abort, leading to a bytecode verification error.

There is still an issue with a missing warning, added #14320 for that.
wrwg added a commit that referenced this issue Aug 19, 2024
* [compiler-v2] Adding and verifying some missing v1 tests

This goes through all urgent bugs in the spreadsheet from issue #13747 and verifies that they have been resolved with recent changes.

* [compiler-v2] Do not pop non-droppables on stack flush

Fixes #14233

On stack flush we must not pop values which cannot be dropped. Rather they should be stored in temps. This faciltates scenarios where an `abort` happens before the end of a block. Without this, we pop the value before the abort, leading to a bytecode verification error.

There is still an issue with a missing warning, added #14320 for that.

* Addressing reviewer feedback

* Fix wrong naming
@brmataptos brmataptos moved this from 🆕 New to Assigned in Move Language and Runtime Sep 5, 2024
@sausagee sausagee added the stale-exempt Prevents issues from being automatically marked and closed as stale label Sep 5, 2024
@brmataptos
Copy link
Contributor

This seems redundant with #11707

@github-project-automation github-project-automation bot moved this from Assigned to ✅ Done in Move Language and Runtime Sep 14, 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 stale-exempt Prevents issues from being automatically marked and closed as stale
Projects
Status: Done
Development

No branches or pull requests

4 participants