Skip to content

Conversation

beetrees
Copy link
Contributor

Adds unsafe to a few internal functions that have safety requirements but were previously not marked as unsafe. Specifically:

  • std::sys::pal::unix::thread::min_stack_size needs to be unsafe as __pthread_get_minstack might dereference the passed pointer. All callers currently pass a valid initialised libc::pthread_attr_t.
  • std::thread::Thread::new (and new_inner) need to be unsafe as it requires the passed thread name to be valid UTF-8, otherwise Thread::name will trigger undefined behaviour. I've taken the opportunity to split out the unnamed thread case into a separate new_unnamed function to make the safety requirement clearer. All callers meet the safety requirement now that Revert "Use OS thread name by default" #123505 has been merged.

@rustbot
Copy link
Collaborator

rustbot commented Apr 13, 2024

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 13, 2024
@rust-log-analyzer

This comment has been minimized.

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 13, 2024

📌 Commit 126c762 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 13, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 14, 2024
…mulacrum

Add missing `unsafe` to some internal `std` functions

Adds `unsafe` to a few internal functions that have safety requirements but were previously not marked as `unsafe`. Specifically:

- `std::sys::pal::unix::thread::min_stack_size` needs to be `unsafe` as `__pthread_get_minstack` might dereference the passed pointer. All callers currently pass a valid initialised `libc::pthread_attr_t`.
- `std::thread::Thread::new` (and `new_inner`) need to be `unsafe` as it requires the passed thread name to be valid UTF-8, otherwise `Thread::name` will trigger undefined behaviour. I've taken the opportunity to split out the unnamed thread case into a separate `new_unnamed` function to make the safety requirement clearer. All callers meet the safety requirement now that rust-lang#123505 has been merged.
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#123651 (Thread local updates for idiomatic examples)
 - rust-lang#123699 (run-make-support: tidy up support library)
 - rust-lang#123779 (OpenBSD fix long socket addresses)
 - rust-lang#123803 (Fix `VecDeque::shrink_to` UB when `handle_alloc_error` unwinds.)
 - rust-lang#123875 (Doc: replace x with y for hexa-decimal fmt)
 - rust-lang#123879 (Add missing `unsafe` to some internal `std` functions)
 - rust-lang#123889 (reduce tidy overheads in run-make checks)
 - rust-lang#123898 (Generic associated consts: Check regions earlier when comparing impl with trait item def)
 - rust-lang#123902 (compiletest: Update rustfix to 0.8.1)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#123651 (Thread local updates for idiomatic examples)
 - rust-lang#123699 (run-make-support: tidy up support library)
 - rust-lang#123779 (OpenBSD fix long socket addresses)
 - rust-lang#123875 (Doc: replace x with y for hexa-decimal fmt)
 - rust-lang#123879 (Add missing `unsafe` to some internal `std` functions)
 - rust-lang#123889 (reduce tidy overheads in run-make checks)
 - rust-lang#123898 (Generic associated consts: Check regions earlier when comparing impl with trait item def)
 - rust-lang#123902 (compiletest: Update rustfix to 0.8.1)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2ba0c62 into rust-lang:master Apr 14, 2024
@rustbot rustbot added this to the 1.79.0 milestone Apr 14, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2024
Rollup merge of rust-lang#123879 - beetrees:missing-unsafe, r=Mark-Simulacrum

Add missing `unsafe` to some internal `std` functions

Adds `unsafe` to a few internal functions that have safety requirements but were previously not marked as `unsafe`. Specifically:

- `std::sys::pal::unix::thread::min_stack_size` needs to be `unsafe` as `__pthread_get_minstack` might dereference the passed pointer. All callers currently pass a valid initialised `libc::pthread_attr_t`.
- `std::thread::Thread::new` (and `new_inner`) need to be `unsafe` as it requires the passed thread name to be valid UTF-8, otherwise `Thread::name` will trigger undefined behaviour. I've taken the opportunity to split out the unnamed thread case into a separate `new_unnamed` function to make the safety requirement clearer. All callers meet the safety requirement now that rust-lang#123505 has been merged.
@beetrees beetrees deleted the missing-unsafe branch April 14, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-unix Operating system: Unix-like S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants