From 5676bd51aeaa6b05735dc888a2e41770a4d7b162 Mon Sep 17 00:00:00 2001 From: Midas Lambrichts Date: Fri, 26 Mar 2021 23:16:22 +0100 Subject: [PATCH 1/7] Break when there is a mismatch in the type count When other errors are generated, there can be a mismatch between the amount of input types in MIR, and the amount in the function itself. Break from the comparative loop if this is the case to prevent out-of-bounds. --- .../rustc_mir/src/borrow_check/type_check/input_output.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/compiler/rustc_mir/src/borrow_check/type_check/input_output.rs b/compiler/rustc_mir/src/borrow_check/type_check/input_output.rs index 77d9136622458..1bb447d105781 100644 --- a/compiler/rustc_mir/src/borrow_check/type_check/input_output.rs +++ b/compiler/rustc_mir/src/borrow_check/type_check/input_output.rs @@ -70,6 +70,12 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { // Equate expected input tys with those in the MIR. for (argument_index, &normalized_input_ty) in normalized_input_tys.iter().enumerate() { + if argument_index + 1 >= body.local_decls.len() { + self.tcx() + .sess + .delay_span_bug(body.span, "found more normalized_input_ty than local_decls"); + break; + } // In MIR, argument N is stored in local N+1. let local = Local::new(argument_index + 1); From 2e4215cb72a256b4843323ea5c9f1552e2c13a43 Mon Sep 17 00:00:00 2001 From: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> Date: Thu, 1 Apr 2021 00:52:02 -0400 Subject: [PATCH 2/7] Fix minor typo in once.rs --- library/std/src/sync/once.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sync/once.rs b/library/std/src/sync/once.rs index 2e5f843fc43cb..a24a5cb2ae39f 100644 --- a/library/std/src/sync/once.rs +++ b/library/std/src/sync/once.rs @@ -471,7 +471,7 @@ fn wait(state_and_queue: &AtomicUsize, mut current_state: usize) { // If the managing thread happens to signal and unpark us before we // can park ourselves, the result could be this thread never gets // unparked. Luckily `park` comes with the guarantee that if it got - // an `unpark` just before on an unparked thread is does not park. + // an `unpark` just before on an unparked thread it does not park. thread::park(); } break; From a38d3cb17aed86e9cf977b571926954224c43869 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Thu, 1 Apr 2021 20:43:44 +0900 Subject: [PATCH 3/7] Add my new email address to .mailmap --- .mailmap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index 3f23aed31a833..1019710dc9793 100644 --- a/.mailmap +++ b/.mailmap @@ -286,7 +286,7 @@ Xuefeng Wu Xuefeng Wu Xuefeng Wu XuefengWu York Xiang Youngsoo Son -Yuki Okushi +Yuki Okushi Zach Pomerantz Zack Corr Zack Slayton From 2d813b26092ac67933de06009e2eede04b056923 Mon Sep 17 00:00:00 2001 From: Midas Lambrichts Date: Thu, 1 Apr 2021 20:54:57 +0200 Subject: [PATCH 4/7] Add a test that triggers the out-of-bounds ICE. Add a test that has the right input to trigger an out-of-bounds error when in MIR the local_decls and the normalized_input_tys don't match in amount. --- .../issue-83499-input-output-iteration-ice.rs | 10 +++++++++ ...ue-83499-input-output-iteration-ice.stderr | 21 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 src/test/ui/mir/issue-83499-input-output-iteration-ice.rs create mode 100644 src/test/ui/mir/issue-83499-input-output-iteration-ice.stderr diff --git a/src/test/ui/mir/issue-83499-input-output-iteration-ice.rs b/src/test/ui/mir/issue-83499-input-output-iteration-ice.rs new file mode 100644 index 0000000000000..4d404d015ec0b --- /dev/null +++ b/src/test/ui/mir/issue-83499-input-output-iteration-ice.rs @@ -0,0 +1,10 @@ +// Test that when in MIR the amount of local_decls and amount of normalized_input_tys don't match +// that an out-of-bounds access does not occur. +#![feature(c_variadic)] + +fn main() {} + +fn foo(_: Bar, ...) -> impl {} +//~^ ERROR only foreign or `unsafe extern "C" functions may be C-variadic +//~| ERROR cannot find type `Bar` in this scope +//~| ERROR at least one trait must be specified diff --git a/src/test/ui/mir/issue-83499-input-output-iteration-ice.stderr b/src/test/ui/mir/issue-83499-input-output-iteration-ice.stderr new file mode 100644 index 0000000000000..eb172684899cf --- /dev/null +++ b/src/test/ui/mir/issue-83499-input-output-iteration-ice.stderr @@ -0,0 +1,21 @@ +error: only foreign or `unsafe extern "C" functions may be C-variadic + --> $DIR/issue-83499-input-output-iteration-ice.rs:7:16 + | +LL | fn foo(_: Bar, ...) -> impl {} + | ^^^ + +error: at least one trait must be specified + --> $DIR/issue-83499-input-output-iteration-ice.rs:7:24 + | +LL | fn foo(_: Bar, ...) -> impl {} + | ^^^^ + +error[E0412]: cannot find type `Bar` in this scope + --> $DIR/issue-83499-input-output-iteration-ice.rs:7:11 + | +LL | fn foo(_: Bar, ...) -> impl {} + | ^^^ not found in this scope + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0412`. From f13135070cf8b973673f2383e283e3d8af26a794 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 1 Apr 2021 21:55:06 +0200 Subject: [PATCH 5/7] Add test to ensure search tabs behaviour --- ...rch-tab-selection-if-current-is-empty.goml | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 src/test/rustdoc-gui/search-tab-selection-if-current-is-empty.goml diff --git a/src/test/rustdoc-gui/search-tab-selection-if-current-is-empty.goml b/src/test/rustdoc-gui/search-tab-selection-if-current-is-empty.goml new file mode 100644 index 0000000000000..a4df102d245be --- /dev/null +++ b/src/test/rustdoc-gui/search-tab-selection-if-current-is-empty.goml @@ -0,0 +1,21 @@ +goto: file://|DOC_PATH|/index.html +write: (".search-input", "Foo") +// Waiting for the search results to appear... +wait-for: "#titles" +assert: ("#titles > button:nth-of-type(1)", "class", "selected") + +// To go back to the original "state" +goto: file://|DOC_PATH|/index.html +write: (".search-input", "-> String") +// Waiting for the search results to appear... +wait-for: "#titles" +// With this search, only the last tab shouldn't be empty so it should be selected. +assert: ("#titles > button:nth-of-type(3)", "class", "selected") + +// To go back to the original "state" +goto: file://|DOC_PATH|/index.html +write: (".search-input", "-> Something") +// Waiting for the search results to appear... +wait-for: "#titles" +// With this search, all the tabs are empty so the first one should remain selected. +assert: ("#titles > button:nth-of-type(1)", "class", "selected") From cc69c652072ed8c544b288b2c45a183e57dd052d Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 31 Mar 2021 22:33:07 -0400 Subject: [PATCH 6/7] Don't load all extern crates unconditionally Instead, only load the crates that are linked to with intra-doc links. This doesn't help very much with any of rustdoc's fundamental issues with freezing the resolver, but it at least fixes a stable-to-stable regression, and makes the crate loading model somewhat more consistent with rustc's. --- src/librustdoc/core.rs | 86 ++++++++++++------- src/librustdoc/lib.rs | 4 +- .../passes/collect_intra_doc_links.rs | 4 +- src/librustdoc/passes/mod.rs | 2 +- src/test/rustdoc-ui/auxiliary/panic-item.rs | 15 ++++ src/test/rustdoc-ui/unused-extern-crate.rs | 3 + .../auxiliary/issue-66159-1.rs | 0 .../extern-crate-only-used-in-link.rs | 8 ++ src/test/rustdoc/issue-66159.rs | 10 --- src/tools/compiletest/src/header.rs | 4 +- 10 files changed, 88 insertions(+), 48 deletions(-) create mode 100644 src/test/rustdoc-ui/auxiliary/panic-item.rs create mode 100644 src/test/rustdoc-ui/unused-extern-crate.rs rename src/test/rustdoc/{ => intra-doc}/auxiliary/issue-66159-1.rs (100%) create mode 100644 src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs delete mode 100644 src/test/rustdoc/issue-66159.rs diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 5a022b2d40c56..3d2165343572e 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -1,3 +1,4 @@ +use rustc_ast as ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::{self, Lrc}; use rustc_driver::abort_on_err; @@ -22,7 +23,7 @@ use rustc_session::DiagnosticOutput; use rustc_session::Session; use rustc_span::source_map; use rustc_span::symbol::sym; -use rustc_span::{Span, DUMMY_SP}; +use rustc_span::Span; use std::cell::RefCell; use std::collections::hash_map::Entry; @@ -348,42 +349,65 @@ crate fn create_config( } crate fn create_resolver<'a>( - externs: config::Externs, queries: &Queries<'a>, sess: &Session, ) -> Rc> { - let extern_names: Vec = externs - .iter() - .filter(|(_, entry)| entry.add_prelude) - .map(|(name, _)| name) - .cloned() - .collect(); - let parts = abort_on_err(queries.expansion(), sess).peek(); - let resolver = parts.1.borrow(); - - // Before we actually clone it, let's force all the extern'd crates to - // actually be loaded, just in case they're only referred to inside - // intra-doc links - resolver.borrow_mut().access(|resolver| { - sess.time("load_extern_crates", || { - for extern_name in &extern_names { - debug!("loading extern crate {}", extern_name); - if let Err(()) = resolver - .resolve_str_path_error( - DUMMY_SP, - extern_name, - TypeNS, - LocalDefId { local_def_index: CRATE_DEF_INDEX }.to_def_id(), - ) { - warn!("unable to resolve external crate {} (do you have an unused `--extern` crate?)", extern_name) - } + let (krate, resolver, _) = &*parts; + let resolver = resolver.borrow().clone(); + + // Letting the resolver escape at the end of the function leads to inconsistencies between the + // crates the TyCtxt sees and the resolver sees (because the resolver could load more crates + // after escaping). Hopefully `IntraLinkCrateLoader` gets all the crates we need ... + struct IntraLinkCrateLoader { + current_mod: DefId, + resolver: Rc>, + } + impl ast::visit::Visitor<'_> for IntraLinkCrateLoader { + fn visit_attribute(&mut self, attr: &ast::Attribute) { + use crate::html::markdown::{markdown_links, MarkdownLink}; + use crate::passes::collect_intra_doc_links::Disambiguator; + + if let Some(doc) = attr.doc_str() { + for MarkdownLink { link, .. } in markdown_links(&doc.as_str()) { + // FIXME: this misses a *lot* of the preprocessing done in collect_intra_doc_links + // I think most of it shouldn't be necessary since we only need the crate prefix? + let path_str = match Disambiguator::from_str(&link) { + Ok(x) => x.map_or(link.as_str(), |(_, p)| p), + Err(_) => continue, + }; + self.resolver.borrow_mut().access(|resolver| { + let _ = resolver.resolve_str_path_error( + attr.span, + path_str, + TypeNS, + self.current_mod, + ); + }); + } } - }); - }); + ast::visit::walk_attribute(self, attr); + } + + fn visit_item(&mut self, item: &ast::Item) { + use rustc_ast_lowering::ResolverAstLowering; + + if let ast::ItemKind::Mod(..) = item.kind { + let new_mod = + self.resolver.borrow_mut().access(|resolver| resolver.local_def_id(item.id)); + let old_mod = mem::replace(&mut self.current_mod, new_mod.to_def_id()); + ast::visit::walk_item(self, item); + self.current_mod = old_mod; + } else { + ast::visit::walk_item(self, item); + } + } + } + let crate_id = LocalDefId { local_def_index: CRATE_DEF_INDEX }.to_def_id(); + let mut loader = IntraLinkCrateLoader { current_mod: crate_id, resolver }; + ast::visit::walk_crate(&mut loader, krate); - // Now we're good to clone the resolver because everything should be loaded - resolver.clone() + loader.resolver } crate fn run_global_ctxt( diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index dabc21e3a447c..a3b83b9701f56 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -31,6 +31,7 @@ extern crate tracing; // // Dependencies listed in Cargo.toml do not need `extern crate`. extern crate rustc_ast; +extern crate rustc_ast_lowering; extern crate rustc_ast_pretty; extern crate rustc_attr; extern crate rustc_data_structures; @@ -637,7 +638,6 @@ fn main_options(options: config::Options) -> MainResult { let default_passes = options.default_passes; let output_format = options.output_format; // FIXME: fix this clone (especially render_options) - let externs = options.externs.clone(); let manual_passes = options.manual_passes.clone(); let render_options = options.render_options.clone(); let config = core::create_config(options); @@ -649,7 +649,7 @@ fn main_options(options: config::Options) -> MainResult { // We need to hold on to the complete resolver, so we cause everything to be // cloned for the analysis passes to use. Suboptimal, but necessary in the // current architecture. - let resolver = core::create_resolver(externs, queries, &sess); + let resolver = core::create_resolver(queries, &sess); if sess.has_errors() { sess.fatal("Compilation failed, aborting rustdoc"); diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 55978ca551b05..437f42b26dd11 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1517,7 +1517,7 @@ fn range_between_backticks(ori_link: &MarkdownLink) -> Range { #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] /// Disambiguators for a link. -enum Disambiguator { +crate enum Disambiguator { /// `prim@` /// /// This is buggy, see @@ -1546,7 +1546,7 @@ impl Disambiguator { /// This returns `Ok(Some(...))` if a disambiguator was found, /// `Ok(None)` if no disambiguator was found, or `Err(...)` /// if there was a problem with the disambiguator. - fn from_str(link: &str) -> Result, (String, Range)> { + crate fn from_str(link: &str) -> Result, (String, Range)> { use Disambiguator::{Kind, Namespace as NS, Primitive}; if let Some(idx) = link.find('@') { diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 4c639c8496db3..755217a4629f4 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -30,7 +30,7 @@ crate use self::unindent_comments::UNINDENT_COMMENTS; mod propagate_doc_cfg; crate use self::propagate_doc_cfg::PROPAGATE_DOC_CFG; -mod collect_intra_doc_links; +crate mod collect_intra_doc_links; crate use self::collect_intra_doc_links::COLLECT_INTRA_DOC_LINKS; mod doc_test_lints; diff --git a/src/test/rustdoc-ui/auxiliary/panic-item.rs b/src/test/rustdoc-ui/auxiliary/panic-item.rs new file mode 100644 index 0000000000000..ef8e912738c1a --- /dev/null +++ b/src/test/rustdoc-ui/auxiliary/panic-item.rs @@ -0,0 +1,15 @@ +#![no_std] +#![feature(lang_items)] + +use core::panic::PanicInfo; +use core::sync::atomic::{self, Ordering}; + +#[panic_handler] +fn panic(_info: &PanicInfo) -> ! { + loop { + atomic::compiler_fence(Ordering::SeqCst); + } +} + +#[lang = "eh_personality"] +fn foo() {} diff --git a/src/test/rustdoc-ui/unused-extern-crate.rs b/src/test/rustdoc-ui/unused-extern-crate.rs new file mode 100644 index 0000000000000..f703a18379074 --- /dev/null +++ b/src/test/rustdoc-ui/unused-extern-crate.rs @@ -0,0 +1,3 @@ +// check-pass +// aux-crate:panic_item=panic-item.rs +// @has unused_extern_crate/index.html diff --git a/src/test/rustdoc/auxiliary/issue-66159-1.rs b/src/test/rustdoc/intra-doc/auxiliary/issue-66159-1.rs similarity index 100% rename from src/test/rustdoc/auxiliary/issue-66159-1.rs rename to src/test/rustdoc/intra-doc/auxiliary/issue-66159-1.rs diff --git a/src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs b/src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs new file mode 100644 index 0000000000000..0964c79de068f --- /dev/null +++ b/src/test/rustdoc/intra-doc/extern-crate-only-used-in-link.rs @@ -0,0 +1,8 @@ +// aux-build:issue-66159-1.rs +// aux-crate:priv:issue_66159_1=issue-66159-1.rs +// build-aux-docs +// compile-flags:-Z unstable-options + +// @has extern_crate_only_used_in_link/index.html +// @has - '//a[@href="../issue_66159_1/struct.Something.html"]' 'issue_66159_1::Something' +//! [issue_66159_1::Something] diff --git a/src/test/rustdoc/issue-66159.rs b/src/test/rustdoc/issue-66159.rs deleted file mode 100644 index 003d079a470c0..0000000000000 --- a/src/test/rustdoc/issue-66159.rs +++ /dev/null @@ -1,10 +0,0 @@ -// aux-crate:priv:issue_66159_1=issue-66159-1.rs -// compile-flags:-Z unstable-options - -// The issue was an ICE which meant that we never actually generated the docs -// so if we have generated the docs, we're okay. -// Since we don't generate the docs for the auxiliary files, we can't actually -// verify that the struct is linked correctly. - -// @has issue_66159/index.html -//! [issue_66159_1::Something] diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 83ea676e8f4ee..531a23d76a27b 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -708,8 +708,8 @@ impl Config { self.parse_name_value_directive(line, "aux-crate").map(|r| { let mut parts = r.trim().splitn(2, '='); ( - parts.next().expect("aux-crate name").to_string(), - parts.next().expect("aux-crate value").to_string(), + parts.next().expect("missing aux-crate name (e.g. log=log.rs)").to_string(), + parts.next().expect("missing aux-crate value (e.g. log=log.rs)").to_string(), ) }) } From ca14abbab1821d20d4d326af2acec916ccc0806e Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Sat, 27 Mar 2021 11:12:38 -0600 Subject: [PATCH 7/7] Fix stack overflow detection on FreeBSD 11.1+ Beginning with FreeBSD 10.4 and 11.1, there is one guard page by default. And the stack autoresizes, so if Rust allocates its own guard page, then FreeBSD's will simply move up one page. The best solution is to just use the OS's guard page. --- library/std/src/sys/unix/thread.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index 01a12dcf5a2af..b8f43caec32a3 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/library/std/src/sys/unix/thread.rs @@ -343,6 +343,20 @@ pub mod guard { // it can eventually grow to. It cannot be used to determine // the position of kernel's stack guard. None + } else if cfg!(target_os = "freebsd") { + // FreeBSD's stack autogrows, and optionally includes a guard page + // at the bottom. If we try to remap the bottom of the stack + // ourselves, FreeBSD's guard page moves upwards. So we'll just use + // the builtin guard page. + let stackaddr = get_stack_start_aligned()?; + let guardaddr = stackaddr as usize; + // Technically the number of guard pages is tunable and controlled + // by the security.bsd.stack_guard_page sysctl, but there are + // few reasons to change it from the default. The default value has + // been 1 ever since FreeBSD 11.1 and 10.4. + const GUARD_PAGES: usize = 1; + let guard = guardaddr..guardaddr + GUARD_PAGES * page_size; + Some(guard) } else { // Reallocate the last page of the stack. // This ensures SIGBUS will be raised on @@ -371,9 +385,8 @@ pub mod guard { } let guardaddr = stackaddr as usize; - let offset = if cfg!(target_os = "freebsd") { 2 } else { 1 }; - Some(guardaddr..guardaddr + offset * page_size) + Some(guardaddr..guardaddr + page_size) } } @@ -417,11 +430,7 @@ pub mod guard { assert_eq!(libc::pthread_attr_getstack(&attr, &mut stackaddr, &mut size), 0); let stackaddr = stackaddr as usize; - ret = if cfg!(target_os = "freebsd") { - // FIXME does freebsd really fault *below* the guard addr? - let guardaddr = stackaddr - guardsize; - Some(guardaddr - PAGE_SIZE.load(Ordering::Relaxed)..guardaddr) - } else if cfg!(target_os = "netbsd") { + ret = if cfg!(any(target_os = "freebsd", target_os = "netbsd")) { Some(stackaddr - guardsize..stackaddr) } else if cfg!(all(target_os = "linux", target_env = "musl")) { Some(stackaddr - guardsize..stackaddr)