From ee6e705d91a7b19aa2d029b21396c70e2129f741 Mon Sep 17 00:00:00 2001 From: Diggory Blake Date: Sun, 24 May 2020 13:11:12 +0100 Subject: [PATCH 1/8] Fix UB in Arc Use raw pointers to avoid making any assertions about the data field. --- src/liballoc/sync.rs | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 2bcf763354247..385506dc7a13d 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -866,12 +866,10 @@ impl Arc { unsafe fn drop_slow(&mut self) { // Destroy the data at this time, even though we may not free the box // allocation itself (there may still be weak pointers lying around). - ptr::drop_in_place(&mut self.ptr.as_mut().data); + ptr::drop_in_place(Self::get_mut_unchecked(self)); - if self.inner().weak.fetch_sub(1, Release) == 1 { - acquire!(self.inner().weak); - Global.dealloc(self.ptr.cast(), Layout::for_value(self.ptr.as_ref())) - } + // Drop the weak ref collectively held by all strong references + drop(Weak { ptr: self.ptr }); } #[inline] @@ -1201,7 +1199,7 @@ impl Arc { // As with `get_mut()`, the unsafety is ok because our reference was // either unique to begin with, or became one upon cloning the contents. - unsafe { &mut this.ptr.as_mut().data } + unsafe { Self::get_mut_unchecked(this) } } } @@ -1277,7 +1275,9 @@ impl Arc { #[inline] #[unstable(feature = "get_mut_unchecked", issue = "63292")] pub unsafe fn get_mut_unchecked(this: &mut Self) -> &mut T { - &mut this.ptr.as_mut().data + // We are careful to *not* create a reference covering the "count" fields, as + // this would alias with concurrent access to the reference counts (e.g. by `Weak`). + &mut (*this.ptr.as_ptr()).data } /// Determine whether this is the unique reference (including weak refs) to @@ -1568,6 +1568,13 @@ impl Weak { } } +/// Helper type to allow accessing the reference counts without +/// making any assertions about the data field. +struct WeakInner<'a> { + weak: &'a atomic::AtomicUsize, + strong: &'a atomic::AtomicUsize, +} + impl Weak { /// Attempts to upgrade the `Weak` pointer to an [`Arc`], delaying /// dropping of the inner value if successful. @@ -1673,8 +1680,18 @@ impl Weak { /// Returns `None` when the pointer is dangling and there is no allocated `ArcInner`, /// (i.e., when this `Weak` was created by `Weak::new`). #[inline] - fn inner(&self) -> Option<&ArcInner> { - if is_dangling(self.ptr) { None } else { Some(unsafe { self.ptr.as_ref() }) } + fn inner(&self) -> Option> { + if is_dangling(self.ptr) { + None + } else { + // We are careful to *not* create a reference covering the "data" field, as + // the field may be mutated concurrently (for example, if the last `Arc` + // is dropped, the data field will be dropped in-place). + Some(unsafe { + let ptr = self.ptr.as_ptr(); + WeakInner { strong: &(*ptr).strong, weak: &(*ptr).weak } + }) + } } /// Returns `true` if the two `Weak`s point to the same allocation (similar to From 91dcbbbf50baa02d0757085bb5a9bd69bae5a5a4 Mon Sep 17 00:00:00 2001 From: Samrat Man Singh Date: Mon, 25 May 2020 20:45:26 +0530 Subject: [PATCH 2/8] Allow unlabeled breaks from desugared `?` in labeled blocks --- src/librustc_passes/loops.rs | 3 ++- .../ui/label/label_break_value_desugared_break.rs | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/label/label_break_value_desugared_break.rs diff --git a/src/librustc_passes/loops.rs b/src/librustc_passes/loops.rs index 09b3d44020d81..767a6909d31d4 100644 --- a/src/librustc_passes/loops.rs +++ b/src/librustc_passes/loops.rs @@ -9,6 +9,7 @@ use rustc_middle::hir::map::Map; use rustc_middle::ty::query::Providers; use rustc_middle::ty::TyCtxt; use rustc_session::Session; +use rustc_span::hygiene::DesugaringKind; use rustc_span::Span; #[derive(Clone, Copy, Debug, PartialEq)] @@ -203,7 +204,7 @@ impl<'a, 'hir> CheckLoopVisitor<'a, 'hir> { label: &Destination, cf_type: &str, ) -> bool { - if self.cx == LabeledBlock { + if !span.is_desugaring(DesugaringKind::QuestionMark) && self.cx == LabeledBlock { if label.label.is_none() { struct_span_err!( self.sess, diff --git a/src/test/ui/label/label_break_value_desugared_break.rs b/src/test/ui/label/label_break_value_desugared_break.rs new file mode 100644 index 0000000000000..de883b61111ce --- /dev/null +++ b/src/test/ui/label/label_break_value_desugared_break.rs @@ -0,0 +1,12 @@ +// compile-flags: --edition 2018 +#![feature(label_break_value, try_blocks)] + +// run-pass +fn main() { + let _: Result<(), ()> = try { + 'foo: { + Err(())?; + break 'foo; + } + }; +} From 0fdbfd3f05cdca7cdbde839c34cdcdf040009e9e Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 25 May 2020 17:17:27 -0700 Subject: [PATCH 3/8] Update books --- src/doc/book | 2 +- src/doc/edition-guide | 2 +- src/doc/embedded-book | 2 +- src/doc/reference | 2 +- src/doc/rust-by-example | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/doc/book b/src/doc/book index 6247be15a7f75..e8a4714a9d8a6 160000 --- a/src/doc/book +++ b/src/doc/book @@ -1 +1 @@ -Subproject commit 6247be15a7f7509559f7981ee2209b9e0cc121df +Subproject commit e8a4714a9d8a6136a59b8e63544e149683876e36 diff --git a/src/doc/edition-guide b/src/doc/edition-guide index 49270740c7a4b..0a8ab50468297 160000 --- a/src/doc/edition-guide +++ b/src/doc/edition-guide @@ -1 +1 @@ -Subproject commit 49270740c7a4bff2763e6bc730b191d45b7d5167 +Subproject commit 0a8ab5046829733eb03df0738c4fafaa9b36b348 diff --git a/src/doc/embedded-book b/src/doc/embedded-book index 366c50a03bed9..5555a97f04ad7 160000 --- a/src/doc/embedded-book +++ b/src/doc/embedded-book @@ -1 +1 @@ -Subproject commit 366c50a03bed928589771eba8a6f18e0c0c01d23 +Subproject commit 5555a97f04ad7974ac6fb8fb47c267c4274adf4a diff --git a/src/doc/reference b/src/doc/reference index 892b928b565e3..becdca9477c9e 160000 --- a/src/doc/reference +++ b/src/doc/reference @@ -1 +1 @@ -Subproject commit 892b928b565e35d25b6f9c47faee03b94bc41489 +Subproject commit becdca9477c9eafa96a4eea5156fe7a2730d9dd2 diff --git a/src/doc/rust-by-example b/src/doc/rust-by-example index ab072b14393cb..7aa82129aa23e 160000 --- a/src/doc/rust-by-example +++ b/src/doc/rust-by-example @@ -1 +1 @@ -Subproject commit ab072b14393cbd9e8a1d1d75879bf51e27217bbb +Subproject commit 7aa82129aa23e7e181efbeb8da03a2a897ef6afc From ffa493ab57fbb44f16efae2515abdde6876cc5c9 Mon Sep 17 00:00:00 2001 From: Jeremy Fitzhardinge Date: Sun, 17 May 2020 01:48:01 -0700 Subject: [PATCH 4/8] Implement warning for unused dependencies. This will print a diagnostic for crates which are mentioned as `--extern` arguments on the command line, but are never referenced from the source. This diagnostic is controlled by `-Wunused-crate-dependencies` or `#![warn(unused_crate_dependencies)]` and is "allow" by default. There are cases where certain crates need to be linked in but are not directly referenced - for example if they are providing symbols for C linkage. In this case the warning can be suppressed with `use needed_crate as _;`. Thanks to @petrochenkov for simplified core. Resolves issue #57274 --- src/librustc_lint/lib.rs | 1 + src/librustc_metadata/creader.rs | 29 +++++++++++++++++++ src/librustc_session/lint/builtin.rs | 7 +++++ .../ui/unused-crate-deps/auxiliary/bar.rs | 1 + .../ui/unused-crate-deps/auxiliary/foo.rs | 5 ++++ src/test/ui/unused-crate-deps/libfib.rs | 21 ++++++++++++++ src/test/ui/unused-crate-deps/libfib.stderr | 10 +++++++ src/test/ui/unused-crate-deps/suppress.rs | 11 +++++++ .../ui/unused-crate-deps/unused-aliases.rs | 13 +++++++++ .../unused-crate-deps/unused-aliases.stderr | 14 +++++++++ .../use_extern_crate_2015.rs | 13 +++++++++ src/test/ui/unused-crate-deps/warn-attr.rs | 10 +++++++ .../ui/unused-crate-deps/warn-attr.stderr | 14 +++++++++ .../unused-crate-deps/warn-cmdline-static.rs | 10 +++++++ .../warn-cmdline-static.stderr | 10 +++++++ src/test/ui/unused-crate-deps/warn-cmdline.rs | 9 ++++++ .../ui/unused-crate-deps/warn-cmdline.stderr | 10 +++++++ 17 files changed, 188 insertions(+) create mode 100644 src/test/ui/unused-crate-deps/auxiliary/bar.rs create mode 100644 src/test/ui/unused-crate-deps/auxiliary/foo.rs create mode 100644 src/test/ui/unused-crate-deps/libfib.rs create mode 100644 src/test/ui/unused-crate-deps/libfib.stderr create mode 100644 src/test/ui/unused-crate-deps/suppress.rs create mode 100644 src/test/ui/unused-crate-deps/unused-aliases.rs create mode 100644 src/test/ui/unused-crate-deps/unused-aliases.stderr create mode 100644 src/test/ui/unused-crate-deps/use_extern_crate_2015.rs create mode 100644 src/test/ui/unused-crate-deps/warn-attr.rs create mode 100644 src/test/ui/unused-crate-deps/warn-attr.stderr create mode 100644 src/test/ui/unused-crate-deps/warn-cmdline-static.rs create mode 100644 src/test/ui/unused-crate-deps/warn-cmdline-static.stderr create mode 100644 src/test/ui/unused-crate-deps/warn-cmdline.rs create mode 100644 src/test/ui/unused-crate-deps/warn-cmdline.stderr diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index b791d313fc4f4..ee27342541c93 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -276,6 +276,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { UNUSED_ALLOCATION, UNUSED_DOC_COMMENTS, UNUSED_EXTERN_CRATES, + UNUSED_CRATE_DEPENDENCIES, UNUSED_FEATURES, UNUSED_LABELS, UNUSED_PARENS, diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index b0220ddd3c38e..db29e9538999a 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -5,6 +5,7 @@ use crate::rmeta::{CrateDep, CrateMetadata, CrateNumMap, CrateRoot, MetadataBlob use rustc_ast::expand::allocator::{global_allocator_spans, AllocatorKind}; use rustc_ast::{ast, attr}; +use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::svh::Svh; use rustc_data_structures::sync::Lrc; use rustc_errors::struct_span_err; @@ -18,6 +19,7 @@ use rustc_middle::middle::cstore::{ }; use rustc_middle::ty::TyCtxt; use rustc_session::config::{self, CrateType}; +use rustc_session::lint; use rustc_session::output::validate_crate_name; use rustc_session::search_paths::PathKind; use rustc_session::{CrateDisambiguator, Session}; @@ -49,6 +51,7 @@ pub struct CrateLoader<'a> { local_crate_name: Symbol, // Mutable output. cstore: CStore, + used_extern_options: FxHashSet, } pub enum LoadedMacro { @@ -205,6 +208,7 @@ impl<'a> CrateLoader<'a> { allocator_kind: None, has_global_allocator: false, }, + used_extern_options: Default::default(), } } @@ -445,6 +449,9 @@ impl<'a> CrateLoader<'a> { dep_kind: DepKind, dep: Option<(&'b CratePaths, &'b CrateDep)>, ) -> CrateNum { + if dep.is_none() { + self.used_extern_options.insert(name); + } self.maybe_resolve_crate(name, span, dep_kind, dep).unwrap_or_else(|err| err.report()) } @@ -839,6 +846,26 @@ impl<'a> CrateLoader<'a> { }); } + fn report_unused_deps(&mut self, krate: &ast::Crate) { + // Make a point span rather than covering the whole file + let span = krate.span.shrink_to_lo(); + // Complain about anything left over + for (name, _) in self.sess.opts.externs.iter() { + if !self.used_extern_options.contains(&Symbol::intern(name)) { + self.sess.parse_sess.buffer_lint( + lint::builtin::UNUSED_CRATE_DEPENDENCIES, + span, + ast::CRATE_NODE_ID, + &format!( + "external crate `{}` unused in `{}`: remove the dependency or add `use {} as _;`", + name, + self.local_crate_name, + name), + ); + } + } + } + pub fn postprocess(&mut self, krate: &ast::Crate) { self.inject_profiler_runtime(); self.inject_allocator_crate(krate); @@ -847,6 +874,8 @@ impl<'a> CrateLoader<'a> { if log_enabled!(log::Level::Info) { dump_crates(&self.cstore); } + + self.report_unused_deps(krate); } pub fn process_extern_crate( diff --git a/src/librustc_session/lint/builtin.rs b/src/librustc_session/lint/builtin.rs index 3d03e46683ed5..66d8908799181 100644 --- a/src/librustc_session/lint/builtin.rs +++ b/src/librustc_session/lint/builtin.rs @@ -71,6 +71,12 @@ declare_lint! { "extern crates that are never used" } +declare_lint! { + pub UNUSED_CRATE_DEPENDENCIES, + Allow, + "crate dependencies that are never used" +} + declare_lint! { pub UNUSED_QUALIFICATIONS, Allow, @@ -523,6 +529,7 @@ declare_lint_pass! { UNCONDITIONAL_PANIC, UNUSED_IMPORTS, UNUSED_EXTERN_CRATES, + UNUSED_CRATE_DEPENDENCIES, UNUSED_QUALIFICATIONS, UNKNOWN_LINTS, UNUSED_VARIABLES, diff --git a/src/test/ui/unused-crate-deps/auxiliary/bar.rs b/src/test/ui/unused-crate-deps/auxiliary/bar.rs new file mode 100644 index 0000000000000..1d3824e7a44f6 --- /dev/null +++ b/src/test/ui/unused-crate-deps/auxiliary/bar.rs @@ -0,0 +1 @@ +pub const BAR: &str = "bar"; diff --git a/src/test/ui/unused-crate-deps/auxiliary/foo.rs b/src/test/ui/unused-crate-deps/auxiliary/foo.rs new file mode 100644 index 0000000000000..0ef03eb9edf0f --- /dev/null +++ b/src/test/ui/unused-crate-deps/auxiliary/foo.rs @@ -0,0 +1,5 @@ +// edition:2018 +// aux-crate:bar=bar.rs + +pub const FOO: &str = "foo"; +pub use bar::BAR; diff --git a/src/test/ui/unused-crate-deps/libfib.rs b/src/test/ui/unused-crate-deps/libfib.rs new file mode 100644 index 0000000000000..c1545dca99f57 --- /dev/null +++ b/src/test/ui/unused-crate-deps/libfib.rs @@ -0,0 +1,21 @@ +// Test warnings for a library crate + +// check-pass +// aux-crate:bar=bar.rs +// compile-flags:--crate-type lib -Wunused-crate-dependencies + +pub fn fib(n: u32) -> Vec { +//~^ WARNING external crate `bar` unused in +let mut prev = 0; + let mut cur = 1; + let mut v = vec![]; + + for _ in 0..n { + v.push(prev); + let n = prev + cur; + prev = cur; + cur = n; + } + + v +} diff --git a/src/test/ui/unused-crate-deps/libfib.stderr b/src/test/ui/unused-crate-deps/libfib.stderr new file mode 100644 index 0000000000000..15833126bd620 --- /dev/null +++ b/src/test/ui/unused-crate-deps/libfib.stderr @@ -0,0 +1,10 @@ +warning: external crate `bar` unused in `libfib`: remove the dependency or add `use bar as _;` + --> $DIR/libfib.rs:7:1 + | +LL | pub fn fib(n: u32) -> Vec { + | ^ + | + = note: requested on the command line with `-W unused-crate-dependencies` + +warning: 1 warning emitted + diff --git a/src/test/ui/unused-crate-deps/suppress.rs b/src/test/ui/unused-crate-deps/suppress.rs new file mode 100644 index 0000000000000..8904d04bc14f7 --- /dev/null +++ b/src/test/ui/unused-crate-deps/suppress.rs @@ -0,0 +1,11 @@ +// Suppress by using crate + +// edition:2018 +// check-pass +// aux-crate:bar=bar.rs + +#![warn(unused_crate_dependencies)] + +use bar as _; + +fn main() {} diff --git a/src/test/ui/unused-crate-deps/unused-aliases.rs b/src/test/ui/unused-crate-deps/unused-aliases.rs new file mode 100644 index 0000000000000..1b7cb9b970e49 --- /dev/null +++ b/src/test/ui/unused-crate-deps/unused-aliases.rs @@ -0,0 +1,13 @@ +// Warn about unused aliased for the crate + +// edition:2018 +// check-pass +// aux-crate:bar=bar.rs +// aux-crate:barbar=bar.rs + +#![warn(unused_crate_dependencies)] +//~^ WARNING external crate `barbar` unused in + +use bar as _; + +fn main() {} diff --git a/src/test/ui/unused-crate-deps/unused-aliases.stderr b/src/test/ui/unused-crate-deps/unused-aliases.stderr new file mode 100644 index 0000000000000..c8c6c4507b0c5 --- /dev/null +++ b/src/test/ui/unused-crate-deps/unused-aliases.stderr @@ -0,0 +1,14 @@ +warning: external crate `barbar` unused in `unused_aliases`: remove the dependency or add `use barbar as _;` + --> $DIR/unused-aliases.rs:8:1 + | +LL | #![warn(unused_crate_dependencies)] + | ^ + | +note: the lint level is defined here + --> $DIR/unused-aliases.rs:8:9 + | +LL | #![warn(unused_crate_dependencies)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: 1 warning emitted + diff --git a/src/test/ui/unused-crate-deps/use_extern_crate_2015.rs b/src/test/ui/unused-crate-deps/use_extern_crate_2015.rs new file mode 100644 index 0000000000000..f15c87fa0b249 --- /dev/null +++ b/src/test/ui/unused-crate-deps/use_extern_crate_2015.rs @@ -0,0 +1,13 @@ +// Suppress by using crate + +// edition:2015 +// check-pass +// aux-crate:bar=bar.rs + +#![warn(unused_crate_dependencies)] + +extern crate bar; + +fn main() { + println!("bar {}", bar::BAR); +} diff --git a/src/test/ui/unused-crate-deps/warn-attr.rs b/src/test/ui/unused-crate-deps/warn-attr.rs new file mode 100644 index 0000000000000..1acb307ab21b3 --- /dev/null +++ b/src/test/ui/unused-crate-deps/warn-attr.rs @@ -0,0 +1,10 @@ +// Check for unused crate dep, no path + +// edition:2018 +// check-pass +// aux-crate:bar=bar.rs + +#![warn(unused_crate_dependencies)] +//~^ WARNING external crate `bar` unused in + +fn main() {} diff --git a/src/test/ui/unused-crate-deps/warn-attr.stderr b/src/test/ui/unused-crate-deps/warn-attr.stderr new file mode 100644 index 0000000000000..0d38315704b11 --- /dev/null +++ b/src/test/ui/unused-crate-deps/warn-attr.stderr @@ -0,0 +1,14 @@ +warning: external crate `bar` unused in `warn_attr`: remove the dependency or add `use bar as _;` + --> $DIR/warn-attr.rs:7:1 + | +LL | #![warn(unused_crate_dependencies)] + | ^ + | +note: the lint level is defined here + --> $DIR/warn-attr.rs:7:9 + | +LL | #![warn(unused_crate_dependencies)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: 1 warning emitted + diff --git a/src/test/ui/unused-crate-deps/warn-cmdline-static.rs b/src/test/ui/unused-crate-deps/warn-cmdline-static.rs new file mode 100644 index 0000000000000..c609529a6c6fb --- /dev/null +++ b/src/test/ui/unused-crate-deps/warn-cmdline-static.rs @@ -0,0 +1,10 @@ +// Check for unused crate dep, no path + +// edition:2018 +// check-pass +// compile-flags: -Wunused-crate-dependencies +// aux-crate:bar=bar.rs +// no-prefer-dynamic + +fn main() {} +//~^ WARNING external crate `bar` unused in diff --git a/src/test/ui/unused-crate-deps/warn-cmdline-static.stderr b/src/test/ui/unused-crate-deps/warn-cmdline-static.stderr new file mode 100644 index 0000000000000..65956461d6439 --- /dev/null +++ b/src/test/ui/unused-crate-deps/warn-cmdline-static.stderr @@ -0,0 +1,10 @@ +warning: external crate `bar` unused in `warn_cmdline_static`: remove the dependency or add `use bar as _;` + --> $DIR/warn-cmdline-static.rs:9:1 + | +LL | fn main() {} + | ^ + | + = note: requested on the command line with `-W unused-crate-dependencies` + +warning: 1 warning emitted + diff --git a/src/test/ui/unused-crate-deps/warn-cmdline.rs b/src/test/ui/unused-crate-deps/warn-cmdline.rs new file mode 100644 index 0000000000000..3bae61c3ea2cc --- /dev/null +++ b/src/test/ui/unused-crate-deps/warn-cmdline.rs @@ -0,0 +1,9 @@ +// Check for unused crate dep, no path + +// edition:2018 +// check-pass +// compile-flags: -Wunused-crate-dependencies +// aux-crate:bar=bar.rs + +fn main() {} +//~^ WARNING external crate `bar` unused in diff --git a/src/test/ui/unused-crate-deps/warn-cmdline.stderr b/src/test/ui/unused-crate-deps/warn-cmdline.stderr new file mode 100644 index 0000000000000..ea675ba9a1eb1 --- /dev/null +++ b/src/test/ui/unused-crate-deps/warn-cmdline.stderr @@ -0,0 +1,10 @@ +warning: external crate `bar` unused in `warn_cmdline`: remove the dependency or add `use bar as _;` + --> $DIR/warn-cmdline.rs:8:1 + | +LL | fn main() {} + | ^ + | + = note: requested on the command line with `-W unused-crate-dependencies` + +warning: 1 warning emitted + From 695623a44cba66796964a5aa643e15da9f2154bd Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 26 May 2020 10:33:40 +0200 Subject: [PATCH 5/8] Add working example for E0617 explanation --- src/librustc_error_codes/error_codes/E0617.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/librustc_error_codes/error_codes/E0617.md b/src/librustc_error_codes/error_codes/E0617.md index f4357ff755e29..874fc40ed0b70 100644 --- a/src/librustc_error_codes/error_codes/E0617.md +++ b/src/librustc_error_codes/error_codes/E0617.md @@ -17,3 +17,14 @@ Certain Rust types must be cast before passing them to a variadic function, because of arcane ABI rules dictated by the C standard. To fix the error, cast the value to the type specified by the error message (which you may need to import from `std::os::raw`). + +In this case, `c_double` has the same size as `f64` so we can use it directly: + +``` +# extern { +# fn printf(c: *const i8, ...); +# } +unsafe { + printf(::std::ptr::null(), 0f64); // ok! +} +``` From 6faa82be42df548877315768184f8cfba111772e Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Tue, 26 May 2020 11:04:34 +0100 Subject: [PATCH 6/8] Eagerly lower asm sub-expressions to HIR even if there is an error Fixes #72570 --- src/librustc_ast_lowering/expr.rs | 68 ++++++++++++++++--------------- src/test/ui/asm/issue-72570.rs | 7 ++++ 2 files changed, 42 insertions(+), 33 deletions(-) create mode 100644 src/test/ui/asm/issue-72570.rs diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index 5bcd111706f35..d8002bd3e19ae 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -974,20 +974,18 @@ impl<'hir> LoweringContext<'_, 'hir> { } fn lower_expr_asm(&mut self, sp: Span, asm: &InlineAsm) -> hir::ExprKind<'hir> { - let asm_arch = if let Some(asm_arch) = self.sess.asm_arch { - asm_arch - } else { + if self.sess.asm_arch.is_none() { struct_span_err!(self.sess, sp, E0472, "asm! is unsupported on this target").emit(); - return hir::ExprKind::Err; }; - if asm.options.contains(InlineAsmOptions::ATT_SYNTAX) { - match asm_arch { - asm::InlineAsmArch::X86 | asm::InlineAsmArch::X86_64 => {} - _ => self - .sess - .struct_span_err(sp, "the `att_syntax` option is only supported on x86") - .emit(), - } + if asm.options.contains(InlineAsmOptions::ATT_SYNTAX) + && !matches!( + self.sess.asm_arch, + Some(asm::InlineAsmArch::X86 | asm::InlineAsmArch::X86_64) + ) + { + self.sess + .struct_span_err(sp, "the `att_syntax` option is only supported on x86") + .emit(); } // Lower operands to HIR, filter_map skips any operands with invalid @@ -1001,10 +999,8 @@ impl<'hir> LoweringContext<'_, 'hir> { Some(match reg { InlineAsmRegOrRegClass::Reg(s) => asm::InlineAsmRegOrRegClass::Reg( asm::InlineAsmReg::parse( - asm_arch, - |feature| { - self.sess.target_features.contains(&Symbol::intern(feature)) - }, + sess.asm_arch?, + |feature| sess.target_features.contains(&Symbol::intern(feature)), s, ) .map_err(|e| { @@ -1015,7 +1011,7 @@ impl<'hir> LoweringContext<'_, 'hir> { ), InlineAsmRegOrRegClass::RegClass(s) => { asm::InlineAsmRegOrRegClass::RegClass( - asm::InlineAsmRegClass::parse(asm_arch, s) + asm::InlineAsmRegClass::parse(sess.asm_arch?, s) .map_err(|e| { let msg = format!( "invalid register class `{}`: {}", @@ -1029,33 +1025,38 @@ impl<'hir> LoweringContext<'_, 'hir> { } }) }; - let op = match op { - InlineAsmOperand::In { reg, expr } => hir::InlineAsmOperand::In { - reg: lower_reg(*reg)?, + + // lower_reg is executed last because we need to lower all + // sub-expressions even if we throw them away later. + let op = match *op { + InlineAsmOperand::In { reg, ref expr } => hir::InlineAsmOperand::In { expr: self.lower_expr_mut(expr), + reg: lower_reg(reg)?, }, - InlineAsmOperand::Out { reg, late, expr } => hir::InlineAsmOperand::Out { - reg: lower_reg(*reg)?, - late: *late, + InlineAsmOperand::Out { reg, late, ref expr } => hir::InlineAsmOperand::Out { + late, expr: expr.as_ref().map(|expr| self.lower_expr_mut(expr)), + reg: lower_reg(reg)?, }, - InlineAsmOperand::InOut { reg, late, expr } => hir::InlineAsmOperand::InOut { - reg: lower_reg(*reg)?, - late: *late, - expr: self.lower_expr_mut(expr), - }, - InlineAsmOperand::SplitInOut { reg, late, in_expr, out_expr } => { + InlineAsmOperand::InOut { reg, late, ref expr } => { + hir::InlineAsmOperand::InOut { + late, + expr: self.lower_expr_mut(expr), + reg: lower_reg(reg)?, + } + } + InlineAsmOperand::SplitInOut { reg, late, ref in_expr, ref out_expr } => { hir::InlineAsmOperand::SplitInOut { - reg: lower_reg(*reg)?, - late: *late, + late, in_expr: self.lower_expr_mut(in_expr), out_expr: out_expr.as_ref().map(|expr| self.lower_expr_mut(expr)), + reg: lower_reg(reg)?, } } - InlineAsmOperand::Const { expr } => { + InlineAsmOperand::Const { ref expr } => { hir::InlineAsmOperand::Const { expr: self.lower_expr_mut(expr) } } - InlineAsmOperand::Sym { expr } => { + InlineAsmOperand::Sym { ref expr } => { hir::InlineAsmOperand::Sym { expr: self.lower_expr_mut(expr) } } }; @@ -1069,6 +1070,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } // Validate template modifiers against the register classes for the operands + let asm_arch = sess.asm_arch.unwrap(); for p in &asm.template { if let InlineAsmTemplatePiece::Placeholder { operand_idx, diff --git a/src/test/ui/asm/issue-72570.rs b/src/test/ui/asm/issue-72570.rs new file mode 100644 index 0000000000000..df508e9045819 --- /dev/null +++ b/src/test/ui/asm/issue-72570.rs @@ -0,0 +1,7 @@ +#![feature(asm)] + +fn main() { + unsafe { + asm!("", in("invalid") "".len()); + } +} From 7aa8946c7271f7077df537ed8e3e0b544252c466 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Tue, 26 May 2020 11:13:04 +0100 Subject: [PATCH 7/8] Update src/librustc_ast_lowering/expr.rs Co-authored-by: Oliver Scherer --- src/librustc_ast_lowering/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index d8002bd3e19ae..b7f2e9a9050df 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -976,7 +976,7 @@ impl<'hir> LoweringContext<'_, 'hir> { fn lower_expr_asm(&mut self, sp: Span, asm: &InlineAsm) -> hir::ExprKind<'hir> { if self.sess.asm_arch.is_none() { struct_span_err!(self.sess, sp, E0472, "asm! is unsupported on this target").emit(); - }; + } if asm.options.contains(InlineAsmOptions::ATT_SYNTAX) && !matches!( self.sess.asm_arch, From de53276aac3f9cb89fff5d9db3741e8c7852920a Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Tue, 26 May 2020 11:27:27 +0100 Subject: [PATCH 8/8] Fix test --- src/test/ui/asm/issue-72570.rs | 3 +++ src/test/ui/asm/issue-72570.stderr | 8 ++++++++ 2 files changed, 11 insertions(+) create mode 100644 src/test/ui/asm/issue-72570.stderr diff --git a/src/test/ui/asm/issue-72570.rs b/src/test/ui/asm/issue-72570.rs index df508e9045819..f34525a664ebe 100644 --- a/src/test/ui/asm/issue-72570.rs +++ b/src/test/ui/asm/issue-72570.rs @@ -1,7 +1,10 @@ +// only-x86_64 + #![feature(asm)] fn main() { unsafe { asm!("", in("invalid") "".len()); + //~^ ERROR: invalid register `invalid`: unknown register } } diff --git a/src/test/ui/asm/issue-72570.stderr b/src/test/ui/asm/issue-72570.stderr new file mode 100644 index 0000000000000..49013a23ced2d --- /dev/null +++ b/src/test/ui/asm/issue-72570.stderr @@ -0,0 +1,8 @@ +error: invalid register `invalid`: unknown register + --> $DIR/issue-72570.rs:7:18 + | +LL | asm!("", in("invalid") "".len()); + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error +