From 675b585931d870ad4207d60ef43179390526a2fc Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 3 May 2020 11:40:45 +0200 Subject: [PATCH 1/4] Remove clippy from some leftover lists of "possibly failing" tools --- src/bootstrap/test.rs | 2 +- src/bootstrap/toolstate.rs | 1 - src/tools/publish_toolstate.py | 5 ----- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 90b8b5eea947f..0cf47d20ead06 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -528,7 +528,7 @@ impl Step for Clippy { host, "test", "src/tools/clippy", - SourceType::Submodule, + SourceType::InTree, &[], ); diff --git a/src/bootstrap/toolstate.rs b/src/bootstrap/toolstate.rs index e6560771c0ee7..cd8ce4881b1dd 100644 --- a/src/bootstrap/toolstate.rs +++ b/src/bootstrap/toolstate.rs @@ -78,7 +78,6 @@ static STABLE_TOOLS: &[(&str, &str)] = &[ ("edition-guide", "src/doc/edition-guide"), ("rls", "src/tools/rls"), ("rustfmt", "src/tools/rustfmt"), - ("clippy-driver", "src/tools/clippy"), ]; // These tools are permitted to not build on the beta/stable channels. diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 2da62b6bd99f6..988a226706dc0 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -25,10 +25,6 @@ # read privileges on it). CI will fail otherwise. MAINTAINERS = { 'miri': {'oli-obk', 'RalfJung', 'eddyb'}, - 'clippy-driver': { - 'Manishearth', 'llogiq', 'mcarton', 'oli-obk', 'phansch', 'flip1995', - 'yaahc', - }, 'rls': {'Xanewok'}, 'rustfmt': {'topecongiro'}, 'book': {'carols10cents', 'steveklabnik'}, @@ -45,7 +41,6 @@ REPOS = { 'miri': 'https://github.com/rust-lang/miri', - 'clippy-driver': 'https://github.com/rust-lang/rust-clippy', 'rls': 'https://github.com/rust-lang/rls', 'rustfmt': 'https://github.com/rust-lang/rustfmt', 'book': 'https://github.com/rust-lang/book', From cda994633ee109639b9c4c12c20e2aacb6a879cd Mon Sep 17 00:00:00 2001 From: Sam Elliott Date: Mon, 4 May 2020 12:08:35 +0100 Subject: [PATCH 2/4] Add Option to Force Unwind Tables When panic != unwind, `nounwind` is added to all functions for a target. This can cause issues when a panic happens with RUST_BACKTRACE=1, as there needs to be a way to reconstruct the backtrace. There are three possible sources of this information: forcing frame pointers (for which an option exists already), debug info (for which an option exists), or unwind tables. Especially for embedded devices, forcing frame pointers can have code size overheads (RISC-V sees ~10% overheads, ARM sees ~2-3% overheads). In code, it can be the case that debug info is not kept, so it is useful to provide this third option, unwind tables, that users can use to reconstruct the call stack. Reconstructing this stack is harder than with frame pointers, but it is still possible. This commit adds a compiler option which allows a user to force the addition of unwind tables. Unwind tables cannot be disabled on targets that require them for correctness, or when using `-C panic=unwind`. --- src/doc/rustc/src/codegen-options/index.md | 12 +++++ src/librustc_codegen_llvm/allocator.rs | 2 +- src/librustc_codegen_llvm/attributes.rs | 5 +-- src/librustc_interface/tests.rs | 1 + src/librustc_session/options.rs | 2 + src/librustc_session/session.rs | 44 +++++++++++++++++++ src/test/codegen/force-unwind-tables.rs | 7 +++ .../unwind-tables-panic-required.rs | 10 +++++ .../unwind-tables-target-required.rs | 11 +++++ 9 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 src/test/codegen/force-unwind-tables.rs create mode 100644 src/test/compile-fail/unwind-tables-panic-required.rs create mode 100644 src/test/compile-fail/unwind-tables-target-required.rs diff --git a/src/doc/rustc/src/codegen-options/index.md b/src/doc/rustc/src/codegen-options/index.md index 08b5ab1081704..4ffa6207b97b1 100644 --- a/src/doc/rustc/src/codegen-options/index.md +++ b/src/doc/rustc/src/codegen-options/index.md @@ -98,6 +98,18 @@ values: The default behaviour, if frame pointers are not force-enabled, depends on the target. +## force-unwind-tables + +This flag forces the generation of unwind tables. It takes one of the following +values: + +* `y`, `yes`, `on`, or no value: Unwind tables are forced to be generated. +* `n`, `no`, or `off`: Unwind tables are not forced to be generated. If unwind + tables are required by the target or `-C panic=unwind`, an error will be + emitted. + +The default if not specified depends on the target. + ## incremental This flag allows you to enable incremental compilation, which allows `rustc` diff --git a/src/librustc_codegen_llvm/allocator.rs b/src/librustc_codegen_llvm/allocator.rs index a78546571e291..bc1d9e1818c2f 100644 --- a/src/librustc_codegen_llvm/allocator.rs +++ b/src/librustc_codegen_llvm/allocator.rs @@ -54,7 +54,7 @@ pub(crate) unsafe fn codegen(tcx: TyCtxt<'_>, mods: &mut ModuleLlvm, kind: Alloc if tcx.sess.target.target.options.default_hidden_visibility { llvm::LLVMRustSetVisibility(llfn, llvm::Visibility::Hidden); } - if tcx.sess.target.target.options.requires_uwtable { + if tcx.sess.must_emit_unwind_tables() { attributes::emit_uwtable(llfn, true); } diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index fc357ebb05d62..64412843f6def 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -13,7 +13,6 @@ use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, TyCtxt}; use rustc_session::config::{OptLevel, Sanitizer}; use rustc_session::Session; -use rustc_target::spec::PanicStrategy; use crate::attributes; use crate::llvm::AttributePlace::Function; @@ -271,9 +270,7 @@ pub fn from_fn_attrs(cx: &CodegenCx<'ll, 'tcx>, llfn: &'ll Value, instance: ty:: // // You can also find more info on why Windows is whitelisted here in: // https://bugzilla.mozilla.org/show_bug.cgi?id=1302078 - if cx.sess().panic_strategy() == PanicStrategy::Unwind - || cx.sess().target.target.options.requires_uwtable - { + if cx.sess().must_emit_unwind_tables() { attributes::emit_uwtable(llfn, true); } diff --git a/src/librustc_interface/tests.rs b/src/librustc_interface/tests.rs index 0a200426e38ea..f600b1dbf54ac 100644 --- a/src/librustc_interface/tests.rs +++ b/src/librustc_interface/tests.rs @@ -415,6 +415,7 @@ fn test_codegen_options_tracking_hash() { tracked!(debuginfo, 0xdeadbeef); tracked!(embed_bitcode, false); tracked!(force_frame_pointers, Some(false)); + tracked!(force_unwind_tables, Some(true)); tracked!(inline_threshold, Some(0xf007ba11)); tracked!(linker_plugin_lto, LinkerPluginLto::LinkerPluginAuto); tracked!(llvm_args, vec![String::from("1"), String::from("2")]); diff --git a/src/librustc_session/options.rs b/src/librustc_session/options.rs index b03fc00d93db2..984d47956ca59 100644 --- a/src/librustc_session/options.rs +++ b/src/librustc_session/options.rs @@ -668,6 +668,8 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options, "extra data to put in each output filename"), force_frame_pointers: Option = (None, parse_opt_bool, [TRACKED], "force use of the frame pointers"), + force_unwind_tables: Option = (None, parse_opt_bool, [TRACKED], + "force use of unwind tables"), incremental: Option = (None, parse_opt_string, [UNTRACKED], "enable incremental compilation"), inline_threshold: Option = (None, parse_opt_uint, [TRACKED], diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index 69e1b46de4df7..871893d45651a 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -601,6 +601,33 @@ impl Session { } } + pub fn must_emit_unwind_tables(&self) -> bool { + // This is used to control the emission of the `uwtable` attribute on + // LLVM functions. + // + // At the very least, unwind tables are needed when compiling with + // `-C panic=unwind`. + // + // On some targets (including windows), however, exceptions include + // other events such as illegal instructions, segfaults, etc. This means + // that on Windows we end up still needing unwind tables even if the `-C + // panic=abort` flag is passed. + // + // You can also find more info on why Windows needs unwind tables in: + // https://bugzilla.mozilla.org/show_bug.cgi?id=1302078 + // + // If a target requires unwind tables, then they must be emitted. + // Otherwise, we can defer to the `-C force-unwind-tables=` + // value, if it is provided, or disable them, if not. + if self.panic_strategy() == PanicStrategy::Unwind { + true + } else if self.target.target.options.requires_uwtable { + true + } else { + self.opts.cg.force_unwind_tables.unwrap_or(false) + } + } + /// Returns the symbol name for the registrar function, /// given the crate `Svh` and the function `DefIndex`. pub fn generate_plugin_registrar_symbol(&self, disambiguator: CrateDisambiguator) -> String { @@ -1178,6 +1205,23 @@ fn validate_commandline_args_with_session_available(sess: &Session) { } } + // Unwind tables cannot be disabled if the target requires them. + if let Some(include_uwtables) = sess.opts.cg.force_unwind_tables { + if sess.panic_strategy() == PanicStrategy::Unwind && !include_uwtables { + sess.err( + "panic=unwind requires unwind tables, they cannot be disabled \ + with `-C force-unwind-tables=no`.", + ); + } + + if sess.target.target.options.requires_uwtable && !include_uwtables { + sess.err( + "target requires unwind tables, they cannot be disabled with \ + `-C force-unwind-tables=no`.", + ); + } + } + // PGO does not work reliably with panic=unwind on Windows. Let's make it // an error to combine the two for now. It always runs into an assertions // if LLVM is built with assertions, but without assertions it sometimes diff --git a/src/test/codegen/force-unwind-tables.rs b/src/test/codegen/force-unwind-tables.rs new file mode 100644 index 0000000000000..fbaf38d69df7f --- /dev/null +++ b/src/test/codegen/force-unwind-tables.rs @@ -0,0 +1,7 @@ +// min-llvm-version 8.0 +// compile-flags: -C no-prepopulate-passes -C force-unwind-tables=y + +#![crate_type="lib"] + +// CHECK: attributes #{{.*}} uwtable +pub fn foo() {} diff --git a/src/test/compile-fail/unwind-tables-panic-required.rs b/src/test/compile-fail/unwind-tables-panic-required.rs new file mode 100644 index 0000000000000..314d9e778d5ae --- /dev/null +++ b/src/test/compile-fail/unwind-tables-panic-required.rs @@ -0,0 +1,10 @@ +// Tests that the compiler errors if the user tries to turn off unwind tables +// when they are required. +// +// compile-flags: -C panic=unwind -C force-unwind-tables=no +// ignore-tidy-linelength +// +// error-pattern: panic=unwind requires unwind tables, they cannot be disabled with `-C force-unwind-tables=no`. + +pub fn main() { +} diff --git a/src/test/compile-fail/unwind-tables-target-required.rs b/src/test/compile-fail/unwind-tables-target-required.rs new file mode 100644 index 0000000000000..14c1789376414 --- /dev/null +++ b/src/test/compile-fail/unwind-tables-target-required.rs @@ -0,0 +1,11 @@ +// Tests that the compiler errors if the user tries to turn off unwind tables +// when they are required. +// +// only-x86_64-windows-msvc +// compile-flags: -C force-unwind-tables=no +// ignore-tidy-linelength +// +// error-pattern: target requires unwind tables, they cannot be disabled with `-C force-unwind-tables=no`. + +pub fn main() { +} From 0b35692f5ede1734e7861a538c41240eafc3f925 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 4 May 2020 13:45:06 -0700 Subject: [PATCH 3/4] Improve docs for embed-bitcode and linker-plugin-lto Follow-up from #71716 I wasn't able to add in time. --- src/doc/rustc/src/codegen-options/index.md | 44 ++++++++++++++++++---- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/src/doc/rustc/src/codegen-options/index.md b/src/doc/rustc/src/codegen-options/index.md index 08b5ab1081704..a0b435f369dbb 100644 --- a/src/doc/rustc/src/codegen-options/index.md +++ b/src/doc/rustc/src/codegen-options/index.md @@ -62,24 +62,40 @@ the linker. ## embed-bitcode -This flag controls whether or not the compiler puts LLVM bitcode into generated -rlibs. It takes one of the following values: +This flag controls whether or not the compiler embeds LLVM bitcode into object +files. It takes one of the following values: * `y`, `yes`, `on`, or no value: put bitcode in rlibs (the default). * `n`, `no`, or `off`: omit bitcode from rlibs. -LLVM bitcode is only needed when link-time optimization (LTO) is being -performed, but it is enabled by default for backwards compatibility reasons. +LLVM bitcode is required when rustc is performing link-time optimization (LTO). +It is also required on some targets like iOS ones where vendors look for LLVM +bitcode. Embedded bitcode will appear in rustc-generated object files inside of +a section whose name is defined by the target platform. Most of the time this is +`.llvmbc`. The use of `-C embed-bitcode=no` can significantly improve compile times and -reduce generated file sizes. For these reasons, Cargo uses `-C -embed-bitcode=no` whenever possible. Likewise, if you are building directly -with `rustc` we recommend using `-C embed-bitcode=no` whenever you are not -using LTO. +reduce generated file sizes if your compilation does not actually need bitcode +(e.g. if you're not compiling for iOS or you're not performing LTO). For these +reasons, Cargo uses `-C embed-bitcode=no` whenever possible. Likewise, if you +are building directly with `rustc` we recommend using `-C embed-bitcode=no` +whenever you are not using LTO. If combined with `-C lto`, `-C embed-bitcode=no` will cause `rustc` to abort at start-up, because the combination is invalid. +> **Note**: if you're building Rust code with LTO then you probably don't even +> need the `embed-bitcode` option turned on. You'll likely want to use +> `-Clinker-plugin-lto` instead which skips generating object files entirely and +> simply replaces object files with LLVM bitcode. The only purpose for +> `-Cembed-bitcode` is when you're generating an rlib that is both being used +> with and without LTO. For example Rust's standard library ships with embedded +> bitcode since users link to it both with and without LTO. +> +> This also may make you wonder why the default is `yes` for this option. The +> reason for that is that it's how it was for rustc 1.44 and prior. In 1.45 this +> option was added to turn off what had always been the default. + ## extra-filename This option allows you to put extra data in each output filename. It takes a @@ -187,6 +203,18 @@ the following values: * `n`, `no`, or `off`: disable linker plugin LTO (the default). * A path to the linker plugin. +More specifically this flag will cause the compiler to replace its typical +object file output with LLVM bitcode files. For example an rlib produced with +`-Clinker-plugin-lto` will still have `*.o` files in it, but they'll all be LLVM +bitcode instead of actual machine code. It is expected that the native platform +linker is capable of loading these LLVM bitcode files and generating code at +link time (typically after performing optimizations). + +Note that rustc can also read its own object files produced with +`-Clinker-plugin-lto`. If an rlib is only ever going to get used later with a +`-Clto` compilation then you can pass `-Clinker-plugin-lto` to speed up +compilation and avoid generating object files that aren't used. + ## llvm-args This flag can be used to pass a list of arguments directly to LLVM. From ca72352e604408142c96a16e1ab9fed4916453de Mon Sep 17 00:00:00 2001 From: mibac138 <5672750+mibac138@users.noreply.github.com> Date: Mon, 4 May 2020 18:33:05 +0200 Subject: [PATCH 4/4] Suggest removing semicolon in last expression only if it's type is known --- src/librustc_typeck/check/mod.rs | 4 +++- src/test/ui/issues/issue-43162.stderr | 3 --- src/test/ui/typeck/issue-67971.rs | 9 +++++++++ src/test/ui/typeck/issue-67971.stderr | 18 ++++++++++++++++++ 4 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/typeck/issue-67971.rs create mode 100644 src/test/ui/typeck/issue-67971.stderr diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index c431745198822..b8f1fb00de9ef 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5387,7 +5387,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => return None, }; let last_expr_ty = self.node_ty(last_expr.hir_id); - if self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err() { + if matches!(last_expr_ty.kind, ty::Error) + || self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err() + { return None; } let original_span = original_sp(last_stmt.span, blk.span); diff --git a/src/test/ui/issues/issue-43162.stderr b/src/test/ui/issues/issue-43162.stderr index 0ed3d27c65b6e..a443db40732ac 100644 --- a/src/test/ui/issues/issue-43162.stderr +++ b/src/test/ui/issues/issue-43162.stderr @@ -17,9 +17,6 @@ LL | fn foo() -> bool { | --- ^^^^ expected `bool`, found `()` | | | implicitly returns `()` as its body has no tail or `return` expression -LL | -LL | break true; - | - help: consider removing this semicolon error: aborting due to 3 previous errors diff --git a/src/test/ui/typeck/issue-67971.rs b/src/test/ui/typeck/issue-67971.rs new file mode 100644 index 0000000000000..8bf725cb5ee38 --- /dev/null +++ b/src/test/ui/typeck/issue-67971.rs @@ -0,0 +1,9 @@ +struct S {} + +fn foo(ctx: &mut S) -> String { //~ ERROR mismatched types + // Don't suggest to remove semicolon as it won't fix anything + ctx.sleep = 0; + //~^ ERROR no field `sleep` on type `&mut S` +} + +fn main() {} diff --git a/src/test/ui/typeck/issue-67971.stderr b/src/test/ui/typeck/issue-67971.stderr new file mode 100644 index 0000000000000..36ad3fcb342a8 --- /dev/null +++ b/src/test/ui/typeck/issue-67971.stderr @@ -0,0 +1,18 @@ +error[E0609]: no field `sleep` on type `&mut S` + --> $DIR/issue-67971.rs:5:9 + | +LL | ctx.sleep = 0; + | ^^^^^ unknown field + +error[E0308]: mismatched types + --> $DIR/issue-67971.rs:3:24 + | +LL | fn foo(ctx: &mut S) -> String { + | --- ^^^^^^ expected struct `std::string::String`, found `()` + | | + | implicitly returns `()` as its body has no tail or `return` expression + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0308, E0609. +For more information about an error, try `rustc --explain E0308`.