rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.65k stars 12.75k forks source link

OOB index in async fn evades linter #112850

Open fuse117 opened 1 year ago

fuse117 commented 1 year ago

I am hitting index out of bounds panics when indexing into arrays in macros inside aync functions. An example is the following.

async fn foo() {
    let x: [u8; 1] = [0];
    rprintln!("{:x} {:x}", x[0], x[1]);
}
thread 'main' panicked at 'index out of bounds: the len is 1 but the index is 1', src/main.rs:18:34

This playground showcases the issue. The actual macro I am using is rprintln from rtt-target

matthiaskrgr commented 1 year ago

you have an array x of length 1 : [0]

x[0] returns the 0th / "first" element in the array and x[1] returns the 1th "second" element of the array. This is just a classic array off-by-one bug in your code ^^

pub fn main() {
    let arr: [u8; 1] = [0];
    let _ = arr[1]; // BOOM
}
fuse117 commented 1 year ago

Yes, I know. It is a contrived example. I would expect the compiler to catch it. Remove the async and it does. See this playground

jamesmunns commented 1 year ago

So, @matthiaskrgr, the issue here is that the compile time (lint? check?) DOES fire for sync code, and does NOT fire for async code. I minimized further and the macro use is not needed, it's only sync vs async:

use futures::executor::block_on;

fn main() {
    let _ = block_on(lola());
    // let _ = lol();
}

fn lol() {
    let x: [u8; 1] = [0];
    println!("{}", x[1]);
}

async fn lola() {
    let x: [u8; 1] = [0];
    println!("{}", x[1]);
}

playground link.

If you uncomment the line that calls lol(), the code no longer compiles, whereas with lola(), it is a runtime error with no warning at compile time.

jamesmunns commented 1 year ago

It seems this check is unconditional_panic, so perhaps it would make sense to change the title to "unconditional_panic lint does not detect out of bounds access for async functions at compile time" or similar, to make the issue more clear.

matthiaskrgr commented 1 year ago

Possibly there is some const propagation that shuts off as soon as it sees anything async

saethlin commented 1 year ago

The const prop lint is tripped up by almost anything. For example, this program doesn't produce a const prop error:

fn main() {
    let arr = [0];
    let sli = &arr[..];
    sli[1];
}

But also yes, generators are special-cased in addition: https://github.com/rust-lang/rust/blob/4457658f68b5c165cd76d24b8147a8f77b9f660e/compiler/rustc_mir_transform/src/const_prop_lint.rs#L58-L64

vincenzopalazzo commented 1 year ago

@rustbot claim

vincenzopalazzo commented 1 year ago

But also yes, generators are special-cased in addition:

Yes this is the problem, I should understand why we skip this for generators in MIR and try to fix this asp

if you compile the compiler without the code https://github.com/rust-lang/rust/blob/4457658f68b5c165cd76d24b8147a8f77b9f660e/compiler/rustc_mir_transform/src/const_prop_lint.rs#L58-L64 all works for this simple example.