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.

Sign up for GitHub

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

Jump to bottom

Deny unsafe ops in unsafe fns in std::sys_common #73928

Closed
wants to merge 1 commit into from
Closed

Deny unsafe ops in unsafe fns in std::sys_common #73928

wants to merge 1 commit into from

Conversation

LeSeulArtichaut
Copy link
Contributor

Helps with #73904.

I wasn't able to properly document some unsafety used in thread_local.rs, because the system-dependent unsafe functions it uses aren't documented. I'll be happy to help if any team member has some time to walk me through those functions, but I think that can be done in a separate PR.

This should be merged after (or alongside) #73909 because #73909 enables the #![feature(unsafe_block_in_unsafe_fn)] needed for this PR.

@rust-highfive
Copy link
Collaborator

r? @withoutboats

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 1, 2020
@LeSeulArtichaut LeSeulArtichaut mentioned this pull request Jul 1, 2020
Open
22 tasks
@rust-highfive

This comment has been minimized.

Sign in to view
@LeSeulArtichaut LeSeulArtichaut force-pushed the unsafe-libstd branch 3 times, most recently from 91df7d3 to 65c4b9c Compare July 14, 2020 14:25
@bors

This comment has been minimized.

Sign in to view
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 28, 2020
@bors

This comment has been minimized.

Sign in to view
@Dylan-DPC-zz
Copy link

@LeSeulArtichaut any updates on this?

@crlf0710
Copy link
Member

Ping from triage. There's still merge conflicts. Any updates?

@LeSeulArtichaut
Copy link
Contributor Author

LeSeulArtichaut commented Sep 11, 2020 •
edited
Loading

Sorry for being so slow to respond. I'm currently rebasing this PR, I should be able to update this PR soon.

@LeSeulArtichaut LeSeulArtichaut force-pushed the unsafe-libstd branch 2 times, most recently from 8eda097 to 20e0f12 Compare September 13, 2020 08:54
@LeSeulArtichaut
Copy link
Contributor Author

Resolved the conflicts, and CI should pass now.

@LeSeulArtichaut LeSeulArtichaut added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 13, 2020
@jyn514 jyn514 added T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 15, 2020
Mark-Simulacrum
Mark-Simulacrum reviewed Sep 15, 2020
View reviewed changes
library/std/src/sys_common/alloc.rs
// Docs for GlobalAlloc::realloc require this to be valid:
let new_layout = Layout::from_size_align_unchecked(new_size, old_layout.align());
// SAFETY: as stated in docs for GlobalAlloc::realloc, the caller
// must guarantee that `new_size` is valid for a `Layout`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please put any guarantees the caller must make in a documentation comment on the function.

@LeSeulArtichaut LeSeulArtichaut added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 16, 2020
@bors
Copy link
Contributor

bors commented Oct 2, 2020

☔ The latest upstream changes (presumably #77436) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@LeSeulArtichaut LeSeulArtichaut removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 16, 2020
@LeSeulArtichaut LeSeulArtichaut added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 16, 2020
@crlf0710
Copy link
Member

crlf0710 commented Nov 6, 2020

@LeSeulArtichaut Ping from triage, any updates on this?

1 similar comment
@JohnCSimon
Copy link
Member

@LeSeulArtichaut Ping from triage, any updates on this?

@crlf0710
Copy link
Member

@LeSeulArtichaut I'm gonna close this due to inactivity. Feel free to reopen or create a new PR when you're ready to work on this. Thanks!

@crlf0710 crlf0710 closed this Dec 11, 2020
@LeSeulArtichaut LeSeulArtichaut deleted the unsafe-libstd branch February 18, 2021 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mark-Simulacrum Mark-Simulacrum Mark-Simulacrum left review comments

Assignees

@withoutboats withoutboats

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@LeSeulArtichaut @rust-highfive @bors @Dylan-DPC-zz @crlf0710 @JohnCSimon @Mark-Simulacrum @withoutboats @jyn514