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.

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 8 pull requests #83537

Closed
wants to merge 29 commits into from

Conversation

Dylan-DPC-zz
Copy link

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Aaron1011 and others added 29 commits March 15, 2021 22:37
Background:

Over the last year, pidfd support was added to the Linux kernel. This
allows interacting with other processes. In particular, this allows
waiting on a child process with a timeout in a race-free way, bypassing
all of the awful signal-handler tricks that are usually required.

Pidfds can be obtained for a child process (as well as any other
process) via the `pidfd_open` syscall. Unfortunately, this requires
several conditions to hold in order to be race-free (i.e. the pid is not
reused).
Per `man pidfd_open`:

```
· the disposition of SIGCHLD has not been explicitly set to SIG_IGN
 (see sigaction(2));

· the SA_NOCLDWAIT flag was not specified while establishing a han‐
 dler for SIGCHLD or while setting the disposition of that signal to
 SIG_DFL (see sigaction(2)); and

· the zombie process was not reaped elsewhere in the program (e.g.,
 either by an asynchronously executed signal handler or by wait(2)
 or similar in another thread).

If any of these conditions does not hold, then the child process
(along with a PID file descriptor that refers to it) should instead
be created using clone(2) with the CLONE_PIDFD flag.
```

Sadly, these conditions are impossible to guarantee once any libraries
are used. For example, C code runnng in a different thread could call
`wait()`, which is impossible to detect from Rust code trying to open a
pidfd.

While pid reuse issues should (hopefully) be rare in practice, we can do
better. By passing the `CLONE_PIDFD` flag to `clone()` or `clone3()`, we
can obtain a pidfd for the child process in a guaranteed race-free
manner.

This PR:

This PR adds Linux-specific process extension methods to allow obtaining
pidfds for processes spawned via the standard `Command` API. Other than
being made available to user code, the standard library does not make
use of these pidfds in any way. In particular, the implementation of
`Child::wait` is completely unchanged.

Two Linux-specific helper methods are added: `CommandExt::create_pidfd`
and `ChildExt::pidfd`. These methods are intended to serve as a building
block for libraries to build higher-level abstractions - in particular,
waiting on a process with a timeout.

I've included a basic test, which verifies that pidfds are created iff
the `create_pidfd` method is used. This test is somewhat special - it
should always succeed on systems with the `clone3` system call
available, and always fail on systems without `clone3` available. I'm
not sure how to best ensure this programatically.

This PR relies on the newer `clone3` system call to pass the `CLONE_FD`,
rather than the older `clone` system call. `clone3` was added to Linux
in the same release as pidfds, so this shouldn't unnecessarily limit the
kernel versions that this code supports.

Unresolved questions:
* What should the name of the feature gate be for these newly added
  methods?
* Should the `pidfd` method distinguish between an error occurring
  and `create_pidfd` not being called?
Co-authored-by: bjorn3 <[email protected]>
Changelog: https://github.com/Amanieu/parking_lot/blob/master/CHANGELOG.md#parking_lot_core-083-2021-02-12
The full log:
```
Removing cloudabi v0.1.0
Updating parking_lot v0.11.0 -> v0.11.1
Updating parking_lot_core v0.8.0 -> v0.8.3
```
Commit range: llogiq/bytecount@b0f5fba...8dcd437

The full log:
```
Updating bytecount v0.6.0 -> v0.6.2
Adding libm v0.1.4
Removing packed_simd v0.3.3
Adding packed_simd_2 v0.3.4
```
When building with multiple codegen units the test case can fail with
only a subset of all errors. Use a single codegen unit as a workaround.
Fixes '\\' handling in format strings.

Fixes rust-lang#83340
When the character next to `{}` is "shifted" (when mapping a byte index
in the format string to span) we should avoid shifting the span end
index, so first map the index of `}` to span, then bump the span,
instead of first mapping the next byte index to a span (which causes
bumping the end span too much).

Regression test added.

Fixes rust-lang#83344
This should be a nice small quality of life improvement when looking at
`config.toml.example` on GitHub or looking at diffs of it in PRs.
This error probably almost never happens, but we should still use the
diagnostic infrastructure. My guess is that the error was added back
before rustdoc used the rustc diagnostic infrastructure (it was all
`println!` and `eprintln!` back then!) and since it likely rarely occurs
and this code doesn't change that much, no one thought to transition it
to using diagnostics.

Note that the old error was actually a warning (it didn't stop the rest
of doc building). It seems very unlikely that this would fail without
the rest of the doc build failing, so it makes more sense for it to be a
hard error.

The error looks like this:

    error: failed to render source code for `src/test/rustdoc/smart-punct.rs`: "bar": foo
      --> src/test/rustdoc/smart-punct.rs:3:1
       |
    3  | / #![crate_name = "foo"]
    4  | |
    5  | | //! This is the "start" of the 'document'! How'd you know that "it's" ...
    6  | | //!
    ...  |
    22 | | //! I say "don't smart-punct me -- please!"
    23 | | //! ```
       | |_______^

I wasn't sure how to trigger the error, so to create that message I
temporarily made rustdoc always emit it. That's also why it says "bar"
and "foo" instead of a real error message.

Note that the span of the diagnostic starts at line 3 because line 1 of
that file is a (non-doc) comment and line 2 is a blank line.
The test calls libc::getpid() in the pre_exec hook and asserts that the returned value is different from the PID of the parent.
However, libc::getpid() returns the wrong value.
Before version 2.25, glibc caches the PID of the current process with the goal of avoiding additional syscalls.
The cached value is only updated when the wrapper functions for fork or clone are called.
In PR rust-lang#81825 we switch to directly using the clone3 syscall.
Thus, the cache is not updated and getpid returns the PID of the parent.
source: https://man7.org/linux/man-pages/man2/getpid.2.html#NOTES
Add Linux-specific pidfd process extensions (take 2)

Continuation of rust-lang#77168.
I addressed the following concerns from the original PR:

- make `CommandExt` and `ChildExt` sealed traits
- wrap file descriptors in `PidFd` struct representing ownership over the fd
- add `take_pidfd` to take the fd out of `Child`
- close fd when dropped

Tracking Issue: rust-lang#82971
rustdoc: Use diagnostics for error when including sources

This error probably almost never happens, but we should still use the
diagnostic infrastructure. My guess is that the error was added back
before rustdoc used the rustc diagnostic infrastructure (it was all
`println!` and `eprintln!` back then!) and since it likely rarely occurs
and this code doesn't change that much, no one thought to transition it
to using diagnostics.

Note that the old error was actually a warning (it didn't stop the rest
of doc building). It seems very unlikely that this would fail without
the rest of the doc build failing, so it makes more sense for it to be a
hard error.

The error looks like this:

    error: failed to render source code for `src/test/rustdoc/smart-punct.rs`: "bar": foo
      --> src/test/rustdoc/smart-punct.rs:3:1
       |
    3  | / #![crate_name = "foo"]
    4  | |
    5  | | //! This is the "start" of the 'document'! How'd you know that "it's" ...
    6  | | //!
    ...  |
    22 | | //! I say "don't smart-punct me -- please!"
    23 | | //! ```
       | |_______^

I wasn't sure how to trigger the error, so to create that message I
temporarily made rustdoc always emit it. That's also why it says "bar"
and "foo" instead of a real error message.

Note that the span of the diagnostic starts at line 3 because line 1 of
that file is a (non-doc) comment and line 2 is a blank line.
…acrum

Remove/replace some outdated crates from the dependency tree

- Remove `cloudabi` by updating `parking_lot` to 0.11.1.
- Replace `packed_simd` with `packed_simd2` by updating `bytecount` to 0.6.2.
Fixes to inline assmebly tests

* Join test thread to make assertion effective in sym.rs test case
* Use a single codegen unit to reduce non-determinism in srcloc.rs test rust-lang#82886
Simplify and fix byte skipping in format! string parser

Fixes '\\' handling in format strings.

Fixes rust-lang#83340
format macro argument parsing fix

When the character next to `{}` is "shifted" (when mapping a byte index
in the format string to span) we should avoid shifting the span end
index, so first map the index of `}` to span, then bump the span,
instead of first mapping the next byte index to a span (which causes
bumping the end span too much).

Regression test added.

Fixes rust-lang#83344

---

r? `@estebank`
Make # pretty print format easier to discover

# Rationale:

I use (cargo cult?) three formats in rust:  `{}`, debug `{:?}`, and pretty-print debug `{:#?}`. I discovered `{:#?}` in some blog post or guide when I started working in Rust. While `#` is documented I think it is hard to discover. So taking the good advice of `@carols10cents`  I am trying to improve the docs with a PR

As a reminder "pretty print" means that where `{:?}` will print something like
```
foo: { b1: 1, b2: 2}
```

`{:#?}` will prints something like
```
foo {
  b1: 1
  b2: 3
}
```

# Changes
Add an example to `fmt` to try and make it easier to discover `#`
…s, r=Mark-Simulacrum

Tell GitHub to highlight `config.toml.example` as TOML

This should be a nice small quality of life improvement when looking at
`config.toml.example` on GitHub or looking at diffs of it in PRs.
@rustbot rustbot added the rollup A PR which is a rollup label Mar 26, 2021
@Dylan-DPC-zz
Copy link
Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Mar 26, 2021

📌 Commit 296f148 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 Mar 26, 2021
@bors
Copy link
Contributor

bors commented Mar 27, 2021

⌛ Testing commit 296f148 with merge d19bae3ed5989bfbab767bac97d58784bbdcd756...

@rust-log-analyzer
Copy link
Collaborator

The job i686-gnu-nopt failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
test [ui] ui/conditional-compilation/cfg-arg-invalid-5.rs ... ok
test [ui] ui/conditional-compilation/cfg-attr-crate-2.rs ... ok
test [ui] ui/command/command-argv0-debug.rs ... ok
test [ui] ui/conditional-compilation/cfg-attr-multi-invalid-1.rs ... ok
test [ui] ui/command/command-create-pidfd.rs ... ok
test [ui] ui/conditional-compilation/cfg-attr-invalid-predicate.rs ... ok
test [ui] ui/conditional-compilation/cfg-attr-multi-invalid-2.rs ... ok
test [ui] ui/command/command-uid-gid.rs ... ok
test [ui] ui/conditional-compilation/cfg-attr-cfg-2.rs ... ok
---
failures:

---- [ui] ui/abi/stack-probes-lto.rs stdout ----

error: test compilation failed although it shouldn't!
status: signal: 6 (core dumped)
command: "/checkout/obj/build/i686-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/abi/stack-probes-lto.rs" "-Zthreads=1" "--target=i686-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "-o" "/checkout/obj/build/i686-unknown-linux-gnu/test/ui/abi/stack-probes-lto/a" "-Crpath" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/i686-unknown-linux-gnu/native/rust-test-helpers" "-C" "lto" "-L" "/checkout/obj/build/i686-unknown-linux-gnu/test/ui/abi/stack-probes-lto/auxiliary"
------------------------------------------

------------------------------------------
Some tests failed in compiletest suite=ui mode=ui host=i686-unknown-linux-gnu target=i686-unknown-linux-gnu
Some tests failed in compiletest suite=ui mode=ui host=i686-unknown-linux-gnu target=i686-unknown-linux-gnu
stderr:
------------------------------------------
rustc: /checkout/src/llvm-project/llvm/lib/CodeGen/RegAllocFast.cpp:883: void {anonymous}::RegAllocFast::defineVirtReg(llvm::MachineInstr&, unsigned int, llvm::Register, bool): Assertion `PhysReg != 0 && "Register not assigned"' failed.
error: ran out of registers during register allocation

------------------------------------------



---- [ui] ui/panic-runtime/lto-unwind.rs stdout ----

error: test compilation failed although it shouldn't!
status: signal: 6 (core dumped)
command: "/checkout/obj/build/i686-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/panic-runtime/lto-unwind.rs" "-Zthreads=1" "--target=i686-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "-o" "/checkout/obj/build/i686-unknown-linux-gnu/test/ui/panic-runtime/lto-unwind/a" "-Crpath" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/i686-unknown-linux-gnu/native/rust-test-helpers" "-C" "lto" "-C" "panic=unwind" "-L" "/checkout/obj/build/i686-unknown-linux-gnu/test/ui/panic-runtime/lto-unwind/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
rustc: /checkout/src/llvm-project/llvm/lib/CodeGen/RegAllocFast.cpp:883: void {anonymous}::RegAllocFast::defineVirtReg(llvm::MachineInstr&, unsigned int, llvm::Register, bool): Assertion `PhysReg != 0 && "Register not assigned"' failed.
------------------------------------------



---
test result: FAILED. 11584 passed; 2 failed; 132 ignored; 0 measured; 0 filtered out; finished in 121.12s



command did not execute successfully: "/checkout/obj/build/i686-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/i686-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/i686-unknown-linux-gnu/stage2/lib/rustlib/i686-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/i686-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/i686-unknown-linux-gnu/test/ui" "--stage-id" "stage2-i686-unknown-linux-gnu" "--suite" "ui" "--mode" "ui" "--target" "i686-unknown-linux-gnu" "--host" "i686-unknown-linux-gnu" "--llvm-filecheck" "/checkout/obj/build/i686-unknown-linux-gnu/llvm/build/bin/FileCheck" "--host-rustcflags" "-Crpath -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/i686-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/i686-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python3" "--lldb-python" "/usr/bin/python3" "--gdb" "/usr/bin/gdb" "--llvm-version" "12.0.0-rust-1.53.0-nightly" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter cfguard codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver dwarflinker engine executionengine extensions filecheck frontendopenacc frontendopenmp fuzzmutate globalisel hellonew hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interfacestub interpreter ipo irreader jitlink libdriver lineeditor linker lto mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts object objectyaml option orcjit orcshared orctargetprocess passes powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo runtimedyld scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target textapi transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info xray" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test
Build completed unsuccessfully in 0:23:01

@bors
Copy link
Contributor

bors commented Mar 27, 2021

💔 Test failed - checks-actions

@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 Mar 27, 2021
@JohnTitor
Copy link
Member

I'm 90% sure that #81825 caused the rollup failure but not 100% so I'm not going to r- just to confirm.

@joshtriplett
Copy link
Member

"ran out of registers during register allocation" doesn't seem very likely to be #81825.

@JohnTitor
Copy link
Member

@joshtriplett Thanks for checking, yeah, just failed: #81825 (comment)

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.