Pre-RFC: Rust Safety Standard

[hxuhack]

This example by itself does not compromise memory safety. To actually trigger memory safety issues, it must be used together with other unsafe features. Have you seen such cases in real-world projects? If yes, the project might end up with an unnecessary proliferation of unsafe code.

[tczajka]

@hxuhack

This example by itself does not compromise memory safety. To actually trigger memory safety issues, it must be used together with other unsafe features.

The unsafe keyword is ultimately about documenting API requirements and guarantees. The example has API guarantees. Violating guarantees can in turn lead to memory safety violations down the line.

For example, if new_even_unchecked wasn't unsafe, a user of my module might do the following and run into undefined behavior:

fn user() {
    let x = new_even_unchecked(5);
    let y = even_to_u32(x);
    
    match y % 2 {
        0 => println!("all good"),
        _ => {
            /// Safety: We know y is even thanks to `even_to_u32` documentation.
            unsafe { unreachable_unchecked() }
        }
    }
}

[hxuhack]

From my understanding, you cannot trigger undefined behavior without using the unsafe function unreachable_unchecked. Is that correct? In this way, it satisfies the soundness requirement of Rust:

We say that a library (or an individual function) is sound if it is impossible for safe code to cause Undefined Behavior using its public API.

[tczajka]

I'm not saying my library is unsound, I'm saying it's buggy if new_even_unchecked isn't marked unsafe.

But I can also give an example where your "Function Safety Rule 1" causes unsoundness. Suppose I add this to my library:

pub fn half(a: EvenNumber) -> u32 {
    match a.0 % 2 {
        0 => a.0 / 2,
        _ => {
            /// Safety: even number modulo 2 is always 0.
            unsafe { unreachable_unchecked() }
        }
    }
}

new_even_unchecked should be marked unsafe, otherwise half will be unsound.

[hxuhack]

Can we regard this as a problem related to structs rather than free functions? It is connected to the type invariant of the struct and unsafe constructors (if you consider EvenNumber(x) as constructing an instance via the literal constructor of the struct EvenNumber), rather than free functions. new_even_unchecked is a caller of the literal constructor.

If you agree, I will add discussions in Section 3 about such cases.

[tczajka]

Here are a bunch of other issues with your rules:

@hxuhack

Function Safety Rule 2: If a function contains unsafe code, it may be declared safe only if all conditions required for the safe use of that unsafe code are met.

You can never violate conditions for calling unsafe APIs. Defining such a function unsafe doesn't help. Such a function has undefined behavior and is just unsound even if you define it unsafe.

Generally whether a function is unsafe or not is orthogonal to whether it can call unsafe APIs. A function is unsafe when it has requirements on its callers. A function can call unsafe APIs when it has satisfied requirements of those APIs. These are two separate sets of requirements and can be totally unrelated.

@hxuhack

Struct Safety Rule 1: If none associated functions of a struct contain unsafe code, the struct cannot cause undefined behavior and all its associated functions can be declared as safe.

Wrong for the same reason as discussed previously for free functions. There is no need to differentiate rules for free functions vs associated functions. They have the same rules.

I could make new_even_unchecked and to_u32 methods of EvenNumber, and in order to make this code correct I would have to declare new_even_unchecked unsafe. If I don't, to_u32 is buggy because it violates its documented guarantees.

@hxuhack

Struct Safety Rule 3: A constructor can be declared safe if it guarantees that the type invariant of the struct is satisfied; otherwise, it must be declared unsafe.

This contradicts safety rule 1. A "constructor" is an associated function.

@hxuhack

Struct Safety Rule 4: A method that contains no unsafe code can be declared safe if it does not violate the type invariant of the struct.

This also contradicts safety rule 1.

Also all this is too specific. Other reasons for unsafe requirements can exist. For example an associated function on struct A might have to be careful not to violate invariants of a different struct B, of a global variable, etc.

@hxuhack

Struct Safety Rule 5: If a method contains unsafe code, it can be declared safe only if both of the following conditions are met:

• 1. It does not violate the type invariant of the struct.
• 2. All safety requirements of its internal unsafe code are satisfied.

Same thing as in "function safety rule 2". Violating safety requirements of internal unsafe code doesn't just make a function unsafe, it makes the function unsound. Marking it unsafe doesn't solve the problem. You always have to satisfy safety requirements of any unsafe APIs you call, even in functions that are themselves unsafe.

[hxuhack]

Thanks. I see where the disagreement comes from. While you view the problem from the following perspective:

A function is unsafe when it has requirements on its callers.

I am thinking in terms of the presence of root unsafe components, such as a raw pointer dereference, that can propagate upwards. These two perspectives are neither contradictory nor orthogonal; they just lead to very different ways of expressing things. I’ll fix it and see if you find it acceptable.

[robofinch]

@ais523

I've long thought that the answer to this is "no" because a safe trait is not supposed to be able to cause unsoundness if implemented incorrectly, and one example of implementing the trait incorrectly would be by writing an unsafe method with the wrong preconditions

Well, a wrong implementation of Read could dereference a null pointer. The actual constraint is that safe implementations can have whatever logic errors they wish, but must still be sound. So, a safe trait can have unsafe methods, it’s just that the caller would not be able to depend on what exactly the implementer does (beyond it being sound).

@ais523

Due to being a safe trait, an incorrect implementation is not allowed to create undefined behaviour. This trait has been implemented incorrectly in a sense (the unsafe precondition is not the expected one), but the implementation complies with its safety conditions.

I would count “not being able to be soundly called on inputs that the trait said that callers can provide to this trait method” as an example of unsoundness. Loosening conditions is fine, strengthening them is not.

The motivation for having a safe trait with an unsafe method is to allow the callee to depend on the preconditions being satisfied by the caller, for performance and whatnot. I’ve been seeing unsafe trait as placing restrictions on the callee and unsafe fn as placing restrictions on the caller with preconditions and on the callee with postconditions. Since a safe trait cannot place safety requirements on implementers, its unsafe trait methods cannot have postconditions (beyond those expressed in the type system) that can be relied on by unsafe code, but I see no reason why preconditions shouldn’t be allowed.

[ia0]

@ais523

Nothing is forcing an implementation of that trait to have the same safety conditions, though.

It doesn't need to. Either the implementation does not use unsafe operations and then the type system proves that it has the safe version of the signature which is a subtype of all unsafe versions that don't have correctness guarantees (otherwise the trait would be unsafe), or the implementation does use unsafe operations in which case that's the responsibility of those unsafe blocks to only use the documentation safety requirements like any other unsafe function would.

@ais523

and it could be implemented like tihs:

trait Foo {
    /// Safety: `xs` must be at least five elements long.
    unsafe fn get_unchecked(xs: &[Self], _i: usize) -> &Self {
        xs.get_unchecked(4)

This example is extremely confusing. Did you mean impl Foo for Bar or did you really mean a default implementation? In the first case, you can't document safety requirements on an implementation. That's something that goes on the specification because safety requirements is part of the specification. In the second case, I don't see any issue. This is sound.

[tczajka]

@ais523

Nothing is forcing an implementation of that trait to have the same safety conditions, though.

I don't know if it's documented anywhere, but I think implementers of a trait are meant to have the same (or weaker) safety conditions as specified in the trait. Those safety conditions are part of the API. Traits are meant to be used in generic code, after all.

[scottmcm]

@ais523

Something that should probably be listed explicitly: can a safe trait ever have an unsafe method?

Yes, it can.

An unsafe trait is a requirement on the implementer; an unsafe fn is a requirement on the caller.

It's perfectly possible to implement an unsafe fn in a way that doesn't need unsafe code as it doesn't actually use the safety precondition.

Liskov substitution principle - Wikipedia means the implementing type certainly can't add new safety preconditions, but it absolutely can remove them.

[vague]

@increasing

You have to look at the whole module to determine that.

I've implemented such a feature for that

cc Share how safety tags are combined with function grouping/navigation

[hxuhack]

I’ve added Section 2: Rationale, and updated the rules for both free functions and structs. Could you take a look and let me know if this version works for you?

[ais523]

I think this is a good description of the underlying problem.

The safety requirements on an unsafe method are, in effect, part of its signature.

When using safe methods, the compiler automatically checks to ensure that the signature of the implemented method matches the signature of the declaration in the trait – if you try to implement the method with the wrong signature, you get a compile error.

When using unsafe methods, the compiler does not automatically do that check (primarily because the safety requirements are written in #[doc] attributes in English, so ensuring that they match would be very difficult). So writing a safety requirement that doesn't match the trait could lead to unsoundness even if everything else is individually sound.

This creates a task of "check that the safety requirement matches" when using an unsafe method, and the question boils down to "when/where is this task performed?". It can't be checked by the compiler automatically, so it has to be checked by hand. Normally, a requirement on a trait implementation that has to be checked by hand means that the trait itself is marked unsafe (so that the need to write unsafe impl when implementing it is the clue that there's a requirement that needs checking). In this case, though, it would be possible to add a general rule "when implementing an unsafe method, you must follow the safety requirements given by the trait – you can't create your own". The keyword unsafe is still involved (although it's being used in the opposite sense to usual – usually writing unsafe fn places obligations on the callers rather than on the implementation, but in this case there's a requirement on the implementation too).

This implies certain things about programming style, e.g. it suggests that implementations of trait methods should not contain a safety section in their documentation (and probably that Clippy should lint against attempts to add one) because the relevant safety requirements are specified on the trait rather than the implementation.

(All this might seem like nit-picking – but this sort of RFC is exactly the sort of place where we would want to pin down all these unstated rules, even if they're obvious.)

[hxuhack]

@tczajka

You can never violate conditions for calling unsafe APIs. Defining such a function unsafe doesn't help. Such a function has undefined behavior and is just unsound even if you define it unsafe.

In this case, we must declare the function unsafe and propagate the safety requirements to the caller. If the safety requirements, together with the caller's code, ensure that the callee's safety contract is upheld, then the caller is sound, even though it is marked unsafe.

[hxuhack]

@tczajka

Wrong for the same reason as discussed previously for free functions. There is no need to differentiate rules for free functions vs associated functions. They have the same rules.

I could make new_even_unchecked and to_u32 methods of EvenNumber, and in order to make this code correct I would have to declare new_even_unchecked unsafe. If I don't, to_u32 is buggy because it violates its documented guarantees.

I have removed this rule to avoid confusing and potential conflicts.

[hxuhack]

@tczajka

Also all this is too specific. Other reasons for unsafe requirements can exist. For example an associated function on struct A might have to be careful not to violate invariants of a different struct B, of a global variable, etc.

Actually, this is a main reason that motivates this RFC. We have to decouple the safety responsibilities to provide actionable safety guidance; see Section 2.3 for details.

[yaksher]

@hxuhack

/// # Safety:
/// - `x` must be properly aligned for `u32`.
/// - `T` must not have padding in its first 4 bytes.
pub unsafe fn bar<T>(x: T) {
    let p: *const u32 = &x as *const T as *const u32;

The safety requirement here should be something like "T has size at least 4 and does not have padding in its first 4 bytes"—as written, T = u8 would satisfy the requirements while being unsound.

[hxuhack]

T = u8 does not satisfy the alignment requirement. Note that according to type layout, the size of a value is always a multiple of its alignment.

[ais523]

u32 iisn't hard-guaranteed to have 4-byte alignment, though; in theory it could be less. (I don't know of any platforms where it is less; but there are, e.g., platforms where the alignment of u64 is 4 rather than the usual 8.) So this would be unsound if T had the same less-than-4 alignment as u32 but a smaller size.