RFC: Explicit safe interfaces for unsafe code

[nmsmith]

This RFC proposes a reworking of Rust's unsafe features to force the programmer to declare (and think about) safety boundaries when they write unsafe code. The goal is to reduce mistakes when working with unsafe code that could ultimately affect program stability and security.

View the formatted version here:
https://github.com/ecl3ctic/rfcs/blob/master/active/0000-explicit-safe-interfaces.md

[huonw]

(FWIW, these aren't being rendered as markdown lists, I think they need to be 1., 2. (i.e. ., not )) and possibly need a newline before the first item.)

[nmsmith]

Fixed it up, thanks.

[bharrisau]

I'd still like to see an RFC for a lint annotating the purpose of an unsafe {} (RawDeref, UnsafeFn, StaticMut) before any other solutions to document unsafe (the #[safe_interface] is really just a lint).

Requiring unsafe blocks in unsafe functions is an interesting idea - but it will get a bit redundant when you are calling an unsafe function from an unsafe function (e.g. a unsafe fn that just wraps a call to an intrinsic). But for the other causes of unsafety it might make sense.

[nmsmith]

@bharrisau #[safe_interface] is not for annotating blocks however, it's for forcing the programmer to declare and acknowledge a safety boundary at the function level. A lint for unsafe {} won't achieve the same goal.

As for requiring unsafe blocks in unsafe functions, this idea has floated around before and gotten some support. The main idea of it is to make it easier to identify which operations in the function are the unsafe ones. It also turns the unsafe keyword on the function signature into a flag, rather than another version of unsafe {}, so unsafe {} becomes the sole location of dirty things.

[bharrisau]

@ecl3ctic Yes, but I imagine this would be implemented as an unsafe_annotation lint that would trigger for functions that contain unsafe blocks unless they were either unsafe or had a #[safe_interface] annotation. Probably defaulting to warn.

Your RFC is really covering three things - require unsafe blocks in unsafe functions, add unsafe fields to structs, and implement a lint and one/some annotations to help document unsafe parts.

And the logic behind having unsafe blocks in unsafe functions is so you can say "the unsafety is here, but I can't contain it to just this function - some of it leaks out".

[nmsmith]

@bharrisau I understand what you're saying, but I don't think a lint is a powerful enough solution. I believe it should be a standardized error, with no option for warn, so that all functions containing unsafe code are properly documented with one of two choices: "this function is unsafe, and I have qualified it as such", or "I claim this function has a safe interface, so I override the requirement for the unsafe qualifier".

An unsafe block lying within a safe function without documentation should not be permitted under any circumstances. It doesn't tell you anything about where the safety boundaries are or how the function should be treated. A programmer can easily put one in and forget about it, leaving it unclear whether the function is really safe. Even if they're used correctly, the lack of clarity makes the code harder to maintain.

[ballard26]

-1: This is the problem with using 'unsafe' for a keyword in cases like this. The only thing 'unsafe' means is that it lies outside of the compiler's ability to verify it. Hence the programer has to state that they verified it independently to the compiler in order to compile it. Adding more extensive warning walls to a notion as simple as that is just useless boilerplate.

[bharrisau]

@ecl3ctic Agreed - in terms of an intellectual point. In practice it will be much more flexible to have it as a lint and projects that want to be hard on safety can #[forbid(unsafe_annotation)].

@ballard26 And for situations where the programmer can't state that it is verified in the scope of that one function? i.e. where they need to 'leak' some of the unsafety for other functions to provide the safety guarantee.

[glaebhoerl]

First, a minor syntactic nit: I don't think safe_interface should be an attribute. If it's of fundamental semantic significance, which it is, then it should also be a first-class part of the syntax. (I'm not sure what that syntax should be - perhaps this would be the right place to use trusted?)

Thinking out loud:

There are a few things about the current design of unsafe which bother me, which I think are very similar to (or even the same as) the motivation for this proposal:

• In fancy theory-speak, unsafe is both the introduction and elimination form for unsafety (by analogy with ADTs, both the constructor and the deconstructor). It's like a self-annihilating particle. It's supposed to mean both "these operations are unsafe" and "but their composition is safe".

• Partly as a consequence, it's not always obvious how widely to scope a given unsafe block. On the one hand, unsafe { } should be scoped as narrowly as possible, to avoid accidentally allowing unsafe operations in what should've been safe code. On the other hand, taking this to its logical conclusion and enclosing only the individual unsafe operation itself in the block, let bar = unsafe { transmute(foo) }, let baz = unsafe { *raw_ptr }, or so on, is also clearly wrong -- at that point unsafe { } is only serving as a syntactic marker, without any kind of semantic meaning.

  The theoretically correct answer here is that the scope of unsafe { } should be exactly the narrowest possible scope such that the totality of the enclosed code is actually safe. But within a function body, taking all of the local and global invariants and reasoning and the borrow checker into account, this isn't always so easy to think about, and the language itself does absolutely nothing to provide any guidance: you can write unsafe around individual operations, or unsafe around the whole body of the function, and the compiler just doesn't give a fig either way.

What this proposal is saying is that unsafe should only mean "these operations are unsafe", and "but their composition is safe" should be stated separately, at the whole function level (with safe_interface).

This seems to be broadly the right kind of idea to me, even if I'm not entirely sure about the particulars. Two specific observations:

• With unsafe now only having a single purpose, to mark unsafe operations, it's now unambiguously the correct choice to scope it as narrowly as possible. Given this, it might be a good idea to allow writing it without braces to refer only to the single operation which follows it: let bar = unsafe transmute(foo);, let baz = unsafe *raw_ptr;, and so on.
• This design loses some granularity: it's no longer possible to cordon off a small part of a function as "containing all of the unsafety"; if you want to express that, you have to factor it out into a separate helper function which you can mark with safe_interface (or trusted, or whatever). This is not necessarily a bad thing: it may even be desirable. But it's worth thinking about.

[shadowmint]

I'm all for requiring unsafe { ... } blocks inside unsafe functions, as this narrows the scope of the unsafe actions and make them more visible when debugging (I also agree with @glaebhoerl that narrowing this down further to a statement level would be useful, although less visible).

However, practically speaking I don't see any meaningful benefit of the #[safe_interface] attribute.

"programmer assurance that a function is safe" is a meaningless statement to me. Obviously no one is going to deliberately write code that is unsafe and broken. By putting it into an unsafe block the programmer is acknowledging that they are stepping outside of the safety rules; practically speaking what does an additional attribute achieve?

At best we could argue that it may encourage some people to use unsafe blocks less; at worst, it has no impact at all; people will simple add the attribute and hide the unsafe behavior just like before.

If idiomatic code does not hide unsafe blocks in function, but rather declares the function unsafe, put it in the style guide and perhaps add a compiler lint like for camel case names.

If we want to make it illegal to have hidden unsafe blocks in a function, make unsafe on function markers mandatory.

This 'it's not ok, but it's actually ok if you like' achieves nothing.

[rpjohnst]

-1, this is just extra boilerplate. @bharrisau "where they need to 'leak' some of the unsafety for other functions to provide the safety guarantee" is what unsafe functions are for in the first place. A non-unsafe function (either using unsafe blocks directly, indirectly, or not) is a declaration by its writer that it is safe.

If you use an unsafe block, you are saying that its contents are safe in any context that function can be called in. If that's not the case, don't use an unsafe block- use an unsafe function.

[lilyball]

👎 This solves nothing. An unsafe {} block is already supposed to contain all unsafety within it, meaning any inputs to the enclosing function should already be validated by the time control passes into the unsafe {} block. All this does is add extra noise.

Also, this is trying to sneak unsafe fields back in, even though that was part of a different (closed) RFC. Unsafe fields are orthogonal to the rest of this RFC, and should not be included. Personally, I think we should revisit the topic of unsafe fields, after the long debates on static mut safety (unsafe fields would fix the UnsafeCell.value issue, which is a big hole in our safety right now). But that should be done as a separate RFC.

[bachm]

I think this RFC is a bit misguided. unsafe blocks already encourage/remind the programmer to be careful. I don't think adding extra annotation improves the situation in any way.

[nmsmith]

@glaebhoerl

First, a minor syntactic nit: I don't think safe_interface should be an attribute. If it's of fundamental semantic significance, which it is, then it should also be a first-class part of the syntax.

It could be, but since its effect is to have no effect (it obviates unsafe), it isn't part of the function signature. The users of the function should be able to treat it like any other safe function.

it's no longer possible to cordon off a small part of a function as "containing all of the unsafety"

I was considering this at first, but I decided against it for two reasons:

1. Putting the safety barriers within the function body might clutter the code (this might not be a big deal in practice).
2. If the barrier is a block within the function body, the inputs and outputs are much harder to see. With the barrier at the function level, all the inputs and outputs are expressed in the function signature (besides statics).

@shadowmint

practically speaking I don't see any meaningful benefit of the #[safe_interface] attribute.

The point is to make the declaration of safety or unsafety explicit. Currently when you put an unsafe block in a safe function the rule is "assume the function is still safe" by default. Under the proposed new rules, it becomes "error by default; have the programmer choose explicitly between safe and unsafe". This will catch people who accidentally (or through lack of understanding) simply drop an unsafe {} block in a safe function to perform some action and then forget about it. It also clearly documents the original intent for future maintainers, so they don't make the wrong assumption. "Assume it's safe" is a dangerous default.

If idiomatic code does not hide unsafe blocks in function, but rather declares the function unsafe, put it in the style guide and perhaps add a compiler lint like for camel case names.

Correct code declares the function unsafe (unless they proclaim a safe interface). This has nothing to do with idiomatic Rust, it's about valid Rust. That's why the proposal forces the programmer to make the choice.

@kballard

This solves nothing. An unsafe {} block is already supposed to contain all unsafety within it, meaning any inputs to the enclosing function should already be validated by the time control passes into the unsafe {} block.

Ideally yes, except nobody seems to be doing this, as evidenced by numerous people claiming unsafe means other things. My proposal splits the input validation (safe interface) requirement into a separate place, and makes it explicit so that the programmer is unable to forget about it and the code is ultimately clearer and safer to maintain. It will make unsafe easier to understand, and will make it harder to make mistakes.

Also, this is trying to sneak unsafe fields back in, even though that was part of a different (closed) RFC.

I'm not sneaking anything in. I did acknowledge that it seems tangential to the rest of the proposal, but it's an important part of the "overhaul" I'm proposing, and Rust's unsafe features certainly aren't complete without it. I'm fine for that to be considered separately (again) if people think it should be. The rest of the RFC can go ahead without it, if we want to leave those holes for now.

[shadowmint]

@ecl3ctic

Correct code declares the function unsafe...

There are plenty of examples of FFI code (eg. malloc) which are accurately safe because they return Option. Are you suggesting these should be tagged unsafe?

The weakness of the decorator is that there are cases where it's valid to have an unsafe block in a function that is safe. The decorator is a work around for these, but by existing it explicitly shows that the 'general rule' that 'functions with unsafe blocks should be unsafe' isn't true.

It should be a lint or a compile error.

A compile error that you can suppress with a decorator doesn't make any sense.

[nmsmith]

There are plenty of examples of FFI code (eg. malloc) which are accurately safe because they return Option. Are you suggesting these should be tagged unsafe?

No, absolutely not! If a function has a safe interface like that, that's when you override the requirement for an unsafe tag with #[safe_interface] to declare (promise) it is safe and to tell the compiler to treat it as a safe function.

I don't completely understand the rest of what you're saying. But to address

it explicitly shows that the 'general rule' that 'functions with unsafe blocks should be unsafe' isn't true

Functions with unsafe blocks should be whatever they need to be. The proposal is to assume they're nothing until the programmer says they're something. Currently we assume they're safe.

[shadowmint]

@ecl3ctic perhaps I've misunderstood.

From the RFC it seemed to be that:

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

Would be a compile error?

Mandate that functions containing unsafe blocks be marked as unsafe by default.

However, by adding the #[safe_interface] decorator, you allow this to compile.

Introduce a #[safe_interface] attribute which permits the exclusion of the unsafe qualifier on functions

So the identical code; one with #[safe_interface] compiles, one without #[safe_interface] does not compile.

ie. You are suppressing a compile error with an attribute.

Isn't that like creating an #[ignore_lifetimes] attribute that lets you mark a function with invalid lifetimes as valid, so the compiler will compile it no questions asked?

I can't think of any other examples in rust where you can suppress compile errors in this way off the top of my head?

[lilyball]

Ideally yes, except nobody seems to be doing this, as evidenced by numerous people claiming unsafe means other things.

What other things are they claiming it means? unsafe's mechanism is to disable some of the compiler's restrictions around things the compiler cannot guarantee are safe. That's what it does. But what the rule for using unsafe is that it must not leak unsafety outside of its scope. If it does, then either its scope needs to be expanded, or ultimately the function it's in needs to become unsafe. Doing anything else breaks the safety guarantees of the language.

unsafe is, by its very nature, a thing that the compiler cannot verify. Since the compiler cannot verify it, then it doesn't really make any sense to try and introduce extra boilerplate around it. That doesn't help the compiler in any way provide more guarantees than it does today, and it won't in any way affect the behavior of existing code. All it does is make unsafe more annoying to use.

As near as I can tell, the only motivation you have for making this change is to try and force programmers to double-check that they're not doing something wrong when they use unsafe. I don't think that's going to work, though; anyone who doesn't already validate inputs before using them in an unsafe block isn't going to start doing so merely because they have to add the extra boilerplate of a #[safe_interface] attribute.

---

The only change that I feel like would actually make a difference is to change the unsafe-block lint default from allow to warn. That way the compiler will issue a warning on every use of an unsafe block, requiring you to #[allow(unsafe-block)] to squelch it. Yes, some people will slap a #![allow(unsafe-block)] at the top of their crate and be done with it, but you can't force people to think about safety if they don't want to. For everyone else, we can encourage the style of attaching #[allow(unsafe-block)] to the enclosing function, similar to your #[safe_interface].

This solution is better than #[safe_interface] for several reasons:

1. It requires no change to the behavior of the language, which is good because I feel like unsafe blocks and unsafe functions already have the correct semantics.
2. It can be disabled on a per-crate or per-module scope if desired (i.e. if your module uses a lot of unsafe code, you can go ahead and disable it for the whole module to cut down on boilerplate).
3. It's a warning, rather than a semantic change to the language or an error (requiring the function to be unsafe when it's not is an error). That seems the most appropriate thing to use for the goal of making the programmer think.

[nmsmith]

@shadowmint

Yes,

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

defaults to a compile error because the programmer hasn't specified whether the function interface is safe or unsafe. Annotating the function with unsafe declares it is unsafe, and annotating with #[safe_interface] declares it is safe. The reason it is an attribute instead of a keyword is that it doesn't form part of the function signature, since it's implying the function is safe which is already true when you omit the unsafe keyword. It's also asymmetric in that it's a programmer guarantee which is effectively overruling the compiler and the safety system.

I also suggested #[override(safe)] to emphasize what the programmer is doing here. It's the point where things go wrong; where safe code meets unsafe code, and where the programmer needs to make sure they don't mess up. I'm claiming it's so important that it needs to be explicit and mandatory, rather than a lint or just "good practice". We currently don't have clear interfaces between our safe and our unsafe code, and assuming all uses of unsafe code are safe by default is madness.

[nmsmith]

@kballard

What other things are they claiming it means?

You can read comments around any discussion of unsafe to find the misconceptions. I don't want to target anyone, but there's a variety of interpretations that (I believe) are incorrect and dangerous. The worst ones are "unsafe just means be really careful when you write the code".

But what the rule for using unsafe is that it must not leak unsafety outside of its scope.

Yes, I agree, or the scope of the enclosing function at least, since nobody is going to wrap a whole 100 line function in unsafe {} even if they technically should.

Since the compiler cannot verify it, then it doesn't really make any sense to try and introduce extra boilerplate around it. That doesn't help the compiler in any way provide more guarantees than it does today

As near as I can tell, the only motivation you have for making this change is to try and force programmers to double-check that they're not doing something wrong when they use unsafe

It's easy today to write some unsafe code to get the results you want, then forget about it because now your program "just works", or at least appears to. Assume the enclosing function is part of some large complex struct, and you go back to the code a month later to make changes. You have same unsafe code sitting there, but the implications of it are unclear. Is that function really going to be safe to call in all the new code you are writing, or was it just safe for what you were doing before? It wasn't documented whether the function exposed a safe interface, because it "just worked" back then and we didn't care beyond that. At no point did we make the choice between safe and unsafe, and the compiler didn't force us or ask us to label the function unsafe because of its contents.

The proposal forces the programmer to do a little more thinking beyond "this works", and leaves it documented whether that function is really safe (by the programmer's word) or actually unsafe. There's no default. Yes the world isn't perfect and there will obviously be times where someone puts anything down to shut the compiler up or just gets it wrong, but that's unavoidable with any solution, and this solution is vastly better than what we have today.

[rpjohnst]

assuming all uses of unsafe code are safe by default is madness.

No, it's the only sane use of unsafe code. What you're describing is a cultural problem, not a technical one- people who will slap unsafes around just to make things work will do the exact same thing with a #[safe_interface] attribute, and get more irritated by it. This problem needs to be solved by clearly explaining the purpose of unsafe in tutorials and documentation.

[thestinger]

Boilerplate teaches people that the compiler is there to get in their way rather than to help them. The unsafe block already documents that there is unsafe code and that it must be correct within the surrounding context. I expect that many memory safety bugs will be in safe code called by an unsafe block anyway, so code in unsafe blocks is hardly the only trusted code. The trusted code consists of everything in unsafe blocks and the graph of functions they call.

[lilyball]

The proposal forces the programmer to do a little more thinking beyond "this works", and leaves it documented whether that function is really safe (by the programmer's word) or actually unsafe.

No it doesn't, and that's the problem. You put for this RFC because you are asserting this to be true, but there is no reason to believe that requiring the #[safe_interface] boilerplate will do anything at all to change the behavior of people who use unsafe without thinking about safety.

If people want to write code unsafely, technical countermeasures will not help. As @rpjohnst stated, this is a cultural problem, and the only solution is to fix documentation to stress the importance of using unsafe correctly, and to set a community expectation that unsafe will be used responsibly and correctly.

[alexcrichton]

Thanks for taking the time to write up your thoughts into an RFC! We discussed this at today's triage meeting, and the conclusion we reached is that the complexity additions to the language don't necessarily outweigh the potential benefits of a new system of treading unsafe code. Many comments have also pointed out that this isn't necessarily a silver bullet to help writing unsafe code.

With this in mind, we've decided to close this for now.

[nmsmith]

Since having written this, I do admit that it isn't a perfect solution (and a better solution may not even exist). Perhaps a big effort towards teaching how to write unsafe code properly will ameliorate most of my concerns.