Skip to content

Commit

Permalink
Auto merge of #74613 - Mark-Simulacrum:revert-gimli, r=nnethercote
Browse files Browse the repository at this point in the history
Revert libbacktrace -> gimli

This reverts 4cbd265 028f8d7 13db3cc d7a36d8 (and technically 79673d3 but it's made empty by previous reverts).

The current plan is to land this PR as a temporary change, so that we can get a better handle on the regressions introduced by it. Trying to fix/examine them in master is difficult, and we want to be better able to evaluate them without impact to other PRs being landed in the mean time.

That said, it is currently *my* belief that gimli, in one form or another, will need to land sometime soon. I think it's quite likely that it may slip a week or two, but I would personally push for re-landing it then "regardless" of the regressions. We should try to focus efforts on understanding and removing as much of the performance impact as possible, as everyone pretty much agrees that it should be quite minimal (and entirely in the linker, basically).

r? @nnethercote
  • Loading branch information
bors committed Jul 23, 2020
2 parents 2bbfa02 + b747a33 commit 371917a
Show file tree
Hide file tree
Showing 14 changed files with 55 additions and 108 deletions.
3 changes: 0 additions & 3 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,3 @@
[submodule "src/tools/rust-analyzer"]
path = src/tools/rust-analyzer
url = https://github.com/rust-analyzer/rust-analyzer.git
[submodule "src/backtrace"]
path = src/backtrace
url = https://github.com/rust-lang/backtrace-rs.git
88 changes: 29 additions & 59 deletions Cargo.lock
Original file line number Diff line number Diff line change
@@ -1,26 +1,10 @@
# This file is automatically @generated by Cargo.
# It is not intended for manual editing.
[[package]]
name = "addr2line"
version = "0.13.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1b6a2d3371669ab3ca9797670853d61402b03d0b4b9ebf33d677dfa720203072"
dependencies = [
"compiler_builtins",
"gimli",
"rustc-std-workspace-alloc",
"rustc-std-workspace-core",
]

[[package]]
name = "adler"
version = "0.2.2"
name = "adler32"
version = "1.0.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ccc9a9dd069569f212bc4330af9f17c4afb5e8ce185e83dbb14f1349dda18b10"
dependencies = [
"compiler_builtins",
"rustc-std-workspace-core",
]
checksum = "7e522997b529f05601e05166c07ed17789691f562762c7f3b987263d2dedee5c"

[[package]]
name = "aho-corasick"
Expand Down Expand Up @@ -141,14 +125,28 @@ checksum = "f8aac770f1885fd7e387acedd76065302551364496e46b3dd00860b2f8359b9d"

[[package]]
name = "backtrace"
version = "0.3.50"
version = "0.3.46"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b1e692897359247cc6bb902933361652380af0f1b7651ae5c5013407f30e109e"
dependencies = [
"addr2line",
"backtrace-sys",
"cfg-if",
"compiler_builtins",
"libc",
"miniz_oxide",
"object",
"rustc-demangle",
"rustc-std-workspace-core",
]

[[package]]
name = "backtrace-sys"
version = "0.1.37"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "18fbebbe1c9d1f383a9cc7e8ccdb471b91c8d024ee9c2ca5b5346121fe8b4399"
dependencies = [
"cc",
"compiler_builtins",
"libc",
"rustc-std-workspace-core",
]

[[package]]
Expand Down Expand Up @@ -690,9 +688,9 @@ dependencies = [

[[package]]
name = "crc32fast"
version = "1.2.0"
version = "1.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ba125de2af0df55319f41944744ad91c71113bf74a4646efff39afe1f6842db1"
checksum = "e91d5240c6975ef33aeb5f148f35275c25eda8e8a5f95abe421978b05b8bf192"
dependencies = [
"cfg-if",
]
Expand Down Expand Up @@ -1025,9 +1023,9 @@ checksum = "37ab347416e802de484e4d03c7316c48f1ecb56574dfd4a46a80f173ce1de04d"

[[package]]
name = "flate2"
version = "1.0.16"
version = "1.0.12"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "68c90b0fc46cf89d227cc78b40e494ff81287a92dd07631e5af0d06fe3cf885e"
checksum = "ad3c5233c9a940c8719031b423d7e6c16af66e031cb0420b0896f5245bf181d3"
dependencies = [
"cfg-if",
"crc32fast",
Expand Down Expand Up @@ -1161,17 +1159,6 @@ dependencies = [
"wasi",
]

[[package]]
name = "gimli"
version = "0.22.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "aaf91faf136cb47367fa430cd46e37a788775e7fa104f8b4bcb3861dc389b724"
dependencies = [
"compiler_builtins",
"rustc-std-workspace-alloc",
"rustc-std-workspace-core",
]

[[package]]
name = "git2"
version = "0.13.5"
Expand Down Expand Up @@ -1832,14 +1819,11 @@ dependencies = [

[[package]]
name = "miniz_oxide"
version = "0.4.0"
version = "0.3.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "be0f75932c1f6cfae3c04000e40114adf955636e19040f9c0a2c380702aa1c7f"
checksum = "6f3f74f726ae935c3f514300cc6773a0c9492abc5e972d42ba0c0ebb88757625"
dependencies = [
"adler",
"compiler_builtins",
"rustc-std-workspace-alloc",
"rustc-std-workspace-core",
"adler32",
]

[[package]]
Expand Down Expand Up @@ -1971,17 +1955,6 @@ dependencies = [
"libc",
]

[[package]]
name = "object"
version = "0.20.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1ab52be62400ca80aa00285d25253d7f7c437b7375c4de678f5405d3afe82ca5"
dependencies = [
"compiler_builtins",
"rustc-std-workspace-alloc",
"rustc-std-workspace-core",
]

[[package]]
name = "once_cell"
version = "1.1.0"
Expand Down Expand Up @@ -4373,8 +4346,8 @@ dependencies = [
name = "std"
version = "0.0.0"
dependencies = [
"addr2line",
"alloc",
"backtrace",
"cfg-if",
"compiler_builtins",
"core",
Expand All @@ -4383,13 +4356,10 @@ dependencies = [
"hashbrown",
"hermit-abi",
"libc",
"miniz_oxide",
"object",
"panic_abort",
"panic_unwind",
"profiler_builtins",
"rand 0.7.3",
"rustc-demangle",
"unwind",
"wasi",
]
Expand Down
6 changes: 0 additions & 6 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,5 @@ rustc-std-workspace-core = { path = 'src/tools/rustc-std-workspace-core' }
rustc-std-workspace-alloc = { path = 'src/tools/rustc-std-workspace-alloc' }
rustc-std-workspace-std = { path = 'src/tools/rustc-std-workspace-std' }

# This crate's integration with libstd is a bit wonky, so we use a submodule
# instead of a crates.io dependency. Make sure everything else in the repo is
# also using the submodule, however, so we can avoid duplicate copies of the
# source code for this crate.
backtrace = { path = "src/backtrace" }

[patch."https://github.com/rust-lang/rust-clippy"]
clippy_lints = { path = "src/tools/clippy/clippy_lints" }
1 change: 0 additions & 1 deletion rustfmt.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ ignore = [
"src/tools/rust-analyzer",
"src/tools/rust-installer",
"src/tools/rustfmt",
"src/backtrace",

# We do not format this file as it is externally sourced and auto-generated.
"src/libstd/sys/cloudabi/abi/cloudabi.rs",
Expand Down
1 change: 0 additions & 1 deletion src/backtrace
Submodule backtrace deleted from 5965cf
1 change: 0 additions & 1 deletion src/bootstrap/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,6 @@ impl Step for Src {
// (essentially libstd and all of its path dependencies)
let std_src_dirs = [
"src/build_helper",
"src/backtrace/src",
"src/liballoc",
"src/libcore",
"src/libpanic_abort",
Expand Down
23 changes: 9 additions & 14 deletions src/libstd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,11 @@ profiler_builtins = { path = "../libprofiler_builtins", optional = true }
unwind = { path = "../libunwind" }
hashbrown = { version = "0.6.2", default-features = false, features = ['rustc-dep-of-std'] }

# Dependencies of the `backtrace` crate
addr2line = { version = "0.13.0", optional = true, default-features = false }
rustc-demangle = { version = "0.1.4", features = ['rustc-dep-of-std'] }
miniz_oxide = { version = "0.4.0", optional = true, default-features = false }
[dependencies.object]
version = "0.20"
optional = true
default-features = false
features = ['read_core', 'elf', 'macho', 'pe']
[dependencies.backtrace_rs]
package = "backtrace"
version = "0.3.46"
default-features = false # without the libstd `backtrace` feature, stub out everything
features = [ "rustc-dep-of-std" ] # enable build support for integrating into libstd

[dev-dependencies]
rand = "0.7"
Expand All @@ -51,12 +47,11 @@ wasi = { version = "0.9.0", features = ['rustc-dep-of-std'], default-features =

[features]
backtrace = [
"gimli-symbolize",
'addr2line/rustc-dep-of-std',
'object/rustc-dep-of-std',
'miniz_oxide/rustc-dep-of-std',
"backtrace_rs/dbghelp", # backtrace/symbolize on MSVC
"backtrace_rs/libbacktrace", # symbolize on most platforms
"backtrace_rs/libunwind", # backtrace on most platforms
"backtrace_rs/dladdr", # symbolize on platforms w/o libbacktrace
]
gimli-symbolize = []

panic-unwind = ["panic_unwind"]
profiler = ["profiler_builtins"]
Expand Down
21 changes: 11 additions & 10 deletions src/libstd/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,15 @@
// `Backtrace`, but that's a relatively small price to pay relative to capturing
// a backtrace or actually symbolizing it.

use crate::backtrace_rs::{self, BytesOrWideString};
use crate::env;
use crate::ffi::c_void;
use crate::fmt;
use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst};
use crate::sync::Mutex;
use crate::sys_common::backtrace::{lock, output_filename};
use crate::vec::Vec;
use backtrace::BytesOrWideString;
use backtrace_rs as backtrace;

/// A captured OS thread stack backtrace.
///
Expand Down Expand Up @@ -149,7 +150,7 @@ struct BacktraceFrame {
}

enum RawFrame {
Actual(backtrace_rs::Frame),
Actual(backtrace::Frame),
#[cfg(test)]
Fake,
}
Expand Down Expand Up @@ -196,7 +197,7 @@ impl fmt::Debug for BacktraceSymbol {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(fmt, "{{ ")?;

if let Some(fn_name) = self.name.as_ref().map(|b| backtrace_rs::SymbolName::new(b)) {
if let Some(fn_name) = self.name.as_ref().map(|b| backtrace::SymbolName::new(b)) {
write!(fmt, "fn: \"{:#}\"", fn_name)?;
} else {
write!(fmt, "fn: <unknown>")?;
Expand All @@ -222,7 +223,7 @@ impl fmt::Debug for BytesOrWide {
BytesOrWide::Bytes(w) => BytesOrWideString::Bytes(w),
BytesOrWide::Wide(w) => BytesOrWideString::Wide(w),
},
backtrace_rs::PrintFmt::Short,
backtrace::PrintFmt::Short,
crate::env::current_dir().as_ref().ok(),
)
}
Expand Down Expand Up @@ -304,7 +305,7 @@ impl Backtrace {
let mut frames = Vec::new();
let mut actual_start = None;
unsafe {
backtrace_rs::trace_unsynchronized(|frame| {
backtrace::trace_unsynchronized(|frame| {
frames.push(BacktraceFrame {
frame: RawFrame::Actual(frame.clone()),
symbols: Vec::new(),
Expand Down Expand Up @@ -355,9 +356,9 @@ impl fmt::Display for Backtrace {

let full = fmt.alternate();
let (frames, style) = if full {
(&capture.frames[..], backtrace_rs::PrintFmt::Full)
(&capture.frames[..], backtrace::PrintFmt::Full)
} else {
(&capture.frames[capture.actual_start..], backtrace_rs::PrintFmt::Short)
(&capture.frames[capture.actual_start..], backtrace::PrintFmt::Short)
};

// When printing paths we try to strip the cwd if it exists, otherwise
Expand All @@ -369,7 +370,7 @@ impl fmt::Display for Backtrace {
output_filename(fmt, path, style, cwd.as_ref().ok())
};

let mut f = backtrace_rs::BacktraceFmt::new(fmt, style, &mut print_path);
let mut f = backtrace::BacktraceFmt::new(fmt, style, &mut print_path);
f.add_context()?;
for frame in frames {
let mut f = f.frame();
Expand All @@ -379,7 +380,7 @@ impl fmt::Display for Backtrace {
for symbol in frame.symbols.iter() {
f.print_raw(
frame.frame.ip(),
symbol.name.as_ref().map(|b| backtrace_rs::SymbolName::new(b)),
symbol.name.as_ref().map(|b| backtrace::SymbolName::new(b)),
symbol.filename.as_ref().map(|b| match b {
BytesOrWide::Bytes(w) => BytesOrWideString::Bytes(w),
BytesOrWide::Wide(w) => BytesOrWideString::Wide(w),
Expand Down Expand Up @@ -414,7 +415,7 @@ impl Capture {
RawFrame::Fake => unimplemented!(),
};
unsafe {
backtrace_rs::resolve_frame_unsynchronized(frame, |symbol| {
backtrace::resolve_frame_unsynchronized(frame, |symbol| {
symbols.push(BacktraceSymbol {
name: symbol.name().map(|m| m.as_bytes().to_vec()),
filename: symbol.filename_raw().map(|b| match b {
Expand Down
1 change: 0 additions & 1 deletion src/libstd/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,4 @@ fn main() {
println!("cargo:rustc-cfg=feature=\"restricted-std\"");
}
println!("cargo:rustc-env=STD_ENV_ARCH={}", env::var("CARGO_CFG_TARGET_ARCH").unwrap());
println!("cargo:rustc-cfg=backtrace_in_libstd");
}
4 changes: 0 additions & 4 deletions src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,10 +511,6 @@ mod panicking;
// compiler
pub mod rt;

#[path = "../backtrace/src/lib.rs"]
#[allow(dead_code, unused_attributes)]
mod backtrace_rs;

// Pull in the `std_detect` crate directly into libstd. The contents of
// `std_detect` are in a different repository: rust-lang/stdarch.
//
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ fn default_hook(info: &PanicInfo<'_>) {
// If this is a double panic, make sure that we print a backtrace
// for this panic. Otherwise only print it if logging is enabled.
let backtrace_env = if panic_count::get() >= 2 {
RustBacktrace::Print(crate::backtrace_rs::PrintFmt::Full)
RustBacktrace::Print(backtrace_rs::PrintFmt::Full)
} else {
backtrace::rust_backtrace_env()
};
Expand Down
3 changes: 2 additions & 1 deletion src/libstd/sys_common/backtrace.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::backtrace_rs::{self, BacktraceFmt, BytesOrWideString, PrintFmt};
use crate::borrow::Cow;
/// Common code for printing the backtrace in the same way across the different
/// supported platforms.
Expand All @@ -10,6 +9,8 @@ use crate::path::{self, Path, PathBuf};
use crate::sync::atomic::{self, Ordering};
use crate::sys::mutex::Mutex;

use backtrace_rs::{BacktraceFmt, BytesOrWideString, PrintFmt};

/// Max number of frames to print.
const MAX_NB_FRAMES: usize = 100;

Expand Down
Loading

0 comments on commit 371917a

Please sign in to comment.