Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify role of unsafe functions #179

Open
epage opened this Issue Sep 10, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@epage
Copy link

epage commented Sep 10, 2018

The only things the guidelines say about unsafe functions are:

For getters that do runtime validation such as bounds checking, consider adding unsafe _unchecked variants. Typically those will have the following signatures.

Unsafe functions should be documented with a "Safety" section that explains all invariants that the caller is responsible for upholding to use the function correctly.

Should unsafe be reserved for compiler Undefined Behavior or also bypassing API invariant checks?

@epage

This comment has been minimized.

Copy link
Author

epage commented Sep 10, 2018

Hints towards API invariants:

For getters that do runtime validation such as bounds checking, consider adding unsafe _unchecked variants. Typically those will have the following signatures.

https://rust-lang-nursery.github.io/api-guidelines/print.html#getter-names-follow-rust-convention-c-getter

The second type of operation that requires an unsafe block is calls to unsafe functions. Unsafe functions and methods look exactly like regular functions and methods, but they have an extra unsafe before the rest of the definition. The unsafe keyword in this context indicates the function has requirements we need to uphold when we call this function, because Rust can’t guarantee we’ve met these requirements. By calling an unsafe function within an unsafe block, we’re saying that we’ve read this function’s documentation and take responsibility for upholding the function’s contracts.
...
By inserting the unsafe block around our call to dangerous, we’re asserting to Rust that we’ve read the function’s documentation, we understand how to use it properly, and we’ve verified that we’re fulfilling the contract of the function.

https://doc.rust-lang.org/book/second-edition/ch19-01-unsafe-rust.html

@epage

This comment has been minimized.

Copy link
Author

epage commented Sep 10, 2018

Hints towards undefined behavior:

The reason these operations are relegated to Unsafe is that misusing any of these things will cause the ever dreaded Undefined Behavior. Invoking Undefined Behavior gives the compiler full rights to do arbitrarily bad things to your program.

https://doc.rust-lang.org/nomicon/what-unsafe-does.html

it is trickier to get unsafe code correct because the compiler can’t help uphold memory safety.

https://doc.rust-lang.org/book/second-edition/ch19-01-unsafe-rust.html

@epage

This comment has been minimized.

Copy link
Author

epage commented Sep 10, 2018

An example of a way to break invariants of an API that doesn't result in UB, and as such is explicitly not marked as unsafe: https://doc.rust-lang.org/std/collections/hash_map/struct.HashMap.html

It is required that the keys implement the Eq and Hash traits, although this can frequently be achieved by using #[derive(PartialEq, Eq, Hash)]. If you implement these yourself, it is important that the following property holds:

k1 == k2 -> hash(k1) == hash(k2)

In other words, if two keys are equal, their hashes must be equal.

It is a logic error for a key to be modified in such a way that the key's hash, as determined by the Hash trait, or its equality, as determined by the Eq trait, changes while it is in the map. This is normally only possible through Cell, RefCell, global state, I/O, or unsafe code.

@KodrAus

This comment has been minimized.

Copy link
Collaborator

KodrAus commented Sep 10, 2018

Can we think of any examples of *_unchecked variants within the standard library or elsewhere in the ecosystem that aren't specifically geared around the potential to land in undefined behaviour?

@epage

This comment has been minimized.

Copy link
Author

epage commented Jan 10, 2019

Can we think of any examples of *_unchecked variants within the standard library or elsewhere in the ecosystem that aren't specifically geared around the potential to land in undefined behaviour?

I missed the "in the ecosystem" part of your question. While I don't know of an existing use, this came up because of crate-ci/escargot#6 where I am concerned about the brittleness of some functions and am considering marking them unsafe which led to the creation of this issue.

I do see value in other cases. For example, I remember a rustls talk where it was mentioned that calls to a special function need to be watched for in code reviews to make sure they don't introduce security holes. When I heard this, my automatic reaction was that unsafe would help contributors and maintainers in their effort on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.