Lint against unsafe fn with no unsafe expressions

1

Are there any valid cases where one would make a function unsafe even though it contains no unsafe expressions? If not, I feel it would be good to produce a lint warning against it, in the same way we do for unsafe blocks without unsafe expressions.

If I write:

fn foo() {
    unsafe { just_safe_stuff(); }
}

The compiler will tell me

  |
2 |     unsafe {
  |     ^^^^^^ unnecessary `unsafe` block
  |
  = note: #[warn(unused_unsafe)] on by default

But if I write

unsafe fn foo() {
    just_safe_stuff();
}

The compiler says nothing.

Any arguments for or against creating a new warn-by-default unused_unsafe_fn lint?

2 Likes
2

I think the argument for doing this is that we should aim for least privilege (and a small trusted computing base) and that unnecessarily keeping things unsafe fn when it is no longer necessary would invite adding unsafe bits either knowingly or even worse unknowingly later.

We can likely just extend unused_unsafe to do this; a new name is probably overkill.

1 Like
3

No, I don’t think you can do this in general. unsafe is ultimately about enforcing invariants, and if calling a function could break those invariants such that it could lead to UB anywhere else, then unsafe is appropriate even if the particular implementation of that function doesn’t itself require the use of other unsafe blocks.

For example: https://docs.rs/pcre2/0.1.1/pcre2/bytes/struct.RegexBuilder.html#method.disable_utf_check — And its source shows no obvious requirement for unsafe: https://docs.rs/pcre2/0.1.1/src/pcre2/bytes.rs.html#286-289 — Instead, the requirement is documented as part of the method’s contract.

14 Likes
4 
impl<T> Vec<T> {
  // this function is unsafe, because after calling it, you could use other (safe) operations on Vec to access uninitialized memory
  unsafe fn set_len(len: usize) {
    self.len = len;
  }
}
22 Likes
5

See also any number of new_unchecked inherents that construct an object without checking an invariant, e.g. NonZero::new_unchecked.

6 Likes
6

It is possible to get UB when handling with atomic ordering even without unsafe

7

Related: https://github.com/rust-lang/rfcs/pull/2585

2 Likes
8

Many valid points here. And yes, that RFC is awesome, really hoping for it to be merged.

9

Yeah here's a snippit of one:

  pub const unsafe fn cast<Z>(self) -> VolAddress<Z> {
    VolAddress {
      address: self.address,
      marker: PhantomData,
    }
  }

Casting this pointer type to a new type is just as dangerous as making a fresh value, since you basically are making a fresh value. It might not be aligned, it might not be valid bits. So we mark it unsafe, because it's better to offload the unsafe to value creation rather than value use whenever possible (and it's possible here).

1 Like
10

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.