Prevent Getting a Raw Pointer to a Temporary

[rylev]

What it does

The borrow checker prevents users from borrowing content from temporaries.

The following code...

fn mistake() {
    let hello = String::from("hello").as_str();
    hello.to_string();
}

gives this compiler error:

error[E0716]: temporary value dropped while borrowed
 --> src/main.rs:4:17
  |
4 |     let hello = String::from("hello").as_str();
  |                 ^^^^^^^^^^^^^^^^^^^^^         - temporary value is freed at the end of this statement
  |                 |
  |                 creates a temporary which is freed while still in use
5 |     hello.to_string();
  |     ----- borrow later used here
  |
  = note: consider using a `let` binding to create a longer lived value

However, the same mistakes with raw pointers is not caught. This is almost always a mistake which can lead to use after free.

Luckily, most standard library APIs are designed to not make this mistake possible, but for some like CString::as_raw there is already a clippy lint.

However, for custom smart pointer types, this issue is not caught by clippy (although it is caught by miri).

Categories (optional)

• Kind: clippy::correctness

What is the advantage of the recommended code over the original code

The use of a pointer to a temporary is almost guaranteed to be incorrect.

Drawbacks

None.

[oli-obk]

We could generalize the CString optimization to a MIR optimization that finds any !Copy stored in temporaries that get a reference taken and end up with a raw pointer somewhere in the process. Let me untangle this sentence:

1. We want to catch some_expression_creating_cstring.as_ptr()
2. We don't want to catch Box::into_raw(whatev)

These two cases are distinguishable because as_ptr takes a reference and into_raw takes a value.

Now there may be situations where the temporary on which as_ptr is invoked was created from a borrowed value (think SomeRefWrapper::new(&something).as_ptr()). And in fact, CString::new does just that, it takes a &str, so we can't just plainly stop linting on temporaries whose creating expression took a reference to something longer lived. While we could check that the owned temporary satisfies a 'static bound, this would also cover cases where the user writes Something::new(&raw something).as_ptr(), which could be fine, as they put a raw pointer into Something and thus have no lifetimes.

So... in order to find something which has no false alarms (so we can make it deny), but also still triggers on realistic code that has bugs (so the lint is still useful and covers all the CString cases we're covering now), we could trigger on all temporary expressions that

• are 'static,
• didn't require raw pointers to get created,
• get their reference taken for a method/function call and result in a raw pointer

We can additionally/alternatively create a #[clippy::foo] attribute that users (and libstd) can apply to their as_ptr like functions so we lint them whenever they are used on a temporary value.

[wesleywiser]

Generalizing this lint to work in all of the erroneous cases would be great! In the short term, it might be good to extend the set of types the existing lint applies to from just CString to other container types in the standard library which I suspect is a lot easier. Someone today asked me why their code isn't working:

let buffer = vec![0; 1024].as_ptr(); 
unsafe { 
    c_func(..., buffer, 1024);
}

and I was sure Clippy caught this but it doesn't.

[flip1995]

The temporary_cstring_as_ptr lint has now been uplifted to rustc. Maybe this extension should also be implemented in rustc. Maybe this extension to the lint should also be implemented there?