Skip to content
New issue

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

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

Already on GitHub? # to your account

Rollup of 3 pull requests #70676

Closed
wants to merge 11 commits into from

Conversation

Dylan-DPC-zz
Copy link

Successful merges:

Failed merges:

r? @ghost

KamilaBorowska and others added 11 commits March 22, 2020 20:10
Before this commit `-C codegen-units` would just get silently be
ignored if `-C incremental` was specified too. After this commit
one can control the number of codegen units generated during
incremental compilation. The default is rather high at 256, so most
crates won't see a difference unless explicitly opting into a lower
count.
…atsakis

Make the rustc respect the `-C codegen-units` flag in incremental mode.

This PR implements (the as of yet unapproved) major change proposal at rust-lang/compiler-team#245. See the description there for background and rationale.

The changes are pretty straightforward and should be easy to rebase if the proposal gets accepted at some point.

r? @nikomatsakis cc @pnkfelix
Implement Hash for Infallible

https://www.reddit.com/r/rust/comments/fmllgx/never_crate_stable_alternative_to/ lists not implementing `Hash` as a reason for the `never` crate. I see no reason not to implement `Hash` for `Infallible`, so might as well do it.

No changes necessary for `!`, because `!` already implements `Hash` (see rust-lang#51404).
…fix, r=Amanieu

Fix double-free and undefined behaviour in libstd::syn::unix::Thread::new

While working on concurrency support for Miri, I found that the `libstd::syn::unix::Thread::new` method has two potential problems: double-free and undefined behaviour.

**Double-free** could occur if the following events happened (credit for pointing this out goes to @RalfJung):

1.  The call to `pthread_create` successfully launched a new thread that executed to completion and deallocated `p`.
2.  The call to `pthread_attr_destroy` returned a non-zero value causing the `assert_eq!` to panic.
3.  Since `mem::forget(p)` was not yet executed, the destructor of `p` would be executed and cause a double-free.

As far as I understand, this code also violates the stacked-borrows aliasing rules and thus would result in **undefined behaviour** if these rules were adopted.  The problem is that the ownership of `p` is passed to the newly created thread before the call to `mem::forget`. Since the call to `mem::forget` is still a call, it counts as a use of `p` and triggers UB.

This pull request changes the code to use `mem::ManuallyDrop` instead of `mem::forget`. As a consequence, in case of a panic, `p` would be potentially leaked, which while undesirable is probably better than double-free or undefined behaviour.
@Dylan-DPC-zz
Copy link
Author

@bors r+ p=3 rollup=never

@bors
Copy link
Contributor

bors commented Apr 1, 2020

📌 Commit 1cdf210 has been approved by Dylan-DPC

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 1, 2020
@Dylan-DPC-zz
Copy link
Author

@bors p=4

@bors
Copy link
Contributor

bors commented Apr 2, 2020

⌛ Testing commit 1cdf210 with merge fcb9e03a725bbb8d4ed92bf4d1c8b6650f095e91...

@bors
Copy link
Contributor

bors commented Apr 2, 2020

💔 Test failed - checks-azure

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 2, 2020
@RalfJung
Copy link
Member

RalfJung commented Apr 2, 2020

#70597 got r-.

@RalfJung RalfJung closed this Apr 2, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants