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.

4d2d3fc5da/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)
This commit is contained in:
bors 2018-02-14 16:05:19 +00:00
commit 3ec5a99aaa
4 changed files with 34 additions and 34 deletions

View file

@ -600,25 +600,9 @@ impl<'a> Builder<'a> {
// //
// FIXME: the guard against msvc shouldn't need to be here // FIXME: the guard against msvc shouldn't need to be here
if !target.contains("msvc") { if !target.contains("msvc") {
let ccache = self.config.ccache.as_ref(); let cc = self.cc(target);
let ccacheify = |s: &Path| { cargo.env(format!("CC_{}", target), cc)
let ccache = match ccache { .env("CC", cc);
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 cflags = self.cflags(target).join(" "); let cflags = self.cflags(target).join(" ");
cargo.env(format!("CFLAGS_{}", target), cflags.clone()) cargo.env(format!("CFLAGS_{}", target), cflags.clone())
@ -633,9 +617,8 @@ impl<'a> Builder<'a> {
} }
if let Ok(cxx) = self.cxx(target) { if let Ok(cxx) = self.cxx(target) {
let cxx = ccacheify(&cxx); cargo.env(format!("CXX_{}", target), cxx)
cargo.env(format!("CXX_{}", target), &cxx) .env("CXX", cxx)
.env("CXX", &cxx)
.env(format!("CXXFLAGS_{}", target), cflags.clone()) .env(format!("CXXFLAGS_{}", target), cflags.clone())
.env("CXXFLAGS", cflags); .env("CXXFLAGS", cflags);
} }

View file

@ -1246,15 +1246,18 @@ macro_rules! iterator {
{ {
// The addition might panic on overflow // The addition might panic on overflow
// Use the len of the slice to hint optimizer to remove result index bounds check. // 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| { self.try_fold(0, move |i, x| {
if predicate(x) { Err(i) } if predicate(x) { Err(i) }
else { Ok(i + 1) } else { Ok(i + 1) }
}).err() }).err()
.map(|i| { // // FIXME(#48116/#45964):
unsafe { assume(i < n) }; // // This assume() causes misoptimization on LLVM 6.
i // // Commented out until it is fixed again.
}) // .map(|i| {
// unsafe { assume(i < n) };
// i
// })
} }
#[inline] #[inline]
@ -1271,10 +1274,13 @@ macro_rules! iterator {
if predicate(x) { Err(i) } if predicate(x) { Err(i) }
else { Ok(i) } else { Ok(i) }
}).err() }).err()
.map(|i| { // // FIXME(#48116/#45964):
unsafe { assume(i < n) }; // // This assume() causes misoptimization on LLVM 6.
i // // Commented out until it is fixed again.
}) // .map(|i| {
// unsafe { assume(i < n) };
// i
// })
} }
} }

View file

@ -1026,6 +1026,8 @@ fn import_path_to_string(names: &[SpannedIdent],
if names.is_empty() { if names.is_empty() {
import_directive_subclass_to_string(subclass) import_directive_subclass_to_string(subclass)
} else { } else {
// FIXME: Remove this entire logic after #48116 is fixed.
//
// Note that this code looks a little wonky, it's currently here to // Note that this code looks a little wonky, it's currently here to
// hopefully help debug #48116, but otherwise isn't intended to // hopefully help debug #48116, but otherwise isn't intended to
// cause any problems. // cause any problems.
@ -1034,8 +1036,17 @@ fn import_path_to_string(names: &[SpannedIdent],
names_to_string(names), names_to_string(names),
import_directive_subclass_to_string(subclass), import_directive_subclass_to_string(subclass),
); );
assert!(!names.is_empty()); if names.is_empty() || x.starts_with("::") {
assert!(!x.starts_with("::")); span_bug!(
span,
"invalid name `{}` at {:?}; global = {}, names = {:?}, subclass = {:?}",
x,
span,
global,
names,
subclass
);
}
return x return x
} }
} }

View file

@ -250,6 +250,7 @@ impl<'test> TestCx<'test> {
fn run_cfail_test(&self) { fn run_cfail_test(&self) {
let proc_res = self.compile_test(); let proc_res = self.compile_test();
self.check_if_test_should_compile(&proc_res); 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 output_to_check = self.get_output(&proc_res);
let expected_errors = errors::load_errors(&self.testpaths.file, self.revision); 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_error_patterns(&output_to_check, &proc_res);
} }
self.check_no_compiler_crash(&proc_res);
self.check_forbid_output(&output_to_check, &proc_res); self.check_forbid_output(&output_to_check, &proc_res);
} }