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 on previously working code (potentially due to nested arrays?) #4383

Closed
dkales opened this issue Feb 15, 2024 · 2 comments · Fixed by #4903
Closed

ICE on previously working code (potentially due to nested arrays?) #4383

dkales opened this issue Feb 15, 2024 · 2 comments · Fixed by #4903
Labels
bug Something isn't working

Comments

@dkales
Copy link

dkales commented Feb 15, 2024

Aim

Checkout https://github.com/TaceoLabs/noir-aes, a basic implementation of AES-128 and execute nargo test.
This worked fine with nargo v0.22.0, but has an ICE with v0.23.0.

Expected Behavior

Runs the tests and does not crash.

Bug

[aes] Testing test_aes_key_schedule... The application panicked (crashed).
Message:  internal error: entered unreachable code: ICE: Should have been in cache v80929 Instruction { instruction: Id(77801), position: 0, typ: Numeric(Unsigned { bit_size: 8 }) }
Location: compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs:1336

Under v0.24.0 there is a different error:

[aes] Testing test_aes_key_schedule... error: Internal Consistency Evaluators Errors:

                    This is likely a bug. Consider Opening an issue at https://github.com/noir-lang/noir/issues
  ┌─ /home/dkales/repos/noir_libs/noir-aes/src/lib.nr:1:1
  │
1 │ use dep::std;
  │ - ICE: Expected "a numeric value", found "[Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 })), Var(AcirVar(381), NumericType(Unsigned { bit_size: 8 }))]"

To Reproduce

  1. git clone https://github.com/TaceoLabs/noir-aes && cd noir-aes
  2. noirup -v v0.23.0 (or noirup -v v0.24.0)
  3. nargo test
  4. Observe ICE.

Project Impact

None

Impact Context

No response

Workaround

None

Workaround Description

No response

Additional Context

The changelog of 0.23 talks about nested slices being banned, however the example just uses nested arrays, so I don;t know if this is related.

Installation Method

Binary (noirup default)

Nargo Version

nargo version = 0.23.0 noirc version = 0.23.0+5be9f9d7e2f39ca228df10e5a530474af0331704 (git version hash: 5be9f9d, is dirty: false)

NoirJS Version

--

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@dkales dkales added the bug Something isn't working label Feb 15, 2024
@akonior
Copy link
Contributor

akonior commented Apr 23, 2024

I've got similar error trying to run nested arrays test:

#[test]
fn test_array_equal() {
    assert([[1]] == [[1]]);
}
Testing test_array_equal... error: Internal Consistency Evaluators Errors:

                    This is likely a bug. Consider opening an issue at https://github.com/noir-lang/noir/issues
  ┌─ /private/tmp/test_nest_array/src/main.nr:1:1
  │
1 │ fn main() {
  │ - ICE: Expected "a numeric value", found "[Var(AcirVar(1), NumericType(NativeField))]"
  │
  = Call stack:
    1. /private/tmp/test_nest_array/src/main.nr:6:12

@jfecher
Copy link
Contributor

jfecher commented Apr 24, 2024

@akonior Thanks for the small example, it was very helpful in fixing the issue. I've made #4903 which should fix your issue and @dkales's issue as well.

github-merge-queue bot pushed a commit that referenced this issue Apr 24, 2024
# Description

## Problem\*

Resolves #4383 (both examples in the issue)

## Summary\*

We were relying on the built in array equality for arrays in SSA
previously, but this did not recurse on the array element in the case of
nested arrays. I've just removed this since it is no longer needed. We
now use the impl Eq for arrays in the stdlib instead since the built in
version provided no speedup anyway.

## Additional Context



## 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.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <tom@tomfren.ch>
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
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants