Skip to content

Support linting c-like fn calls that take pointer-to-slice and length #148664

@estebank

Description

@estebank

Given code like:

use std::env;
use std::process;

fn main() {
    let args: Vec<String> = env::args().collect();
    
    if args.len() != 3 {
        eprintln!("Usage: {} <string> <length>", args[0]);
        process::exit(1);
    }
    
    let buffer = &args[1];
    let Ok(length) = args[2].parse::<usize>() else {
        eprintln!("Error: length must be a valid integer");
        process::exit(1);
    };

    unsafe {
        libc::write(1, buffer.as_ptr() as *const libc::c_void, length);
    }
}

you can cause undefined behavior. It does get caught by miri:

error: Undefined Behavior: memory access failed: attempting to access 1000 bytes, but got alloc1 which is only 4 bytes from the end of the allocation
  --> src/main.rs:21:9
   |
21 |         libc::write(1, buffer.as_ptr() as *const libc::c_void, length);
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

But we could leverage the #[diagnostic::*] namespace to annotate functions that have this kind of C API with independent pointer and length, and then lint if the length argument isn't directly derived from buffer. I would go as far as making the lint simple in its analysis of that and check that length is either buffer.length() or std::cmp::max(buffer.len(), length) and trigger otherwise (and ask people to enable it for anything more complex, at least at the start?).

The annotation would likely look something like:

#[diagnostic::c_buffer_length(arg2, arg3)]
pub fn write(arg1: c_int, arg2: *const c_void, arg3: c_int) -> c_int;

and the output be something like

error: when calling `write`, `length` must be derived from `buffer` to avoid accidental out-of-bounds access
  --> src/main.rs:21:9
   |
LL |   let buffer = &args[1];
   |       ------ `buffer` created here
LL |    let Ok(length) = args[2].parse::<usize>() else {
   |           ------ `length` created here, unrelated to `buffer`
..
LL |         libc::write(1, buffer.as_ptr() as *const libc::c_void, length);
   |                        ---------------                         ^^^^^^ must be related to `buffer`
   |                        |
   |                        a pointer to `buffer` was captured here
   |
   = help: this can cause a bug in the program: `write` could perform an invalid operation, and cause Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
help: consider using the length of the buffer being passed in directly
   |
LL -         libc::write(1, buffer.as_ptr() as *const libc::c_void, length);
LL +         libc::write(1, buffer.as_ptr() as *const libc::c_void, buffer.length());
   |
help: alternatively, if you desire to constrain the buffer up to `length`, then ensure that you account for `length` being longer than the buffer
   |
LL |         libc::write(1, buffer.as_ptr() as *const libc::c_void, std::cmp::max(length, buffer.length()));
   |                                                                ++++++++++++++      ++++++++++++++++++
   |

This lint would is not a reason not to use miri, just a way to catch a possible mistake earlier.

Inspired by https://nitter.net/filpizlo/status/1984366437390303265

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-diagnosticsArea: Messages for errors, warnings, and lintsA-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.D-papercutDiagnostics: An error or lint that needs small tweaks.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions