From d4fc3ec208186f0a9de8d05a6802c95b07e058fd Mon Sep 17 00:00:00 2001 From: Diggory Blake Date: Wed, 26 Aug 2015 01:44:55 +0100 Subject: [PATCH] Add line numbers to windows-gnu backtraces Fix formatting Remove unused imports Refactor Fix msvc build Fix line lengths Formatting Enable backtrace tests Fix using directive on mac pwd info Work-around buildbot PWD bug, and fix libbacktrace configuration Use alternative to `env -u` which is not supported on bitrig Disable tests on 32-bit windows gnu --- mk/rt.mk | 23 +++++-- src/libbacktrace/configure | 2 +- src/libstd/sys/common/backtrace.rs | 30 ++++++++- .../printing => common/gnu}/libbacktrace.rs | 10 +-- src/libstd/sys/common/gnu/mod.rs | 15 +++++ src/libstd/sys/common/mod.rs | 4 ++ src/libstd/sys/unix/backtrace/mod.rs | 29 --------- .../sys/unix/backtrace/printing/dladdr.rs | 2 +- src/libstd/sys/unix/backtrace/printing/gnu.rs | 11 ++++ src/libstd/sys/unix/backtrace/printing/mod.rs | 4 +- src/libstd/sys/windows/backtrace.rs | 61 ++++++++----------- src/libstd/sys/windows/printing/gnu.rs | 22 +++++++ src/libstd/sys/windows/printing/msvc.rs | 42 +++++++++++++ src/test/run-pass/backtrace-debuginfo.rs | 23 ++++--- src/test/run-pass/backtrace.rs | 7 ++- 15 files changed, 191 insertions(+), 94 deletions(-) rename src/libstd/sys/{unix/backtrace/printing => common/gnu}/libbacktrace.rs (96%) create mode 100644 src/libstd/sys/common/gnu/mod.rs create mode 100644 src/libstd/sys/unix/backtrace/printing/gnu.rs create mode 100644 src/libstd/sys/windows/printing/gnu.rs create mode 100644 src/libstd/sys/windows/printing/msvc.rs diff --git a/mk/rt.mk b/mk/rt.mk index 75051f9184f..1f60aaed473 100644 --- a/mk/rt.mk +++ b/mk/rt.mk @@ -259,8 +259,10 @@ BACKTRACE_NAME_$(1) := $$(call CFG_STATIC_LIB_NAME_$(1),backtrace) BACKTRACE_LIB_$(1) := $$(RT_OUTPUT_DIR_$(1))/$$(BACKTRACE_NAME_$(1)) BACKTRACE_BUILD_DIR_$(1) := $$(RT_OUTPUT_DIR_$(1))/libbacktrace -# We don't use this on platforms that aren't linux-based, so just make the file -# available, the compilation of libstd won't actually build it. +# We don't use this on platforms that aren't linux-based (with the exception of +# msys2/mingw builds on windows, which use it to read the dwarf debug +# information) so just make the file available, the compilation of libstd won't +# actually build it. ifeq ($$(findstring darwin,$$(OSTYPE_$(1))),darwin) # See comment above $$(BACKTRACE_LIB_$(1)): @@ -273,7 +275,7 @@ $$(BACKTRACE_LIB_$(1)): touch $$@ else -ifeq ($$(CFG_WINDOWSY_$(1)),1) +ifeq ($$(findstring msvc,$(1)),msvc) # See comment above $$(BACKTRACE_LIB_$(1)): touch $$@ @@ -296,16 +298,25 @@ endif # ./configure script. This is done to force libbacktrace to *not* use the # atomic/sync functionality because it pulls in unnecessary dependencies and we # never use it anyway. +# +# We also use `env PWD=` to clear the PWD environment variable, and then +# execute the command in a new shell. This is necessary to workaround a +# buildbot/msys2 bug: the shell is launched with PWD set to a windows-style path, +# which results in all further uses of `pwd` also printing a windows-style path, +# which breaks libbacktrace's configure script. Clearing PWD within the same +# shell is not sufficient. + $$(BACKTRACE_BUILD_DIR_$(1))/Makefile: $$(BACKTRACE_DEPS) $$(MKFILE_DEPS) @$$(call E, configure: libbacktrace for $(1)) $$(Q)rm -rf $$(BACKTRACE_BUILD_DIR_$(1)) $$(Q)mkdir -p $$(BACKTRACE_BUILD_DIR_$(1)) - $$(Q)(cd $$(BACKTRACE_BUILD_DIR_$(1)) && \ + $$(Q)(cd $$(BACKTRACE_BUILD_DIR_$(1)) && env \ + PWD= \ CC="$$(CC_$(1))" \ AR="$$(AR_$(1))" \ RANLIB="$$(AR_$(1)) s" \ CFLAGS="$$(CFG_GCCISH_CFLAGS_$(1):-Werror=) -fno-stack-protector" \ - $(S)src/libbacktrace/configure --target=$(1) --host=$(CFG_BUILD)) + $(S)src/libbacktrace/configure --build=$(CFG_GNU_TRIPLE_$(CFG_BUILD)) --host=$(CFG_GNU_TRIPLE_$(1))) $$(Q)echo '#undef HAVE_ATOMIC_FUNCTIONS' >> \ $$(BACKTRACE_BUILD_DIR_$(1))/config.h $$(Q)echo '#undef HAVE_SYNC_FUNCTIONS' >> \ @@ -317,7 +328,7 @@ $$(BACKTRACE_LIB_$(1)): $$(BACKTRACE_BUILD_DIR_$(1))/Makefile $$(MKFILE_DEPS) INCDIR=$(S)src/libbacktrace $$(Q)cp $$(BACKTRACE_BUILD_DIR_$(1))/.libs/libbacktrace.a $$@ -endif # endif for windowsy +endif # endif for msvc endif # endif for ios endif # endif for darwin diff --git a/src/libbacktrace/configure b/src/libbacktrace/configure index 5b108d612b8..d9e8075845f 100755 --- a/src/libbacktrace/configure +++ b/src/libbacktrace/configure @@ -3974,7 +3974,7 @@ am_lf=' ' case `pwd` in *[\\\"\#\$\&\'\`$am_lf]*) - as_fn_error "unsafe absolute working directory name" "$LINENO" 5;; + as_fn_error "unsafe absolute working directory name: \``pwd`'" "$LINENO" 5;; esac case $srcdir in *[\\\"\#\$\&\'\`$am_lf\ \ ]*) diff --git a/src/libstd/sys/common/backtrace.rs b/src/libstd/sys/common/backtrace.rs index 7f56afc9e1f..7845714e7f6 100644 --- a/src/libstd/sys/common/backtrace.rs +++ b/src/libstd/sys/common/backtrace.rs @@ -8,9 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use io::prelude::*; - use io; +use io::prelude::*; +use str; +use libc; #[cfg(target_pointer_width = "64")] pub const HEX_WIDTH: usize = 18; @@ -18,6 +19,31 @@ pub const HEX_WIDTH: usize = 18; #[cfg(target_pointer_width = "32")] pub const HEX_WIDTH: usize = 10; + +// These output functions should now be used everywhere to ensure consistency. +pub fn output(w: &mut Write, idx: isize, addr: *mut libc::c_void, + s: Option<&[u8]>) -> io::Result<()> { + try!(write!(w, " {:2}: {:2$?} - ", idx, addr, HEX_WIDTH)); + match s.and_then(|s| str::from_utf8(s).ok()) { + Some(string) => try!(demangle(w, string)), + None => try!(write!(w, "")), + } + w.write_all(&['\n' as u8]) +} + +#[allow(dead_code)] +pub fn output_fileline(w: &mut Write, file: &[u8], line: libc::c_int, + more: bool) -> io::Result<()> { + let file = str::from_utf8(file).unwrap_or(""); + // prior line: " ##: {:2$} - func" + try!(write!(w, " {:3$}at {}:{}", "", file, line, HEX_WIDTH)); + if more { + try!(write!(w, " <... and possibly more>")); + } + w.write_all(&['\n' as u8]) +} + + // All rust symbols are in theory lists of "::"-separated identifiers. Some // assemblers, however, can't handle these characters in symbol names. To get // around this, we use C++-style mangling. The mangling method is: diff --git a/src/libstd/sys/unix/backtrace/printing/libbacktrace.rs b/src/libstd/sys/common/gnu/libbacktrace.rs similarity index 96% rename from src/libstd/sys/unix/backtrace/printing/libbacktrace.rs rename to src/libstd/sys/common/gnu/libbacktrace.rs index 5640eb81f2a..7a2ca0a9f09 100644 --- a/src/libstd/sys/unix/backtrace/printing/libbacktrace.rs +++ b/src/libstd/sys/common/gnu/libbacktrace.rs @@ -11,13 +11,12 @@ use io; use io::prelude::*; use libc; +use sys_common::backtrace::{output, output_fileline}; -use sys::backtrace::{output, output_fileline}; pub fn print(w: &mut Write, idx: isize, addr: *mut libc::c_void, symaddr: *mut libc::c_void) -> io::Result<()> { use env; use ffi::CStr; - use os::unix::prelude::*; use ptr; //////////////////////////////////////////////////////////////////////// @@ -129,14 +128,15 @@ pub fn print(w: &mut Write, idx: isize, addr: *mut libc::c_void, let selfname = if cfg!(target_os = "freebsd") || cfg!(target_os = "dragonfly") || cfg!(target_os = "bitrig") || - cfg!(target_os = "openbsd") { + cfg!(target_os = "openbsd") || + cfg!(target_os = "windows") { env::current_exe().ok() } else { None }; - let filename = match selfname { + let filename = match selfname.as_ref().and_then(|s| s.as_os_str().to_bytes()) { Some(path) => { - let bytes = path.as_os_str().as_bytes(); + let bytes = path; if bytes.len() < LAST_FILENAME.len() { let i = bytes.iter(); for (slot, val) in LAST_FILENAME.iter_mut().zip(i) { diff --git a/src/libstd/sys/common/gnu/mod.rs b/src/libstd/sys/common/gnu/mod.rs new file mode 100644 index 00000000000..3a8cf2d8425 --- /dev/null +++ b/src/libstd/sys/common/gnu/mod.rs @@ -0,0 +1,15 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(missing_docs)] +#![allow(non_camel_case_types)] +#![allow(non_snake_case)] + +pub mod libbacktrace; diff --git a/src/libstd/sys/common/mod.rs b/src/libstd/sys/common/mod.rs index aca1f42c529..b8074235fb2 100644 --- a/src/libstd/sys/common/mod.rs +++ b/src/libstd/sys/common/mod.rs @@ -23,6 +23,10 @@ pub mod thread_info; pub mod thread_local; pub mod wtf8; +#[cfg(any(all(unix, not(any(target_os = "macos", target_os = "ios"))), + all(windows, target_env = "gnu")))] +pub mod gnu; + // common error constructors /// A trait for viewing representations from std types diff --git a/src/libstd/sys/unix/backtrace/mod.rs b/src/libstd/sys/unix/backtrace/mod.rs index 79833c1cacb..d7c05e513f6 100644 --- a/src/libstd/sys/unix/backtrace/mod.rs +++ b/src/libstd/sys/unix/backtrace/mod.rs @@ -85,36 +85,7 @@ pub use self::tracing::write; -use io; -use io::prelude::*; -use libc; -use str; - -use sys_common::backtrace::{demangle, HEX_WIDTH}; - // tracing impls: mod tracing; // symbol resolvers: mod printing; - -pub fn output(w: &mut Write, idx: isize, addr: *mut libc::c_void, - s: Option<&[u8]>) -> io::Result<()> { - try!(write!(w, " {:2}: {:2$?} - ", idx, addr, HEX_WIDTH)); - match s.and_then(|s| str::from_utf8(s).ok()) { - Some(string) => try!(demangle(w, string)), - None => try!(write!(w, "")), - } - w.write_all(&['\n' as u8]) -} - -#[allow(dead_code)] -pub fn output_fileline(w: &mut Write, file: &[u8], line: libc::c_int, - more: bool) -> io::Result<()> { - let file = str::from_utf8(file).unwrap_or(""); - // prior line: " ##: {:2$} - func" - try!(write!(w, " {:3$}at {}:{}", "", file, line, HEX_WIDTH)); - if more { - try!(write!(w, " <... and possibly more>")); - } - w.write_all(&['\n' as u8]) -} diff --git a/src/libstd/sys/unix/backtrace/printing/dladdr.rs b/src/libstd/sys/unix/backtrace/printing/dladdr.rs index 2bca56921d5..d9b759dc673 100644 --- a/src/libstd/sys/unix/backtrace/printing/dladdr.rs +++ b/src/libstd/sys/unix/backtrace/printing/dladdr.rs @@ -14,7 +14,7 @@ use libc; pub fn print(w: &mut Write, idx: isize, addr: *mut libc::c_void, _symaddr: *mut libc::c_void) -> io::Result<()> { - use sys::backtrace::{output}; + use sys_common::backtrace::{output}; use intrinsics; use ffi::CStr; diff --git a/src/libstd/sys/unix/backtrace/printing/gnu.rs b/src/libstd/sys/unix/backtrace/printing/gnu.rs new file mode 100644 index 00000000000..fb06fbedaf5 --- /dev/null +++ b/src/libstd/sys/unix/backtrace/printing/gnu.rs @@ -0,0 +1,11 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +pub use sys_common::gnu::libbacktrace::print; diff --git a/src/libstd/sys/unix/backtrace/printing/mod.rs b/src/libstd/sys/unix/backtrace/printing/mod.rs index a3bd45566f1..e09832c231e 100644 --- a/src/libstd/sys/unix/backtrace/printing/mod.rs +++ b/src/libstd/sys/unix/backtrace/printing/mod.rs @@ -8,12 +8,12 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -pub use self::imp::*; +pub use self::imp::print; #[cfg(any(target_os = "macos", target_os = "ios"))] #[path = "dladdr.rs"] mod imp; #[cfg(not(any(target_os = "macos", target_os = "ios")))] -#[path = "libbacktrace.rs"] +#[path = "gnu.rs"] mod imp; diff --git a/src/libstd/sys/windows/backtrace.rs b/src/libstd/sys/windows/backtrace.rs index d36ca709c5c..35e3c1d4663 100644 --- a/src/libstd/sys/windows/backtrace.rs +++ b/src/libstd/sys/windows/backtrace.rs @@ -27,17 +27,28 @@ use io::prelude::*; use dynamic_lib::DynamicLibrary; -use ffi::CStr; use intrinsics; use io; use libc; -use mem; use path::Path; use ptr; -use str; use sync::StaticMutex; -use sys_common::backtrace::*; +macro_rules! sym{ ($lib:expr, $e:expr, $t:ident) => (unsafe { + let lib = $lib; + match lib.symbol($e) { + Ok(f) => $crate::mem::transmute::<*mut u8, $t>(f), + Err(..) => return Ok(()) + } +}) } + +#[cfg(target_env = "msvc")] +#[path = "printing/msvc.rs"] +mod printing; + +#[cfg(target_env = "gnu")] +#[path = "printing/gnu.rs"] +mod printing; #[allow(non_snake_case)] extern "system" { @@ -302,23 +313,15 @@ pub fn write(w: &mut Write) -> io::Result<()> { // Open up dbghelp.dll, we don't link to it explicitly because it can't // always be found. Additionally, it's nice having fewer dependencies. let path = Path::new("dbghelp.dll"); - let lib = match DynamicLibrary::open(Some(&path)) { + let dbghelp = match DynamicLibrary::open(Some(&path)) { Ok(lib) => lib, Err(..) => return Ok(()), }; - macro_rules! sym{ ($e:expr, $t:ident) => (unsafe { - match lib.symbol($e) { - Ok(f) => mem::transmute::<*mut u8, $t>(f), - Err(..) => return Ok(()) - } - }) } - // Fetch the symbols necessary from dbghelp.dll - let SymFromAddr = sym!("SymFromAddr", SymFromAddrFn); - let SymInitialize = sym!("SymInitialize", SymInitializeFn); - let SymCleanup = sym!("SymCleanup", SymCleanupFn); - let StackWalk64 = sym!("StackWalk64", StackWalk64Fn); + let SymInitialize = sym!(&dbghelp, "SymInitialize", SymInitializeFn); + let SymCleanup = sym!(&dbghelp, "SymCleanup", SymCleanupFn); + let StackWalk64 = sym!(&dbghelp, "StackWalk64", StackWalk64Fn); // Allocate necessary structures for doing the stack walk let process = unsafe { GetCurrentProcess() }; @@ -334,7 +337,9 @@ pub fn write(w: &mut Write) -> io::Result<()> { let _c = Cleanup { handle: process, SymCleanup: SymCleanup }; // And now that we're done with all the setup, do the stack walking! - let mut i = 0; + // Start from -1 to avoid printing this stack frame, which will + // always be exactly the same. + let mut i = -1; try!(write!(w, "stack backtrace:\n")); while StackWalk64(image, process, thread, &mut frame, &mut context, ptr::null_mut(), @@ -346,28 +351,10 @@ pub fn write(w: &mut Write) -> io::Result<()> { frame.AddrReturn.Offset == 0 { break } i += 1; - try!(write!(w, " {:2}: {:#2$x}", i, addr, HEX_WIDTH)); - let mut info: SYMBOL_INFO = unsafe { intrinsics::init() }; - info.MaxNameLen = MAX_SYM_NAME as libc::c_ulong; - // the struct size in C. the value is different to - // `size_of::() - MAX_SYM_NAME + 1` (== 81) - // due to struct alignment. - info.SizeOfStruct = 88; - let mut displacement = 0u64; - let ret = SymFromAddr(process, addr as u64, &mut displacement, - &mut info); - - if ret == libc::TRUE { - try!(write!(w, " - ")); - let ptr = info.Name.as_ptr() as *const libc::c_char; - let bytes = unsafe { CStr::from_ptr(ptr).to_bytes() }; - match str::from_utf8(bytes) { - Ok(s) => try!(demangle(w, s)), - Err(..) => try!(w.write_all(&bytes[..bytes.len()-1])), - } + if i >= 0 { + try!(printing::print(w, i, addr-1, &dbghelp, process)); } - try!(w.write_all(&['\n' as u8])); } Ok(()) diff --git a/src/libstd/sys/windows/printing/gnu.rs b/src/libstd/sys/windows/printing/gnu.rs new file mode 100644 index 00000000000..8d3c93bb7b1 --- /dev/null +++ b/src/libstd/sys/windows/printing/gnu.rs @@ -0,0 +1,22 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use dynamic_lib::DynamicLibrary; +use io; +use io::prelude::*; +use libc; + +use sys_common::gnu::libbacktrace; + +pub fn print(w: &mut Write, i: isize, addr: u64, _: &DynamicLibrary, _: libc::HANDLE) + -> io::Result<()> { + let addr = addr as usize as *mut libc::c_void; + libbacktrace::print(w, i, addr, addr) +} diff --git a/src/libstd/sys/windows/printing/msvc.rs b/src/libstd/sys/windows/printing/msvc.rs new file mode 100644 index 00000000000..25cef04ca96 --- /dev/null +++ b/src/libstd/sys/windows/printing/msvc.rs @@ -0,0 +1,42 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use sys_common::backtrace::output; +use ffi::CStr; +use dynamic_lib::DynamicLibrary; +use super::{SymFromAddrFn, SYMBOL_INFO, MAX_SYM_NAME}; +use io; +use io::prelude::*; +use intrinsics; +use libc; + +pub fn print(w: &mut Write, i: isize, addr: u64, dbghelp: &DynamicLibrary, process: libc::HANDLE) + -> io::Result<()> { + let SymFromAddr = sym!(dbghelp, "SymFromAddr", SymFromAddrFn); + + let mut info: SYMBOL_INFO = unsafe { intrinsics::init() }; + info.MaxNameLen = MAX_SYM_NAME as libc::c_ulong; + // the struct size in C. the value is different to + // `size_of::() - MAX_SYM_NAME + 1` (== 81) + // due to struct alignment. + info.SizeOfStruct = 88; + + let mut displacement = 0u64; + let ret = SymFromAddr(process, addr as u64, &mut displacement, &mut info); + + let name = if ret == libc::TRUE { + let ptr = info.Name.as_ptr() as *const libc::c_char; + Some(unsafe { CStr::from_ptr(ptr).to_bytes() }) + } else { + None + }; + + output(w, i, addr as usize as *mut libc::c_void, name) +} diff --git a/src/test/run-pass/backtrace-debuginfo.rs b/src/test/run-pass/backtrace-debuginfo.rs index f889e381dc4..5feca9422f6 100644 --- a/src/test/run-pass/backtrace-debuginfo.rs +++ b/src/test/run-pass/backtrace-debuginfo.rs @@ -27,11 +27,12 @@ macro_rules! pos { () => ((file!(), line!())) } -#[cfg(all(unix, - not(target_os = "macos"), - not(target_os = "ios"), - not(target_os = "android"), - not(all(target_os = "linux", target_arch = "arm"))))] +#[cfg(any(all(unix, + not(target_os = "macos"), + not(target_os = "ios"), + not(target_os = "android"), + not(all(target_os = "linux", target_arch = "arm"))), + all(windows, target_env = "gnu", not(target_arch = "x86"))))] macro_rules! dump_and_die { ($($pos:expr),*) => ({ // FIXME(#18285): we cannot include the current position because @@ -42,11 +43,12 @@ macro_rules! dump_and_die { } // this does not work on Windows, Android, OSX or iOS -#[cfg(any(not(unix), - target_os = "macos", - target_os = "ios", - target_os = "android", - all(target_os = "linux", target_arch = "arm")))] +#[cfg(not(any(all(unix, + not(target_os = "macos"), + not(target_os = "ios"), + not(target_os = "android"), + not(all(target_os = "linux", target_arch = "arm"))), + all(windows, target_env = "gnu", not(target_arch = "x86")))))] macro_rules! dump_and_die { ($($pos:expr),*) => ({ let _ = [$($pos),*]; }) } @@ -165,3 +167,4 @@ fn main() { run_test(&args[0]); } } + diff --git a/src/test/run-pass/backtrace.rs b/src/test/run-pass/backtrace.rs index 3f4849dbcb2..5d65f9eb2be 100644 --- a/src/test/run-pass/backtrace.rs +++ b/src/test/run-pass/backtrace.rs @@ -9,8 +9,8 @@ // except according to those terms. // no-pretty-expanded FIXME #15189 -// ignore-windows FIXME #13259 // ignore-android FIXME #17520 +// ignore-msvc FIXME #28133 use std::env; use std::process::{Command, Stdio}; @@ -89,6 +89,7 @@ fn runtest(me: &str) { "bad output4: {}", s); } +#[cfg(not(all(windows, target_arch = "x86")))] fn main() { let args: Vec = env::args().collect(); if args.len() >= 2 && args[1] == "fail" { @@ -99,3 +100,7 @@ fn main() { runtest(&args[0]); } } + +// See issue 28218 +#[cfg(all(windows, target_arch = "x86"))] +fn main() {}