noir icon indicating copy to clipboard operation
noir copied to clipboard

Dead Instruction Elimination pass removes out of bounds array sets

Open Thunkar opened this issue 8 months ago • 1 comments

Aim

While moving some tests from constrained to unconstrained so they run faster (we're only checking function logic and assume circuit generation is fine), the code that used to work started failing with "Array index out of bounds" errors.

Expected Behavior

Behavior should be consistent between constrained and unconstrained contexts.

Bug

Out of bounds errors can be masked in certain scenarios, specifically when copying array positions from one to another. However, illegal acceses are always detected as expected in unconstrained functions.

To Reproduce

// Taken from noir-protocol-circuits
fn arr_copy_slice<T, N, M>(src: [T; N], mut dst: [T; M], offset: u32) -> [T; M] {
    for i in 0..dst.len() {
        // We are illegally accessing the src array here if dst.len() > src.len()
        dst[i] = src[i + offset];
    }
    dst
}

unconstrained fn boom<N, M>(src: [Field; N], dst: [Field; M]) -> [Field; M] {
    arr_copy_slice(src, dst, 0)
}


fn main() {
    // This shouldn't work
    let small_array_src = [1; 5];
    let mut big_array_dst = [0; 6];
    let mut result = arr_copy_slice(small_array_src, big_array_dst, 0);

    // Uncomment for fireworks. This *always* get caught in unconstrained land
    //let result = boom();

    // This is fine
    let first = result[0];
    dep::std::println(first);
    // This is also fine, since we are overwriting the "illegal" position.
    // BUT if we comment this line out, fireworks again (since we are now "using" the illegal position)
    result[5] = 1;

    dep::std::println(result[5]);
}

Project Impact

Nice-to-have

Impact Context

No response

Workaround

None

Workaround Description

No response

Additional Context

No response

Installation Method

Compiled from source

Nargo Version

No response

NoirJS Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

Thunkar avatar Jun 20 '24 09:06 Thunkar