noir-lang / noir

Noir is a domain specific language for zero knowledge proofs
https://noir-lang.org
Apache License 2.0
897 stars 200 forks source link

Using an untyped global as array length crashes the compiler #6046

Closed Thjarkgh closed 2 months ago

Thjarkgh commented 2 months ago

Aim

I want to be able to derive globals from other globals, and if I accidentially include a typo, I want a error message indicating the line with the faulty global definition.

Expected Behavior

When I derive a global from a non-existing global, I want the compiler to mark that line as an error (IntelliSense should underline it red as well)

Bug

The Noir LSP simply panics:

The application panicked (crashed). Message: index out of bounds: the len is 4918 but the index is 18446744073709551615 Location: compiler/noirc_frontend/src/node_interner.rs:1145

To Reproduce

Did not reproduce it from scratch, but can be easily reproduced in any working project:

global SERIALIZED_OBSTACLES_ARRAY_SIZE = 3; global SERIALIZED_ENEMY_OBSTACLES_ARRAY_SIZE = SERIALIED_OBSTACLES_ARRAY_SIZE + 1;

After a few seconds Intellisense should stop working and a popup should complain about the LSP having crashed 5 times and will not be restarted anymore. In case you are not using Intellisense:

nargo compile

This will cause Noir to panic as well and show the same error message

Workaround

None

Workaround Description

No response

Additional Context

Very annoying, as it is really hard to guess what caused the panic. The missing Z in the above example was really hard to spot. I only noticed it after checking the node_interner.rs file in the Noir github. Only thanks to the compiler source code I was able to find out that I should be looking for an invalid global definition. An error, at least indicating that a global definition caused this would help a lot!

Project Impact

Nice-to-have

Blocker Context

No response

Nargo Version

noirc version = 0.34.0+359caafac5e489901d9ff02b08d1a688178d9b0a

NoirJS Version

0.32.0

Proving Backend Tooling & Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

asterite commented 2 months ago

Thank you for the bug report.

I tried it with the program you mention:

global SERIALIZED_OBSTACLES_ARRAY_SIZE = 3;
global SERIALIZED_ENEMY_OBSTACLES_ARRAY_SIZE = SERIALIED_OBSTACLES_ARRAY_SIZE + 1;

and there is no crash.

I think I know why the crash might happen, but it might be easier to fix it if you could provide the exact code that triggers the crash.

Thank you!

Thjarkgh commented 2 months ago

Well, the total code is a little long - you can clone it from: https://github.com/Thjarkgh/skirmish

compile it and see that it works

then go to circuit/src/main.nr

remove the Z in line 111 (as reported in the original bug report) nargo build get the panic

add the Z again notice that everything works just fine again

asterite commented 2 months ago

Thanks! I was able to reproduce the issue. Here's a code that reproduces it:

global BAR = OOPS;
global X: [Field; BAR] = [];

I'll send a fix soon :-)

Thjarkgh commented 2 months ago

Ah! Yes makes sense, has to be used as an array size, that it? Yes, defined all these things as globals to be able to later on change it more easily (some of these array sizes are subject to change in future versions - as long as everything goes as planed). Thanks for your effort!