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

std: Don't abort process when printing panics in tests #69959

Merged
merged 1 commit into from
Mar 19, 2020

Conversation

alexcrichton
Copy link
Member

This commit fixes an issue when using set_print and friends, notably
used by libtest, to avoid aborting the process if printing panics. This
previously panicked due to borrowing a mutable RefCell twice, and this
is worked around by borrowing these cells for less time, instead
taking out and removing contents temporarily.

Closes #69558

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(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 Mar 12, 2020
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

r=me if my understanding is right

I guess this just adds another case where our thread-local set_error/set_print functions aren't a great fit but that seems largely fine.

if let Some(mut w) = prev {
let result = w.write_fmt(args);
*s.borrow_mut() = Some(w);
return result;
Copy link
Member

Choose a reason for hiding this comment

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

Just to make sure I'm understanding correctly, the reason this does not need a try_borrow_mut anymore is because there's no possibility of a recursive lock, right? I believe the try_borrow was added in e2fd2df FWIW.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right yeah the scope of the borrow should always be one line of code which doesn't execute other arbitrary lines of code (e.g. an arbitrary Write implementation). As a result we should be guaranteed that it always succeeds.

@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

I agree! This is all probably great reasons why set_print and set_panic should never be stabilized :)

@bors
Copy link
Contributor

bors commented Mar 12, 2020

📌 Commit 96e4d12afc8a362227ad14e5ab0ec2bb7f223ded has been approved by Mark-Simulacrum

@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 Mar 12, 2020
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-7 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2020-03-13T00:11:50.4004635Z ========================== Starting Command Output ===========================
2020-03-13T00:11:50.4009724Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/e65549fc-f3a2-43f5-bd31-9778b331eb38.sh
2020-03-13T00:11:50.4010244Z 
2020-03-13T00:11:50.4015688Z ##[section]Finishing: Disable git automatic line ending conversion
2020-03-13T00:11:50.4036227Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/69959/merge to s
2020-03-13T00:11:50.4039837Z Task         : Get sources
2020-03-13T00:11:50.4040164Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-03-13T00:11:50.4040476Z Version      : 1.0.0
2020-03-13T00:11:50.4040709Z Author       : Microsoft
---
2020-03-13T00:11:52.0716008Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-03-13T00:11:52.0723011Z ##[command]git config gc.auto 0
2020-03-13T00:11:52.0727109Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-03-13T00:11:52.0730851Z ##[command]git config --get-all http.proxy
2020-03-13T00:11:52.0738363Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/69959/merge:refs/remotes/pull/69959/merge
---
2020-03-13T01:13:44.1160235Z .................................................................................................... 1700/9768
2020-03-13T01:13:48.8824813Z .................................................................................................... 1800/9768
2020-03-13T01:14:00.9097765Z ................................................................i................................... 1900/9768
2020-03-13T01:14:07.9461442Z .................................................................................................... 2000/9768
2020-03-13T01:14:22.8855272Z ......................................................iiiii......................................... 2100/9768
2020-03-13T01:14:33.9202109Z .................................................................................................... 2300/9768
2020-03-13T01:14:36.2451588Z .................................................................................................... 2400/9768
2020-03-13T01:14:39.4876200Z .................................................................................................... 2500/9768
2020-03-13T01:15:02.2874536Z .................................................................................................... 2600/9768
---
2020-03-13T01:17:46.2605339Z .........................i...............i.......................................................... 5000/9768
2020-03-13T01:17:55.7941505Z .................................................................................................... 5100/9768
2020-03-13T01:18:01.7887477Z ....................................................................i............................... 5200/9768
2020-03-13T01:18:07.9432604Z .................................................................................................... 5300/9768
2020-03-13T01:18:17.2392178Z .................................................ii.ii........i...i................................. 5400/9768
2020-03-13T01:18:25.2613870Z .................................................................................................... 5600/9768
2020-03-13T01:18:34.8343126Z .................................................................................................... 5700/9768
2020-03-13T01:18:41.1022154Z ........................................i........................................................... 5800/9768
2020-03-13T01:18:47.3449274Z .................................................................................................... 5900/9768
2020-03-13T01:18:47.3449274Z .................................................................................................... 5900/9768
2020-03-13T01:18:57.8534899Z .................................................................................................... 6000/9768
2020-03-13T01:19:06.8664761Z .................................ii...i..ii...........i............................................. 6100/9768
2020-03-13T01:19:24.0654383Z .................................................................................................... 6300/9768
2020-03-13T01:19:30.7963735Z .................................................................................................... 6400/9768
2020-03-13T01:19:30.7963735Z .................................................................................................... 6400/9768
2020-03-13T01:19:41.5744701Z ................................................................i..ii............................... 6500/9768
2020-03-13T01:20:07.9999352Z .................................................................................................... 6700/9768
2020-03-13T01:20:13.4654198Z ...F...........................................................i.................................... 6800/9768
2020-03-13T01:20:15.5016464Z .................................................................................................... 6900/9768
2020-03-13T01:20:17.6180658Z .................................................................................................i.. 7000/9768
---
2020-03-13T01:21:58.0885574Z .................................................................................................... 7700/9768
2020-03-13T01:22:02.8135435Z .................................................................................................... 7800/9768
2020-03-13T01:22:08.8148388Z .................................................................................................... 7900/9768
2020-03-13T01:22:14.9261391Z ...............................................i.................................................... 8000/9768
2020-03-13T01:22:25.1675339Z ................................................................................................iiii 8100/9768
2020-03-13T01:22:30.9585422Z iiiiii.i............................................................................................ 8200/9768
2020-03-13T01:22:45.2116046Z .................................................................................................... 8400/9768
2020-03-13T01:22:56.3109461Z .................................................................................................... 8500/9768
2020-03-13T01:23:08.6425767Z .................................................................................................... 8600/9768
2020-03-13T01:23:14.2908320Z .................................................................................................... 8700/9768
---
2020-03-13T01:25:07.3724621Z ---- [ui] ui/panic-while-printing.rs stdout ----
2020-03-13T01:25:07.3724962Z 
2020-03-13T01:25:07.3725183Z error: ui test compiled successfully!
2020-03-13T01:25:07.3725463Z status: exit code: 0
2020-03-13T01:25:07.3727704Z command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/panic-while-printing.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/panic-while-printing" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-A" "unused" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/panic-while-printing/auxiliary"
2020-03-13T01:25:07.3729515Z ------------------------------------------
2020-03-13T01:25:07.3729715Z 
2020-03-13T01:25:07.3730148Z ------------------------------------------
2020-03-13T01:25:07.3730385Z stderr:
---
2020-03-13T01:25:07.3732185Z ---- [ui] ui/test-panic-while-printing.rs stdout ----
2020-03-13T01:25:07.3732423Z 
2020-03-13T01:25:07.3732628Z error: ui test compiled successfully!
2020-03-13T01:25:07.3732884Z status: exit code: 0
2020-03-13T01:25:07.3735147Z command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/test-panic-while-printing.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/test-panic-while-printing" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-A" "unused" "--test" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/test-panic-while-printing/auxiliary"
2020-03-13T01:25:07.3737000Z ------------------------------------------
2020-03-13T01:25:07.3737196Z 
2020-03-13T01:25:07.3737608Z ------------------------------------------
2020-03-13T01:25:07.3737855Z stderr:
---
2020-03-13T01:25:07.3761123Z thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:348:22
2020-03-13T01:25:07.3761642Z note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
2020-03-13T01:25:07.3772990Z 
2020-03-13T01:25:07.3773355Z 
2020-03-13T01:25:07.3781180Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-7/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "7.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
2020-03-13T01:25:07.3788399Z 
2020-03-13T01:25:07.3788715Z 
2020-03-13T01:25:07.3799132Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
2020-03-13T01:25:07.3799662Z Build completed unsuccessfully in 1:07:35
2020-03-13T01:25:07.3799662Z Build completed unsuccessfully in 1:07:35
2020-03-13T01:25:07.3891667Z == clock drift check ==
2020-03-13T01:25:07.3910595Z   local time: Fri Mar 13 01:25:07 UTC 2020
2020-03-13T01:25:07.5586037Z   network time: Fri, 13 Mar 2020 01:25:07 GMT
2020-03-13T01:25:07.5589903Z == end clock drift check ==
2020-03-13T01:25:08.0097481Z 
2020-03-13T01:25:08.0171738Z ##[error]Bash exited with code '1'.
2020-03-13T01:25:08.0186592Z ##[section]Finishing: Run build
2020-03-13T01:25:08.0239671Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/69959/merge to s
2020-03-13T01:25:08.0245835Z Task         : Get sources
2020-03-13T01:25:08.0246225Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-03-13T01:25:08.0246603Z Version      : 1.0.0
2020-03-13T01:25:08.0246858Z Author       : Microsoft
2020-03-13T01:25:08.0246858Z Author       : Microsoft
2020-03-13T01:25:08.0247271Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-03-13T01:25:08.0247751Z ==============================================================================
2020-03-13T01:25:08.3939365Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-03-13T01:25:08.3983538Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/69959/merge to s
2020-03-13T01:25:08.4081597Z Cleaning up task key
2020-03-13T01:25:08.4083191Z Start cleaning up orphan processes.
2020-03-13T01:25:08.4278160Z Terminate orphan process: pid (4619) (python)
2020-03-13T01:25:08.4539944Z ##[section]Finishing: Finalize Job

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @rust-lang/infra. (Feature Requests)

@Mark-Simulacrum
Copy link
Member

@bors r-

Looks like some tests are passing now and are expected to fail?

  • ui/panic-while-printing.rs
  • ui/test-panic-while-printing.rs

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 13, 2020
@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

Oh just forgot the annotation to say they're expected to pass

@bors
Copy link
Contributor

bors commented Mar 13, 2020

📌 Commit 7d31795 has been approved by Mark-Simulacrum

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 13, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 14, 2020
…Mark-Simulacrum

std: Don't abort process when printing panics in tests

This commit fixes an issue when using `set_print` and friends, notably
used by libtest, to avoid aborting the process if printing panics. This
previously panicked due to borrowing a mutable `RefCell` twice, and this
is worked around by borrowing these cells for less time, instead
taking out and removing contents temporarily.

Closes rust-lang#69558
@Dylan-DPC-zz
Copy link

Failed in rollup #70008
@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 15, 2020
@Kixiron Kixiron 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 Mar 16, 2020
@JohnTitor JohnTitor 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 Mar 16, 2020
@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Mar 17, 2020

📌 Commit 773c04c has been approved by Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Mar 17, 2020

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 17, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 17, 2020
…Mark-Simulacrum

std: Don't abort process when printing panics in tests

This commit fixes an issue when using `set_print` and friends, notably
used by libtest, to avoid aborting the process if printing panics. This
previously panicked due to borrowing a mutable `RefCell` twice, and this
is worked around by borrowing these cells for less time, instead
taking out and removing contents temporarily.

Closes rust-lang#69558
@ehuss
Copy link
Contributor

ehuss commented Mar 18, 2020

@bors r-
Failed in #70085 on wasm32-unknown-unknown

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 18, 2020
This commit fixes an issue when using `set_print` and friends, notably
used by libtest, to avoid aborting the process if printing panics. This
previously panicked due to borrowing a mutable `RefCell` twice, and this
is worked around by borrowing these cells for less time, instead
taking out and removing contents temporarily.

Closes rust-lang#69558
@alexcrichton
Copy link
Member Author

@bors: r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Mar 18, 2020

📌 Commit d5b6a20 has been approved by Mark-Simulacrum

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 18, 2020
Centril added a commit to Centril/rust that referenced this pull request Mar 19, 2020
…Mark-Simulacrum

std: Don't abort process when printing panics in tests

This commit fixes an issue when using `set_print` and friends, notably
used by libtest, to avoid aborting the process if printing panics. This
previously panicked due to borrowing a mutable `RefCell` twice, and this
is worked around by borrowing these cells for less time, instead
taking out and removing contents temporarily.

Closes rust-lang#69558
bors added a commit that referenced this pull request Mar 19, 2020
Rollup of 9 pull requests

Successful merges:

 - #69036 (rustc: don't resolve Instances which would produce malformed shims.)
 - #69443 (tidy: Better license checks.)
 - #69814 (Smaller and more correct generator codegen)
 - #69929 (Regenerate tables for Unicode 13.0.0)
 - #69959 (std: Don't abort process when printing panics in tests)
 - #69969 (unix: Set a guard page at the end of signal stacks)
 - #70005 ([rustdoc] Improve visibility for code blocks warnings)
 - #70088 (Use copy bound in atomic operations to generate simpler MIR)
 - #70095 (Implement -Zlink-native-libraries)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Mar 19, 2020
Rollup of 9 pull requests

Successful merges:

 - #68941 (Properly handle Spans that reference imported SourceFiles)
 - #69036 (rustc: don't resolve Instances which would produce malformed shims.)
 - #69443 (tidy: Better license checks.)
 - #69814 (Smaller and more correct generator codegen)
 - #69929 (Regenerate tables for Unicode 13.0.0)
 - #69959 (std: Don't abort process when printing panics in tests)
 - #69969 (unix: Set a guard page at the end of signal stacks)
 - #70005 ([rustdoc] Improve visibility for code blocks warnings)
 - #70088 (Use copy bound in atomic operations to generate simpler MIR)

Failed merges:

r? @ghost
@bors bors merged commit 73c3a49 into rust-lang:master Mar 19, 2020
@alexcrichton alexcrichton deleted the fix-panic-in-print branch July 23, 2020 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display in test fails with SIGILL (illegal instruction)
8 participants