Rollup merge of rust-lang#37410 - TimNN:gdb-next-gen, r=alexcrichton

Support GDB with native rust support

This PR aims at making the debuginfo tests pass with the newer GDB versions which have native rust support (RGDB)

To that end debuginfo tests now support three GDB prefixes:
- `gdb` applicable to both GDB varieties
- `gdbg` (**G**eneric) only applicable to the old GDB
- `gdbr` (**R**ust) only applicable to the new RGDB

Whether the GDB has rust support is detected based on it's version: GDB >= 7.11.10 is considered to have rust support.

---

**Test updates**

All tests have been updated to the new GDB version. Note that some tests currently require the GDB trunk<sup>1</sup>.

---

**How to move forward with this PR:**

I propose the following steps for moving forward with this PR:
- [x] Validate the general approach of this PR (the `gdb-`, `gdbg-` and `gdbr-` split)
- [x] Validate the approach taken for updating the debuginfo tests (I've checked this since there's (almost) no `set language c` left, which was my main concern)
- [x] Determine how to distinguish between the new and old GDB (and implement that)
- [ ] Add one or more non-gating build bots with the new GDB (blocked on the previous item, can happen after this PR has been merged)
- [ ] If the new bots pass the tests, gate on them
- [x] \(Maybe) update the remaining tests to run without `set syntax c` (in a separate PR)
- [ ] \(Maybe) add tests specifically for the new GDB (in a separate PR / open an issue about this)

I'm not completely sure about the build bot related steps (cc @alexcrichton), the current approach was suggested to prevent any downtime / broken build time between a new GDB gating builder being added and this PR being merged.

---

**Suboptimal RGDB Output**

I've found several places where the output of RGDB is not ideal. They are tagged with `// FIXME`, here is an overview:
- [x] Trait references are not syntactically correct: `type_names::&Trait2<...>` (**WontFix**: the issue is minor and from @Manishearth below: "to properly address the trait issue we should wait for trait object support")
- [x] Univariant enums are printed as `<error reading variable>` (**Fixed** in GDB trunk<sup>1<sup>)
- [x] Unions are printed as `<error reading variable>` (**Fixed** in GDB trunk<sup>1</sup>)
- [x] "Nil Enums" (`enum Foo {}`) are printed as `<error reading variable>` (**WontFix**: the are not supposed to exist)
- [x] I have found no alternative for `sizeof(var)` in rust mode, so had to resort to `set language c` (**Fixed** in GDB trunk<sup>1</sup>)
- [x] I have found not way of interpreting a value as a specific enum variant, so had to resort to `set language c` (**Fixed** in GDB trunk<sup>1</sup>)
- [x] Before the initial `run` command, gdb does not realise it should be in rust mode (thus, if one want's to print statics before the run one has to explicitly `set language rust`) (maybe this is intended / expected behaviour, if so, someone please tell me ;) (**"Expected"** / current behaviour of GDB: picks up jemalloc, see rust-lang#37410 (comment))

<sup>1</sup>: Or rather in @Manishearth's trunk, waiting to be merged upstream.

---

cc @alexcrichton, @michaelwoerister, rust-lang#36323

configure

@@ -868,13 +868,6 @@ then
     fi
 fi
 
-if [ -n "$CFG_GDB" ]
-then
-    # Store GDB's version
-    CFG_GDB_VERSION=$($CFG_GDB --version 2>/dev/null | head -1)
-    putvar CFG_GDB_VERSION
-fi
-
 if [ -n "$CFG_LLDB" ]
 then
     # Store LLDB's version

mk/tests.mk

@@ -648,7 +648,7 @@ CTEST_COMMON_ARGS$(1)-T-$(2)-H-$(3) = \
         --host $(3) \
 	--docck-python $$(CFG_PYTHON) \
 	--lldb-python $$(CFG_LLDB_PYTHON) \
-        --gdb-version="$(CFG_GDB_VERSION)" \
+        --gdb="$(CFG_GDB)" \
         --lldb-version="$(CFG_LLDB_VERSION)" \
         --llvm-version="$$(LLVM_VERSION_$(3))" \
         --android-cross-path=$(CFG_ARM_LINUX_ANDROIDEABI_NDK) \

src/bootstrap/check.rs

@@ -168,8 +168,8 @@ pub fn compiletest(build: &Build,
         cmd.arg("--lldb-python").arg(python_default);
     }
 
-    if let Some(ref vers) = build.gdb_version {
-        cmd.arg("--gdb-version").arg(vers);
+    if let Some(ref gdb) = build.config.gdb {
+        cmd.arg("--gdb").arg(gdb);
     }
     if let Some(ref vers) = build.lldb_version {
         cmd.arg("--lldb-version").arg(vers);

src/bootstrap/config.rs

@@ -23,6 +23,7 @@ use std::process;
 use num_cpus;
 use rustc_serialize::Decodable;
 use toml::{Parser, Decoder, Value};
+use util::push_exe_path;
 
 /// Global configuration for the entire build and/or bootstrap.
 ///
@@ -86,6 +87,7 @@ pub struct Config {
     pub mandir: Option<String>,
     pub codegen_tests: bool,
     pub nodejs: Option<PathBuf>,
+    pub gdb: Option<PathBuf>,
 }
 
 /// Per-target configuration stored in the global configuration structure.
@@ -123,6 +125,7 @@ struct Build {
     compiler_docs: Option<bool>,
     docs: Option<bool>,
     submodules: Option<bool>,
+    gdb: Option<String>,
 }
 
 /// TOML representation of how the LLVM build is configured.
@@ -227,6 +230,7 @@ impl Config {
         }
         config.rustc = build.rustc.map(PathBuf::from);
         config.cargo = build.cargo.map(PathBuf::from);
+        config.gdb = build.gdb.map(PathBuf::from);
         set(&mut config.compiler_docs, build.compiler_docs);
         set(&mut config.docs, build.docs);
         set(&mut config.submodules, build.submodules);
@@ -356,44 +360,47 @@ impl Config {
                                        .collect();
                 }
                 "CFG_MUSL_ROOT" if value.len() > 0 => {
-                    self.musl_root = Some(PathBuf::from(value));
+                    self.musl_root = Some(parse_configure_path(value));
                 }
                 "CFG_MUSL_ROOT_X86_64" if value.len() > 0 => {
                     let target = "x86_64-unknown-linux-musl".to_string();
                     let target = self.target_config.entry(target)
                                      .or_insert(Target::default());
-                    target.musl_root = Some(PathBuf::from(value));
+                    target.musl_root = Some(parse_configure_path(value));
                 }
                 "CFG_MUSL_ROOT_I686" if value.len() > 0 => {
                     let target = "i686-unknown-linux-musl".to_string();
                     let target = self.target_config.entry(target)
                                      .or_insert(Target::default());
-                    target.musl_root = Some(PathBuf::from(value));
+                    target.musl_root = Some(parse_configure_path(value));
                 }
                 "CFG_MUSL_ROOT_ARM" if value.len() > 0 => {
                     let target = "arm-unknown-linux-musleabi".to_string();
                     let target = self.target_config.entry(target)
                                      .or_insert(Target::default());
-                    target.musl_root = Some(PathBuf::from(value));
+                    target.musl_root = Some(parse_configure_path(value));
                 }
                 "CFG_MUSL_ROOT_ARMHF" if value.len() > 0 => {
                     let target = "arm-unknown-linux-musleabihf".to_string();
                     let target = self.target_config.entry(target)
                                      .or_insert(Target::default());
-                    target.musl_root = Some(PathBuf::from(value));
+                    target.musl_root = Some(parse_configure_path(value));
                 }
                 "CFG_MUSL_ROOT_ARMV7" if value.len() > 0 => {
                     let target = "armv7-unknown-linux-musleabihf".to_string();
                     let target = self.target_config.entry(target)
                                      .or_insert(Target::default());
-                    target.musl_root = Some(PathBuf::from(value));
+                    target.musl_root = Some(parse_configure_path(value));
                 }
                 "CFG_DEFAULT_AR" if value.len() > 0 => {
                     self.rustc_default_ar = Some(value.to_string());
                 }
                 "CFG_DEFAULT_LINKER" if value.len() > 0 => {
                     self.rustc_default_linker = Some(value.to_string());
                 }
+                "CFG_GDB" if value.len() > 0 => {
+                    self.gdb = Some(parse_configure_path(value));
+                }
                 "CFG_RELEASE_CHANNEL" => {
                     self.channel = value.to_string();
                 }
@@ -412,48 +419,72 @@ impl Config {
                 "CFG_LLVM_ROOT" if value.len() > 0 => {
                     let target = self.target_config.entry(self.build.clone())
                                      .or_insert(Target::default());
-                    let root = PathBuf::from(value);
-                    target.llvm_config = Some(root.join("bin/llvm-config"));
+                    let root = parse_configure_path(value);
+                    target.llvm_config = Some(push_exe_path(root, &["bin", "llvm-config"]));
                 }
                 "CFG_JEMALLOC_ROOT" if value.len() > 0 => {
                     let target = self.target_config.entry(self.build.clone())
                                      .or_insert(Target::default());
-                    target.jemalloc = Some(PathBuf::from(value));
+                    target.jemalloc = Some(parse_configure_path(value));
                 }
                 "CFG_ARM_LINUX_ANDROIDEABI_NDK" if value.len() > 0 => {
                     let target = "arm-linux-androideabi".to_string();
                     let target = self.target_config.entry(target)
                                      .or_insert(Target::default());
-                    target.ndk = Some(PathBuf::from(value));
+                    target.ndk = Some(parse_configure_path(value));
                 }
                 "CFG_ARMV7_LINUX_ANDROIDEABI_NDK" if value.len() > 0 => {
                     let target = "armv7-linux-androideabi".to_string();
                     let target = self.target_config.entry(target)
                                      .or_insert(Target::default());
-                    target.ndk = Some(PathBuf::from(value));
+                    target.ndk = Some(parse_configure_path(value));
                 }
                 "CFG_I686_LINUX_ANDROID_NDK" if value.len() > 0 => {
                     let target = "i686-linux-android".to_string();
                     let target = self.target_config.entry(target)
                                      .or_insert(Target::default());
-                    target.ndk = Some(PathBuf::from(value));
+                    target.ndk = Some(parse_configure_path(value));
                 }
                 "CFG_AARCH64_LINUX_ANDROID_NDK" if value.len() > 0 => {
                     let target = "aarch64-linux-android".to_string();
                     let target = self.target_config.entry(target)
                                      .or_insert(Target::default());
-                    target.ndk = Some(PathBuf::from(value));
+                    target.ndk = Some(parse_configure_path(value));
                 }
                 "CFG_LOCAL_RUST_ROOT" if value.len() > 0 => {
-                    self.rustc = Some(PathBuf::from(value).join("bin/rustc"));
-                    self.cargo = Some(PathBuf::from(value).join("bin/cargo"));
+                    let path = parse_configure_path(value);
+                    self.rustc = Some(push_exe_path(path.clone(), &["bin", "rustc"]));
+                    self.cargo = Some(push_exe_path(path, &["bin", "cargo"]));
                 }
                 _ => {}
             }
         }
     }
 }
 
+#[cfg(not(windows))]
+fn parse_configure_path(path: &str) -> PathBuf {
+    path.into()
+}
+
+#[cfg(windows)]
+fn parse_configure_path(path: &str) -> PathBuf {
+    // on windows, configure produces unix style paths e.g. /c/some/path but we
+    // only want real windows paths
+
+    use build_helper;
+
+    // '/' is invalid in windows paths, so we can detect unix paths by the presence of it
+    if !path.contains('/') {
+        return path.into();
+    }
+
+    let win_path = build_helper::output(Command::new("cygpath").arg("-w").arg(path));
+    let win_path = win_path.trim();
+
+    win_path.into()
+}
+
 fn set<T>(field: &mut T, val: Option<T>) {
     if let Some(v) = val {
         *field = v;

src/bootstrap/config.toml.example

@@ -79,6 +79,9 @@
 # Indicate whether submodules are managed and updated automatically.
 #submodules = true
 
+# The path to (or name of) the GDB executable to use
+#gdb = "gdb"
+
 # =============================================================================
 # Options for compiling Rust code itself
 # =============================================================================

src/bootstrap/lib.rs

@@ -123,7 +123,6 @@ pub struct Build {
     bootstrap_key_stage0: String,
 
     // Probed tools at runtime
-    gdb_version: Option<String>,
     lldb_version: Option<String>,
     lldb_python_dir: Option<String>,
 
@@ -196,7 +195,6 @@ impl Build {
             package_vers: String::new(),
             cc: HashMap::new(),
             cxx: HashMap::new(),
-            gdb_version: None,
             lldb_version: None,
             lldb_python_dir: None,
         }

src/bootstrap/sanity.rs

@@ -92,6 +92,12 @@ pub fn check(build: &mut Build) {
         need_cmd(s.as_ref());
     }
 
+    if let Some(ref gdb) = build.config.gdb {
+        need_cmd(gdb.as_ref());
+    } else {
+        build.config.gdb = have_cmd("gdb".as_ref());
+    }
+
     // We're gonna build some custom C code here and there, host triples
     // also build some C++ shims for LLVM so we need a C++ compiler.
     for target in build.config.target.iter() {
@@ -198,7 +204,6 @@ $ pacman -R cmake && pacman -S mingw-w64-x86_64-cmake
                    .to_string()
         })
     };
-    build.gdb_version = run(Command::new("gdb").arg("--version")).ok();
     build.lldb_version = run(Command::new("lldb").arg("--version")).ok();
     if build.lldb_version.is_some() {
         build.lldb_python_dir = run(Command::new("lldb").arg("-P")).ok();

src/bootstrap/util.rs

@@ -172,3 +172,21 @@ pub fn dylib_path() -> Vec<PathBuf> {
     env::split_paths(&env::var_os(dylib_path_var()).unwrap_or(OsString::new()))
         .collect()
 }
+
+/// `push` all components to `buf`. On windows, append `.exe` to the last component.
+pub fn push_exe_path(mut buf: PathBuf, components: &[&str]) -> PathBuf {
+    let (&file, components) = components.split_last().expect("at least one component required");
+    let mut file = file.to_owned();
+
+    if cfg!(windows) {
+        file.push_str(".exe");
+    }
+
+    for c in components {
+        buf.push(c);
+    }
+
+    buf.push(file);
+
+    buf
+}

src/test/debuginfo/associated-types.rs

@@ -16,7 +16,8 @@
 // gdb-command:run
 
 // gdb-command:print arg
-// gdb-check:$1 = {b = -1, b1 = 0}
+// gdbg-check:$1 = {b = -1, b1 = 0}
+// gdbr-check:$1 = associated_types::Struct<i32> {b: -1, b1: 0}
 // gdb-command:continue
 
 // gdb-command:print inferred
@@ -30,7 +31,8 @@
 // gdb-command:continue
 
 // gdb-command:print arg
-// gdb-check:$5 = {__0 = 4, __1 = 5}
+// gdbg-check:$5 = {__0 = 4, __1 = 5}
+// gdbr-check:$5 = (4, 5)
 // gdb-command:continue
 
 // gdb-command:print a

src/test/debuginfo/basic-types-globals-metadata.rs

@@ -12,33 +12,47 @@
 
 // compile-flags:-g
 // gdb-command:run
-// gdb-command:whatis 'basic_types_globals_metadata::B'
+// gdbg-command:whatis 'basic_types_globals_metadata::B'
+// gdbr-command:whatis basic_types_globals_metadata::B
 // gdb-check:type = bool
-// gdb-command:whatis 'basic_types_globals_metadata::I'
+// gdbg-command:whatis 'basic_types_globals_metadata::I'
+// gdbr-command:whatis basic_types_globals_metadata::I
 // gdb-check:type = isize
-// gdb-command:whatis 'basic_types_globals_metadata::C'
+// gdbg-command:whatis 'basic_types_globals_metadata::C'
+// gdbr-command:whatis basic_types_globals_metadata::C
 // gdb-check:type = char
-// gdb-command:whatis 'basic_types_globals_metadata::I8'
+// gdbg-command:whatis 'basic_types_globals_metadata::I8'
+// gdbr-command:whatis basic_types_globals_metadata::I8
 // gdb-check:type = i8
-// gdb-command:whatis 'basic_types_globals_metadata::I16'
+// gdbg-command:whatis 'basic_types_globals_metadata::I16'
+// gdbr-command:whatis basic_types_globals_metadata::I16
 // gdb-check:type = i16
-// gdb-command:whatis 'basic_types_globals_metadata::I32'
+// gdbg-command:whatis 'basic_types_globals_metadata::I32'
+// gdbr-command:whatis basic_types_globals_metadata::I32
 // gdb-check:type = i32
-// gdb-command:whatis 'basic_types_globals_metadata::I64'
+// gdbg-command:whatis 'basic_types_globals_metadata::I64'
+// gdbr-command:whatis basic_types_globals_metadata::I64
 // gdb-check:type = i64
-// gdb-command:whatis 'basic_types_globals_metadata::U'
+// gdbg-command:whatis 'basic_types_globals_metadata::U'
+// gdbr-command:whatis basic_types_globals_metadata::U
 // gdb-check:type = usize
-// gdb-command:whatis 'basic_types_globals_metadata::U8'
+// gdbg-command:whatis 'basic_types_globals_metadata::U8'
+// gdbr-command:whatis basic_types_globals_metadata::U8
 // gdb-check:type = u8
-// gdb-command:whatis 'basic_types_globals_metadata::U16'
+// gdbg-command:whatis 'basic_types_globals_metadata::U16'
+// gdbr-command:whatis basic_types_globals_metadata::U16
 // gdb-check:type = u16
-// gdb-command:whatis 'basic_types_globals_metadata::U32'
+// gdbg-command:whatis 'basic_types_globals_metadata::U32'
+// gdbr-command:whatis basic_types_globals_metadata::U32
 // gdb-check:type = u32
-// gdb-command:whatis 'basic_types_globals_metadata::U64'
+// gdbg-command:whatis 'basic_types_globals_metadata::U64'
+// gdbr-command:whatis basic_types_globals_metadata::U64
 // gdb-check:type = u64
-// gdb-command:whatis 'basic_types_globals_metadata::F32'
+// gdbg-command:whatis 'basic_types_globals_metadata::F32'
+// gdbr-command:whatis basic_types_globals_metadata::F32
 // gdb-check:type = f32
-// gdb-command:whatis 'basic_types_globals_metadata::F64'
+// gdbg-command:whatis 'basic_types_globals_metadata::F64'
+// gdbr-command:whatis basic_types_globals_metadata::F64
 // gdb-check:type = f64
 // gdb-command:continue