From 8b7f726aea629ac96ee4cd991efb47b6b7288a79 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Perennou Date: Tue, 18 Sep 2018 14:58:11 +0200 Subject: [PATCH 1/3] rustbuild: drop color handling Let cargo handle that for us Signed-off-by: Marc-Antoine Perennou --- src/bootstrap/bin/rustc.rs | 9 --------- src/bootstrap/compile.rs | 34 +--------------------------------- 2 files changed, 1 insertion(+), 42 deletions(-) diff --git a/src/bootstrap/bin/rustc.rs b/src/bootstrap/bin/rustc.rs index f30f34acf5c6c..b89976eca26c4 100644 --- a/src/bootstrap/bin/rustc.rs +++ b/src/bootstrap/bin/rustc.rs @@ -291,15 +291,6 @@ fn main() { cmd.arg("-Z").arg("verify-llvm-ir"); } - let color = match env::var("RUSTC_COLOR") { - Ok(s) => usize::from_str(&s).expect("RUSTC_COLOR should be an integer"), - Err(_) => 0, - }; - - if color != 0 { - cmd.arg("--color=always"); - } - if env::var_os("RUSTC_DENY_WARNINGS").is_some() && env::var_os("RUSTC_EXTERNAL_TOOL").is_none() { cmd.arg("-Dwarnings"); diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 4205d5aebffa1..608f2c982c2a5 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -29,7 +29,7 @@ use build_helper::{output, mtime, up_to_date}; use filetime::FileTime; use serde_json; -use util::{exe, libdir, is_dylib, CiEnv}; +use util::{exe, libdir, is_dylib}; use {Compiler, Mode, GitRepo}; use native; @@ -1034,29 +1034,6 @@ pub fn add_to_sysroot(builder: &Builder, sysroot_dst: &Path, stamp: &Path) { } } -// Avoiding a dependency on winapi to keep compile times down -#[cfg(unix)] -fn stderr_isatty() -> bool { - use libc; - unsafe { libc::isatty(libc::STDERR_FILENO) != 0 } -} -#[cfg(windows)] -fn stderr_isatty() -> bool { - type DWORD = u32; - type BOOL = i32; - type HANDLE = *mut u8; - const STD_ERROR_HANDLE: DWORD = -12i32 as DWORD; - extern "system" { - fn GetStdHandle(which: DWORD) -> HANDLE; - fn GetConsoleMode(hConsoleHandle: HANDLE, lpMode: *mut DWORD) -> BOOL; - } - unsafe { - let handle = GetStdHandle(STD_ERROR_HANDLE); - let mut out = 0; - GetConsoleMode(handle, &mut out) != 0 - } -} - pub fn run_cargo(builder: &Builder, cargo: &mut Command, tail_args: Vec, @@ -1218,15 +1195,6 @@ pub fn stream_cargo( cargo.arg("--message-format").arg("json") .stdout(Stdio::piped()); - if stderr_isatty() && builder.ci_env == CiEnv::None && - // if the terminal is reported as dumb, then we don't want to enable color for rustc - env::var_os("TERM").map(|t| t != *"dumb").unwrap_or(true) { - // since we pass message-format=json to cargo, we need to tell the rustc - // wrapper to give us colored output if necessary. This is because we - // only want Cargo's JSON output, not rustcs. - cargo.env("RUSTC_COLOR", "1"); - } - for arg in tail_args { cargo.arg(arg); } From 002581f6278957d3d0f35483749488ff3b6a9558 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 15 Sep 2018 23:26:04 +0300 Subject: [PATCH 2/3] avoid leaking host details in proc macro metadata decoding proc macro crates are essentially implemented as dynamic libraries using a dlopen-based ABI. They are also Rust crates, so they have 2 worlds - the "host" world in which they are defined, and the "target" world in which they are used. For all the "target" world knows, the proc macro crate might not even be implemented in Rust, so leaks of details from the host to the target must be avoided for correctness. Because the "host" DefId space is different from the "target" DefId space, any leak involving a DefId will have a nonsensical or out-of-bounds DefKey, and will cause all sorts of crashes. This PR fixes all leaks I have found in `decoder`. In particular, #54059 was caused by host native libraries leaking into the target, which feels like it might even be a correctness issue if it doesn't cause an ICE. Fixes #54059 --- src/librustc_metadata/cstore_impl.rs | 6 -- src/librustc_metadata/decoder.rs | 71 ++++++++++++++----- .../auxiliary/issue_54059.rs | 59 +++++++++++++++ src/test/incremental-fulldeps/issue-54059.rs | 18 +++++ 4 files changed, 130 insertions(+), 24 deletions(-) create mode 100644 src/test/incremental-fulldeps/auxiliary/issue_54059.rs create mode 100644 src/test/incremental-fulldeps/issue-54059.rs diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 87a32b5a53e7a..4382a3c12c961 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -259,12 +259,6 @@ provide! { <'tcx> tcx, def_id, other, cdata, let cnum = cdata.cnum; assert!(cnum != LOCAL_CRATE); - // If this crate is a custom derive crate, then we're not even going to - // link those in so we skip those crates. - if cdata.root.macro_derive_registrar.is_some() { - return Arc::new(Vec::new()) - } - Arc::new(cdata.exported_symbols(tcx)) } } diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 9907df7ed0240..43b03cb863c74 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -692,6 +692,8 @@ impl<'a, 'tcx> CrateMetadata { /// Iterates over all the stability attributes in the given crate. pub fn get_lib_features(&self) -> Vec<(ast::Name, Option)> { + // FIXME: For a proc macro crate, not sure whether we should return the "host" + // features or an empty Vec. Both don't cause ICEs. self.root .lib_features .decode(self) @@ -700,11 +702,16 @@ impl<'a, 'tcx> CrateMetadata { /// Iterates over the language items in the given crate. pub fn get_lang_items(&self) -> Vec<(DefId, usize)> { - self.root - .lang_items - .decode(self) - .map(|(def_index, index)| (self.local_def_id(def_index), index)) - .collect() + if self.proc_macros.is_some() { + // Proc macro crates do not export any lang-items to the target. + vec![] + } else { + self.root + .lang_items + .decode(self) + .map(|(def_index, index)| (self.local_def_id(def_index), index)) + .collect() + } } /// Iterates over each child of the given item. @@ -978,12 +985,16 @@ impl<'a, 'tcx> CrateMetadata { pub fn get_implementations_for_trait(&self, filter: Option, result: &mut Vec) { + if self.proc_macros.is_some() { + // proc-macro crates export no trait impls. + return + } + // Do a reverse lookup beforehand to avoid touching the crate_num // hash map in the loop below. let filter = match filter.map(|def_id| self.reverse_translate_def_id(def_id)) { Some(Some(def_id)) => Some((def_id.krate.as_u32(), def_id.index)), Some(None) => return, - None if self.proc_macros.is_some() => return, None => None, }; @@ -1016,11 +1027,21 @@ impl<'a, 'tcx> CrateMetadata { pub fn get_native_libraries(&self, sess: &Session) -> Vec { - self.root.native_libraries.decode((self, sess)).collect() + if self.proc_macros.is_some() { + // Proc macro crates do not have any *target* native libraries. + vec![] + } else { + self.root.native_libraries.decode((self, sess)).collect() + } } pub fn get_foreign_modules(&self, sess: &Session) -> Vec { - self.root.foreign_modules.decode((self, sess)).collect() + if self.proc_macros.is_some() { + // Proc macro crates do not have any *target* foreign modules. + vec![] + } else { + self.root.foreign_modules.decode((self, sess)).collect() + } } pub fn get_dylib_dependency_formats(&self) -> Vec<(CrateNum, LinkagePreference)> { @@ -1036,10 +1057,15 @@ impl<'a, 'tcx> CrateMetadata { } pub fn get_missing_lang_items(&self) -> Vec { - self.root - .lang_items_missing - .decode(self) - .collect() + if self.proc_macros.is_some() { + // Proc macro crates do not depend on any target weak lang-items. + vec![] + } else { + self.root + .lang_items_missing + .decode(self) + .collect() + } } pub fn get_fn_arg_names(&self, id: DefIndex) -> Vec { @@ -1055,10 +1081,16 @@ impl<'a, 'tcx> CrateMetadata { pub fn exported_symbols(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Vec<(ExportedSymbol<'tcx>, SymbolExportLevel)> { - let lazy_seq: LazySeq<(ExportedSymbol<'tcx>, SymbolExportLevel)> = - LazySeq::with_position_and_length(self.root.exported_symbols.position, - self.root.exported_symbols.len); - lazy_seq.decode((self, tcx)).collect() + if self.proc_macros.is_some() { + // If this crate is a custom derive crate, then we're not even going to + // link those in so we skip those crates. + vec![] + } else { + let lazy_seq: LazySeq<(ExportedSymbol<'tcx>, SymbolExportLevel)> = + LazySeq::with_position_and_length(self.root.exported_symbols.position, + self.root.exported_symbols.len); + lazy_seq.decode((self, tcx)).collect() + } } pub fn get_rendered_const(&self, id: DefIndex) -> String { @@ -1149,9 +1181,12 @@ impl<'a, 'tcx> CrateMetadata { /// file they represent, just information about length, line breaks, and /// multibyte characters. This information is enough to generate valid debuginfo /// for items inlined from other crates. + /// + /// Proc macro crates don't currently export spans, so this function does not have + /// to work for them. pub fn imported_source_files(&'a self, - local_source_map: &source_map::SourceMap) - -> ReadGuard<'a, Vec> { + local_source_map: &source_map::SourceMap) + -> ReadGuard<'a, Vec> { { let source_files = self.source_map_import_info.borrow(); if !source_files.is_empty() { diff --git a/src/test/incremental-fulldeps/auxiliary/issue_54059.rs b/src/test/incremental-fulldeps/auxiliary/issue_54059.rs new file mode 100644 index 0000000000000..ec0d044d4bf8d --- /dev/null +++ b/src/test/incremental-fulldeps/auxiliary/issue_54059.rs @@ -0,0 +1,59 @@ +// Copyright 2018 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. + +// no-prefer-dynamic + +// check that having extern "C" functions in a proc macro doesn't crash. + +#![crate_type="proc-macro"] +#![allow(non_snake_case)] + +extern crate proc_macro; + +macro_rules! proc_macro_tokenstream { + () => { + ::proc_macro::TokenStream + }; +} + +macro_rules! proc_macro_expr_impl { + ($( + $( #[$attr:meta] )* + pub fn $func:ident($input:ident: &str) -> String $body:block + )+) => { + $( + // Parses an input that looks like: + // + // ``` + // #[allow(unused)] + // enum ProcMacroHack { + // Input = (stringify!(ARGS), 0).1, + // } + // ``` + $( #[$attr] )* + #[proc_macro_derive($func)] + pub fn $func(input: proc_macro_tokenstream!()) -> proc_macro_tokenstream!() { + unsafe { rust_dbg_extern_identity_u64(0); } + panic!() + } + )+ + }; +} + +proc_macro_expr_impl! { + pub fn base2_impl(input: &str) -> String { + panic!() + } +} + +#[link(name="rust_test_helpers")] +extern "C" { + pub fn rust_dbg_extern_identity_u64(v: u64) -> u64; +} diff --git a/src/test/incremental-fulldeps/issue-54059.rs b/src/test/incremental-fulldeps/issue-54059.rs new file mode 100644 index 0000000000000..5f0c7db64fe7d --- /dev/null +++ b/src/test/incremental-fulldeps/issue-54059.rs @@ -0,0 +1,18 @@ +// Copyright 2018 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. + +// aux-build:issue_54059.rs +// ignore-stage1 +// ignore-wasm32-bare no libc for ffi testing +// revisions: rpass1 + +extern crate issue_54059; + +fn main() {} From ce782b934b96940495b619e1b3149cb058c6d089 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 21 Sep 2018 10:05:23 -0700 Subject: [PATCH 3/3] Ignore new test on Windows --- src/test/incremental-fulldeps/issue-54059.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/incremental-fulldeps/issue-54059.rs b/src/test/incremental-fulldeps/issue-54059.rs index 5f0c7db64fe7d..dee1e0abf61f2 100644 --- a/src/test/incremental-fulldeps/issue-54059.rs +++ b/src/test/incremental-fulldeps/issue-54059.rs @@ -11,6 +11,7 @@ // aux-build:issue_54059.rs // ignore-stage1 // ignore-wasm32-bare no libc for ffi testing +// ignore-windows - dealing with weird symbols issues on dylibs isn't worth it // revisions: rpass1 extern crate issue_54059;