Rust: `#![deny(unsafe_op_in_unsafe_fn)]` in libstd

[LeSeulArtichaut]

The goal of this effort is to #![deny(unsafe_op_in_unsafe_fn)] in all of libstd, as proposed in rust-lang/compiler-team#317. This means enclosing unsafe operations in unsafe functions inside unsafe blocks, and documenting them as much as possible.
However, as libstd contains more than 100,000 lines (!!!) and 600 unsafe functions, this should be done step by step, and by multiple people. This issue is meant as a way of tracking and synchronizing progress.

cc @nikomatsakis @RalfJung

Mentoring instructions (or rather, suggested workflow)

Please first leave a comment here stating that you want to work on file xxx.rs or module xxx, to make sure that this implements Sync.

You'll first want to add a #![deny(unsafe_op_in_unsafe_fn)] attribute in the scope you'll be working on.
Then, add unsafe blocks around unsafe operations in unsafe functions. These unsafe operations can be found either by searching for unsafe fns, or by running ./x.py check src/libstd and looking at the errors.
When adding an unsafe block, try to explain why it is safe in a safety comment before the unsafe block. This should look like:

// SAFETY: explain why `unsf` is safe here...
unsafe {
    unsf();
}

Example PRs: #72709 (for liballoc) and #73622 (for libcore)

TODO list

• [x] alloc.rs (@poliorcetics, #74333)
• [x] fs.rs (@eltonlaw, #73909)
• [x] panicking.rs (@poliorcetics, #74200)
• [x] path.rs (@hellow554, #73963)
• [x] primitive_docs.rs
• [x] process.rs (@hellow554, #73955)
• [ ] sys_common/ (@LeSeulArtichaut, #73928)
• [x] net/tcp.rs (@ryr3, #73962)
• [ ] io/ (@ryr3)
• [x] ffi/c_str.rs (@poliorcetics, #74062)
• [x] sync/mpsc (@poliorcetics, #74278)
• [x] thread/ (@poliorcetics, #74225)
• [ ] sys/
  
  
  
  • [ ] sys/cloudabi (@chansuke, #75115)
  
  
  • [ ] sys/hermit (@maekawatoshiki, #74979)
  
  
  • [x] sys/sgx (@Caduser2020)
  
  
  • [ ] sys/unix (@euclio)
  
  
  • [x] sys/unsupported (@m-ou-se, #77722)
  
  
  • [ ] sys/vxworks
  
  
  • [ ] sys/wasm (@chansuke, #74477)
  
  
  • [x] sys/wasi (@Amjad50, #75971)
  
  
  • [ ] sys/windows (@carbotaniuman, #76676)
  
  

[euclio]

I'd like to work on sys/unix.

[eltonlaw]

I'd like to work on fs.rs

[ryr3]

I'd like to take up net/tcp.rs

[hellow554]

Took care of process.rs in #73955

What do you expect for primitive_docs.rs?

[LeSeulArtichaut]

What do you expect for primitive_docs.rs?

I built that list by running ripgrep to find occurrences of "unsafe fn" and blindly copied it. You’re right, there’s nothing to do, obviously since everything is comments :D

[retep998]

I'd like to be pinged on any PRs that touch windows bits.

[hellow554]

FYI: #73909 has been merged and now #![feature(unsafe_block_in_unsafe_fn)] is activated in libstd/lib.rs

[ryr3]

Can I try working on the io part once this is merged?
This one-liner to open the files that contain unsafe fns might be useful to someone working on this. Just thought I'd share!

grep -l -d recurse 'unsafe fn' io | xargs vim # Or your editor

[LeSeulArtichaut]

Can I try working on the io part once this is merged?

@ryr3 please go ahead!

[ryr3]

I started to work on it and saw that some files required other crates which need to be changed first.
For example, stdio imports thread/local.rs which has many unsafe fns. This leads to an error when running ./x.py check src/libstd
Is there any way to get around this, or will thread/local.rs have to be fixed first?

[LeSeulArtichaut]

I started to work on it and saw that some files required other crates which need to be changed first.
For example, stdio imports thread/local.rs which has many unsafe fns. This leads to an error when running ./x.py check src/libstd
Is there any way to get around this, or will thread/local.rs have to be fixed first?

I think that's only macros being problematic. You cannot put unsafe blocks in the macro, because it will make use sites which aren't using #[deny(unsafe_op_in_unsafe_fn)] fail to compile because of the "unnecessary unsafe block" warning. And there's no quick fix, because the macros defined in thread/local.rs are also used in other crates so you can't just slap an #[allow(unsafe_op_in_unsafe_fn)] in the macro. I think this is definitely an issue, but which is going to disappear when we stabilize #![feature(unsafe_block_in_unsafe_fn)]. cc @nikomatsakis for advice on this.

[LeSeulArtichaut]

Also cc @RalfJung as this problem might have an impact on the design of the feature.

[RalfJung]

For unsafe blocks in macros, AFAIK it is fairly common to add #[allow(unused_unsafe)] to avoid this lint. That is a problem even without unsafe_op_in_unsafe_fn. See for example here.

That should work here as well, right?

[LeSeulArtichaut]

For unsafe blocks in macros, AFAIK it is fairly common to add #[allow(unused_unsafe)] to avoid this lint. That is a problem even without unsafe_op_in_unsafe_fn. See for example here.

That should work here as well, right?

Ohhhh, I didn’t even think about that! Yeah, IIRC adding an unsafe block in the macro along with an #[allow(unused_unsafe)] would totally work.

[poliorcetics]

I'm going to try and do ffi/c_str.rs.

[poliorcetics]

I'll start panicking.rs today.

[poliorcetics]

Starting libstd/thread/ right now.

[poliorcetics]

Starting libstd/sync right now.

[poliorcetics]

I will do libstd/alloc.rs today.

[chansuke]

I'd like to work on sys/wasm.

[LeSeulArtichaut]

@ryr3 Any updates on io/? I see you committed on your fork, but you didn’t open a PR.

[pnadon]

Is there a file in particular someone would recommend I help out with? I have yet to contribute to Rust but I am very interested in doing so!

If there are no suggestions, I will do sys/wasi

[LeSeulArtichaut]

@pnadon Thanks for your help! I have no particular advice on what to work on. If you have any particular knowledge for an environment, go ahead, but otherwise pick any of them.
The remaining files are only internal system-dependent APIs so it might be a little harder because of the lack of clear documentation. If you find yourself stuck the best would be to come on Zulip so you can have a discussion with people knowledgeable for these platforms.

[pnadon]

You'll first want to add a #![deny(unsafe_op_in_unsafe_fn)] attribute in the scope you'll be working on.
Then, add unsafe blocks around unsafe operations in unsafe functions. These unsafe operations can be found either by searching for unsafe fns, or by running ./x.py check src/libstd and looking at the errors.
When adding an unsafe block, try to explain why it is safe in a safety comment before the unsafe block. This should look like:

Something strange happened while I was applying the changes.. I used ripgrep to search through sys/wasi to find all files with a mention of "unsafe". From there, I added #![deny(unsafe_op_in_unsafe_fn)] at the top of the file, and then ran ./x.py check src/libstd to check for errors, which surprisingly resulted in a successful compile. Should I run extra checks or am I going about this the wrong way?

[LeSeulArtichaut]

@pnadon lint levels get "inherited" in submodules, so you probably only need to add one in the parent module (probably mod.rs file?)
I think checking doesn’t work here since you’re not compiling for a wasi environment. I’m not sure how you can work around that, maybe you can use cargo directly and check for a wasm target?

[pnadon]

@pnadon lint levels get "inherited" in submodules, so you probably only need to add one in the parent module (probably mod.rs file?)
I think checking doesn’t work here since you’re not compiling for a wasi environment. I’m not sure how you can work around that, maybe you can use cargo directly and check for a wasm target?

Ok sounds good, I'll look into this!

[maekawatoshiki]

I'd like to work on sys/hermit

[chansuke]

I'd like to work on sys/cloudabi

[pnadon]

I apologize for this, but I should probably give up my responsibilities for sys/wasi, as I am busy with another issue.

I may take up the task again once I am finished with the other issue.

[Amjad50]

I would like to take sys/wasi

[carbotaniuman]

I'll take sys/windows.

[Caduser2020]

I would like to take sys/sgx.

[m-ou-se]

sys/unsupported is missing in the list. That one was not much work: https://github.com/rust-lang/rust/pull/77722

sys/vxworks will be mostly fixed by https://github.com/rust-lang/rust/pull/77666

[Caduser2020]

sys/sgx merged in #77346.

[Mark-Simulacrum]

I have adjusted the E- tag here to E-hard, because for the most part, correctly checking the requirements and determining the appropriate safety comment to write is not an easy task. It might be a medium task (unclear), but it feels hard to me.