diff options
| author | bors <bors@rust-lang.org> | 2018-02-14 16:05:19 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2018-02-14 16:05:19 +0000 |
| commit | 3ec5a99aaa0084d97a9e845b34fdf03d1462c475 (patch) | |
| tree | 8c5e904998498ba5c058f1676ce1194d6dde0d6f /src | |
| parent | 4d2d3fc5dadf894a8ad709a5860a549f2c0b1032 (diff) | |
| parent | e0da9902a1ab8e620be45470130cd72e31d54fc2 (diff) | |
Auto merge of #48209 - kennytm:try-fix-48116, r=alexcrichton
Try to fix 48116 and 48192 The bug #48116 happens because of a misoptimization of the `import_path_to_string` function, where a `names` slice is empty but the `!names.is_empty()` branch is executed. https://github.com/rust-lang/rust/blob/4d2d3fc5dadf894a8ad709a5860a549f2c0b1032/src/librustc_resolve/resolve_imports.rs#L1015-L1042 Yesterday, @eddyb had locally reproduced the bug, and [came across the `position` function](https://mozilla.logbot.info/rust-infra/20180214#c14296834) where the `assume()` call is found to be suspicious. We have *not* concluded that this `assume()` causes #48116, but given [the reputation of `assume()`](https://github.com/rust-lang/rust/pull/45501#issuecomment-340159627), this seems higher relevant. Here we try to see if commenting it out can fix the errors. Later @alexcrichton has bisected and found a potential bug [in the LLVM side](https://github.com/rust-lang/rust/issues/48116#issuecomment-365624777). We are currently testing if reverting that LLVM commit is enough to stop the bug. If true, this PR can be reverted (keep the `assume()`) and we could backport the LLVM patch instead. (This PR also includes an earlier commit from #48127 for help debugging ICE happening in compile-fail/parse-fail tests.) The PR also reverts #48059, which seems to cause #48192. r? @alexcrichton cc @eddyb, @arthurprs (#47333)
Diffstat (limited to 'src')
| -rw-r--r-- | src/bootstrap/builder.rs | 27 | ||||
| -rw-r--r-- | src/libcore/slice/mod.rs | 24 | ||||
| -rw-r--r-- | src/librustc_resolve/resolve_imports.rs | 15 | ||||
| -rw-r--r-- | src/tools/compiletest/src/runtest.rs | 2 |
4 files changed, 34 insertions, 34 deletions
diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 74dd4a6fa01..03630dfbed3 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -600,25 +600,9 @@ impl<'a> Builder<'a> { // // FIXME: the guard against msvc shouldn't need to be here if !target.contains("msvc") { - let ccache = self.config.ccache.as_ref(); - let ccacheify = |s: &Path| { - let ccache = match ccache { - Some(ref s) => s, - None => return s.display().to_string(), - }; - // FIXME: the cc-rs crate only recognizes the literal strings - // `ccache` and `sccache` when doing caching compilations, so we - // mirror that here. It should probably be fixed upstream to - // accept a new env var or otherwise work with custom ccache - // vars. - match &ccache[..] { - "ccache" | "sccache" => format!("{} {}", ccache, s.display()), - _ => s.display().to_string(), - } - }; - let cc = ccacheify(&self.cc(target)); - cargo.env(format!("CC_{}", target), &cc) - .env("CC", &cc); + let cc = self.cc(target); + cargo.env(format!("CC_{}", target), cc) + .env("CC", cc); let cflags = self.cflags(target).join(" "); cargo.env(format!("CFLAGS_{}", target), cflags.clone()) @@ -633,9 +617,8 @@ impl<'a> Builder<'a> { } if let Ok(cxx) = self.cxx(target) { - let cxx = ccacheify(&cxx); - cargo.env(format!("CXX_{}", target), &cxx) - .env("CXX", &cxx) + cargo.env(format!("CXX_{}", target), cxx) + .env("CXX", cxx) .env(format!("CXXFLAGS_{}", target), cflags.clone()) .env("CXXFLAGS", cflags); } diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index aacbbd5058e..ac390313a67 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -1246,15 +1246,18 @@ macro_rules! iterator { { // The addition might panic on overflow // Use the len of the slice to hint optimizer to remove result index bounds check. - let n = make_slice!(self.ptr, self.end).len(); + let _n = make_slice!(self.ptr, self.end).len(); self.try_fold(0, move |i, x| { if predicate(x) { Err(i) } else { Ok(i + 1) } }).err() - .map(|i| { - unsafe { assume(i < n) }; - i - }) + // // FIXME(#48116/#45964): + // // This assume() causes misoptimization on LLVM 6. + // // Commented out until it is fixed again. + // .map(|i| { + // unsafe { assume(i < n) }; + // i + // }) } #[inline] @@ -1271,10 +1274,13 @@ macro_rules! iterator { if predicate(x) { Err(i) } else { Ok(i) } }).err() - .map(|i| { - unsafe { assume(i < n) }; - i - }) + // // FIXME(#48116/#45964): + // // This assume() causes misoptimization on LLVM 6. + // // Commented out until it is fixed again. + // .map(|i| { + // unsafe { assume(i < n) }; + // i + // }) } } diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 8cb25f449b6..a8070c553bd 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -1026,6 +1026,8 @@ fn import_path_to_string(names: &[SpannedIdent], if names.is_empty() { import_directive_subclass_to_string(subclass) } else { + // FIXME: Remove this entire logic after #48116 is fixed. + // // Note that this code looks a little wonky, it's currently here to // hopefully help debug #48116, but otherwise isn't intended to // cause any problems. @@ -1034,8 +1036,17 @@ fn import_path_to_string(names: &[SpannedIdent], names_to_string(names), import_directive_subclass_to_string(subclass), ); - assert!(!names.is_empty()); - assert!(!x.starts_with("::")); + if names.is_empty() || x.starts_with("::") { + span_bug!( + span, + "invalid name `{}` at {:?}; global = {}, names = {:?}, subclass = {:?}", + x, + span, + global, + names, + subclass + ); + } return x } } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index a87809dd7bc..bef085e17ea 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -250,6 +250,7 @@ impl<'test> TestCx<'test> { fn run_cfail_test(&self) { let proc_res = self.compile_test(); self.check_if_test_should_compile(&proc_res); + self.check_no_compiler_crash(&proc_res); let output_to_check = self.get_output(&proc_res); let expected_errors = errors::load_errors(&self.testpaths.file, self.revision); @@ -262,7 +263,6 @@ impl<'test> TestCx<'test> { self.check_error_patterns(&output_to_check, &proc_res); } - self.check_no_compiler_crash(&proc_res); self.check_forbid_output(&output_to_check, &proc_res); } |
