Skip to content

Commit

Permalink
Auto merge of #58464 - jethrogb:jb/std-test-panic-output, r=alexcrichton
Browse files Browse the repository at this point in the history
Use the correct stderr when testing libstd

When compiling the unit tests for libstd, there are two copies of `std` in existence, see [lib.rs](https://github.com/rust-lang/rust/blob/919cf42/src/libstd/lib.rs#L335-L341). This means there are two copies of everything, including thread local variable definitions. Before this PR, it's possible that libtest would configure a stderr sink in one of those copies, whereas the panic logic would inspect the sink in the other copy, resulting in libtest missing the relevant panic message. This PR makes sure that when testing, the panic logic always accesses the stderr sink from “realstd”, using the same logic that libtest uses.
  • Loading branch information
bors committed Mar 3, 2019
2 parents 0ea2271 + c0e8cf9 commit 7dbba3d
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 19 deletions.
14 changes: 14 additions & 0 deletions src/libstd/io/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,20 @@ impl<B: BufRead + ?Sized> BufRead for Box<B> {
}
}

// Used by panicking::default_hook
#[cfg(test)]
/// This impl is only used by printing logic, so any error returned is always
/// of kind `Other`, and should be ignored.
impl Write for Box<dyn (::realstd::io::Write) + Send> {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
(**self).write(buf).map_err(|_| ErrorKind::Other.into())
}

fn flush(&mut self) -> io::Result<()> {
(**self).flush().map_err(|_| ErrorKind::Other.into())
}
}

// =============================================================================
// In-memory buffer implementations

Expand Down
16 changes: 14 additions & 2 deletions src/libstd/io/stdio.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![cfg_attr(test, allow(unused))]

use crate::io::prelude::*;

use crate::cell::RefCell;
Expand All @@ -16,6 +18,13 @@ thread_local! {
}
}

/// Stderr used by eprint! and eprintln! macros, and panics
thread_local! {
static LOCAL_STDERR: RefCell<Option<Box<dyn Write + Send>>> = {
RefCell::new(None)
}
}

/// A handle to a raw instance of the standard input stream of this process.
///
/// This handle is not synchronized or buffered in any fashion. Constructed via
Expand Down Expand Up @@ -668,7 +677,6 @@ impl fmt::Debug for StderrLock<'_> {
issue = "0")]
#[doc(hidden)]
pub fn set_panic(sink: Option<Box<dyn Write + Send>>) -> Option<Box<dyn Write + Send>> {
use crate::panicking::LOCAL_STDERR;
use crate::mem;
LOCAL_STDERR.with(move |slot| {
mem::replace(&mut *slot.borrow_mut(), sink)
Expand Down Expand Up @@ -740,6 +748,7 @@ where
reason = "implementation detail which may disappear or be replaced at any time",
issue = "0")]
#[doc(hidden)]
#[cfg(not(test))]
pub fn _print(args: fmt::Arguments) {
print_to(args, &LOCAL_STDOUT, stdout, "stdout");
}
Expand All @@ -748,11 +757,14 @@ pub fn _print(args: fmt::Arguments) {
reason = "implementation detail which may disappear or be replaced at any time",
issue = "0")]
#[doc(hidden)]
#[cfg(not(test))]
pub fn _eprint(args: fmt::Arguments) {
use crate::panicking::LOCAL_STDERR;
print_to(args, &LOCAL_STDERR, stderr, "stderr");
}

#[cfg(test)]
pub use realstd::io::{_eprint, _print};

#[cfg(test)]
mod tests {
use crate::panic::{UnwindSafe, RefUnwindSafe};
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@
// std may use features in a platform-specific way
#![allow(unused_features)]

#![cfg_attr(test, feature(test, update_panic_count))]
#![cfg_attr(test, feature(print_internals, set_stdio, test, update_panic_count))]
#![cfg_attr(all(target_vendor = "fortanix", target_env = "sgx"),
feature(global_asm, range_contains, slice_index_methods,
decl_macro, coerce_unsized, sgx_platform, ptr_wrapping_offset_from))]
Expand Down
28 changes: 12 additions & 16 deletions src/libstd/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,9 @@
//! * Executing a panic up to doing the actual implementation
//! * Shims around "try"
use core::panic::BoxMeUp;
use core::panic::{PanicInfo, Location};

use crate::io::prelude::*;
use core::panic::{BoxMeUp, PanicInfo, Location};

use crate::any::Any;
use crate::cell::RefCell;
use crate::fmt;
use crate::intrinsics;
use crate::mem;
Expand All @@ -25,11 +21,12 @@ use crate::sys_common::thread_info;
use crate::sys_common::util;
use crate::thread;

thread_local! {
pub static LOCAL_STDERR: RefCell<Option<Box<dyn Write + Send>>> = {
RefCell::new(None)
}
}
#[cfg(not(test))]
use crate::io::set_panic;
// make sure to use the stderr output configured
// by libtest in the real copy of std
#[cfg(test)]
use realstd::io::set_panic;

// Binary interface to the panic runtime that the standard library depends on.
//
Expand Down Expand Up @@ -205,12 +202,11 @@ fn default_hook(info: &PanicInfo) {
}
};

if let Some(mut local) = LOCAL_STDERR.with(|s| s.borrow_mut().take()) {
write(&mut *local);
let mut s = Some(local);
LOCAL_STDERR.with(|slot| {
*slot.borrow_mut() = s.take();
});
if let Some(mut local) = set_panic(None) {
// NB. In `cfg(test)` this uses the forwarding impl
// for `Box<dyn (::realstd::io::Write) + Send>`.
write(&mut local);
set_panic(Some(local));
} else if let Some(mut out) = panic_output() {
write(&mut out);
}
Expand Down

0 comments on commit 7dbba3d

Please sign in to comment.