From 49d78fcd901700c5a14e19a6679db1646b5ca901 Mon Sep 17 00:00:00 2001 From: Erin Power Date: Tue, 21 Jan 2020 13:13:26 +0100 Subject: [PATCH 01/14] Add GitHub issue templates --- .github/ISSUE_TEMPLATE/blank_issue.md | 4 ++ .github/ISSUE_TEMPLATE/bug_report.md | 44 ++++++++++++++++++ .github/ISSUE_TEMPLATE/config.yml | 5 ++ .github/ISSUE_TEMPLATE/ice.md | 52 +++++++++++++++++++++ .github/ISSUE_TEMPLATE/tracking_issue.md | 58 ++++++++++++++++++++++++ 5 files changed, 163 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/blank_issue.md create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/config.yml create mode 100644 .github/ISSUE_TEMPLATE/ice.md create mode 100644 .github/ISSUE_TEMPLATE/tracking_issue.md diff --git a/.github/ISSUE_TEMPLATE/blank_issue.md b/.github/ISSUE_TEMPLATE/blank_issue.md new file mode 100644 index 0000000000000..9aef3ebe637a1 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/blank_issue.md @@ -0,0 +1,4 @@ +--- +name: Blank Issue +about: Create a blank issue. +--- diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 0000000000000..5675579bc964a --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,44 @@ +--- +name: Bug Report +about: Create a bug report for Rust. +labels: C-bug +--- + + +I tried this code: + +```rust + +``` + +I expected to see this happen: *explanation* + +Instead, this happened: *explanation* + +### Meta + + +`rustc --version --verbose`: +``` + +``` + + +
Backtrace +

+ +``` + +``` + +

+
diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000000000..bd7dc0ac95c1f --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,5 @@ +blank_issues_enabled: true +contact_links: + - name: Rust Programming Language Forum + url: https://users.rust-lang.org + about: Please ask and answer questions about Rust here. diff --git a/.github/ISSUE_TEMPLATE/ice.md b/.github/ISSUE_TEMPLATE/ice.md new file mode 100644 index 0000000000000..e669e4912f8c9 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/ice.md @@ -0,0 +1,52 @@ +--- +name: Internal Compiler Error +about: Create a report for an internal compiler error in rustc. +labels: C-bug, I-ICE, T-compiler +--- + + +### Code + +``` + +``` + + +### Meta + + +`rustc --version --verbose`: +``` + +``` + +### Error output + +``` + +``` + + +
Backtrace +

+ +``` + +``` + +

+
+ diff --git a/.github/ISSUE_TEMPLATE/tracking_issue.md b/.github/ISSUE_TEMPLATE/tracking_issue.md new file mode 100644 index 0000000000000..f93591204cd98 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/tracking_issue.md @@ -0,0 +1,58 @@ +--- +name: Tracking Issue +about: A tracking issue for a feature in Rust. +title: Tracking Issue for XXX +labels: C-tracking-issue +--- + + +This is a tracking issue for the RFC "XXX" (rust-lang/rfcs#NNN). +The feature gate for the issue is `#![feature(FFF)]`. + +### About tracking issues + +Tracking issues are used to record the overall progress of implementation. +They are also uses as hubs connecting to other relevant issues, e.g., bugs or open design questions. +A tracking issue is however *not* meant for large scale discussion, questions, or bug reports about a feature. +Instead, open a dedicated issue for the specific matter and add the relevant feature gate label. + +### Steps + + +- [ ] Implement the RFC (cc @rust-lang/XXX -- can anyone write up mentoring + instructions?) +- [ ] Adjust documentation ([see instructions on rustc-guide][doc-guide]) +- [ ] Stabilization PR ([see instructions on rustc-guide][stabilization-guide]) + +[stabilization-guide]: https://rust-lang.github.io/rustc-guide/stabilization_guide.html#stabilization-pr +[doc-guide]: https://rust-lang.github.io/rustc-guide/stabilization_guide.html#documentation-prs + +### Unresolved Questions + + +XXX --- list all the "unresolved questions" found in the RFC to ensure they are +not forgotten + +### Implementation history + + From b846b42c8dcf052eabda71d416a986a7891093f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Sun, 12 Jan 2020 00:00:00 +0000 Subject: [PATCH 02/14] Selectively disable sanitizer instrumentation Add `no_sanitize` attribute that allows to opt out from sanitizer instrumentation in an annotated function. --- .../src/language-features/no-sanitize.md | 29 ++++++++++++++ src/librustc/middle/codegen_fn_attrs.rs | 6 +++ src/librustc_codegen_llvm/attributes.rs | 20 ++++++++++ src/librustc_codegen_llvm/declare.rs | 16 -------- src/librustc_feature/active.rs | 3 ++ src/librustc_feature/builtin_attrs.rs | 5 +++ src/librustc_mir/transform/inline.rs | 23 +++++++++++ src/librustc_session/lint/builtin.rs | 7 ++++ src/librustc_span/symbol.rs | 4 ++ src/librustc_typeck/collect.rs | 39 ++++++++++++++++++- .../codegen/sanitizer-no-sanitize-inlining.rs | 32 +++++++++++++++ src/test/codegen/sanitizer-no-sanitize.rs | 29 ++++++++++++++ .../feature-gates/feature-gate-no_sanitize.rs | 4 ++ .../feature-gate-no_sanitize.stderr | 12 ++++++ src/test/ui/invalid/invalid-no-sanitize.rs | 5 +++ .../ui/invalid/invalid-no-sanitize.stderr | 10 +++++ src/test/ui/sanitize-inline-always.rs | 15 +++++++ src/test/ui/sanitize-inline-always.stderr | 13 +++++++ 18 files changed, 255 insertions(+), 17 deletions(-) create mode 100644 src/doc/unstable-book/src/language-features/no-sanitize.md create mode 100644 src/test/codegen/sanitizer-no-sanitize-inlining.rs create mode 100644 src/test/codegen/sanitizer-no-sanitize.rs create mode 100644 src/test/ui/feature-gates/feature-gate-no_sanitize.rs create mode 100644 src/test/ui/feature-gates/feature-gate-no_sanitize.stderr create mode 100644 src/test/ui/invalid/invalid-no-sanitize.rs create mode 100644 src/test/ui/invalid/invalid-no-sanitize.stderr create mode 100644 src/test/ui/sanitize-inline-always.rs create mode 100644 src/test/ui/sanitize-inline-always.stderr diff --git a/src/doc/unstable-book/src/language-features/no-sanitize.md b/src/doc/unstable-book/src/language-features/no-sanitize.md new file mode 100644 index 0000000000000..28c683934d4ed --- /dev/null +++ b/src/doc/unstable-book/src/language-features/no-sanitize.md @@ -0,0 +1,29 @@ +# `no_sanitize` + +The tracking issue for this feature is: [#39699] + +[#39699]: https://github.com/rust-lang/rust/issues/39699 + +------------------------ + +The `no_sanitize` attribute can be used to selectively disable sanitizer +instrumentation in an annotated function. This might be useful to: avoid +instrumentation overhead in a performance critical function, or avoid +instrumenting code that contains constructs unsupported by given sanitizer. + +The precise effect of this annotation depends on particular sanitizer in use. +For example, with `no_sanitize(thread)`, the thread sanitizer will no longer +instrument non-atomic store / load operations, but it will instrument atomic +operations to avoid reporting false positives and provide meaning full stack +traces. + +## Examples + +``` rust +#![feature(no_sanitize)] + +#[no_sanitize(address)] +fn foo() { + // ... +} +``` diff --git a/src/librustc/middle/codegen_fn_attrs.rs b/src/librustc/middle/codegen_fn_attrs.rs index 9f8c20208616b..5eef3fb3c5765 100644 --- a/src/librustc/middle/codegen_fn_attrs.rs +++ b/src/librustc/middle/codegen_fn_attrs.rs @@ -72,6 +72,12 @@ bitflags! { const FFI_RETURNS_TWICE = 1 << 10; /// `#[track_caller]`: allow access to the caller location const TRACK_CALLER = 1 << 11; + /// `#[no_sanitize(address)]`: disables address sanitizer instrumentation + const NO_SANITIZE_ADDRESS = 1 << 12; + /// `#[no_sanitize(memory)]`: disables memory sanitizer instrumentation + const NO_SANITIZE_MEMORY = 1 << 13; + /// `#[no_sanitize(thread)]`: disables thread sanitizer instrumentation + const NO_SANITIZE_THREAD = 1 << 14; } } diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index e3920d99c90bc..3e23df09c6684 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -288,6 +288,26 @@ pub fn from_fn_attrs( if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::ALLOCATOR) { Attribute::NoAlias.apply_llfn(llvm::AttributePlace::ReturnValue, llfn); } + if let Some(ref sanitizer) = cx.tcx.sess.opts.debugging_opts.sanitizer { + match *sanitizer { + Sanitizer::Address => { + if !codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_ADDRESS) { + llvm::Attribute::SanitizeAddress.apply_llfn(Function, llfn); + } + } + Sanitizer::Memory => { + if !codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_MEMORY) { + llvm::Attribute::SanitizeMemory.apply_llfn(Function, llfn); + } + } + Sanitizer::Thread => { + if !codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_THREAD) { + llvm::Attribute::SanitizeThread.apply_llfn(Function, llfn); + } + } + Sanitizer::Leak => {} + } + } unwind( llfn, diff --git a/src/librustc_codegen_llvm/declare.rs b/src/librustc_codegen_llvm/declare.rs index bb06b52162186..691f32dd85a05 100644 --- a/src/librustc_codegen_llvm/declare.rs +++ b/src/librustc_codegen_llvm/declare.rs @@ -19,7 +19,6 @@ use crate::llvm::AttributePlace::Function; use crate::type_::Type; use crate::value::Value; use log::debug; -use rustc::session::config::Sanitizer; use rustc::ty::Ty; use rustc_codegen_ssa::traits::*; use rustc_data_structures::small_c_str::SmallCStr; @@ -47,21 +46,6 @@ fn declare_raw_fn( llvm::Attribute::NoRedZone.apply_llfn(Function, llfn); } - if let Some(ref sanitizer) = cx.tcx.sess.opts.debugging_opts.sanitizer { - match *sanitizer { - Sanitizer::Address => { - llvm::Attribute::SanitizeAddress.apply_llfn(Function, llfn); - } - Sanitizer::Memory => { - llvm::Attribute::SanitizeMemory.apply_llfn(Function, llfn); - } - Sanitizer::Thread => { - llvm::Attribute::SanitizeThread.apply_llfn(Function, llfn); - } - _ => {} - } - } - attributes::default_optimisation_attrs(cx.tcx.sess, llfn); attributes::non_lazy_bind(cx.sess(), llfn); llfn diff --git a/src/librustc_feature/active.rs b/src/librustc_feature/active.rs index 4ae79f9ccaa6c..d7fd15a8a7b5f 100644 --- a/src/librustc_feature/active.rs +++ b/src/librustc_feature/active.rs @@ -541,6 +541,9 @@ declare_features! ( /// Allows `T: ?const Trait` syntax in bounds. (active, const_trait_bound_opt_out, "1.42.0", Some(67794), None), + /// Allows the use of `no_sanitize` attribute. + (active, no_sanitize, "1.42.0", Some(39699), None), + // ------------------------------------------------------------------------- // feature-group-end: actual feature gates // ------------------------------------------------------------------------- diff --git a/src/librustc_feature/builtin_attrs.rs b/src/librustc_feature/builtin_attrs.rs index a38726e3de81f..e2e061c185c03 100644 --- a/src/librustc_feature/builtin_attrs.rs +++ b/src/librustc_feature/builtin_attrs.rs @@ -261,6 +261,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ungated!(cold, Whitelisted, template!(Word)), ungated!(no_builtins, Whitelisted, template!(Word)), ungated!(target_feature, Whitelisted, template!(List: r#"enable = "name""#)), + gated!( + no_sanitize, Whitelisted, + template!(List: "address, memory, thread"), + experimental!(no_sanitize) + ), // FIXME: #14408 whitelist docs since rustdoc looks at them ungated!(doc, Whitelisted, template!(List: "hidden|inline|...", NameValueStr: "string")), diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index a3cafcb576323..b6802505df73f 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -8,6 +8,7 @@ use rustc_index::vec::{Idx, IndexVec}; use rustc::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc::mir::visit::*; use rustc::mir::*; +use rustc::session::config::Sanitizer; use rustc::ty::subst::{InternalSubsts, Subst, SubstsRef}; use rustc::ty::{self, Instance, InstanceDef, ParamEnv, Ty, TyCtxt, TypeFoldable}; @@ -228,6 +229,28 @@ impl Inliner<'tcx> { return false; } + // Avoid inlining functions marked as no_sanitize if sanitizer is enabled, + // since instrumentation might be enabled and performed on the caller. + match self.tcx.sess.opts.debugging_opts.sanitizer { + Some(Sanitizer::Address) => { + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_ADDRESS) { + return false; + } + } + Some(Sanitizer::Memory) => { + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_MEMORY) { + return false; + } + } + Some(Sanitizer::Thread) => { + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_THREAD) { + return false; + } + } + Some(Sanitizer::Leak) => {} + None => {} + } + let hinted = match codegen_fn_attrs.inline { // Just treat inline(always) as a hint for now, // there are cases that prevent inlining that we diff --git a/src/librustc_session/lint/builtin.rs b/src/librustc_session/lint/builtin.rs index c326061100b06..a61ab5b5e1755 100644 --- a/src/librustc_session/lint/builtin.rs +++ b/src/librustc_session/lint/builtin.rs @@ -474,6 +474,12 @@ declare_lint! { }; } +declare_lint! { + pub INLINE_NO_SANITIZE, + Warn, + "detects incompatible use of `#[inline(always)]` and `#[no_sanitize(...)]`", +} + declare_lint_pass! { /// Does nothing as a lint pass, but registers some `Lint`s /// that are used by other parts of the compiler. @@ -537,5 +543,6 @@ declare_lint_pass! { MUTABLE_BORROW_RESERVATION_CONFLICT, INDIRECT_STRUCTURAL_MATCH, SOFT_UNSTABLE, + INLINE_NO_SANITIZE, ] } diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index e4f8b5a014389..31100cc573df1 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -120,6 +120,7 @@ symbols! { abi_vectorcall, abi_x86_interrupt, aborts, + address, add_with_overflow, advanced_slice_patterns, adx_target_feature, @@ -445,6 +446,7 @@ symbols! { mem_uninitialized, mem_zeroed, member_constraints, + memory, message, meta, min_align_of, @@ -487,6 +489,7 @@ symbols! { None, non_exhaustive, non_modrs_mods, + no_sanitize, no_stack_check, no_start, no_std, @@ -721,6 +724,7 @@ symbols! { test_removed_feature, test_runner, then_with, + thread, thread_local, tool_attributes, tool_lints, diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 4d812d2621c61..79cd769569e85 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -2743,6 +2743,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { let mut inline_span = None; let mut link_ordinal_span = None; + let mut no_sanitize_span = None; for attr in attrs.iter() { if attr.check_name(sym::cold) { codegen_fn_attrs.flags |= CodegenFnAttrFlags::COLD; @@ -2832,6 +2833,24 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { if let ordinal @ Some(_) = check_link_ordinal(tcx, attr) { codegen_fn_attrs.link_ordinal = ordinal; } + } else if attr.check_name(sym::no_sanitize) { + no_sanitize_span = Some(attr.span); + if let Some(list) = attr.meta_item_list() { + for item in list.iter() { + if item.check_name(sym::address) { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_SANITIZE_ADDRESS; + } else if item.check_name(sym::memory) { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_SANITIZE_MEMORY; + } else if item.check_name(sym::thread) { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_SANITIZE_THREAD; + } else { + tcx.sess + .struct_span_err(item.span(), "invalid argument for `no_sanitize`") + .note("expected one of: `address`, `memory` or `thread`") + .emit(); + } + } + } } } @@ -2911,7 +2930,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { // purpose functions as they wouldn't have the right target features // enabled. For that reason we also forbid #[inline(always)] as it can't be // respected. - if codegen_fn_attrs.target_features.len() > 0 { if codegen_fn_attrs.inline == InlineAttr::Always { if let Some(span) = inline_span { @@ -2924,6 +2942,25 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { } } + let no_sanitize_flags = CodegenFnAttrFlags::NO_SANITIZE_ADDRESS + | CodegenFnAttrFlags::NO_SANITIZE_MEMORY + | CodegenFnAttrFlags::NO_SANITIZE_THREAD; + if codegen_fn_attrs.flags.intersects(no_sanitize_flags) { + if codegen_fn_attrs.inline == InlineAttr::Always { + if let (Some(no_sanitize_span), Some(inline_span)) = (no_sanitize_span, inline_span) { + let hir_id = tcx.hir().as_local_hir_id(id).unwrap(); + tcx.struct_span_lint_hir( + lint::builtin::INLINE_NO_SANITIZE, + hir_id, + no_sanitize_span, + "`no_sanitize` will have no effect after inlining", + ) + .span_note(inline_span, "inlining requested here") + .emit(); + } + } + } + // Weak lang items have the same semantics as "std internal" symbols in the // sense that they're preserved through all our LTO passes and only // strippable by the linker. diff --git a/src/test/codegen/sanitizer-no-sanitize-inlining.rs b/src/test/codegen/sanitizer-no-sanitize-inlining.rs new file mode 100644 index 0000000000000..d96e76618d325 --- /dev/null +++ b/src/test/codegen/sanitizer-no-sanitize-inlining.rs @@ -0,0 +1,32 @@ +// Verifies that no_sanitize attribute prevents inlining when +// given sanitizer is enabled, but has no effect on inlining otherwise. +// +// needs-sanitizer-support +// only-x86_64 +// +// revisions: ASAN LSAN +// +//[ASAN] compile-flags: -Zsanitizer=address -C opt-level=3 -Z mir-opt-level=3 +//[LSAN] compile-flags: -Zsanitizer=leak -C opt-level=3 -Z mir-opt-level=3 + +#![crate_type="lib"] +#![feature(no_sanitize)] + +// ASAN-LABEL: define void @test +// ASAN: tail call fastcc void @random_inline +// ASAN: } +// +// LSAN-LABEL: define void @test +// LSAN-NO: call +// LSAN: } +#[no_mangle] +pub fn test(n: &mut u32) { + random_inline(n); +} + +#[no_sanitize(address)] +#[inline] +#[no_mangle] +pub fn random_inline(n: &mut u32) { + *n = 42; +} diff --git a/src/test/codegen/sanitizer-no-sanitize.rs b/src/test/codegen/sanitizer-no-sanitize.rs new file mode 100644 index 0000000000000..dfceb28c8dd10 --- /dev/null +++ b/src/test/codegen/sanitizer-no-sanitize.rs @@ -0,0 +1,29 @@ +// Verifies that no_sanitze attribute can be used to +// selectively disable sanitizer instrumentation. +// +// needs-sanitizer-support +// compile-flags: -Zsanitizer=address + +#![crate_type="lib"] +#![feature(no_sanitize)] + +// CHECK-LABEL: ; sanitizer_no_sanitize::unsanitized +// CHECK-NEXT: ; Function Attrs: +// CHECK-NOT: sanitize_address +// CHECK: start: +// CHECK-NOT: call void @__asan_report_load +// CHECK: } +#[no_sanitize(address)] +pub fn unsanitized(b: &mut u8) -> u8 { + *b +} + +// CHECK-LABEL: ; sanitizer_no_sanitize::sanitized +// CHECK-NEXT: ; Function Attrs: +// CHECK: sanitize_address +// CHECK: start: +// CHECK: call void @__asan_report_load +// CHECK: } +pub fn sanitized(b: &mut u8) -> u8 { + *b +} diff --git a/src/test/ui/feature-gates/feature-gate-no_sanitize.rs b/src/test/ui/feature-gates/feature-gate-no_sanitize.rs new file mode 100644 index 0000000000000..66a9263e13a53 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-no_sanitize.rs @@ -0,0 +1,4 @@ +#[no_sanitize(address)] +//~^ the `#[no_sanitize]` attribute is an experimental feature +fn main() { +} diff --git a/src/test/ui/feature-gates/feature-gate-no_sanitize.stderr b/src/test/ui/feature-gates/feature-gate-no_sanitize.stderr new file mode 100644 index 0000000000000..7359cf03652bc --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-no_sanitize.stderr @@ -0,0 +1,12 @@ +error[E0658]: the `#[no_sanitize]` attribute is an experimental feature + --> $DIR/feature-gate-no_sanitize.rs:1:1 + | +LL | #[no_sanitize(address)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: for more information, see https://github.com/rust-lang/rust/issues/39699 + = help: add `#![feature(no_sanitize)]` to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/invalid/invalid-no-sanitize.rs b/src/test/ui/invalid/invalid-no-sanitize.rs new file mode 100644 index 0000000000000..b52e3cc83fab2 --- /dev/null +++ b/src/test/ui/invalid/invalid-no-sanitize.rs @@ -0,0 +1,5 @@ +#![feature(no_sanitize)] + +#[no_sanitize(brontosaurus)] //~ ERROR invalid argument +fn main() { +} diff --git a/src/test/ui/invalid/invalid-no-sanitize.stderr b/src/test/ui/invalid/invalid-no-sanitize.stderr new file mode 100644 index 0000000000000..e9983e5fbd24d --- /dev/null +++ b/src/test/ui/invalid/invalid-no-sanitize.stderr @@ -0,0 +1,10 @@ +error: invalid argument for `no_sanitize` + --> $DIR/invalid-no-sanitize.rs:3:15 + | +LL | #[no_sanitize(brontosaurus)] + | ^^^^^^^^^^^^ + | + = note: expected one of: `address`, `memory` or `thread` + +error: aborting due to previous error + diff --git a/src/test/ui/sanitize-inline-always.rs b/src/test/ui/sanitize-inline-always.rs new file mode 100644 index 0000000000000..52dc557818039 --- /dev/null +++ b/src/test/ui/sanitize-inline-always.rs @@ -0,0 +1,15 @@ +// check-pass + +#![feature(no_sanitize)] + +#[inline(always)] +//~^ NOTE inlining requested here +#[no_sanitize(address)] +//~^ WARN will have no effect after inlining +//~| NOTE on by default +fn x() { +} + +fn main() { + x() +} diff --git a/src/test/ui/sanitize-inline-always.stderr b/src/test/ui/sanitize-inline-always.stderr new file mode 100644 index 0000000000000..50b9474baa2d6 --- /dev/null +++ b/src/test/ui/sanitize-inline-always.stderr @@ -0,0 +1,13 @@ +warning: `no_sanitize` will have no effect after inlining + --> $DIR/sanitize-inline-always.rs:7:1 + | +LL | #[no_sanitize(address)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(inline_no_sanitize)]` on by default +note: inlining requested here + --> $DIR/sanitize-inline-always.rs:5:1 + | +LL | #[inline(always)] + | ^^^^^^^^^^^^^^^^^ + From 1caa8755e523d2c4d3d8d4cfd7be86f86cac3810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Thu, 16 Jan 2020 00:00:00 +0000 Subject: [PATCH 03/14] Apply LLVM sanitize attributes to generated entry wrapper --- src/librustc_codegen_llvm/attributes.rs | 46 ++++++++++++++----------- src/librustc_codegen_llvm/base.rs | 7 ++-- src/librustc_codegen_ssa/base.rs | 22 ++++++------ 3 files changed, 43 insertions(+), 32 deletions(-) diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index 3e23df09c6684..a9e4fdba03036 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -46,6 +46,31 @@ fn inline(cx: &CodegenCx<'ll, '_>, val: &'ll Value, inline: InlineAttr) { }; } +/// Apply LLVM sanitize attributes. +#[inline] +pub fn sanitize(cx: &CodegenCx<'ll, '_>, codegen_fn_flags: CodegenFnAttrFlags, llfn: &'ll Value) { + if let Some(ref sanitizer) = cx.tcx.sess.opts.debugging_opts.sanitizer { + match *sanitizer { + Sanitizer::Address => { + if !codegen_fn_flags.contains(CodegenFnAttrFlags::NO_SANITIZE_ADDRESS) { + llvm::Attribute::SanitizeAddress.apply_llfn(Function, llfn); + } + } + Sanitizer::Memory => { + if !codegen_fn_flags.contains(CodegenFnAttrFlags::NO_SANITIZE_MEMORY) { + llvm::Attribute::SanitizeMemory.apply_llfn(Function, llfn); + } + } + Sanitizer::Thread => { + if !codegen_fn_flags.contains(CodegenFnAttrFlags::NO_SANITIZE_THREAD) { + llvm::Attribute::SanitizeThread.apply_llfn(Function, llfn); + } + } + Sanitizer::Leak => {} + } + } +} + /// Tell LLVM to emit or not emit the information necessary to unwind the stack for the function. #[inline] pub fn emit_uwtable(val: &'ll Value, emit: bool) { @@ -288,26 +313,7 @@ pub fn from_fn_attrs( if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::ALLOCATOR) { Attribute::NoAlias.apply_llfn(llvm::AttributePlace::ReturnValue, llfn); } - if let Some(ref sanitizer) = cx.tcx.sess.opts.debugging_opts.sanitizer { - match *sanitizer { - Sanitizer::Address => { - if !codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_ADDRESS) { - llvm::Attribute::SanitizeAddress.apply_llfn(Function, llfn); - } - } - Sanitizer::Memory => { - if !codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_MEMORY) { - llvm::Attribute::SanitizeMemory.apply_llfn(Function, llfn); - } - } - Sanitizer::Thread => { - if !codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_THREAD) { - llvm::Attribute::SanitizeThread.apply_llfn(Function, llfn); - } - } - Sanitizer::Leak => {} - } - } + sanitize(cx, codegen_fn_attrs.flags, llfn); unwind( llfn, diff --git a/src/librustc_codegen_llvm/base.rs b/src/librustc_codegen_llvm/base.rs index d3b524c1a1e70..04c084e459eab 100644 --- a/src/librustc_codegen_llvm/base.rs +++ b/src/librustc_codegen_llvm/base.rs @@ -15,6 +15,7 @@ use super::ModuleLlvm; +use crate::attributes; use crate::builder::Builder; use crate::common; use crate::context::CodegenCx; @@ -23,7 +24,7 @@ use crate::metadata; use crate::value::Value; use rustc::dep_graph; -use rustc::middle::codegen_fn_attrs::CodegenFnAttrs; +use rustc::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; use rustc::middle::cstore::EncodedMetadata; use rustc::middle::exported_symbols; use rustc::mir::mono::{Linkage, Visibility}; @@ -131,7 +132,9 @@ pub fn compile_codegen_unit( // If this codegen unit contains the main function, also create the // wrapper here - maybe_create_entry_wrapper::>(&cx); + if let Some(entry) = maybe_create_entry_wrapper::>(&cx) { + attributes::sanitize(&cx, CodegenFnAttrFlags::empty(), entry); + } // Run replace-all-uses-with for statics that need it for &(old_g, new_g) in cx.statics_to_rauw().borrow().iter() { diff --git a/src/librustc_codegen_ssa/base.rs b/src/librustc_codegen_ssa/base.rs index 1f43a4027c5ff..900150913842c 100644 --- a/src/librustc_codegen_ssa/base.rs +++ b/src/librustc_codegen_ssa/base.rs @@ -391,10 +391,12 @@ pub fn codegen_instance<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>>( /// Creates the `main` function which will initialize the rust runtime and call /// users main function. -pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(cx: &'a Bx::CodegenCx) { +pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( + cx: &'a Bx::CodegenCx, +) -> Option { let (main_def_id, span) = match cx.tcx().entry_fn(LOCAL_CRATE) { Some((def_id, _)) => (def_id, cx.tcx().def_span(def_id)), - None => return, + None => return None, }; let instance = Instance::mono(cx.tcx(), main_def_id); @@ -402,17 +404,15 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(cx: &' if !cx.codegen_unit().contains_item(&MonoItem::Fn(instance)) { // We want to create the wrapper in the same codegen unit as Rust's main // function. - return; + return None; } let main_llfn = cx.get_fn_addr(instance); - let et = cx.tcx().entry_fn(LOCAL_CRATE).map(|e| e.1); - match et { - Some(EntryFnType::Main) => create_entry_fn::(cx, span, main_llfn, main_def_id, true), - Some(EntryFnType::Start) => create_entry_fn::(cx, span, main_llfn, main_def_id, false), - None => {} // Do nothing. - } + return cx.tcx().entry_fn(LOCAL_CRATE).map(|(_, et)| { + let use_start_lang_item = EntryFnType::Start != et; + create_entry_fn::(cx, span, main_llfn, main_def_id, use_start_lang_item) + }); fn create_entry_fn<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( cx: &'a Bx::CodegenCx, @@ -420,7 +420,7 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(cx: &' rust_main: Bx::Value, rust_main_def_id: DefId, use_start_lang_item: bool, - ) { + ) -> Bx::Function { // The entry function is either `int main(void)` or `int main(int argc, char **argv)`, // depending on whether the target needs `argc` and `argv` to be passed in. let llfty = if cx.sess().target.target.options.main_needs_argc_argv { @@ -481,6 +481,8 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(cx: &' let result = bx.call(start_fn, &args, None); let cast = bx.intcast(result, cx.type_int(), true); bx.ret(cast); + + llfn } } From 80adde2e337f4e0d784da401b2db37c5d4d3468b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Thu, 6 Feb 2020 00:00:00 +0000 Subject: [PATCH 04/14] Add CodegenFnAttrFlags::NO_SANITIZE_ANY --- src/librustc/middle/codegen_fn_attrs.rs | 2 ++ src/librustc_typeck/collect.rs | 7 +++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/librustc/middle/codegen_fn_attrs.rs b/src/librustc/middle/codegen_fn_attrs.rs index 5eef3fb3c5765..82adcfddc289e 100644 --- a/src/librustc/middle/codegen_fn_attrs.rs +++ b/src/librustc/middle/codegen_fn_attrs.rs @@ -78,6 +78,8 @@ bitflags! { const NO_SANITIZE_MEMORY = 1 << 13; /// `#[no_sanitize(thread)]`: disables thread sanitizer instrumentation const NO_SANITIZE_THREAD = 1 << 14; + /// All `#[no_sanitize(...)]` attributes. + const NO_SANITIZE_ANY = Self::NO_SANITIZE_ADDRESS.bits | Self::NO_SANITIZE_MEMORY.bits | Self::NO_SANITIZE_THREAD.bits; } } diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 79cd769569e85..5e916fc533584 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1,3 +1,5 @@ +// ignore-tidy-filelength + //! "Collection" is the process of determining the type and other external //! details of each item in Rust. Collection is specifically concerned //! with *inter-procedural* things -- for example, for a function @@ -2942,10 +2944,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { } } - let no_sanitize_flags = CodegenFnAttrFlags::NO_SANITIZE_ADDRESS - | CodegenFnAttrFlags::NO_SANITIZE_MEMORY - | CodegenFnAttrFlags::NO_SANITIZE_THREAD; - if codegen_fn_attrs.flags.intersects(no_sanitize_flags) { + if codegen_fn_attrs.flags.intersects(CodegenFnAttrFlags::NO_SANITIZE_ANY) { if codegen_fn_attrs.inline == InlineAttr::Always { if let (Some(no_sanitize_span), Some(inline_span)) = (no_sanitize_span, inline_span) { let hir_id = tcx.hir().as_local_hir_id(id).unwrap(); From 63980cd0fb4b0993e71b482e0a14b72b0ca82fa1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 6 Feb 2020 11:59:29 +0100 Subject: [PATCH 05/14] Add a Hir wrapper type --- src/librustc/hir/mod.rs | 32 ++++++++++++++++++++++++++ src/librustc/ty/context.rs | 7 +----- src/librustc_driver/pretty.rs | 10 ++++---- src/librustc_metadata/rmeta/encoder.rs | 4 ++-- src/librustc_passes/check_const.rs | 3 ++- src/librustc_passes/entry.rs | 4 ++-- src/librustc_typeck/check/mod.rs | 2 +- src/librustc_typeck/check/pat.rs | 2 +- src/librustdoc/test.rs | 2 +- 9 files changed, 47 insertions(+), 19 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 6275c0aabe896..4a4d9cb81456d 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -7,6 +7,38 @@ pub mod exports; pub mod map; use crate::ty::query::Providers; +use crate::ty::TyCtxt; +use rustc_hir::print; +use std::ops::Deref; + +/// A wrapper type which allows you to access HIR. +#[derive(Clone)] +pub struct Hir<'tcx> { + tcx: TyCtxt<'tcx>, + map: &'tcx map::Map<'tcx>, +} + +impl<'tcx> Deref for Hir<'tcx> { + type Target = &'tcx map::Map<'tcx>; + + #[inline(always)] + fn deref(&self) -> &Self::Target { + &self.map + } +} + +impl<'hir> print::PpAnn for Hir<'hir> { + fn nested(&self, state: &mut print::State<'_>, nested: print::Nested) { + self.map.nested(state, nested) + } +} + +impl<'tcx> TyCtxt<'tcx> { + #[inline(always)] + pub fn hir(self) -> Hir<'tcx> { + Hir { tcx: self, map: &self.hir_map } + } +} pub fn provide(providers: &mut Providers<'_>) { map::provide(providers); diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index f12032943f91f..5a0cb45d0bbe8 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -966,7 +966,7 @@ pub struct GlobalCtxt<'tcx> { /// Export map produced by name resolution. export_map: FxHashMap>>, - hir_map: hir_map::Map<'tcx>, + pub(crate) hir_map: hir_map::Map<'tcx>, /// A map from `DefPathHash` -> `DefId`. Includes `DefId`s from the local crate /// as well as all upstream crates. Only populated in incremental mode. @@ -1019,11 +1019,6 @@ pub struct GlobalCtxt<'tcx> { } impl<'tcx> TyCtxt<'tcx> { - #[inline(always)] - pub fn hir(self) -> &'tcx hir_map::Map<'tcx> { - &self.hir_map - } - pub fn alloc_steal_mir(self, mir: BodyAndCache<'tcx>) -> &'tcx Steal> { self.arena.alloc(Steal::new(mir)) } diff --git a/src/librustc_driver/pretty.rs b/src/librustc_driver/pretty.rs index 345b03e6db243..4606ef81a360e 100644 --- a/src/librustc_driver/pretty.rs +++ b/src/librustc_driver/pretty.rs @@ -143,7 +143,7 @@ impl<'hir> HirPrinterSupport<'hir> for NoAnn<'hir> { } fn hir_map<'a>(&'a self) -> Option<&'a hir_map::Map<'hir>> { - self.tcx.map(|tcx| tcx.hir()) + self.tcx.map(|tcx| *tcx.hir()) } fn pp_ann<'a>(&'a self) -> &'a dyn pprust_hir::PpAnn { @@ -155,7 +155,7 @@ impl<'hir> pprust::PpAnn for NoAnn<'hir> {} impl<'hir> pprust_hir::PpAnn for NoAnn<'hir> { fn nested(&self, state: &mut pprust_hir::State<'_>, nested: pprust_hir::Nested) { if let Some(tcx) = self.tcx { - pprust_hir::PpAnn::nested(tcx.hir(), state, nested) + pprust_hir::PpAnn::nested(*tcx.hir(), state, nested) } } } @@ -217,7 +217,7 @@ impl<'hir> HirPrinterSupport<'hir> for IdentifiedAnnotation<'hir> { } fn hir_map<'a>(&'a self) -> Option<&'a hir_map::Map<'hir>> { - self.tcx.map(|tcx| tcx.hir()) + self.tcx.map(|tcx| *tcx.hir()) } fn pp_ann<'a>(&'a self) -> &'a dyn pprust_hir::PpAnn { @@ -228,7 +228,7 @@ impl<'hir> HirPrinterSupport<'hir> for IdentifiedAnnotation<'hir> { impl<'hir> pprust_hir::PpAnn for IdentifiedAnnotation<'hir> { fn nested(&self, state: &mut pprust_hir::State<'_>, nested: pprust_hir::Nested) { if let Some(ref tcx) = self.tcx { - pprust_hir::PpAnn::nested(tcx.hir(), state, nested) + pprust_hir::PpAnn::nested(*tcx.hir(), state, nested) } } fn pre(&self, s: &mut pprust_hir::State<'_>, node: pprust_hir::AnnNode<'_>) { @@ -334,7 +334,7 @@ impl<'a, 'tcx> pprust_hir::PpAnn for TypedAnnotation<'a, 'tcx> { if let pprust_hir::Nested::Body(id) = nested { self.tables.set(self.tcx.body_tables(id)); } - pprust_hir::PpAnn::nested(self.tcx.hir(), state, nested); + pprust_hir::PpAnn::nested(*self.tcx.hir(), state, nested); self.tables.set(old_tables); } fn pre(&self, s: &mut pprust_hir::State<'_>, node: pprust_hir::AnnNode<'_>) { diff --git a/src/librustc_metadata/rmeta/encoder.rs b/src/librustc_metadata/rmeta/encoder.rs index 54fbdb14010c9..4133047af78fe 100644 --- a/src/librustc_metadata/rmeta/encoder.rs +++ b/src/librustc_metadata/rmeta/encoder.rs @@ -796,7 +796,7 @@ impl EncodeContext<'tcx> { record!(self.per_def.kind[def_id] <- match trait_item.kind { ty::AssocKind::Const => { let rendered = - hir::print::to_string(self.tcx.hir(), |s| s.print_trait_item(ast_item)); + hir::print::to_string(&self.tcx.hir(), |s| s.print_trait_item(ast_item)); let rendered_const = self.lazy(RenderedConst(rendered)); EntryKind::AssocConst( @@ -1009,7 +1009,7 @@ impl EncodeContext<'tcx> { fn encode_rendered_const_for_body(&mut self, body_id: hir::BodyId) -> Lazy { let body = self.tcx.hir().body(body_id); - let rendered = hir::print::to_string(self.tcx.hir(), |s| s.print_expr(&body.value)); + let rendered = hir::print::to_string(&self.tcx.hir(), |s| s.print_expr(&body.value)); let rendered_const = &RenderedConst(rendered); self.lazy(rendered_const) } diff --git a/src/librustc_passes/check_const.rs b/src/librustc_passes/check_const.rs index faa85f68fab86..b178110f4f954 100644 --- a/src/librustc_passes/check_const.rs +++ b/src/librustc_passes/check_const.rs @@ -8,6 +8,7 @@ //! through, but errors for structured control flow in a `const` should be emitted here. use rustc::hir::map::Map; +use rustc::hir::Hir; use rustc::session::config::nightly_options; use rustc::session::parse::feature_err; use rustc::ty::query::Providers; @@ -74,7 +75,7 @@ enum ConstKind { } impl ConstKind { - fn for_body(body: &hir::Body<'_>, hir_map: &Map<'_>) -> Option { + fn for_body(body: &hir::Body<'_>, hir_map: Hir<'_>) -> Option { let is_const_fn = |id| hir_map.fn_sig_by_hir_id(id).unwrap().header.is_const(); let owner = hir_map.body_owner(body.id()); diff --git a/src/librustc_passes/entry.rs b/src/librustc_passes/entry.rs index d36114fd3b5b5..ebd93e9ab85b8 100644 --- a/src/librustc_passes/entry.rs +++ b/src/librustc_passes/entry.rs @@ -1,4 +1,4 @@ -use rustc::hir::map as hir_map; +use rustc::hir::Hir; use rustc::session::config::EntryFnType; use rustc::session::{config, Session}; use rustc::ty::query::Providers; @@ -15,7 +15,7 @@ use syntax::entry::EntryPointType; struct EntryContext<'a, 'tcx> { session: &'a Session, - map: &'a hir_map::Map<'tcx>, + map: Hir<'tcx>, /// The top-level function called `main`. main_fn: Option<(HirId, Span)>, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index d0275429747b6..62bc6724d0cfe 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -2588,7 +2588,7 @@ fn report_unexpected_variant_res(tcx: TyCtxt<'_>, res: Res, span: Span, qpath: & E0533, "expected unit struct, unit variant or constant, found {} `{}`", res.descr(), - hir::print::to_string(tcx.hir(), |s| s.print_qpath(qpath, false)) + hir::print::to_string(&tcx.hir(), |s| s.print_qpath(qpath, false)) ) .emit(); } diff --git a/src/librustc_typeck/check/pat.rs b/src/librustc_typeck/check/pat.rs index f9dee0e477f79..47baae6860896 100644 --- a/src/librustc_typeck/check/pat.rs +++ b/src/librustc_typeck/check/pat.rs @@ -693,7 +693,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let msg = format!( "expected tuple struct or tuple variant, found {} `{}`", res.descr(), - hir::print::to_string(tcx.hir(), |s| s.print_qpath(qpath, false)), + hir::print::to_string(&tcx.hir(), |s| s.print_qpath(qpath, false)), ); let mut err = struct_span_err!(tcx.sess, pat.span, E0164, "{}", msg); match (res, &pat.kind) { diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 2892c4b153790..ca173fdeb66d4 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -107,7 +107,7 @@ pub fn run(options: Options) -> i32 { let mut hir_collector = HirCollector { sess: compiler.session(), collector: &mut collector, - map: tcx.hir(), + map: *tcx.hir(), codes: ErrorCodes::from( compiler.session().opts.unstable_features.is_nightly_build(), ), From 513e326f5b87a671540ab7959d9205f0cb743491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 6 Feb 2020 12:16:51 +0100 Subject: [PATCH 06/14] Add a `hir_krate` query --- src/librustc/hir/mod.rs | 1 + src/librustc/query/mod.rs | 6 ++++++ src/librustc/ty/query/mod.rs | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 4a4d9cb81456d..3a53c0cb28274 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -41,5 +41,6 @@ impl<'tcx> TyCtxt<'tcx> { } pub fn provide(providers: &mut Providers<'_>) { + providers.hir_crate = |tcx, _| tcx.hir_map.forest.untracked_krate(); map::provide(providers); } diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index 37d5e23535b81..f0ea9ee7b196a 100644 --- a/src/librustc/query/mod.rs +++ b/src/librustc/query/mod.rs @@ -43,6 +43,12 @@ rustc_queries! { } Other { + query hir_crate(key: CrateNum) -> &'tcx Crate<'tcx> { + eval_always + no_hash + desc { "get the crate HIR" } + } + /// Records the type of every item. query type_of(key: DefId) -> Ty<'tcx> { cache_on_disk_if { key.is_local() } diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 973cd81014616..e7b95af103cc9 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -45,7 +45,7 @@ use rustc_data_structures::sync::Lrc; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{CrateNum, DefId, DefIdMap, DefIdSet, DefIndex}; -use rustc_hir::{HirIdSet, ItemLocalId, TraitCandidate}; +use rustc_hir::{Crate, HirIdSet, ItemLocalId, TraitCandidate}; use rustc_index::vec::IndexVec; use rustc_target::spec::PanicStrategy; From 20ce2f6038913058f548f56e1ff1fad29d4df07f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 6 Feb 2020 12:46:26 +0100 Subject: [PATCH 07/14] Move the `krate` method to Hir and remove the Krate dep node --- src/librustc/dep_graph/dep_node.rs | 15 +-------------- src/librustc/hir/map/collector.rs | 9 +++------ src/librustc/hir/map/hir_id_validator.rs | 2 +- src/librustc/hir/map/mod.rs | 9 --------- src/librustc/hir/mod.rs | 8 ++++++++ src/librustc/query/mod.rs | 6 ++++++ src/librustc/ty/query/plumbing.rs | 1 - src/librustc_driver/pretty.rs | 6 +++--- src/librustc_resolve/lifetimes.rs | 2 +- src/librustdoc/test.rs | 2 +- src/test/incremental/crate_hash_reorder.rs | 8 +++----- src/test/incremental/issue-38222.rs | 12 ++++-------- src/test/incremental/krate-inherent.rs | 8 ++++---- src/test/incremental/krate-inlined.rs | 4 ++-- src/test/ui/dep-graph/dep-graph-variance-alias.rs | 9 ++++----- .../ui/dep-graph/dep-graph-variance-alias.stderr | 2 +- 16 files changed, 42 insertions(+), 61 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 9df8e28254cf5..29b94986a5f3a 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -35,7 +35,7 @@ //! "infer" some properties for each kind of `DepNode`: //! //! * Whether a `DepNode` of a given kind has any parameters at all. Some -//! `DepNode`s, like `Krate`, represent global concepts with only one value. +//! `DepNode`s, like `AllLocalTraitImpls`, represent global concepts with only one value. //! * Whether it is possible, in principle, to reconstruct a query key from a //! given `DepNode`. Many `DepKind`s only require a single `DefId` parameter, //! in which case it is possible to map the node's fingerprint back to the @@ -400,19 +400,6 @@ rustc_dep_node_append!([define_dep_nodes!][ <'tcx> // We use this for most things when incr. comp. is turned off. [] Null, - // Represents the `Krate` as a whole (the `hir::Krate` value) (as - // distinct from the krate module). This is basically a hash of - // the entire krate, so if you read from `Krate` (e.g., by calling - // `tcx.hir().krate()`), we will have to assume that any change - // means that you need to be recompiled. This is because the - // `Krate` value gives you access to all other items. To avoid - // this fate, do not call `tcx.hir().krate()`; instead, prefer - // wrappers like `tcx.visit_all_items_in_krate()`. If there is no - // suitable wrapper, you can use `tcx.dep_graph.ignore()` to gain - // access to the krate, but you must remember to add suitable - // edges yourself for the individual items that you read. - [eval_always] Krate, - // Represents the body of a function or method. The def-id is that of the // function/method. [eval_always] HirBody(DefId), diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs index b6be4bb001996..4c922654e02d5 100644 --- a/src/librustc/hir/map/collector.rs +++ b/src/librustc/hir/map/collector.rs @@ -223,12 +223,9 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> { (commandline_args_hash, crate_disambiguator.to_fingerprint()), ); - let (_, crate_hash) = input_dep_node_and_hash( - self.dep_graph, - &mut self.hcx, - DepNode::new_no_params(DepKind::Krate), - crate_hash_input, - ); + let mut stable_hasher = StableHasher::new(); + crate_hash_input.hash_stable(&mut self.hcx, &mut stable_hasher); + let crate_hash: Fingerprint = stable_hasher.finish(); let svh = Svh::new(crate_hash.to_smaller_hash()); (self.map, svh) diff --git a/src/librustc/hir/map/hir_id_validator.rs b/src/librustc/hir/map/hir_id_validator.rs index 76e42b8af2874..da9695ec08a79 100644 --- a/src/librustc/hir/map/hir_id_validator.rs +++ b/src/librustc/hir/map/hir_id_validator.rs @@ -12,7 +12,7 @@ pub fn check_crate(hir_map: &Map<'_>) { let errors = Lock::new(Vec::new()); - par_iter(&hir_map.krate().modules).for_each(|(module_id, _)| { + par_iter(&hir_map.forest.krate.modules).for_each(|(module_id, _)| { let local_def_id = hir_map.local_def_id(*module_id); hir_map.visit_item_likes_in_module( local_def_id, diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 6d7f53133a666..0e74ccc63b8c6 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -140,11 +140,6 @@ impl Forest<'hir> { Forest { krate, dep_graph: dep_graph.clone() } } - pub fn krate(&self) -> &Crate<'hir> { - self.dep_graph.read(DepNode::new_no_params(DepKind::Krate)); - &self.krate - } - /// This is used internally in the dependency tracking system. /// Use the `krate` method to ensure your dependency on the /// crate is tracked. @@ -401,10 +396,6 @@ impl<'hir> Map<'hir> { self.lookup(id).cloned() } - pub fn krate(&self) -> &'hir Crate<'hir> { - self.forest.krate() - } - pub fn item(&self, id: HirId) -> &'hir Item<'hir> { self.read(id); diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 3a53c0cb28274..259cee471603c 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -8,7 +8,9 @@ pub mod map; use crate::ty::query::Providers; use crate::ty::TyCtxt; +use rustc_hir::def_id::LOCAL_CRATE; use rustc_hir::print; +use rustc_hir::Crate; use std::ops::Deref; /// A wrapper type which allows you to access HIR. @@ -18,6 +20,12 @@ pub struct Hir<'tcx> { map: &'tcx map::Map<'tcx>, } +impl<'tcx> Hir<'tcx> { + pub fn krate(&self) -> &'tcx Crate<'tcx> { + self.tcx.hir_crate(LOCAL_CRATE) + } +} + impl<'tcx> Deref for Hir<'tcx> { type Target = &'tcx map::Map<'tcx>; diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index f0ea9ee7b196a..82ff7da13aea8 100644 --- a/src/librustc/query/mod.rs +++ b/src/librustc/query/mod.rs @@ -43,6 +43,12 @@ rustc_queries! { } Other { + // Represents crate as a whole (as distinct from the to-level crate module). + // If you call `hir_crate` (e.g., indirectly by calling `tcx.hir().krate()`), + // we will have to assume that any change means that you need to be recompiled. + // This is because the `hir_crate` query gives you access to all other items. + // To avoid this fate, do not call `tcx.hir().krate()`; instead, + // prefer wrappers like `tcx.visit_all_items_in_krate()`. query hir_crate(key: CrateNum) -> &'tcx Crate<'tcx> { eval_always no_hash diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 117a38c655eaa..6d9fff351e9b8 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1177,7 +1177,6 @@ pub fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool { // These are inputs that are expected to be pre-allocated and that // should therefore always be red or green already. DepKind::AllLocalTraitImpls | - DepKind::Krate | DepKind::CrateMetadata | DepKind::HirBody | DepKind::Hir | diff --git a/src/librustc_driver/pretty.rs b/src/librustc_driver/pretty.rs index 4606ef81a360e..d4f0149049941 100644 --- a/src/librustc_driver/pretty.rs +++ b/src/librustc_driver/pretty.rs @@ -69,19 +69,19 @@ where match *ppmode { PpmNormal => { let annotation = NoAnn { sess: tcx.sess, tcx: Some(tcx) }; - f(&annotation, tcx.hir().forest.krate()) + f(&annotation, tcx.hir().krate()) } PpmIdentified => { let annotation = IdentifiedAnnotation { sess: tcx.sess, tcx: Some(tcx) }; - f(&annotation, tcx.hir().forest.krate()) + f(&annotation, tcx.hir().krate()) } PpmTyped => { abort_on_err(tcx.analysis(LOCAL_CRATE), tcx.sess); let empty_tables = ty::TypeckTables::empty(None); let annotation = TypedAnnotation { tcx, tables: Cell::new(&empty_tables) }; - tcx.dep_graph.with_ignore(|| f(&annotation, tcx.hir().forest.krate())) + tcx.dep_graph.with_ignore(|| f(&annotation, tcx.hir().krate())) } _ => panic!("Should use call_with_pp_support"), } diff --git a/src/librustc_resolve/lifetimes.rs b/src/librustc_resolve/lifetimes.rs index 0ba9b4f17068e..87522d28d1e80 100644 --- a/src/librustc_resolve/lifetimes.rs +++ b/src/librustc_resolve/lifetimes.rs @@ -612,7 +612,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { let parent_id = self.tcx.hir().get_parent_node(hir_id); let parent_impl_id = hir::ImplItemId { hir_id: parent_id }; let parent_trait_id = hir::TraitItemId { hir_id: parent_id }; - let krate = self.tcx.hir().forest.krate(); + let krate = self.tcx.hir().krate(); if !(krate.items.contains_key(&parent_id) || krate.impl_items.contains_key(&parent_impl_id) diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index ca173fdeb66d4..80d3cc05fb7a7 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -87,7 +87,7 @@ pub fn run(options: Options) -> i32 { compiler.enter(|queries| { let lower_to_hir = queries.lower_to_hir()?; - let mut opts = scrape_test_config(lower_to_hir.peek().0.krate()); + let mut opts = scrape_test_config(lower_to_hir.peek().0.untracked_krate()); opts.display_warnings |= options.display_warnings; let enable_per_target_ignores = options.enable_per_target_ignores; let mut collector = Collector::new( diff --git a/src/test/incremental/crate_hash_reorder.rs b/src/test/incremental/crate_hash_reorder.rs index 5aba2a670370c..6e06e67b6682d 100644 --- a/src/test/incremental/crate_hash_reorder.rs +++ b/src/test/incremental/crate_hash_reorder.rs @@ -7,11 +7,9 @@ // Check that reordering otherwise identical items is not considered a // change at all. -#[rustc_clean(label="Krate", cfg="rpass2")] - +#[rustc_clean(label = "hir_crate", cfg = "rpass2")] // But removing an item, naturally, is. -#[rustc_dirty(label="Krate", cfg="rpass3")] - +#[rustc_dirty(label = "hir_crate", cfg = "rpass3")] #[cfg(rpass1)] pub struct X { pub x: u32, @@ -26,4 +24,4 @@ pub struct X { pub x: u32, } -pub fn main() { } +pub fn main() {} diff --git a/src/test/incremental/issue-38222.rs b/src/test/incremental/issue-38222.rs index df08661c1500e..20d4d4200bc1e 100644 --- a/src/test/incremental/issue-38222.rs +++ b/src/test/incremental/issue-38222.rs @@ -1,18 +1,14 @@ -// Test that debuginfo does not introduce a dependency edge to the Krate +// Test that debuginfo does not introduce a dependency edge to the hir_crate // dep-node. // revisions:rpass1 rpass2 // compile-flags: -Z query-dep-graph - #![feature(rustc_attrs)] - - -#![rustc_partition_reused(module="issue_38222-mod1", cfg="rpass2")] - -// If codegen had added a dependency edge to the Krate dep-node, nothing would +#![rustc_partition_reused(module = "issue_38222-mod1", cfg = "rpass2")] +// If codegen had added a dependency edge to the hir_crate dep-node, nothing would // be re-used, so checking that this module was re-used is sufficient. -#![rustc_partition_reused(module="issue_38222", cfg="rpass2")] +#![rustc_partition_reused(module = "issue_38222", cfg = "rpass2")] //[rpass1] compile-flags: -C debuginfo=1 //[rpass2] compile-flags: -C debuginfo=1 diff --git a/src/test/incremental/krate-inherent.rs b/src/test/incremental/krate-inherent.rs index 6e791eacdf37a..2c04e110525a6 100644 --- a/src/test/incremental/krate-inherent.rs +++ b/src/test/incremental/krate-inherent.rs @@ -4,20 +4,20 @@ #![allow(warnings)] #![feature(rustc_attrs)] -#![rustc_partition_reused(module="krate_inherent-x", cfg="cfail2")] +#![rustc_partition_reused(module = "krate_inherent-x", cfg = "cfail2")] #![crate_type = "rlib"] pub mod x { pub struct Foo; impl Foo { - pub fn foo(&self) { } + pub fn foo(&self) {} } pub fn method() { let x: Foo = Foo; - x.foo(); // inherent methods used to add an edge from Krate + x.foo(); // inherent methods used to add an edge from hir_crate } } #[cfg(cfail1)] -pub fn bar() { } // remove this unrelated fn in cfail2, which should not affect `x::method` +pub fn bar() {} // remove this unrelated fn in cfail2, which should not affect `x::method` diff --git a/src/test/incremental/krate-inlined.rs b/src/test/incremental/krate-inlined.rs index dfb18166ae950..6b1db74a37c66 100644 --- a/src/test/incremental/krate-inlined.rs +++ b/src/test/incremental/krate-inlined.rs @@ -1,5 +1,5 @@ // Regr. test that using HIR inlined from another krate does *not* add -// a dependency from the local Krate node. We can't easily test that +// a dependency from the local hir_crate node. We can't easily test that // directly anymore, so now we test that we get reuse. // revisions: rpass1 rpass2 @@ -7,7 +7,7 @@ #![allow(warnings)] #![feature(rustc_attrs)] -#![rustc_partition_reused(module="krate_inlined-x", cfg="rpass2")] +#![rustc_partition_reused(module = "krate_inlined-x", cfg = "rpass2")] fn main() { x::method(); diff --git a/src/test/ui/dep-graph/dep-graph-variance-alias.rs b/src/test/ui/dep-graph/dep-graph-variance-alias.rs index 95645687307a3..927ea5597783a 100644 --- a/src/test/ui/dep-graph/dep-graph-variance-alias.rs +++ b/src/test/ui/dep-graph/dep-graph-variance-alias.rs @@ -6,17 +6,16 @@ #![feature(rustc_attrs)] #![allow(dead_code)] #![allow(unused_variables)] - -fn main() { } +#![rustc_if_this_changed(hir_crate)] +fn main() {} struct Foo { - f: T + f: T, } -#[rustc_if_this_changed(Krate)] type TypeAlias = Foo; #[rustc_then_this_would_need(variances_of)] //~ ERROR OK struct Use { - x: TypeAlias + x: TypeAlias, } diff --git a/src/test/ui/dep-graph/dep-graph-variance-alias.stderr b/src/test/ui/dep-graph/dep-graph-variance-alias.stderr index 554ff455a2073..2422cb9bb2f52 100644 --- a/src/test/ui/dep-graph/dep-graph-variance-alias.stderr +++ b/src/test/ui/dep-graph/dep-graph-variance-alias.stderr @@ -1,5 +1,5 @@ error: OK - --> $DIR/dep-graph-variance-alias.rs:19:1 + --> $DIR/dep-graph-variance-alias.rs:18:1 | LL | #[rustc_then_this_would_need(variances_of)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 623dcb02db0f23270ef4497739dff43ab6f7bcef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 6 Feb 2020 13:41:37 +0100 Subject: [PATCH 08/14] Remove the `Forest` type --- src/librustc/arena.rs | 2 +- src/librustc/hir/map/hir_id_validator.rs | 2 +- src/librustc/hir/map/mod.rs | 89 +++++++++--------------- src/librustc/hir/mod.rs | 2 +- src/librustc/ty/context.rs | 2 +- src/librustc_interface/passes.rs | 12 ++-- src/librustc_interface/queries.rs | 18 ++--- src/librustdoc/test.rs | 2 +- 8 files changed, 51 insertions(+), 78 deletions(-) diff --git a/src/librustc/arena.rs b/src/librustc/arena.rs index 92f5bf87535e6..dd242686d26f2 100644 --- a/src/librustc/arena.rs +++ b/src/librustc/arena.rs @@ -127,7 +127,7 @@ macro_rules! arena_types { [] tys: rustc::ty::TyS<$tcx>, // HIR types - [few] hir_forest: rustc::hir::map::Forest<$tcx>, + [few] hir_krate: rustc_hir::Crate<$tcx>, [] arm: rustc_hir::Arm<$tcx>, [] attribute: syntax::ast::Attribute, [] block: rustc_hir::Block<$tcx>, diff --git a/src/librustc/hir/map/hir_id_validator.rs b/src/librustc/hir/map/hir_id_validator.rs index da9695ec08a79..c721faafbecaf 100644 --- a/src/librustc/hir/map/hir_id_validator.rs +++ b/src/librustc/hir/map/hir_id_validator.rs @@ -12,7 +12,7 @@ pub fn check_crate(hir_map: &Map<'_>) { let errors = Lock::new(Vec::new()); - par_iter(&hir_map.forest.krate.modules).for_each(|(module_id, _)| { + par_iter(&hir_map.krate.modules).for_each(|(module_id, _)| { let local_def_id = hir_map.local_def_id(*module_id); hir_map.visit_item_likes_in_module( local_def_id, diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 0e74ccc63b8c6..7e0e85ea5866f 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -129,25 +129,6 @@ impl<'hir> Entry<'hir> { } } -/// Stores a crate and any number of inlined items from other crates. -pub struct Forest<'hir> { - krate: Crate<'hir>, - pub dep_graph: DepGraph, -} - -impl Forest<'hir> { - pub fn new(krate: Crate<'hir>, dep_graph: &DepGraph) -> Forest<'hir> { - Forest { krate, dep_graph: dep_graph.clone() } - } - - /// This is used internally in the dependency tracking system. - /// Use the `krate` method to ensure your dependency on the - /// crate is tracked. - pub fn untracked_krate(&self) -> &Crate<'hir> { - &self.krate - } -} - /// This type is effectively a `HashMap>`, /// but it is implemented as 2 layers of arrays. /// - first we have `A = IndexVec` mapping `DefIndex`s to an inner value @@ -157,11 +138,8 @@ pub(super) type HirEntryMap<'hir> = IndexVec { - /// The backing storage for all the AST nodes. - pub forest: &'hir Forest<'hir>, + pub krate: &'hir Crate<'hir>, - /// Same as the dep_graph in forest, just available with one fewer - /// deref. This is a gratuitous micro-optimization. pub dep_graph: DepGraph, /// The SVH of the local crate. @@ -212,6 +190,13 @@ impl<'hir> Iterator for ParentHirIterator<'_, 'hir> { } impl<'hir> Map<'hir> { + /// This is used internally in the dependency tracking system. + /// Use the `krate` method to ensure your dependency on the + /// crate is tracked. + pub fn untracked_krate(&self) -> &Crate<'hir> { + &self.krate + } + #[inline] fn lookup(&self, id: HirId) -> Option<&Entry<'hir>> { let local_map = self.map.get(id.owner)?; @@ -399,33 +384,33 @@ impl<'hir> Map<'hir> { pub fn item(&self, id: HirId) -> &'hir Item<'hir> { self.read(id); - // N.B., intentionally bypass `self.forest.krate()` so that we + // N.B., intentionally bypass `self.krate()` so that we // do not trigger a read of the whole krate here - self.forest.krate.item(id) + self.krate.item(id) } pub fn trait_item(&self, id: TraitItemId) -> &'hir TraitItem<'hir> { self.read(id.hir_id); - // N.B., intentionally bypass `self.forest.krate()` so that we + // N.B., intentionally bypass `self.krate()` so that we // do not trigger a read of the whole krate here - self.forest.krate.trait_item(id) + self.krate.trait_item(id) } pub fn impl_item(&self, id: ImplItemId) -> &'hir ImplItem<'hir> { self.read(id.hir_id); - // N.B., intentionally bypass `self.forest.krate()` so that we + // N.B., intentionally bypass `self.krate()` so that we // do not trigger a read of the whole krate here - self.forest.krate.impl_item(id) + self.krate.impl_item(id) } pub fn body(&self, id: BodyId) -> &'hir Body<'hir> { self.read(id.hir_id); - // N.B., intentionally bypass `self.forest.krate()` so that we + // N.B., intentionally bypass `self.krate()` so that we // do not trigger a read of the whole krate here - self.forest.krate.body(id) + self.krate.body(id) } pub fn fn_decl_by_hir_id(&self, hir_id: HirId) -> Option<&'hir FnDecl<'hir>> { @@ -521,9 +506,9 @@ impl<'hir> Map<'hir> { pub fn trait_impls(&self, trait_did: DefId) -> &'hir [HirId] { self.dep_graph.read(DepNode::new_no_params(DepKind::AllLocalTraitImpls)); - // N.B., intentionally bypass `self.forest.krate()` so that we + // N.B., intentionally bypass `self.krate()` so that we // do not trigger a read of the whole krate here - self.forest.krate.trait_impls.get(&trait_did).map_or(&[], |xs| &xs[..]) + self.krate.trait_impls.get(&trait_did).map_or(&[], |xs| &xs[..]) } /// Gets the attributes on the crate. This is preferable to @@ -533,7 +518,7 @@ impl<'hir> Map<'hir> { let def_path_hash = self.definitions.def_path_hash(CRATE_DEF_INDEX); self.dep_graph.read(def_path_hash.to_dep_node(DepKind::Hir)); - &self.forest.krate.attrs + &self.krate.attrs } pub fn get_module(&self, module: DefId) -> (&'hir Mod<'hir>, Span, HirId) { @@ -541,7 +526,7 @@ impl<'hir> Map<'hir> { self.read(hir_id); match self.find_entry(hir_id).unwrap().node { Node::Item(&Item { span, kind: ItemKind::Mod(ref m), .. }) => (m, span, hir_id), - Node::Crate => (&self.forest.krate.module, self.forest.krate.span, hir_id), + Node::Crate => (&self.krate.module, self.krate.span, hir_id), node => panic!("not a module: {:?}", node), } } @@ -558,7 +543,7 @@ impl<'hir> Map<'hir> { // in the expect_* calls the loops below self.read(hir_id); - let module = &self.forest.krate.modules[&hir_id]; + let module = &self.krate.modules[&hir_id]; for id in &module.items { visitor.visit_item(self.expect_item(*id)); @@ -975,7 +960,7 @@ impl<'hir> Map<'hir> { // Unit/tuple structs/variants take the attributes straight from // the struct/variant definition. Some(Node::Ctor(..)) => return self.attrs(self.get_parent_item(id)), - Some(Node::Crate) => Some(&self.forest.krate.attrs[..]), + Some(Node::Crate) => Some(&self.krate.attrs[..]), _ => None, }; attrs.unwrap_or(&[]) @@ -1054,7 +1039,7 @@ impl<'hir> Map<'hir> { Some(Node::Visibility(v)) => bug!("unexpected Visibility {:?}", v), Some(Node::Local(local)) => local.span, Some(Node::MacroDef(macro_def)) => macro_def.span, - Some(Node::Crate) => self.forest.krate.span, + Some(Node::Crate) => self.krate.span, None => bug!("hir::map::Map::span: id not in map: {:?}", hir_id), } } @@ -1222,7 +1207,8 @@ impl Named for ImplItem<'_> { pub fn map_crate<'hir>( sess: &rustc_session::Session, cstore: &CrateStoreDyn, - forest: &'hir Forest<'hir>, + krate: &'hir Crate<'hir>, + dep_graph: DepGraph, definitions: Definitions, ) -> Map<'hir> { let _prof_timer = sess.prof.generic_activity("build_hir_map"); @@ -1235,31 +1221,18 @@ pub fn map_crate<'hir>( .collect(); let (map, crate_hash) = { - let hcx = crate::ich::StableHashingContext::new(sess, &forest.krate, &definitions, cstore); - - let mut collector = NodeCollector::root( - sess, - &forest.krate, - &forest.dep_graph, - &definitions, - &hir_to_node_id, - hcx, - ); - intravisit::walk_crate(&mut collector, &forest.krate); + let hcx = crate::ich::StableHashingContext::new(sess, krate, &definitions, cstore); + + let mut collector = + NodeCollector::root(sess, krate, &dep_graph, &definitions, &hir_to_node_id, hcx); + intravisit::walk_crate(&mut collector, krate); let crate_disambiguator = sess.local_crate_disambiguator(); let cmdline_args = sess.opts.dep_tracking_hash(); collector.finalize_and_compute_crate_hash(crate_disambiguator, cstore, cmdline_args) }; - let map = Map { - forest, - dep_graph: forest.dep_graph.clone(), - crate_hash, - map, - hir_to_node_id, - definitions, - }; + let map = Map { krate, dep_graph, crate_hash, map, hir_to_node_id, definitions }; sess.time("validate_HIR_map", || { hir_id_validator::check_crate(&map); diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 259cee471603c..2e7e8fdd72491 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -49,6 +49,6 @@ impl<'tcx> TyCtxt<'tcx> { } pub fn provide(providers: &mut Providers<'_>) { - providers.hir_crate = |tcx, _| tcx.hir_map.forest.untracked_krate(); + providers.hir_crate = |tcx, _| tcx.hir_map.untracked_krate(); map::provide(providers); } diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 5a0cb45d0bbe8..8979292c86d40 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1323,7 +1323,7 @@ impl<'tcx> TyCtxt<'tcx> { #[inline(always)] pub fn create_stable_hashing_context(self) -> StableHashingContext<'tcx> { - let krate = self.gcx.hir_map.forest.untracked_krate(); + let krate = self.gcx.hir_map.untracked_krate(); StableHashingContext::new(self.sess, krate, self.hir().definitions(), &*self.cstore) } diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index bf8bcd71efa41..6224c4654d695 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -25,6 +25,7 @@ use rustc_data_structures::{box_region_allow_access, declare_box_region_type, pa use rustc_errors::PResult; use rustc_expand::base::ExtCtxt; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; +use rustc_hir::Crate; use rustc_lint::LintStore; use rustc_mir as mir; use rustc_mir_build as mir_build; @@ -422,7 +423,7 @@ pub fn lower_to_hir<'res, 'tcx>( dep_graph: &'res DepGraph, krate: &'res ast::Crate, arena: &'tcx Arena<'tcx>, -) -> Result> { +) -> Crate<'tcx> { // Lower AST to HIR. let hir_crate = rustc_ast_lowering::lower_crate( sess, @@ -437,8 +438,6 @@ pub fn lower_to_hir<'res, 'tcx>( hir_stats::print_hir_stats(&hir_crate); } - let hir_forest = map::Forest::new(hir_crate, &dep_graph); - sess.time("early_lint_checks", || { rustc_lint::check_ast_crate( sess, @@ -455,7 +454,7 @@ pub fn lower_to_hir<'res, 'tcx>( rustc_span::hygiene::clear_syntax_context_map(); } - Ok(hir_forest) + hir_crate } // Returns all the paths that correspond to generated files. @@ -705,7 +704,8 @@ impl<'tcx> QueryContext<'tcx> { pub fn create_global_ctxt<'tcx>( compiler: &'tcx Compiler, lint_store: Lrc, - hir_forest: &'tcx map::Forest<'tcx>, + krate: &'tcx Crate<'tcx>, + dep_graph: DepGraph, mut resolver_outputs: ResolverOutputs, outputs: OutputFilenames, crate_name: &str, @@ -716,7 +716,7 @@ pub fn create_global_ctxt<'tcx>( let defs = mem::take(&mut resolver_outputs.definitions); // Construct the HIR map. - let hir_map = map::map_crate(sess, &*resolver_outputs.cstore, &hir_forest, defs); + let hir_map = map::map_crate(sess, &*resolver_outputs.cstore, krate, dep_graph, defs); let query_result_on_disk_cache = rustc_incremental::load_query_result_cache(sess); diff --git a/src/librustc_interface/queries.rs b/src/librustc_interface/queries.rs index 2ac2845be91b3..720d162ac819e 100644 --- a/src/librustc_interface/queries.rs +++ b/src/librustc_interface/queries.rs @@ -3,7 +3,6 @@ use crate::passes::{self, BoxedResolver, QueryContext}; use rustc::arena::Arena; use rustc::dep_graph::DepGraph; -use rustc::hir::map; use rustc::session::config::{OutputFilenames, OutputType}; use rustc::session::Session; use rustc::ty::steal::Steal; @@ -12,6 +11,7 @@ use rustc::util::common::ErrorReported; use rustc_codegen_utils::codegen_backend::CodegenBackend; use rustc_data_structures::sync::{Lrc, Once, WorkerLocal}; use rustc_hir::def_id::LOCAL_CRATE; +use rustc_hir::Crate; use rustc_incremental::DepGraphFuture; use rustc_lint::LintStore; use std::any::Any; @@ -74,7 +74,7 @@ pub struct Queries<'tcx> { register_plugins: Query<(ast::Crate, Lrc)>, expansion: Query<(ast::Crate, Steal>>, Lrc)>, dep_graph: Query, - lower_to_hir: Query<(&'tcx map::Forest<'tcx>, Steal)>, + lower_to_hir: Query<(&'tcx Crate<'tcx>, Steal)>, prepare_outputs: Query, global_ctxt: Query>, ongoing_codegen: Query>, @@ -207,9 +207,7 @@ impl<'tcx> Queries<'tcx> { }) } - pub fn lower_to_hir( - &'tcx self, - ) -> Result<&Query<(&'tcx map::Forest<'tcx>, Steal)>> { + pub fn lower_to_hir(&'tcx self) -> Result<&Query<(&'tcx Crate<'tcx>, Steal)>> { self.lower_to_hir.compute(|| { let expansion_result = self.expansion()?; let peeked = expansion_result.peek(); @@ -217,14 +215,14 @@ impl<'tcx> Queries<'tcx> { let resolver = peeked.1.steal(); let lint_store = &peeked.2; let hir = resolver.borrow_mut().access(|resolver| { - passes::lower_to_hir( + Ok(passes::lower_to_hir( self.session(), lint_store, resolver, &*self.dep_graph()?.peek(), &krate, &self.arena, - ) + )) })?; let hir = self.arena.alloc(hir); Ok((hir, Steal::new(BoxedResolver::to_resolver_outputs(resolver)))) @@ -253,12 +251,14 @@ impl<'tcx> Queries<'tcx> { let outputs = self.prepare_outputs()?.peek().clone(); let lint_store = self.expansion()?.peek().2.clone(); let hir = self.lower_to_hir()?.peek(); - let (ref hir_forest, ref resolver_outputs) = &*hir; + let dep_graph = self.dep_graph()?.peek().clone(); + let (ref krate, ref resolver_outputs) = &*hir; let _timer = self.session().timer("create_global_ctxt"); Ok(passes::create_global_ctxt( self.compiler, lint_store, - hir_forest, + krate, + dep_graph, resolver_outputs.steal(), outputs, &crate_name, diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 80d3cc05fb7a7..d3e53e1881276 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -87,7 +87,7 @@ pub fn run(options: Options) -> i32 { compiler.enter(|queries| { let lower_to_hir = queries.lower_to_hir()?; - let mut opts = scrape_test_config(lower_to_hir.peek().0.untracked_krate()); + let mut opts = scrape_test_config(lower_to_hir.peek().0); opts.display_warnings |= options.display_warnings; let enable_per_target_ignores = options.enable_per_target_ignores; let mut collector = Collector::new( From dc4fd3d7240e50e6c4c42952c51db021c88a3575 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 6 Feb 2020 14:11:57 +0100 Subject: [PATCH 09/14] Comment tweaks --- src/librustc/query/mod.rs | 2 +- src/librustc/ty/context.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index 82ff7da13aea8..4842719d804da 100644 --- a/src/librustc/query/mod.rs +++ b/src/librustc/query/mod.rs @@ -43,7 +43,7 @@ rustc_queries! { } Other { - // Represents crate as a whole (as distinct from the to-level crate module). + // Represents crate as a whole (as distinct from the top-level crate module). // If you call `hir_crate` (e.g., indirectly by calling `tcx.hir().krate()`), // we will have to assume that any change means that you need to be recompiled. // This is because the `hir_crate` query gives you access to all other items. diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 8979292c86d40..8386058f72ac7 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -966,6 +966,7 @@ pub struct GlobalCtxt<'tcx> { /// Export map produced by name resolution. export_map: FxHashMap>>, + /// This should usually be accessed with the `tcx.hir()` method. pub(crate) hir_map: hir_map::Map<'tcx>, /// A map from `DefPathHash` -> `DefId`. Includes `DefId`s from the local crate From a575495accfe46384df0332be6d9c0a3fb151cbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 6 Feb 2020 17:14:38 +0100 Subject: [PATCH 10/14] Make `krate` private --- src/librustc/hir/map/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index 7e0e85ea5866f..1645420892a75 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -138,7 +138,7 @@ pub(super) type HirEntryMap<'hir> = IndexVec { - pub krate: &'hir Crate<'hir>, + krate: &'hir Crate<'hir>, pub dep_graph: DepGraph, From 861b328f7d62aebd1cfe381eb81bc7e80faf758e Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Thu, 6 Feb 2020 14:43:53 -0800 Subject: [PATCH 11/14] Respect --nocapture in panic=abort test mode --- src/libtest/lib.rs | 26 ++++++++----- src/test/ui/test-panic-abort-nocapture.rs | 39 +++++++++++++++++++ .../ui/test-panic-abort-nocapture.run.stderr | 9 +++++ .../ui/test-panic-abort-nocapture.run.stdout | 23 +++++++++++ 4 files changed, 88 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/test-panic-abort-nocapture.rs create mode 100644 src/test/ui/test-panic-abort-nocapture.run.stderr create mode 100644 src/test/ui/test-panic-abort-nocapture.run.stdout diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index e99473177e838..de001cacbe195 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -63,8 +63,7 @@ use std::{ env, io, io::prelude::Write, panic::{self, catch_unwind, AssertUnwindSafe, PanicInfo}, - process, - process::{Command, Termination}, + process::{self, Command, Termination}, sync::mpsc::{channel, Sender}, sync::{Arc, Mutex}, thread, @@ -457,9 +456,13 @@ pub fn run_test( monitor_ch, opts.time, ), - RunStrategy::SpawnPrimary => { - spawn_test_subprocess(desc, opts.time.is_some(), monitor_ch, opts.time) - } + RunStrategy::SpawnPrimary => spawn_test_subprocess( + desc, + opts.nocapture, + opts.time.is_some(), + monitor_ch, + opts.time, + ), }; // If the platform is single-threaded we're just going to run @@ -558,6 +561,7 @@ fn run_test_in_process( fn spawn_test_subprocess( desc: TestDesc, + nocapture: bool, report_time: bool, monitor_ch: Sender, time_opts: Option, @@ -566,11 +570,15 @@ fn spawn_test_subprocess( let args = env::args().collect::>(); let current_exe = &args[0]; + let mut command = Command::new(current_exe); + command.env(SECONDARY_TEST_INVOKER_VAR, desc.name.as_slice()); + if nocapture { + command.stdout(process::Stdio::inherit()); + command.stderr(process::Stdio::inherit()); + } + let start = report_time.then(Instant::now); - let output = match Command::new(current_exe) - .env(SECONDARY_TEST_INVOKER_VAR, desc.name.as_slice()) - .output() - { + let output = match command.output() { Ok(out) => out, Err(e) => { let err = format!("Failed to spawn {} as child for test: {:?}", args[0], e); diff --git a/src/test/ui/test-panic-abort-nocapture.rs b/src/test/ui/test-panic-abort-nocapture.rs new file mode 100644 index 0000000000000..75f7983865020 --- /dev/null +++ b/src/test/ui/test-panic-abort-nocapture.rs @@ -0,0 +1,39 @@ +// no-prefer-dynamic +// compile-flags: --test -Cpanic=abort -Zpanic_abort_tests +// run-flags: --test-threads=1 --nocapture +// run-fail +// check-run-results +// exec-env:RUST_BACKTRACE=0 + +// ignore-wasm no panic or subprocess support +// ignore-emscripten no panic or subprocess support + +#![cfg(test)] + +use std::io::Write; + +#[test] +fn it_works() { + println!("about to succeed"); + assert_eq!(1 + 1, 2); +} + +#[test] +#[should_panic] +fn it_panics() { + println!("about to panic"); + assert_eq!(1 + 1, 4); +} + +#[test] +fn it_fails() { + println!("about to fail"); + assert_eq!(1 + 1, 4); +} + +#[test] +fn it_writes_to_stdio() { + println!("hello, world"); + writeln!(std::io::stdout(), "testing123").unwrap(); + writeln!(std::io::stderr(), "testing321").unwrap(); +} diff --git a/src/test/ui/test-panic-abort-nocapture.run.stderr b/src/test/ui/test-panic-abort-nocapture.run.stderr new file mode 100644 index 0000000000000..37fbe3d3ff21f --- /dev/null +++ b/src/test/ui/test-panic-abort-nocapture.run.stderr @@ -0,0 +1,9 @@ +thread 'main' panicked at 'assertion failed: `(left == right)` + left: `2`, + right: `4`', $DIR/test-panic-abort-nocapture.rs:31:5 +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace +thread 'main' panicked at 'assertion failed: `(left == right)` + left: `2`, + right: `4`', $DIR/test-panic-abort-nocapture.rs:25:5 +note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace +testing321 diff --git a/src/test/ui/test-panic-abort-nocapture.run.stdout b/src/test/ui/test-panic-abort-nocapture.run.stdout new file mode 100644 index 0000000000000..87a246db5e07b --- /dev/null +++ b/src/test/ui/test-panic-abort-nocapture.run.stdout @@ -0,0 +1,23 @@ + +running 4 tests +test it_fails ... about to fail +FAILED +test it_panics ... about to panic +ok +test it_works ... about to succeed +ok +test it_writes_to_stdio ... hello, world +testing123 +ok + +failures: + +---- it_fails stdout ---- +---- it_fails stderr ---- + + +failures: + it_fails + +test result: FAILED. 3 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out + From c39f2092f727f128c60ac77f0ef9abd88e343f06 Mon Sep 17 00:00:00 2001 From: Hanna Kruppe Date: Thu, 6 Feb 2020 23:45:20 +0100 Subject: [PATCH 12/14] Add myself to .mailmap --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index 6ab6be26cf101..14797092475a2 100644 --- a/.mailmap +++ b/.mailmap @@ -100,6 +100,7 @@ Graydon Hoare Graydon Hoare Guillaume Gomez Guillaume Gomez ggomez Guillaume Gomez Guillaume Gomez +Hanna Kruppe Heather Heather Herman J. Radtke III Herman J. Radtke III From 44edbc0e90dd7364bc427e8a95a588251b9ef560 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Fri, 7 Feb 2020 08:07:06 +0100 Subject: [PATCH 13/14] Remove HashStable impl for ast::Lifetime --- src/librustc/ich/impls_syntax.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/librustc/ich/impls_syntax.rs b/src/librustc/ich/impls_syntax.rs index e1733794b8d72..d1815d5e320db 100644 --- a/src/librustc/ich/impls_syntax.rs +++ b/src/librustc/ich/impls_syntax.rs @@ -12,13 +12,6 @@ use smallvec::SmallVec; impl<'ctx> rustc_target::HashStableContext for StableHashingContext<'ctx> {} -impl<'a> HashStable> for ast::Lifetime { - fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { - self.id.hash_stable(hcx, hasher); - self.ident.hash_stable(hcx, hasher); - } -} - impl<'a> HashStable> for [ast::Attribute] { fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) { if self.len() == 0 { From 26020f506338add6fec610ad8c2cb64c28546cbe Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 7 Feb 2020 13:23:33 +0100 Subject: [PATCH 14/14] clean up E0276 explanation --- src/librustc_error_codes/error_codes/E0276.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_error_codes/error_codes/E0276.md b/src/librustc_error_codes/error_codes/E0276.md index 0e3a613bf9c02..ad76968c5897d 100644 --- a/src/librustc_error_codes/error_codes/E0276.md +++ b/src/librustc_error_codes/error_codes/E0276.md @@ -1,5 +1,6 @@ -This error occurs when a bound in an implementation of a trait does not match -the bounds specified in the original trait. For example: +A trait implementation has stricter requirements than the trait definition. + +Erroneous code example: ```compile_fail,E0276 trait Foo {