-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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.
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
Rollup of 7 pull requests #85032
Closed
Closed
Rollup of 7 pull requests #85032
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This extracts a new `parse_cfg` function that's used between both. - Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)] #[doc(cfg(y))]`. Previously it would be completely ignored. - Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)] #[doc(cfg(x))]`. Previously, the cfg would be ignored. - Pass the cfg predicate through to rustc_expand to be validated Co-authored-by: Vadim Petrochenkov <[email protected]>
Since the beginning of time the `-Ctarget-feature` flag on the command line has largely been passed unmodified to LLVM. Afterwards, though, the `#[target_feature]` attribute was stabilized and some of the names in this attribute do not match the corresponding LLVM name. This is because Rust doesn't always want to stabilize the exact feature name in LLVM for the equivalent functionality in Rust. This creates a situation, however, where in Rust you'd write: #[target_feature(enable = "pclmulqdq")] unsafe fn foo() { // ... } but on the command line you would write: RUSTFLAGS="-Ctarget-feature=+pclmul" cargo build --release This difference is somewhat odd to deal with if you're a newcomer and the situation may be made worse with upcoming features like [WebAssembly SIMD](rust-lang#74372) which may be more prevalent. This commit implements a mapping to translate requests via `-Ctarget-feature` through the same name-mapping functionality that's present for attributes in Rust going to LLVM. This means that `+pclmulqdq` will work on x86 targets where as previously it did not. I've attempted to keep this backwards-compatible where the compiler will just opportunistically attempt to remap features found in `-Ctarget-feature`, but if there's something it doesn't understand it gets passed unmodified to LLVM just as it was before.
Under some conditions, the toolchain will produce a sequence of linker arguments that result in a NEEDED list that puts libc before libgcc_s; e.g., [0] NEEDED 0x2046ba libc.so.1 [1] NEEDED 0x204723 libm.so.2 [2] NEEDED 0x204736 libsocket.so.1 [3] NEEDED 0x20478b libumem.so.1 [4] NEEDED 0x204763 libgcc_s.so.1 Both libc and libgcc_s provide an unwinder implementation, but libgcc_s provides some extra symbols upon which Rust directly depends. If libc is first in the NEEDED list we will find some of those symbols in libc but others in libgcc_s, resulting in undefined behaviour as the two implementations do not use compatible interior data structures. This solution is not perfect, but is the simplest way to produce correct binaries on illumos for now.
Nothing uses the return value. This will make the next changes easier. Signed-off-by: Ian Jackson <[email protected]>
We must change the atomic read on panic entry to `Acquire`, to pick up a possible an `always_panic` on another thread. We add `count` to the names of panic_count::get and ::is_zaero, because now there is another reason why panic ought to maybe abort. Renaming these ensures that we have checked every call site to ensure that they don't need further adjustment. Signed-off-by: Ian Jackson <[email protected]> Co-authored-by: Mara Bos <[email protected]>
As per rust-lang#81858 (comment) Suggested-by: Mara Bos <[email protected]> Signed-off-by: Ian Jackson <[email protected]>
Unwinding after fork() in the child is UB on some platforms, because on those (including musl) malloc can be UB in the child of a multithreaded program, and unwinding must box for the payload. Even if it's safe, unwinding past fork() in the child causes whatever traps the unwind to return twice. This is very strange and clearly not desirable. With the default behaviour of the thread library, this can even result in a panic in the child being transformed into zero exit status (ie, success) as seen in the parent! Fixes rust-lang#79740. Signed-off-by: Ian Jackson <[email protected]>
Signed-off-by: Ian Jackson <[email protected]> Co-authored-by: Mara Bos <[email protected]>
This is safe (does not involve heap allocation) but we don't yet have a test to ensure that stays true. That will come in a moment. Signed-off-by: Ian Jackson <[email protected]> Co-authored-by: Mara Bos <[email protected]>
This tests that we can indeed safely panic after fork, both a raw libc::fork and in a Command pre_exec hook. Signed-off-by: Ian Jackson <[email protected]> Co-authored-by: Mara Bos <[email protected]>
Previoously, if somehow this program got a wrong argument, it would panic in the re-executed child. But that looks like a "success" for this program! We mustn't panic unless everything is great. Signed-off-by: Ian Jackson <[email protected]>
Our existing tests are only on Unix. We want a general one too. Signed-off-by: Ian Jackson <[email protected]>
This test failed on an earlier version of this branch, where this did not work properly, so I know the test works. Signed-off-by: Ian Jackson <[email protected]>
In rust-lang#75979 several inlined modules were split out into multiple files. This PR keeps the multiple files but moves a few things around to organize things in a coherent way.
Do not allocate or unwind after fork ### Objective scenarios * Make (simple) panics safe in `Command::pre_exec_hook`, including most `panic!` calls, `Option::unwrap`, and array bounds check failures. * Make it possible to `libc::fork` and then safely panic in the child (needed for the above, but this requirement means exposing the new raw hook API which the `Command` implementation needs). * In singlethreaded programs, where panic in `pre_exec_hook` is already memory-safe, prevent the double-unwinding malfunction rust-lang#79740. I think we want to make panic after fork safe even though the post-fork child environment is only experienced by users of `unsafe`, beause the subset of Rust in which any panic is UB is really far too hazardous and unnatural. #### Approach * Provide a way for a program to, at runtime, switch to having panics abort. This makes it possible to panic without making *any* heap allocations, which is needed because on some platforms malloc is UB in a child forked from a multithreaded program (see rust-lang#80263 (comment), and maybe also the SuS [spec](https://pubs.opengroup.org/onlinepubs/9699919799/functions/fork.html)). * Make that change in the child spawned by `Command`. * Document the rules comprehensively enough that a programmer has a fighting chance of writing correct code. * Test that this all works as expected (and in particular, that there aren't any heap allocations we missed) Fixes rust-lang#79740 #### Rejected (or previously attempted) approaches * Change the panic machinery to be able to unwind without allocating, at least when the payload and message are both `'static`. This seems like it would be even more subtle. Also that is a potentially-hot path which I don't want to mess with. * Change the existing panic hook mechanism to not convert the message to a `String` before calling the hook. This would be a surprising change for existing code and would not be detected by the type system. * Provide a `raw_panic_hook` function to intercept panics in a way that doesn't allocate. (That was an earlier version of this MR.) ### History This MR could be considered a v2 of rust-lang#80263. Thanks to everyone who commented there. In particular, thanks to `@m-ou-se,` `@Mark-Simulacrum` and `@hyd-dev.` (Tagging you since I think you might be interested in this new MR.) Compared to rust-lang#80263, this MR has very substantial changes and additions. Additionally, I have recently (2021-04-20) completely revised this series following very helpful comments from `@m-ou-se.` r? `@m-ou-se`
…chenkov illumos should put libc last in library search order Under some conditions, the toolchain will produce a sequence of linker arguments that result in a NEEDED list that puts libc before libgcc_s; e.g., [0] NEEDED 0x2046ba libc.so.1 [1] NEEDED 0x204723 libm.so.2 [2] NEEDED 0x204736 libsocket.so.1 [3] NEEDED 0x20478b libumem.so.1 [4] NEEDED 0x204763 libgcc_s.so.1 Both libc and libgcc_s provide an unwinder implementation, but libgcc_s provides some extra symbols upon which Rust directly depends. If libc is first in the NEEDED list we will find some of those symbols in libc but others in libgcc_s, resulting in undefined behaviour as the two implementations do not use compatible interior data structures. This solution is not perfect, but is the simplest way to produce correct binaries on illumos for now.
Unify rustc and rustdoc parsing of `cfg()` This extracts a new `parse_cfg` function that's used between both. - Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)] #[doc(cfg(y))]`. Previously it would be completely ignored. - Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)] #[doc(cfg(x))]`. Previously, the cfg would be ignored. - Pass the cfg predicate through to rustc_expand to be validated Technically this is a breaking change, but doc_cfg is still nightly so I don't think it matters. Fixes rust-lang#84437. r? ```````@petrochenkov```````
Cleanup of `wasm` Some more cleanup of `sys`, this time `wasm` - Reuse `unsupported::args` (functionally equivalent implementation, just an empty iterator). - Split out `atomics` implementation of `wasm::thread`, the non-`atomics` implementation is reused from `unsupported`. - Move all of the `atomics` code to a separate directory `wasm/atomics`. ``@rustbot`` label: +T-libs-impl r? ``@m-ou-se``
…lacrum linker: Avoid library duplication with `/WHOLEARCHIVE` Looks like in rust-lang#72785 I misinterpreted how the `link.exe`'s `/WHOLEARCHIVE` flag works. It's not necessary to write `mylib /WHOLEARCHIVE:mylib` to mark `mylib` as whole archive, `/WHOLEARCHIVE:mylib` alone is enough. https://docs.microsoft.com/en-us/cpp/build/reference/wholearchive-include-all-library-object-files?view=msvc-160
…r=nagisa rustc: Support Rust-specific features in -Ctarget-feature Since the beginning of time the `-Ctarget-feature` flag on the command line has largely been passed unmodified to LLVM. Afterwards, though, the `#[target_feature]` attribute was stabilized and some of the names in this attribute do not match the corresponding LLVM name. This is because Rust doesn't always want to stabilize the exact feature name in LLVM for the equivalent functionality in Rust. This creates a situation, however, where in Rust you'd write: #[target_feature(enable = "pclmulqdq")] unsafe fn foo() { // ... } but on the command line you would write: RUSTFLAGS="-Ctarget-feature=+pclmul" cargo build --release This difference is somewhat odd to deal with if you're a newcomer and the situation may be made worse with upcoming features like [WebAssembly SIMD](rust-lang#74372) which may be more prevalent. This commit implements a mapping to translate requests via `-Ctarget-feature` through the same name-mapping functionality that's present for attributes in Rust going to LLVM. This means that `+pclmulqdq` will work on x86 targets where as previously it did not. I've attempted to keep this backwards-compatible where the compiler will just opportunistically attempt to remap features found in `-Ctarget-feature`, but if there's something it doesn't understand it gets passed unmodified to LLVM just as it was before.
…nagisa Rearrange SGX split module files In rust-lang#75979 several inlined modules were split out into multiple files. This PR keeps the multiple files but moves a few things around to organize things in a coherent way.
@bors r+ rollup=never p=5 |
📌 Commit bd37dbe has been approved by |
⌛ Testing commit bd37dbe with merge b3dc75f1fe34d69cbe95a3fbf0e6a152fb523b69... |
The job Click to see the possible cause of the failure (guessed by this bot)
|
💔 Test failed - checks-actions |
I think the problem is that emscripten counts as Unix but |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
rollup
A PR which is a rollup
S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
cfg()
#84442 (Unify rustc and rustdoc parsing ofcfg()
)wasm
#84655 (Cleanup ofwasm
)/WHOLEARCHIVE
#84866 (linker: Avoid library duplication with/WHOLEARCHIVE
)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup