Skip to content

fix(frontend)!: Ban zero sized arrays and strings as program input#8491

Merged
vezenovm merged 7 commits intomasterfrom
mv/ban-zero-sized-types-as-main-input
May 14, 2025
Merged

fix(frontend)!: Ban zero sized arrays and strings as program input#8491
vezenovm merged 7 commits intomasterfrom
mv/ban-zero-sized-types-as-main-input

Conversation

@vezenovm
Copy link
Copy Markdown
Contributor

Description

Problem*

Resolves #8451

Summary*

I expanded the is_valid_for_program_input cases for array and string by calling a new array_or_string_len_is_not_zero method.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

Copy link
Copy Markdown
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason to allow these anyway

@vezenovm vezenovm changed the title fix(frontend)!: Ban zero sized arrays and strings as array input fix(frontend)!: Ban zero sized arrays and strings as program input May 14, 2025
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 14, 2025

Changes to number of Brillig opcodes executed

Generated at commit: 253d9056a2e4a12fcce6f34589fe5d5b0eb64e97, compared to commit: b657b076bdcfcf627a318237276a917d8e067e0f

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
regression_8261_inliner_max -17 ✅ -8.46%
regression_8261_inliner_min -17 ✅ -8.46%
regression_8261_inliner_zero -17 ✅ -8.46%

Full diff report 👇
Program Brillig opcodes (+/-) %
regression_8261_inliner_max 184 (-17) -8.46%
regression_8261_inliner_min 184 (-17) -8.46%
regression_8261_inliner_zero 184 (-17) -8.46%

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented May 14, 2025

Changes to Brillig bytecode sizes

Generated at commit: 253d9056a2e4a12fcce6f34589fe5d5b0eb64e97, compared to commit: b657b076bdcfcf627a318237276a917d8e067e0f

🧾 Summary (10% most significant diffs)

Program Brillig opcodes (+/-) %
regression_8261_inliner_max -11 ✅ -8.21%
regression_8261_inliner_min -11 ✅ -8.21%
regression_8261_inliner_zero -11 ✅ -8.21%

Full diff report 👇
Program Brillig opcodes (+/-) %
regression_8261_inliner_max 123 (-11) -8.21%
regression_8261_inliner_min 123 (-11) -8.21%
regression_8261_inliner_zero 123 (-11) -8.21%

@vezenovm
Copy link
Copy Markdown
Contributor Author

vezenovm commented May 14, 2025

regression_8261_inliner_max -17 ✅ -8.46%
regression_8261_inliner_min -17 ✅ -8.46%
regression_8261_inliner_zero -17 ✅ -8.46%

A parameter (empty string) was removed from these tests so this makes sense.

@TomAFrench TomAFrench enabled auto-merge May 14, 2025 09:23
@TomAFrench TomAFrench disabled auto-merge May 14, 2025 09:40
@TomAFrench
Copy link
Copy Markdown
Member

@vezenovm there's a couple of test failures here.

@vezenovm
Copy link
Copy Markdown
Contributor Author

@vezenovm there's a couple of test failures here.

Yeah I'm fixing them right now

@vezenovm
Copy link
Copy Markdown
Contributor Author

vezenovm commented May 14, 2025

One caveat of this PR is that if someone provides an incorrect numeric generic type to an array we will trigger this error as well now as the array type will have failed to resolve. This snippet is compile_failure/brillig_vec_to_acir

global DEPTH: Field = 40000;
fn main(x: [u32; DEPTH], y: u32) {
    let mut new_x = Vec::new();
    new_x = clear(x, y);
}
unconstrained fn clear(x: [u32; DEPTH], y: u32) -> Vec<u32> {
    let mut a = Vec::new();
    for i in 0..y {
        a.push(x[i]);
    }
    a
}

It previously had this stderr:

error: The numeric generic is not of type `u32`
  ┌─ src/main.nr:3:12
  │
3 │ fn main(x: [u32; DEPTH], y: u32) {
  │            ------------ expected `u32`, found `Field`
  │

error: The numeric generic is not of type `u32`
  ┌─ src/main.nr:8:27
  │
8 │ unconstrained fn clear(x: [u32; DEPTH], y: u32) -> Vec<u32> {
  │                           ------------ expected `u32`, found `Field`
  │

error: Call to unconstrained function is unsafe and must be in an unconstrained function or unsafe block
  ┌─ src/main.nr:5:13
  │
5 │     new_x = clear(x, y);
  │             -----------
  │

error: Slices cannot be returned from an unconstrained runtime to a constrained runtime
  ┌─ src/main.nr:5:13
  │
5 │     new_x = clear(x, y);
  │             -----------
  │

Aborting due to 4 previous errors

It now has this stderr:

error: The numeric generic is not of type `u32`
  ┌─ src/main.nr:3:12
  │
3 │ fn main(x: [u32; DEPTH], y: u32) {
  │            ------------ expected `u32`, found `Field`
  │

error: Only sized types may be used in the entry point to a program
  ┌─ src/main.nr:3:12
  │
3 │ fn main(x: [u32; DEPTH], y: u32) {
  │            ------------ Slices, references, or any type containing them may not be used in main, contract functions, or foldable functions
  │

error: The numeric generic is not of type `u32`
  ┌─ src/main.nr:8:27
  │
8 │ unconstrained fn clear(x: [u32; DEPTH], y: u32) -> Vec<u32> {
  │                           ------------ expected `u32`, found `Field`
  │

error: Call to unconstrained function is unsafe and must be in an unconstrained function or unsafe block
  ┌─ src/main.nr:5:13
  │
5 │     new_x = clear(x, y);
  │             -----------
  │

error: Slices cannot be returned from an unconstrained runtime to a constrained runtime
  ┌─ src/main.nr:5:13
  │
5 │     new_x = clear(x, y);
  │             -----------
  │

Aborting due to 5 previous errors

For the purpose of the test I switched to using a u32 type for the array input as that is not what this code is testing. But just noting here.

github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 22, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(licm): Account for nested loops being control dependent when
analyzing outer loops (noir-lang/noir#8593)
chore(refactor): Switch unreachable function removal to use centralized
call graph (noir-lang/noir#8578)
chore(test): Allow lambdas in fuzzing
(noir-lang/noir#8584)
chore: use insta for execution_success stdout
(noir-lang/noir#8576)
chore: use generator instead of zero for ec-add predicate
(noir-lang/noir#8552)
fix: use predicate expression as binary result
(noir-lang/noir#8583)
fix(ssa): Do not generate apply functions when no lambda variants exist
(noir-lang/noir#8573)
chore: put `nargo expand` snapshosts in the same directory
(noir-lang/noir#8577)
chore: Use FxHashMap for TypeBindings
(noir-lang/noir#8574)
chore(experimental): use larger stack size for parsing
(noir-lang/noir#8347)
chore: use insta snapshots for compile_failure stderr
(noir-lang/noir#8569)
chore(inlining): Mark functions with <= 10 instructions and no control
flow as inline always (noir-lang/noir#8533)
chore(ssa): Add weighted edges to call graph, move callers and callees
methods to call graph (noir-lang/noir#8513)
fix(frontend): Override to allow empty array input
(noir-lang/noir#8568)
fix: avoid logging all unused params in DIE pass
(noir-lang/noir#8566)
chore: bump external pinned commits
(noir-lang/noir#8562)
chore(deps): bump base-x from 3.0.9 to 3.0.11
(noir-lang/noir#8555)
chore(fuzz): Call function pointers
(noir-lang/noir#8531)
feat: C++ codegen for msgpack
(noir-lang/noir#7716)
feat(performance): brillig array set optimization
(noir-lang/noir#8550)
chore(fuzz): AST fuzzer to use function valued arguments (Part 1)
(noir-lang/noir#8514)
fix(licm): Check whether the loop is executed when hoisting with a
predicate (noir-lang/noir#8546)
feat: Implement $crate (noir-lang/noir#8537)
fix: add offset to ArrayGet
(noir-lang/noir#8536)
chore: remove some unused enum variants and functions
(noir-lang/noir#8538)
fix: disallow `()` in entry points
(noir-lang/noir#8529)
chore: Remove println in ssa interpreter
(noir-lang/noir#8528)
fix: don't overflow when casting signed value to u128
(noir-lang/noir#8526)
chore(performance): Enable hoisting pure with predicate calls
(noir-lang/noir#8522)
feat(fuzz): AST fuzzing with SSA interpreter
(noir-lang/noir#8436)
chore: Add u1 ops to interpreter, convert Value panics to errors
(noir-lang/noir#8469)
chore: Release Noir(1.0.0-beta.6)
(noir-lang/noir#8438)
chore(fuzz): AST generator to add `ctx_limit` to all functions
(noir-lang/noir#8507)
fix(inlining): Use centralized CallGraph structure for inline info
computation (noir-lang/noir#8489)
fix: remove private builtins from `Field` impl
(noir-lang/noir#8496)
feat: primitive types are no longer keywords
(noir-lang/noir#8470)
fix: parenthesized pattern, and correct 1-element tuple printing
(noir-lang/noir#8482)
fix: fix visibility of methods in `std::meta`
(noir-lang/noir#8497)
fix: Change `can_be_main` to be recursive
(noir-lang/noir#8501)
chore: add SSA interpreter test for higher order functions
(noir-lang/noir#8486)
fix(frontend)!: Ban zero sized arrays and strings as program input
(noir-lang/noir#8491)
fix!: remove `to_be_radix` and `to_le_radix` from stdlib interface
(noir-lang/noir#8495)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <[email protected]>
Co-authored-by: Tom French <[email protected]>
AztecBot added a commit to AztecProtocol/aztec-nr that referenced this pull request May 23, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(licm): Account for nested loops being control dependent when
analyzing outer loops (noir-lang/noir#8593)
chore(refactor): Switch unreachable function removal to use centralized
call graph (noir-lang/noir#8578)
chore(test): Allow lambdas in fuzzing
(noir-lang/noir#8584)
chore: use insta for execution_success stdout
(noir-lang/noir#8576)
chore: use generator instead of zero for ec-add predicate
(noir-lang/noir#8552)
fix: use predicate expression as binary result
(noir-lang/noir#8583)
fix(ssa): Do not generate apply functions when no lambda variants exist
(noir-lang/noir#8573)
chore: put `nargo expand` snapshosts in the same directory
(noir-lang/noir#8577)
chore: Use FxHashMap for TypeBindings
(noir-lang/noir#8574)
chore(experimental): use larger stack size for parsing
(noir-lang/noir#8347)
chore: use insta snapshots for compile_failure stderr
(noir-lang/noir#8569)
chore(inlining): Mark functions with <= 10 instructions and no control
flow as inline always (noir-lang/noir#8533)
chore(ssa): Add weighted edges to call graph, move callers and callees
methods to call graph (noir-lang/noir#8513)
fix(frontend): Override to allow empty array input
(noir-lang/noir#8568)
fix: avoid logging all unused params in DIE pass
(noir-lang/noir#8566)
chore: bump external pinned commits
(noir-lang/noir#8562)
chore(deps): bump base-x from 3.0.9 to 3.0.11
(noir-lang/noir#8555)
chore(fuzz): Call function pointers
(noir-lang/noir#8531)
feat: C++ codegen for msgpack
(noir-lang/noir#7716)
feat(performance): brillig array set optimization
(noir-lang/noir#8550)
chore(fuzz): AST fuzzer to use function valued arguments (Part 1)
(noir-lang/noir#8514)
fix(licm): Check whether the loop is executed when hoisting with a
predicate (noir-lang/noir#8546)
feat: Implement $crate (noir-lang/noir#8537)
fix: add offset to ArrayGet
(noir-lang/noir#8536)
chore: remove some unused enum variants and functions
(noir-lang/noir#8538)
fix: disallow `()` in entry points
(noir-lang/noir#8529)
chore: Remove println in ssa interpreter
(noir-lang/noir#8528)
fix: don't overflow when casting signed value to u128
(noir-lang/noir#8526)
chore(performance): Enable hoisting pure with predicate calls
(noir-lang/noir#8522)
feat(fuzz): AST fuzzing with SSA interpreter
(noir-lang/noir#8436)
chore: Add u1 ops to interpreter, convert Value panics to errors
(noir-lang/noir#8469)
chore: Release Noir(1.0.0-beta.6)
(noir-lang/noir#8438)
chore(fuzz): AST generator to add `ctx_limit` to all functions
(noir-lang/noir#8507)
fix(inlining): Use centralized CallGraph structure for inline info
computation (noir-lang/noir#8489)
fix: remove private builtins from `Field` impl
(noir-lang/noir#8496)
feat: primitive types are no longer keywords
(noir-lang/noir#8470)
fix: parenthesized pattern, and correct 1-element tuple printing
(noir-lang/noir#8482)
fix: fix visibility of methods in `std::meta`
(noir-lang/noir#8497)
fix: Change `can_be_main` to be recursive
(noir-lang/noir#8501)
chore: add SSA interpreter test for higher order functions
(noir-lang/noir#8486)
fix(frontend)!: Ban zero sized arrays and strings as program input
(noir-lang/noir#8491)
fix!: remove `to_be_radix` and `to_le_radix` from stdlib interface
(noir-lang/noir#8495)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <[email protected]>
Co-authored-by: Tom French <[email protected]>
Thunkar pushed a commit to AztecProtocol/aztec-packages that referenced this pull request May 23, 2025
Automated pull of nightly from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(licm): Account for nested loops being control dependent when
analyzing outer loops (noir-lang/noir#8593)
chore(refactor): Switch unreachable function removal to use centralized
call graph (noir-lang/noir#8578)
chore(test): Allow lambdas in fuzzing
(noir-lang/noir#8584)
chore: use insta for execution_success stdout
(noir-lang/noir#8576)
chore: use generator instead of zero for ec-add predicate
(noir-lang/noir#8552)
fix: use predicate expression as binary result
(noir-lang/noir#8583)
fix(ssa): Do not generate apply functions when no lambda variants exist
(noir-lang/noir#8573)
chore: put `nargo expand` snapshosts in the same directory
(noir-lang/noir#8577)
chore: Use FxHashMap for TypeBindings
(noir-lang/noir#8574)
chore(experimental): use larger stack size for parsing
(noir-lang/noir#8347)
chore: use insta snapshots for compile_failure stderr
(noir-lang/noir#8569)
chore(inlining): Mark functions with <= 10 instructions and no control
flow as inline always (noir-lang/noir#8533)
chore(ssa): Add weighted edges to call graph, move callers and callees
methods to call graph (noir-lang/noir#8513)
fix(frontend): Override to allow empty array input
(noir-lang/noir#8568)
fix: avoid logging all unused params in DIE pass
(noir-lang/noir#8566)
chore: bump external pinned commits
(noir-lang/noir#8562)
chore(deps): bump base-x from 3.0.9 to 3.0.11
(noir-lang/noir#8555)
chore(fuzz): Call function pointers
(noir-lang/noir#8531)
feat: C++ codegen for msgpack
(noir-lang/noir#7716)
feat(performance): brillig array set optimization
(noir-lang/noir#8550)
chore(fuzz): AST fuzzer to use function valued arguments (Part 1)
(noir-lang/noir#8514)
fix(licm): Check whether the loop is executed when hoisting with a
predicate (noir-lang/noir#8546)
feat: Implement $crate (noir-lang/noir#8537)
fix: add offset to ArrayGet
(noir-lang/noir#8536)
chore: remove some unused enum variants and functions
(noir-lang/noir#8538)
fix: disallow `()` in entry points
(noir-lang/noir#8529)
chore: Remove println in ssa interpreter
(noir-lang/noir#8528)
fix: don't overflow when casting signed value to u128
(noir-lang/noir#8526)
chore(performance): Enable hoisting pure with predicate calls
(noir-lang/noir#8522)
feat(fuzz): AST fuzzing with SSA interpreter
(noir-lang/noir#8436)
chore: Add u1 ops to interpreter, convert Value panics to errors
(noir-lang/noir#8469)
chore: Release Noir(1.0.0-beta.6)
(noir-lang/noir#8438)
chore(fuzz): AST generator to add `ctx_limit` to all functions
(noir-lang/noir#8507)
fix(inlining): Use centralized CallGraph structure for inline info
computation (noir-lang/noir#8489)
fix: remove private builtins from `Field` impl
(noir-lang/noir#8496)
feat: primitive types are no longer keywords
(noir-lang/noir#8470)
fix: parenthesized pattern, and correct 1-element tuple printing
(noir-lang/noir#8482)
fix: fix visibility of methods in `std::meta`
(noir-lang/noir#8497)
fix: Change `can_be_main` to be recursive
(noir-lang/noir#8501)
chore: add SSA interpreter test for higher order functions
(noir-lang/noir#8486)
fix(frontend)!: Ban zero sized arrays and strings as program input
(noir-lang/noir#8491)
fix!: remove `to_be_radix` and `to_le_radix` from stdlib interface
(noir-lang/noir#8495)
END_COMMIT_OVERRIDE

---------

Co-authored-by: AztecBot <[email protected]>
Co-authored-by: Tom French <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compiler crash: call_data and empty arrays

3 participants