about summary refs log tree commit diff
path: root/src
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2018-02-14 16:05:19 +0000
committerbors <bors@rust-lang.org>2018-02-14 16:05:19 +0000
commit3ec5a99aaa0084d97a9e845b34fdf03d1462c475 (patch)
tree8c5e904998498ba5c058f1676ce1194d6dde0d6f /src
parent4d2d3fc5dadf894a8ad709a5860a549f2c0b1032 (diff)
parente0da9902a1ab8e620be45470130cd72e31d54fc2 (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.rs27
-rw-r--r--src/libcore/slice/mod.rs24
-rw-r--r--src/librustc_resolve/resolve_imports.rs15
-rw-r--r--src/tools/compiletest/src/runtest.rs2
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);
     }