Fixing unsafe_op_in_unsafe_fn for std::{os, sys}

[workingjubilee]

In #127744 we begin to deny(unsafe_op_in_unsafe_fn) in the standard library, but there are... many... platforms which all use unsafe code inside unsafe functions. It is unreasonable to fix them all in one step.

Even if it is technically possible to do this in one push that "fixes" it for all the platforms we run tier 1/2 CI on, that would be extremely rude. That would then immediately nuke all tier 3 platforms that aren't a variation on the tier 2 ones. We should at least try to allow people to respond before we do so. After all, we will eventually update the standard library to edition 2024 anyway.

To fix each platform, one at a time, deny(unsafe_op_in_unsafe_fn) should be moved into that platform's support modules (and even individual functions, if necessary). As we can see from the following test sample, you can have a deny, then an allow, then a deny, and it still works correctly. So we will keep the allows at the top level of std::os and std::sys in place.

For now.

In order to not have one giant table of targets that make my eyes blur, I have split the categories in somewhat arbitrary ways. This is primarily based on the heuristic of "if other examples in this category are fixed, are the rest likely to also be fixed?" except for the "uniques" set, which I expect will almost all require individual attention.

berkeley software descendants

• freebsd
• netbsd
• tier 3
  • dragonfly
  • openbsd

darwinian

• macOS
• iOS
• tier 3
  • tvOS
  • visionOS
  • watchOS

linuxen

• android
• linux-gnu
• linux-musl

unixen

• illumos
• redox
• solaris
• tier 3
  • aix
  • haiku
  • hurd
  • nto
  • vxworks - Forbid unused unsafe in vxworks-specific std modules #128539

embedded unix + newlib

Unix-ish impls mostly intended for embedded-ish usage, all using newlib.

• espidf
• horizon - Clean up warnings + unsafe_op_in_unsafe_fn when building std for armv6k-nintendo-3ds #127996
• vita - Fix vita build of std and forbid unsafe in unsafe in the os/vita module #128315

windows-y

• windows-gnu
• windows-gnullvm
• windows-msvc
• win7
• Make os/windows and pal/windows default to #![deny(unsafe_op_in_unsafe_fn)] #127750
• Make more Windows functions #![deny(unsafe_op_in_unsafe_fn)] #127763
• Import c_void rather than using the full path #128131

uniques

We have a grab bag of others here, some of which are "Unix family" but are... estranged cousins, if so.

• fuchsia
• hermit - fix(hermit): deny(unsafe_op_in_unsafe_fn) #128433
• kmc-solid - kmc-solid: #![forbid(unsafe_op_in_unsafe_fn)] #127873
• l4re
• sgx
• teeos - delete #![allow(unsafe_op_in_unsafe_fn)] in teeos #127789
• uefi - std: Forbid unwrapped unsafe ops in xous and uefi modules #127836
• wasm-emscripten
• wasm-unknown
• wasm-wasi
• wasm-wasi-threads
• xous - std: Forbid unwrapped unsafe ops in xous and uefi modules #127836
• zkvm - zkvm: add #[forbid(unsafe_op_in_unsafe_fn)] in stdlib #127833

[workingjubilee]

My current belief is that, like with every edition update so far, we will eventually fix the lint in all relevant places. By "fixed" I mean actually fixing it. However, it is fine if a platform adds more instances of allow for this lint, but I assume that we will eventually remove such allowances.

Pinging first everyone who has their own std::sys::pal submodule, since those are easiest to add #![deny(unsafe_op_in_unsafe_fn)] into their actual submodule, at the moment. You may have nothing to do! Just confirm that and we can mark it off... but do please actually check. At least one platform has started on using deny(unsafe_op_in_unsafe_fn) in some of its submodules, but has at least one instance of code that should fail that lint, yet is neither specifically denied nor allowed.

• hermit: @stlankes @mkroening
• kmc-solid: @kawadakk (is this also the source of the "itron" submodule)?
• sgx: @jethrogb @raoulstrackx @mzohreva
• teeos: @Sword-Destiny @petrochenkov
• uefi: @dvdhrm @nicholasbishop @Ayush1325
• xous: @xobs
• zkvm: @flaub @jbruestle @SchmErik

Also remember to check std::sys::sync and the other "top level" modules of sys for your platform's code in its submodules.

[xobs]

Xous already has #![deny(unsafe_op_in_unsafe_fn)] in sys/pal/xous/mod.rs, and adding it to os/xous/mod.rs does not cause any failures.

Adding it to the top of sys/mod.rs causes failures in:

• library/std/src/sys/pal/common/alloc.rs - std: Unsafe-wrap actually-universal platform code #128089
• library/std/src/sys/personality/dwarf/eh.rs - std: unsafe-wrap personality::dwarf::eh #128055
• library/std/src/sys/personality/dwarf/mod.rs - std: Use read_unaligned for reads from DWARF #127792
• library/std/src/sys/personality/gcc.rs - std: unsafe-wrap personality::gcc #128158
• library/std/src/sys/backtrace.rs - std: Unsafe-wrap actually-universal platform code #128089

But otherwise there are no issues for Xous.

[joboet]

I think we should add #[forbid]s to leaf modules instead and slowly move them up the module tree until we reach the root. Just #[deny]ing the lint stills allows adding new cases of this to modules that have already been fixed.

[Ayush1325]

Adding #![deny(unsafe_op_in_unsafe_fn)] in sys/pal/uefi/mod.rs and os/uefi/mod.rs cause no failures.

[SchmErik]

Thanks for @-ing us @workingjubilee! Adding #![deny(unsafe_op_in_unsafe_fn)] in sys/pal/zkvm/mod.rs resulted in one failure. Here's a small PR to address this: #127833

[workingjubilee]

Identified and sifted out some targets into their own set.

• espressif-espidf: @ivmarkov @MabezDev @SergioGasquez
• nintendo-3ds ("horizon"): @Meziu @AzureMarker @ian-h-chamberlain
• sony-vita: @nikarh @pheki @zetanumbers

Hi, you all are currently listed as maintainers for a target that

• is target_family = "unix"
• is target_env = "newlib"
• uses the std::sys support for a "unix" target
• supports only 32-bit systems
• is officially tier 3 for all variants of the target

Unfortunately, the way the std::sys::pal::unix logic currently is written, it is very deeply entangled between different Unix targets. I am pinging you all as a group, ahead of most other "Unix" targets, because your targets have no CI coverage (so if someone breaks your target, it won't be noticed) and share some of the same architectural considerations. Even if you have little to do, it might be a good idea to factor out some of your implementations so that later passes by other maintainers don't affect your support code.

[kawadakk]

kmc-solid already has #![deny(unsafe_op_in_unsafe_fn)] in std/src/sys/pal/solid/mod.rs and std/src/os/solid/io.rs, and adding it to these target-specific modules causes no failures.

• std/src/os/solid/mod.rs
• std/src/sys/sync/rwlock/solid.rs
• std/src/sys/sync/mutex/itron.rs
• std/src/sys/path/unsupported_backslash.rs (contains no unsafe code, shared with cfg(target_os = "uefi"))

Table

Module path	Source file
std::sys::pal::solid	std/src/sys/pal/solid/mod.rs	Has #![deny(unsafe_op_in_unsafe_fn)]
std::sys::pal::solid::itron	std/src/sys/pal/solid/mod.rs¹	N/A - submodule of std::sys::pal::solid
std::sys::path::unsupported_backslash	std/src/sys/path/unsupported_backslash.rs²	#![deny(unsafe_op_in_unsafe_fn)] can be added without causing compilation failure
std::sys::sync::rwlock::solid	std/src/sys/sync/rwlock/solid.rs	#![deny(unsafe_op_in_unsafe_fn)] can be added without causing compilation failure
std::sys::sync::mutex::itron	std/src/sys/sync/mutex/itron.rs	#![deny(unsafe_op_in_unsafe_fn)] can be added without causing compilation failure
std::os::solid	std/src/os/solid/mod.rs	#![deny(unsafe_op_in_unsafe_fn)] can be added without causing compilation failure
std::os::solid::io	std/src/os/solid/io.rs	Has #![deny(unsafe_op_in_unsafe_fn)]
std::os::solid::ffi	std/src/os/solid/ffi.rs	#![deny(unsafe_op_in_unsafe_fn)] can be added without causing compilation failure

¹ Inline module. Its submodules are implemented in std/src/sys/pal/itron/*.rs.

² Shared with cfg(target_os = "uefi") (Tier 2 or lower).

[workingjubilee]

Thank you for everyone that has responded and gotten fixes in!

It looks like wasm-?-? and wasm-wasi are already deny(unsafe_op_in_unsafe_fn) for their specific code, or obviously conformant, so that's just upgrading a few annotations...

• wasm:
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasm/mod.rs
• wasi:
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/args.rs
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/env.rs
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/fd.rs
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/fs.rs
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/helpers.rs
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/io.rs
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/net.rs
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/os.rs
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/stdio.rs
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/thread.rs
  • https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/time.rs

...expect maybe the... threads code?

cc @g0djan @abrown @loganek can you take a look at https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/thread.rs and the other threads-on-wasi files and make sure they all conform to forbid(unsafe_op_in_unsafe_fn)?

[workingjubilee]

emscripten is littered throughout the "unix" code, like many others.

[workingjubilee]

Pinging the remaining tier 3 Unix maintainers for this. Please be aware that when this lint is enabled, all code which is only compiled for tier 3 targets, but nonetheless violates this lint, will cease compiling without any check from CI.

As these are all considered "Unix" implementations, it is possible the majority or even all of your unsafe fn are shared with tier 2+ targets, so you may not necessarily have to do anything. However, please also be aware that as this issue is fixed, for the Unix code in particular, code that is deemed not-unsafe will be made into safe functions.

• aix: @bzEq @ecnelises
• hurd: @sthibaul
• openbsd: @semarie
• nto: @flba-eb @gh-tr @jonathanpallant @japaric

This entire exercise is really a warm-up for actually having the (un)safety in the standard library's platform interfaces properly documented in the actual code and cross-referencing external sources as appropriate.

[ecnelises]

Thanks for reminding. AIX does not have specific parts to change.

[sthibaul]

Thanks for the heads-up. I added #![deny(unsafe_op_in_unsafe_fn)] to library/std/src/os/hurd/{fs,mod,raw}.rs and it built fine.

[semarie]

library/std/src/os/openbsd/*.rs are fine too. but they will be specific changes need for openbsd in unix parts

[g0djan]

cc @g0djan @abrown @loganek can you take a look at https://github.com/rust-lang/rust/blob/006c8df322e55c14d845e1fe317ca1445c2f8e6b/library/std/src/sys/pal/wasi/thread.rs and the other threads-on-wasi files and make sure they all conform to forbid(unsafe_op_in_unsafe_fn)?

Hey, thanks for reaching out. I see that in this file there're 8 lines which don't compile for wasm32-wasip1-threads when I add #![deny(unsafe_op_in_unsafe_fn)] to it. I'll take a look at the other files and send a PR

[ChrisDenton]

All the tier 1 Windows targets have the Windows specific stuff taken care of. There are still some shared code that needs handling in library\std\src\sys\sync\condvar\futex.rs.

I've not checked tier 3 Windows targets.

[ivmarkov]

Identified and sifted out some targets into their own set.

• espressif-espidf: @ivmarkov @MabezDev @SergioGasquez
• nintendo-3ds ("horizon"): @Meziu @AzureMarker @ian-h-chamberlain
• sony-vita: @nikarh @pheki @zetanumbers

Hi, you all are currently listed as maintainers for a target that

• is target_family = "unix"
• is target_env = "newlib"
• uses the std::sys support for a "unix" target
• supports only 32-bit systems
• is officially tier 3 for all variants of the target

Unfortunately, the way the std::sys::pal::unix logic currently is written, it is very deeply entangled between different Unix targets. I am pinging you all as a group, ahead of most other "Unix" targets, because your targets have no CI coverage (so if someone breaks your target, it won't be noticed) and share some of the same architectural considerations. Even if you have little to do, it might be a good idea to factor out some of your implementations so that later passes by other maintainers don't affect your support code.

@workingjubilee Regarding the ESP-IDF targets:

• Module std::os::espidf and its submodules compile fine in the presence of #![deny(unsafe_op_in_unsafe_fn)]
• However, this was to be expected because it contain 0.0001% of the code for the ESP-IDF targets :)
• Almost all other ESP-IDF code-paths are indeed intermingled with the other #[cfg(unix)] targets in the std::sys::unix::* and std::os::unix::* modules

Factoring out I wouldn't consider ATM, as it would be a gigantic effort, and so far the "intermingling" had worked well for us actually. We occasionally get a build failure once per a couple of months or so, but since we run a cargo -Z build-std nightly CI against downstream ESP-IDF crates, this CI is also catching issues in the STD for our Tier-3 targets "next day" so to say.
(We might consider putting a proper ./x.py build library/std CI in-place for our targets in future, because - as it turns out - cargo -Z build-std is hiding the STD build warnings from us - and in the meantime - building our targets spits out some unused/deprecation wanings I did not realize are in there.)

So I would say I prefer to "take the hit", wait until the Tier-1/2 target maintainers fix the #[cfg(unix)] space, and if there are regressions specific to ESP-IDF after that, we'll submit PRs in a day or two after the regressions pop up in our nightly CI. Given that the "fixes" are basically putting extra unsafe {} blocks here and there, I think we should be able to react very fast. :)

[workingjubilee]

As you wish. :^)

[biabbas]

As seen in pr #128539, deny_unsafeop_in_unsafe_fn in VxWorks specific files does not affect the build( No code where this lint can be applied is present in VxWorks specific files). VxWorks uses most of Unix library code.