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

ICE: unreacheable code #5008

Closed
nventuro opened this issue May 9, 2024 · 3 comments · Fixed by #5009
Closed

ICE: unreacheable code #5008

nventuro opened this issue May 9, 2024 · 3 comments · Fixed by #5009
Labels
bug Something isn't working

Comments

@nventuro
Copy link
Contributor

nventuro commented May 9, 2024

Aim

This is a follow up to #4497. I did some more trimming and arrived at a minimal example that results in a compiler crash.

Bug

The following program crashes:

struct Bar {
    value: Field,
}

struct Foo{
    bar: &mut Bar,
}

impl Foo {
    unconstrained fn crash_fn(self) {}
}

fn main() {
    let foo = Foo { bar: &mut Bar { value: 0 } };

    foo.crash_fn();
}

The error message is

The application panicked (crashed).
Message:  internal error: entered unreachable code: Expected all allocate instructions to be removed before acir_gen
Location: compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs:689

I couldn't distill this down any further. The crash is avoided with any of these changes:

  • if main is unconstrained
  • if crash_fn is not unconstrained
  • if bar takes a & and not a &mut
  • if Bar has no members
  • if crash_fn doesn't take self

So it seems like the bug is related to some odd combination of all of the above.

To Reproduce

Compile the program above.

Project Impact

Blocker

Impact Context

This blocks some currently open PRs, such as AztecProtocol/aztec-packages#4940.

Workaround

None

Workaround Description

No response

Additional Context

No response

Installation Method

Compiled from source

Nargo Version

nargo version = 0.28.0 noirc version = 0.28.0+98eb519766ae219d2f10550944bba83f30f7662c (git version hash: 98eb519766ae219d2f10550944bba83f30f7662c, is dirty: false)

@jfecher
Copy link
Contributor

jfecher commented May 9, 2024

So it seems like the bug is related to some odd combination of all of the above.

Mutable references should be banned from crossing the constrained -> unconstrained boundary. CC @sirasistant. I suspect this somehow getting past that check is what is going wrong here.

@TomAFrench
Copy link
Member

See related issue: #4245

@TomAFrench
Copy link
Member

Looks like the issue is that we only check if the type being passed is a mutable reference rather than it containing any mutable references.

github-merge-queue bot pushed a commit that referenced this issue Jun 20, 2024
… it from constrained code to unconstrained code (#5009)

# Description

## Problem\*

Resolves #5008 

## Summary\*

Thank you to @nventuro for the minimal example and @TomAFrench for
looking into this. As a result it was a quick fix.

## Additional Context

All type system fixes must currently be duplicated in the elaborator so
I've included this there as well.

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: TomAFrench <tom@tomfren.ch>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jun 20, 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
Projects
Status: ✅ Done
3 participants