From 49a5456606967babaa319954a9710dc2eac6c4f0 Mon Sep 17 00:00:00 2001 From: est31 Date: Sun, 20 Feb 2022 03:17:44 +0100 Subject: [PATCH 01/12] tidy: fire less "ignoring file length unneccessarily" warnings This avoids a situation where a file is at the border of the limit, and alternates between hitting the limit and not hitting it, causing a back and forth of addition of the ignore-tidy-linelength directive. As an example, consider the ignore-tidy-filelength of compiler/rustc_typeck/src/collect.rs. It was added in 2ca4964db5d263a8f9222846bd70a7f26cf414cf, removed in 37354ebc9794b0eb14b08c02177e3094c8fe91cd, added again in 448d07683a6defd567996114793a09c9a8aef5df, removed in 3171bd5bf54fb91f7f7df7c40df5adc7d8bd5dea, added in 438826fd1a9a119d00992ede948cdd479431ecbb, and #94142 is going to remove it again. To avoid this back and forth, we exempt files from the unneccessary ignoring warning that have length of at least 70% of the limit. --- src/tools/tidy/src/style.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index ca79c835b9fa..c197acd4828a 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -395,6 +395,9 @@ pub fn check(path: &Path, bad: &mut bool) { ); }; suppressible_tidy_err!(err, skip_file_length, ""); + } else if lines > (LINES * 7) / 10 { + // Just set it to something that doesn't trigger the "unneccessarily ignored" warning. + skip_file_length = Directive::Ignore(true); } if let Directive::Ignore(false) = skip_cr { From f810314bc6ad252430c29636c8ba00acfa81a737 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sun, 20 Feb 2022 08:53:18 +0000 Subject: [PATCH 02/12] solarish current_exe using libc call directly --- library/std/src/sys/unix/os.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/library/std/src/sys/unix/os.rs b/library/std/src/sys/unix/os.rs index b268ef5c3640..0b6cdb923bd6 100644 --- a/library/std/src/sys/unix/os.rs +++ b/library/std/src/sys/unix/os.rs @@ -384,11 +384,8 @@ pub fn current_exe() -> io::Result { if let Ok(path) = crate::fs::read_link("/proc/self/path/a.out") { Ok(path) } else { - extern "C" { - fn getexecname() -> *const c_char; - } unsafe { - let path = getexecname(); + let path = libc::getexecname(); if path.is_null() { Err(io::Error::last_os_error()) } else { From c97f05c4f4a09ce8ec5dd4c11bf58abd533d9dfa Mon Sep 17 00:00:00 2001 From: Nixon Enraght-Moony Date: Sun, 20 Feb 2022 22:44:04 +0000 Subject: [PATCH 03/12] compiletest: Print process output info with less whitespace Before: ``` error: jsondocck failed! status: exit status: 1 command: "/data/ne321/rust/build/x86_64-unknown-linux-gnu/stage0-tools-bin/jsondocck" "--doc-dir" "/data/ne321/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-json/traits/supertrait" "--template" "/data/ne321/rust/src/test/rustdoc-json/traits/supertrait.rs" stdout: ------------------------------------------ ------------------------------------------ stderr: ------------------------------------------ Invalid command: Tried to use the previous path in the first command on line 10 Error: "Jsondocck failed for /data/ne321/rust/src/test/rustdoc-json/traits/supertrait.rs" ------------------------------------------ Rustdoc Output: status: exit status: 0 command: "/data/ne321/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "-L" "/data/ne321/rust/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-L" "/data/ne321/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-json/traits/supertrait/auxiliary" "-o" "/data/ne321/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-json/traits/supertrait" "--deny" "warnings" "/data/ne321/rust/src/test/rustdoc-json/traits/supertrait.rs" "--output-format" "json" "-Zunstable-options" stdout: ------------------------------------------ ------------------------------------------ stderr: ------------------------------------------ ------------------------------------------ ``` After: ``` error: jsondocck failed! status: exit status: 1 command: "/data/ne321/rust/build/x86_64-unknown-linux-gnu/stage0-tools-bin/jsondocck" "--doc-dir" "/data/ne321/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-json/traits/supertrait" "--template" "/data/ne321/rust/src/test/rustdoc-json/traits/supertrait.rs" stdout: none --- stderr ------------------------------- Invalid command: Tried to use the previous path in the first command on line 10 Error: "Jsondocck failed for /data/ne321/rust/src/test/rustdoc-json/traits/supertrait.rs" ------------------------------------------ Rustdoc Output: status: exit status: 0 command: "/data/ne321/rust/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "-L" "/data/ne321/rust/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-L" "/data/ne321/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-json/traits/supertrait/auxiliary" "-o" "/data/ne321/rust/build/x86_64-unknown-linux-gnu/test/rustdoc-json/traits/supertrait" "--deny" "warnings" "/data/ne321/rust/src/test/rustdoc-json/traits/supertrait.rs" "--output-format" "json" "-Zunstable-options" stdout: none stderr: none ``` --- src/tools/compiletest/src/runtest.rs | 34 ++++++++++++++++------------ 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 7fe7db0801b4..3f67a64971b6 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -3753,23 +3753,27 @@ pub struct ProcRes { impl ProcRes { pub fn print_info(&self) { - print!( - "\ - status: {}\n\ - command: {}\n\ - stdout:\n\ - ------------------------------------------\n\ - {}\n\ - ------------------------------------------\n\ - stderr:\n\ - ------------------------------------------\n\ - {}\n\ - ------------------------------------------\n\ - \n", + fn render(name: &str, contents: &str) -> String { + let contents = json::extract_rendered(contents); + let contents = contents.trim(); + if contents.is_empty() { + format!("{name}: none") + } else { + format!( + "\ + --- {name} -------------------------------\n\ + {contents}\n\ + ------------------------------------------", + ) + } + } + + println!( + "status: {}\ncommand: {}\n{}\n{}\n", self.status, self.cmdline, - json::extract_rendered(&self.stdout), - json::extract_rendered(&self.stderr), + render("stdout", &self.stdout), + render("stderr", &self.stderr), ); } From e7730dcb7eb29a10ee73f269f4dc6e9d606db0da Mon Sep 17 00:00:00 2001 From: est31 Date: Mon, 21 Feb 2022 04:38:39 +0100 Subject: [PATCH 04/12] Expand let-else allow tests The #[allow(...)] directive was tested for the body and the pattern, but non-presence of it wasn't tested. Furthermore, it wasn't tested for the expression. We add expression tests as well as ones checking the non-presence of the directive. --- .../ui/let-else/let-else-allow-in-expr.rs | 30 +++++++++++++++++++ .../ui/let-else/let-else-allow-in-expr.stderr | 20 +++++++++++++ src/test/ui/let-else/let-else-allow-unused.rs | 5 ++-- .../ui/let-else/let-else-allow-unused.stderr | 14 +++++++++ src/test/ui/let-else/let-else-check.rs | 5 ++++ src/test/ui/let-else/let-else-check.stderr | 10 +++++-- 6 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/let-else/let-else-allow-in-expr.rs create mode 100644 src/test/ui/let-else/let-else-allow-in-expr.stderr create mode 100644 src/test/ui/let-else/let-else-allow-unused.stderr diff --git a/src/test/ui/let-else/let-else-allow-in-expr.rs b/src/test/ui/let-else/let-else-allow-in-expr.rs new file mode 100644 index 000000000000..39f4c9060fea --- /dev/null +++ b/src/test/ui/let-else/let-else-allow-in-expr.rs @@ -0,0 +1,30 @@ +#![feature(let_else)] + +#![deny(unused_variables)] + +fn main() { + let Some(_): Option = ({ + let x = 1; //~ ERROR unused variable: `x` + Some(1) + }) else { + return; + }; + + #[allow(unused_variables)] + let Some(_): Option = ({ + let x = 1; + Some(1) + }) else { + return; + }; + + let Some(_): Option = ({ + #[allow(unused_variables)] + let x = 1; + Some(1) + }) else { + return; + }; + + let x = 1; //~ ERROR unused variable: `x` +} diff --git a/src/test/ui/let-else/let-else-allow-in-expr.stderr b/src/test/ui/let-else/let-else-allow-in-expr.stderr new file mode 100644 index 000000000000..e86bcbc85002 --- /dev/null +++ b/src/test/ui/let-else/let-else-allow-in-expr.stderr @@ -0,0 +1,20 @@ +error: unused variable: `x` + --> $DIR/let-else-allow-in-expr.rs:7:13 + | +LL | let x = 1; + | ^ help: if this is intentional, prefix it with an underscore: `_x` + | +note: the lint level is defined here + --> $DIR/let-else-allow-in-expr.rs:3:9 + | +LL | #![deny(unused_variables)] + | ^^^^^^^^^^^^^^^^ + +error: unused variable: `x` + --> $DIR/let-else-allow-in-expr.rs:29:9 + | +LL | let x = 1; + | ^ help: if this is intentional, prefix it with an underscore: `_x` + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/let-else/let-else-allow-unused.rs b/src/test/ui/let-else/let-else-allow-unused.rs index bcd8c987628b..86ebacfa7b7d 100644 --- a/src/test/ui/let-else/let-else-allow-unused.rs +++ b/src/test/ui/let-else/let-else-allow-unused.rs @@ -1,4 +1,3 @@ -// check-pass // issue #89807 #![feature(let_else)] @@ -10,5 +9,7 @@ fn main() { #[allow(unused)] let banana = 1; #[allow(unused)] - let Some(chaenomeles) = value else { return }; // OK + let Some(chaenomeles) = value.clone() else { return }; // OK + + let Some(chaenomeles) = value else { return }; //~ ERROR unused variable: `chaenomeles` } diff --git a/src/test/ui/let-else/let-else-allow-unused.stderr b/src/test/ui/let-else/let-else-allow-unused.stderr new file mode 100644 index 000000000000..05b8a9169fb7 --- /dev/null +++ b/src/test/ui/let-else/let-else-allow-unused.stderr @@ -0,0 +1,14 @@ +error: unused variable: `chaenomeles` + --> $DIR/let-else-allow-unused.rs:14:14 + | +LL | let Some(chaenomeles) = value else { return }; + | ^^^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_chaenomeles` + | +note: the lint level is defined here + --> $DIR/let-else-allow-unused.rs:5:8 + | +LL | #[deny(unused_variables)] + | ^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/let-else/let-else-check.rs b/src/test/ui/let-else/let-else-check.rs index ab763447ef7e..9e32cbef742a 100644 --- a/src/test/ui/let-else/let-else-check.rs +++ b/src/test/ui/let-else/let-else-check.rs @@ -10,5 +10,10 @@ fn main() { return; }; + let Some(_): Option = Some(Default::default()) else { + let x = 1; //~ ERROR unused variable: `x` + return; + }; + let x = 1; //~ ERROR unused variable: `x` } diff --git a/src/test/ui/let-else/let-else-check.stderr b/src/test/ui/let-else/let-else-check.stderr index 50e54d320b00..b3da412ec280 100644 --- a/src/test/ui/let-else/let-else-check.stderr +++ b/src/test/ui/let-else/let-else-check.stderr @@ -1,5 +1,5 @@ error: unused variable: `x` - --> $DIR/let-else-check.rs:13:9 + --> $DIR/let-else-check.rs:18:9 | LL | let x = 1; | ^ help: if this is intentional, prefix it with an underscore: `_x` @@ -10,5 +10,11 @@ note: the lint level is defined here LL | #![deny(unused_variables)] | ^^^^^^^^^^^^^^^^ -error: aborting due to previous error +error: unused variable: `x` + --> $DIR/let-else-check.rs:14:13 + | +LL | let x = 1; + | ^ help: if this is intentional, prefix it with an underscore: `_x` + +error: aborting due to 2 previous errors From 5bd71063b3810d977aa376d1e6dd7cec359330cc Mon Sep 17 00:00:00 2001 From: est31 Date: Mon, 21 Feb 2022 04:45:40 +0100 Subject: [PATCH 05/12] Add regression test for #92069 --- src/test/ui/let-else/let-else-slicing-error.rs | 9 +++++++++ src/test/ui/let-else/let-else-slicing-error.stderr | 11 +++++++++++ 2 files changed, 20 insertions(+) create mode 100644 src/test/ui/let-else/let-else-slicing-error.rs create mode 100644 src/test/ui/let-else/let-else-slicing-error.stderr diff --git a/src/test/ui/let-else/let-else-slicing-error.rs b/src/test/ui/let-else/let-else-slicing-error.rs new file mode 100644 index 000000000000..4022656a8f53 --- /dev/null +++ b/src/test/ui/let-else/let-else-slicing-error.rs @@ -0,0 +1,9 @@ +// issue #92069 +#![feature(let_else)] + +fn main() { + let nums = vec![5, 4, 3, 2, 1]; + let [x, y] = nums else { //~ ERROR expected an array or slice + return; + }; +} diff --git a/src/test/ui/let-else/let-else-slicing-error.stderr b/src/test/ui/let-else/let-else-slicing-error.stderr new file mode 100644 index 000000000000..064025e0345b --- /dev/null +++ b/src/test/ui/let-else/let-else-slicing-error.stderr @@ -0,0 +1,11 @@ +error[E0529]: expected an array or slice, found `Vec<{integer}>` + --> $DIR/let-else-slicing-error.rs:6:9 + | +LL | let [x, y] = nums else { + | ^^^^^^ ---- help: consider slicing here: `nums[..]` + | | + | pattern cannot match with input type `Vec<{integer}>` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0529`. From 2e8a7663b45790310adac4f62a88df08821261f1 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Thu, 17 Feb 2022 12:46:47 +0000 Subject: [PATCH 06/12] Simplify gating of BPF w registers behind the alu32 target feature This is already handled by supported_types(). --- compiler/rustc_target/src/asm/bpf.rs | 38 +++++++++------------------- 1 file changed, 12 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_target/src/asm/bpf.rs b/compiler/rustc_target/src/asm/bpf.rs index b4d982f3836b..3b03766a089b 100644 --- a/compiler/rustc_target/src/asm/bpf.rs +++ b/compiler/rustc_target/src/asm/bpf.rs @@ -1,7 +1,6 @@ -use super::{InlineAsmArch, InlineAsmType, Target}; -use rustc_data_structures::stable_set::FxHashSet; +use super::{InlineAsmArch, InlineAsmType}; use rustc_macros::HashStable_Generic; -use rustc_span::{sym, Symbol}; +use rustc_span::Symbol; use std::fmt; def_reg_class! { @@ -43,19 +42,6 @@ impl BpfInlineAsmRegClass { } } -fn only_alu32( - _arch: InlineAsmArch, - target_features: &FxHashSet, - _target: &Target, - _is_clobber: bool, -) -> Result<(), &'static str> { - if !target_features.contains(&sym::alu32) { - Err("register can't be used without the `alu32` target feature") - } else { - Ok(()) - } -} - def_regs! { Bpf BpfInlineAsmReg BpfInlineAsmRegClass { r0: reg = ["r0"], @@ -68,16 +54,16 @@ def_regs! { r7: reg = ["r7"], r8: reg = ["r8"], r9: reg = ["r9"], - w0: wreg = ["w0"] % only_alu32, - w1: wreg = ["w1"] % only_alu32, - w2: wreg = ["w2"] % only_alu32, - w3: wreg = ["w3"] % only_alu32, - w4: wreg = ["w4"] % only_alu32, - w5: wreg = ["w5"] % only_alu32, - w6: wreg = ["w6"] % only_alu32, - w7: wreg = ["w7"] % only_alu32, - w8: wreg = ["w8"] % only_alu32, - w9: wreg = ["w9"] % only_alu32, + w0: wreg = ["w0"], + w1: wreg = ["w1"], + w2: wreg = ["w2"], + w3: wreg = ["w3"], + w4: wreg = ["w4"], + w5: wreg = ["w5"], + w6: wreg = ["w6"], + w7: wreg = ["w7"], + w8: wreg = ["w8"], + w9: wreg = ["w9"], #error = ["r10", "w10"] => "the stack pointer cannot be used as an operand for inline asm", From 1ceb104851c2c3054f680d89f9d9333b5e5110be Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Thu, 17 Feb 2022 14:16:52 +0000 Subject: [PATCH 07/12] On ARM, use relocation_model to detect whether r9 should be reserved The previous approach of checking for the reserve-r9 target feature didn't actually work because LLVM only sets this feature very late when initializing the per-function subtarget. --- compiler/rustc_ast_lowering/src/asm.rs | 2 + .../rustc_codegen_cranelift/src/inline_asm.rs | 8 ++- .../rustc_codegen_ssa/src/target_features.rs | 1 - compiler/rustc_span/src/symbol.rs | 1 - compiler/rustc_target/src/asm/aarch64.rs | 3 +- compiler/rustc_target/src/asm/arm.rs | 21 +++--- compiler/rustc_target/src/asm/mod.rs | 69 ++++++++++--------- compiler/rustc_target/src/asm/riscv.rs | 3 +- compiler/rustc_target/src/asm/x86.rs | 6 +- 9 files changed, 67 insertions(+), 47 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/asm.rs b/compiler/rustc_ast_lowering/src/asm.rs index 0211d7b33673..d7c27059aaea 100644 --- a/compiler/rustc_ast_lowering/src/asm.rs +++ b/compiler/rustc_ast_lowering/src/asm.rs @@ -66,6 +66,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { for (abi_name, abi_span) in &asm.clobber_abis { match asm::InlineAsmClobberAbi::parse( asm_arch, + self.sess.relocation_model(), &self.sess.target_features, &self.sess.target, *abi_name, @@ -134,6 +135,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { asm::InlineAsmRegOrRegClass::Reg(if let Some(asm_arch) = asm_arch { asm::InlineAsmReg::parse( asm_arch, + sess.relocation_model(), &sess.target_features, &sess.target, is_clobber, diff --git a/compiler/rustc_codegen_cranelift/src/inline_asm.rs b/compiler/rustc_codegen_cranelift/src/inline_asm.rs index c242c75ed18f..10c2f06faf3e 100644 --- a/compiler/rustc_codegen_cranelift/src/inline_asm.rs +++ b/compiler/rustc_codegen_cranelift/src/inline_asm.rs @@ -182,7 +182,12 @@ struct InlineAssemblyGenerator<'a, 'tcx> { impl<'tcx> InlineAssemblyGenerator<'_, 'tcx> { fn allocate_registers(&mut self) { let sess = self.tcx.sess; - let map = allocatable_registers(self.arch, &sess.target_features, &sess.target); + let map = allocatable_registers( + self.arch, + sess.relocation_model(), + &sess.target_features, + &sess.target, + ); let mut allocated = FxHashMap::<_, (bool, bool)>::default(); let mut regs = vec![None; self.operands.len()]; @@ -315,6 +320,7 @@ impl<'tcx> InlineAssemblyGenerator<'_, 'tcx> { // Allocate stack slots for saving clobbered registers let abi_clobber = InlineAsmClobberAbi::parse( self.arch, + self.tcx.sess.relocation_model(), &self.tcx.sess.target_features, &self.tcx.sess.target, sym::C, diff --git a/compiler/rustc_codegen_ssa/src/target_features.rs b/compiler/rustc_codegen_ssa/src/target_features.rs index f31b0ee592e9..77166c89735e 100644 --- a/compiler/rustc_codegen_ssa/src/target_features.rs +++ b/compiler/rustc_codegen_ssa/src/target_features.rs @@ -36,7 +36,6 @@ const ARM_ALLOWED_FEATURES: &[(&str, Option)] = &[ // #[target_feature]. ("thumb-mode", Some(sym::arm_target_feature)), ("thumb2", Some(sym::arm_target_feature)), - ("reserve-r9", Some(sym::arm_target_feature)), ]; const AARCH64_ALLOWED_FEATURES: &[(&str, Option)] = &[ diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index c746255e95e1..6767593bbc51 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1122,7 +1122,6 @@ symbols! { repr_packed, repr_simd, repr_transparent, - reserved_r9: "reserved-r9", residual, result, rhs, diff --git a/compiler/rustc_target/src/asm/aarch64.rs b/compiler/rustc_target/src/asm/aarch64.rs index d184ad4e78ae..b4a1b5e91430 100644 --- a/compiler/rustc_target/src/asm/aarch64.rs +++ b/compiler/rustc_target/src/asm/aarch64.rs @@ -1,5 +1,5 @@ use super::{InlineAsmArch, InlineAsmType}; -use crate::spec::Target; +use crate::spec::{Target, RelocModel}; use rustc_data_structures::stable_set::FxHashSet; use rustc_macros::HashStable_Generic; use rustc_span::Symbol; @@ -75,6 +75,7 @@ impl AArch64InlineAsmRegClass { pub fn reserved_x18( _arch: InlineAsmArch, + _reloc_model: RelocModel, _target_features: &FxHashSet, target: &Target, _is_clobber: bool, diff --git a/compiler/rustc_target/src/asm/arm.rs b/compiler/rustc_target/src/asm/arm.rs index b2d5bb3736af..88f2d3f80d2c 100644 --- a/compiler/rustc_target/src/asm/arm.rs +++ b/compiler/rustc_target/src/asm/arm.rs @@ -1,5 +1,5 @@ use super::{InlineAsmArch, InlineAsmType}; -use crate::spec::Target; +use crate::spec::{RelocModel, Target}; use rustc_data_structures::stable_set::FxHashSet; use rustc_macros::HashStable_Generic; use rustc_span::{sym, Symbol}; @@ -67,11 +67,12 @@ fn frame_pointer_is_r7(target_features: &FxHashSet, target: &Target) -> fn frame_pointer_r11( arch: InlineAsmArch, + reloc_model: RelocModel, target_features: &FxHashSet, target: &Target, is_clobber: bool, ) -> Result<(), &'static str> { - not_thumb1(arch, target_features, target, is_clobber)?; + not_thumb1(arch, reloc_model, target_features, target, is_clobber)?; if !frame_pointer_is_r7(target_features, target) { Err("the frame pointer (r11) cannot be used as an operand for inline asm") @@ -82,6 +83,7 @@ fn frame_pointer_r11( fn frame_pointer_r7( _arch: InlineAsmArch, + _reloc_model: RelocModel, target_features: &FxHashSet, target: &Target, _is_clobber: bool, @@ -95,6 +97,7 @@ fn frame_pointer_r7( fn not_thumb1( _arch: InlineAsmArch, + _reloc_model: RelocModel, target_features: &FxHashSet, _target: &Target, is_clobber: bool, @@ -111,18 +114,18 @@ fn not_thumb1( fn reserved_r9( arch: InlineAsmArch, + reloc_model: RelocModel, target_features: &FxHashSet, target: &Target, is_clobber: bool, ) -> Result<(), &'static str> { - not_thumb1(arch, target_features, target, is_clobber)?; + not_thumb1(arch, reloc_model, target_features, target, is_clobber)?; - // We detect this using the reserved-r9 feature instead of using the target - // because the relocation model can be changed with compiler options. - if target_features.contains(&sym::reserved_r9) { - Err("the RWPI static base register (r9) cannot be used as an operand for inline asm") - } else { - Ok(()) + match reloc_model { + RelocModel::Rwpi | RelocModel::RopiRwpi => { + Err("the RWPI static base register (r9) cannot be used as an operand for inline asm") + } + _ => Ok(()), } } diff --git a/compiler/rustc_target/src/asm/mod.rs b/compiler/rustc_target/src/asm/mod.rs index fd95b0338a6e..1bf4747a9705 100644 --- a/compiler/rustc_target/src/asm/mod.rs +++ b/compiler/rustc_target/src/asm/mod.rs @@ -1,5 +1,5 @@ -use crate::abi::Size; use crate::spec::Target; +use crate::{abi::Size, spec::RelocModel}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_macros::HashStable_Generic; use rustc_span::Symbol; @@ -81,6 +81,7 @@ macro_rules! def_regs { pub fn parse( _arch: super::InlineAsmArch, + _reloc_model: crate::spec::RelocModel, _target_features: &rustc_data_structures::fx::FxHashSet, _target: &crate::spec::Target, _is_clobber: bool, @@ -89,7 +90,7 @@ macro_rules! def_regs { match name { $( $($alias)|* | $reg_name => { - $($filter(_arch, _target_features, _target, _is_clobber)?;)? + $($filter(_arch, _reloc_model, _target_features, _target, _is_clobber)?;)? Ok(Self::$reg) } )* @@ -103,6 +104,7 @@ macro_rules! def_regs { pub(super) fn fill_reg_map( _arch: super::InlineAsmArch, + _reloc_model: crate::spec::RelocModel, _target_features: &rustc_data_structures::fx::FxHashSet, _target: &crate::spec::Target, _map: &mut rustc_data_structures::fx::FxHashMap< @@ -113,7 +115,7 @@ macro_rules! def_regs { #[allow(unused_imports)] use super::{InlineAsmReg, InlineAsmRegClass}; $( - if $($filter(_arch, _target_features, _target, false).is_ok() &&)? true { + if $($filter(_arch, _reloc_model, _target_features, _target, false).is_ok() &&)? true { if let Some(set) = _map.get_mut(&InlineAsmRegClass::$arch($arch_regclass::$class)) { set.insert(InlineAsmReg::$arch($arch_reg::$reg)); } @@ -297,6 +299,7 @@ impl InlineAsmReg { pub fn parse( arch: InlineAsmArch, + reloc_model: RelocModel, target_features: &FxHashSet, target: &Target, is_clobber: bool, @@ -307,75 +310,75 @@ impl InlineAsmReg { let name = name.as_str(); Ok(match arch { InlineAsmArch::X86 | InlineAsmArch::X86_64 => { - Self::X86(X86InlineAsmReg::parse(arch, target_features, target, is_clobber, name)?) + Self::X86(X86InlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?) } InlineAsmArch::Arm => { - Self::Arm(ArmInlineAsmReg::parse(arch, target_features, target, is_clobber, name)?) + Self::Arm(ArmInlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?) } InlineAsmArch::AArch64 => Self::AArch64(AArch64InlineAsmReg::parse( arch, - target_features, + reloc_model,target_features, target, is_clobber, name, )?), InlineAsmArch::RiscV32 | InlineAsmArch::RiscV64 => Self::RiscV( - RiscVInlineAsmReg::parse(arch, target_features, target, is_clobber, name)?, + RiscVInlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?, ), InlineAsmArch::Nvptx64 => Self::Nvptx(NvptxInlineAsmReg::parse( arch, - target_features, + reloc_model,target_features, target, is_clobber, name, )?), InlineAsmArch::PowerPC | InlineAsmArch::PowerPC64 => Self::PowerPC( - PowerPCInlineAsmReg::parse(arch, target_features, target, is_clobber, name)?, + PowerPCInlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?, ), InlineAsmArch::Hexagon => Self::Hexagon(HexagonInlineAsmReg::parse( arch, - target_features, + reloc_model,target_features, target, is_clobber, name, )?), InlineAsmArch::Mips | InlineAsmArch::Mips64 => Self::Mips(MipsInlineAsmReg::parse( arch, - target_features, + reloc_model,target_features, target, is_clobber, name, )?), InlineAsmArch::S390x => Self::S390x(S390xInlineAsmReg::parse( arch, - target_features, + reloc_model,target_features, target, is_clobber, name, )?), InlineAsmArch::SpirV => Self::SpirV(SpirVInlineAsmReg::parse( arch, - target_features, + reloc_model,target_features, target, is_clobber, name, )?), InlineAsmArch::Wasm32 | InlineAsmArch::Wasm64 => Self::Wasm(WasmInlineAsmReg::parse( arch, - target_features, + reloc_model,target_features, target, is_clobber, name, )?), InlineAsmArch::Bpf => { - Self::Bpf(BpfInlineAsmReg::parse(arch, target_features, target, is_clobber, name)?) + Self::Bpf(BpfInlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?) } InlineAsmArch::Avr => { - Self::Avr(AvrInlineAsmReg::parse(arch, target_features, target, is_clobber, name)?) + Self::Avr(AvrInlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?) } InlineAsmArch::Msp430 => Self::Msp430(Msp430InlineAsmReg::parse( arch, - target_features, + reloc_model,target_features, target, is_clobber, name, @@ -749,78 +752,79 @@ impl fmt::Display for InlineAsmType { // falling back to an external assembler. pub fn allocatable_registers( arch: InlineAsmArch, + reloc_model: RelocModel, target_features: &FxHashSet, target: &crate::spec::Target, ) -> FxHashMap> { match arch { InlineAsmArch::X86 | InlineAsmArch::X86_64 => { let mut map = x86::regclass_map(); - x86::fill_reg_map(arch, target_features, target, &mut map); + x86::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::Arm => { let mut map = arm::regclass_map(); - arm::fill_reg_map(arch, target_features, target, &mut map); + arm::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::AArch64 => { let mut map = aarch64::regclass_map(); - aarch64::fill_reg_map(arch, target_features, target, &mut map); + aarch64::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::RiscV32 | InlineAsmArch::RiscV64 => { let mut map = riscv::regclass_map(); - riscv::fill_reg_map(arch, target_features, target, &mut map); + riscv::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::Nvptx64 => { let mut map = nvptx::regclass_map(); - nvptx::fill_reg_map(arch, target_features, target, &mut map); + nvptx::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::PowerPC | InlineAsmArch::PowerPC64 => { let mut map = powerpc::regclass_map(); - powerpc::fill_reg_map(arch, target_features, target, &mut map); + powerpc::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::Hexagon => { let mut map = hexagon::regclass_map(); - hexagon::fill_reg_map(arch, target_features, target, &mut map); + hexagon::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::Mips | InlineAsmArch::Mips64 => { let mut map = mips::regclass_map(); - mips::fill_reg_map(arch, target_features, target, &mut map); + mips::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::S390x => { let mut map = s390x::regclass_map(); - s390x::fill_reg_map(arch, target_features, target, &mut map); + s390x::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::SpirV => { let mut map = spirv::regclass_map(); - spirv::fill_reg_map(arch, target_features, target, &mut map); + spirv::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::Wasm32 | InlineAsmArch::Wasm64 => { let mut map = wasm::regclass_map(); - wasm::fill_reg_map(arch, target_features, target, &mut map); + wasm::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::Bpf => { let mut map = bpf::regclass_map(); - bpf::fill_reg_map(arch, target_features, target, &mut map); + bpf::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::Avr => { let mut map = avr::regclass_map(); - avr::fill_reg_map(arch, target_features, target, &mut map); + avr::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } InlineAsmArch::Msp430 => { let mut map = msp430::regclass_map(); - msp430::fill_reg_map(arch, target_features, target, &mut map); + msp430::fill_reg_map(arch, reloc_model, target_features, target, &mut map); map } } @@ -853,6 +857,7 @@ impl InlineAsmClobberAbi { /// clobber ABIs for the target. pub fn parse( arch: InlineAsmArch, + reloc_model: RelocModel, target_features: &FxHashSet, target: &Target, name: Symbol, @@ -878,7 +883,7 @@ impl InlineAsmClobberAbi { }, InlineAsmArch::AArch64 => match name { "C" | "system" | "efiapi" => { - Ok(if aarch64::reserved_x18(arch, target_features, target, true).is_err() { + Ok(if aarch64::reserved_x18(arch, reloc_model, target_features, target, true).is_err() { InlineAsmClobberAbi::AArch64NoX18 } else { InlineAsmClobberAbi::AArch64 diff --git a/compiler/rustc_target/src/asm/riscv.rs b/compiler/rustc_target/src/asm/riscv.rs index e145ba8a16e6..65ce69cb5c0c 100644 --- a/compiler/rustc_target/src/asm/riscv.rs +++ b/compiler/rustc_target/src/asm/riscv.rs @@ -1,5 +1,5 @@ use super::{InlineAsmArch, InlineAsmType}; -use crate::spec::Target; +use crate::spec::{Target, RelocModel}; use rustc_data_structures::stable_set::FxHashSet; use rustc_macros::HashStable_Generic; use rustc_span::{sym, Symbol}; @@ -54,6 +54,7 @@ impl RiscVInlineAsmRegClass { fn not_e( _arch: InlineAsmArch, + _reloc_model: RelocModel, target_features: &FxHashSet, _target: &Target, _is_clobber: bool, diff --git a/compiler/rustc_target/src/asm/x86.rs b/compiler/rustc_target/src/asm/x86.rs index a8ee80ec4ea2..ac6f39f1c955 100644 --- a/compiler/rustc_target/src/asm/x86.rs +++ b/compiler/rustc_target/src/asm/x86.rs @@ -1,5 +1,5 @@ use super::{InlineAsmArch, InlineAsmType}; -use crate::spec::Target; +use crate::spec::{Target, RelocModel}; use rustc_data_structures::stable_set::FxHashSet; use rustc_macros::HashStable_Generic; use rustc_span::Symbol; @@ -139,6 +139,7 @@ impl X86InlineAsmRegClass { fn x86_64_only( arch: InlineAsmArch, + _reloc_model: RelocModel, _target_features: &FxHashSet, _target: &Target, _is_clobber: bool, @@ -152,6 +153,7 @@ fn x86_64_only( fn high_byte( arch: InlineAsmArch, + _reloc_model: RelocModel, _target_features: &FxHashSet, _target: &Target, _is_clobber: bool, @@ -164,6 +166,7 @@ fn high_byte( fn rbx_reserved( arch: InlineAsmArch, + _reloc_model: RelocModel, _target_features: &FxHashSet, _target: &Target, _is_clobber: bool, @@ -179,6 +182,7 @@ fn rbx_reserved( fn esi_reserved( arch: InlineAsmArch, + _reloc_model: RelocModel, _target_features: &FxHashSet, _target: &Target, _is_clobber: bool, From fc41d4bf351b9c39aac58b7d0e307495ddf60dfc Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Thu, 17 Feb 2022 18:16:04 +0000 Subject: [PATCH 08/12] Take CodegenFnAttrs into account when validating asm! register operands Checking of asm! register operands now properly takes function attributes such as #[target_feature] and #[instruction_set] into account. --- compiler/rustc_ast_lowering/src/asm.rs | 28 +-- .../rustc_codegen_cranelift/src/inline_asm.rs | 16 +- compiler/rustc_middle/src/query/mod.rs | 4 + compiler/rustc_passes/src/intrinsicck.rs | 43 ++-- compiler/rustc_target/src/asm/aarch64.rs | 14 +- compiler/rustc_target/src/asm/mod.rs | 200 ++++++++---------- compiler/rustc_typeck/src/collect.rs | 19 ++ src/test/ui/asm/x86_64/bad-reg.rs | 2 - src/test/ui/asm/x86_64/bad-reg.stderr | 32 ++- 9 files changed, 171 insertions(+), 187 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/asm.rs b/compiler/rustc_ast_lowering/src/asm.rs index d7c27059aaea..171cc60dfd77 100644 --- a/compiler/rustc_ast_lowering/src/asm.rs +++ b/compiler/rustc_ast_lowering/src/asm.rs @@ -64,13 +64,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let mut clobber_abis = FxHashMap::default(); if let Some(asm_arch) = asm_arch { for (abi_name, abi_span) in &asm.clobber_abis { - match asm::InlineAsmClobberAbi::parse( - asm_arch, - self.sess.relocation_model(), - &self.sess.target_features, - &self.sess.target, - *abi_name, - ) { + match asm::InlineAsmClobberAbi::parse(asm_arch, &self.sess.target, *abi_name) { Ok(abi) => { // If the abi was already in the list, emit an error match clobber_abis.get(&abi) { @@ -130,18 +124,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { .operands .iter() .map(|(op, op_sp)| { - let lower_reg = |reg, is_clobber| match reg { + let lower_reg = |reg| match reg { InlineAsmRegOrRegClass::Reg(s) => { asm::InlineAsmRegOrRegClass::Reg(if let Some(asm_arch) = asm_arch { - asm::InlineAsmReg::parse( - asm_arch, - sess.relocation_model(), - &sess.target_features, - &sess.target, - is_clobber, - s, - ) - .unwrap_or_else(|e| { + asm::InlineAsmReg::parse(asm_arch, s).unwrap_or_else(|e| { let msg = format!("invalid register `{}`: {}", s.as_str(), e); sess.struct_span_err(*op_sp, &msg).emit(); asm::InlineAsmReg::Err @@ -165,24 +151,24 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let op = match *op { InlineAsmOperand::In { reg, ref expr } => hir::InlineAsmOperand::In { - reg: lower_reg(reg, false), + reg: lower_reg(reg), expr: self.lower_expr_mut(expr), }, InlineAsmOperand::Out { reg, late, ref expr } => hir::InlineAsmOperand::Out { - reg: lower_reg(reg, expr.is_none()), + reg: lower_reg(reg), late, expr: expr.as_ref().map(|expr| self.lower_expr_mut(expr)), }, InlineAsmOperand::InOut { reg, late, ref expr } => { hir::InlineAsmOperand::InOut { - reg: lower_reg(reg, false), + reg: lower_reg(reg), late, expr: self.lower_expr_mut(expr), } } InlineAsmOperand::SplitInOut { reg, late, ref in_expr, ref out_expr } => { hir::InlineAsmOperand::SplitInOut { - reg: lower_reg(reg, false), + reg: lower_reg(reg), late, in_expr: self.lower_expr_mut(in_expr), out_expr: out_expr.as_ref().map(|expr| self.lower_expr_mut(expr)), diff --git a/compiler/rustc_codegen_cranelift/src/inline_asm.rs b/compiler/rustc_codegen_cranelift/src/inline_asm.rs index 10c2f06faf3e..deac5dfd3ec1 100644 --- a/compiler/rustc_codegen_cranelift/src/inline_asm.rs +++ b/compiler/rustc_codegen_cranelift/src/inline_asm.rs @@ -106,6 +106,7 @@ pub(crate) fn codegen_inline_asm<'tcx>( let mut asm_gen = InlineAssemblyGenerator { tcx: fx.tcx, arch: fx.tcx.sess.asm_arch.unwrap(), + enclosing_def_id: fx.instance.def_id(), template, operands, options, @@ -169,6 +170,7 @@ pub(crate) fn codegen_inline_asm<'tcx>( struct InlineAssemblyGenerator<'a, 'tcx> { tcx: TyCtxt<'tcx>, arch: InlineAsmArch, + enclosing_def_id: DefId, template: &'a [InlineAsmTemplatePiece], operands: &'a [InlineAsmOperand<'tcx>], options: InlineAsmOptions, @@ -185,7 +187,7 @@ impl<'tcx> InlineAssemblyGenerator<'_, 'tcx> { let map = allocatable_registers( self.arch, sess.relocation_model(), - &sess.target_features, + self.tcx.asm_target_features(self.enclosing_def_id), &sess.target, ); let mut allocated = FxHashMap::<_, (bool, bool)>::default(); @@ -318,15 +320,9 @@ impl<'tcx> InlineAssemblyGenerator<'_, 'tcx> { let mut new_slot = |x| new_slot_fn(&mut slot_size, x); // Allocate stack slots for saving clobbered registers - let abi_clobber = InlineAsmClobberAbi::parse( - self.arch, - self.tcx.sess.relocation_model(), - &self.tcx.sess.target_features, - &self.tcx.sess.target, - sym::C, - ) - .unwrap() - .clobbered_regs(); + let abi_clobber = InlineAsmClobberAbi::parse(self.arch, &self.tcx.sess.target, sym::C) + .unwrap() + .clobbered_regs(); for (i, reg) in self.registers.iter().enumerate().filter_map(|(i, r)| r.map(|r| (i, r))) { let mut need_save = true; // If the register overlaps with a register clobbered by function call, then diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index be86f1e92744..069dac969c66 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1046,6 +1046,10 @@ rustc_queries! { cache_on_disk_if { true } } + query asm_target_features(def_id: DefId) -> &'tcx FxHashSet { + desc { |tcx| "computing target features for inline asm of `{}`", tcx.def_path_str(def_id) } + } + query fn_arg_names(def_id: DefId) -> &'tcx [rustc_span::symbol::Ident] { desc { |tcx| "looking up function parameter names for `{}`", tcx.def_path_str(def_id) } separate_provide_extern diff --git a/compiler/rustc_passes/src/intrinsicck.rs b/compiler/rustc_passes/src/intrinsicck.rs index bd772d9975b3..d7dde157864a 100644 --- a/compiler/rustc_passes/src/intrinsicck.rs +++ b/compiler/rustc_passes/src/intrinsicck.rs @@ -1,4 +1,5 @@ use rustc_ast::InlineAsmTemplatePiece; +use rustc_data_structures::stable_set::FxHashSet; use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; @@ -138,7 +139,7 @@ impl<'tcx> ExprVisitor<'tcx> { template: &[InlineAsmTemplatePiece], is_input: bool, tied_input: Option<(&hir::Expr<'tcx>, Option)>, - target_features: &[Symbol], + target_features: &FxHashSet, ) -> Option { // Check the type against the allowed types for inline asm. let ty = self.typeck_results.expr_ty_adjusted(expr); @@ -285,9 +286,7 @@ impl<'tcx> ExprVisitor<'tcx> { // (!). In that case we still need the earlier check to verify that the // register class is usable at all. if let Some(feature) = feature { - if !self.tcx.sess.target_features.contains(&feature) - && !target_features.contains(&feature) - { + if !target_features.contains(&feature) { let msg = &format!("`{}` target feature is not enabled", feature); let mut err = self.tcx.sess.struct_span_err(expr.span, msg); err.note(&format!( @@ -347,7 +346,8 @@ impl<'tcx> ExprVisitor<'tcx> { let hir = self.tcx.hir(); let enclosing_id = hir.enclosing_body_owner(hir_id); let enclosing_def_id = hir.local_def_id(enclosing_id).to_def_id(); - let attrs = self.tcx.codegen_fn_attrs(enclosing_def_id); + let target_features = self.tcx.asm_target_features(enclosing_def_id); + let asm_arch = self.tcx.sess.asm_arch.unwrap(); for (idx, (op, op_sp)) in asm.operands.iter().enumerate() { // Validate register classes against currently enabled target // features. We check that at least one type is available for @@ -360,16 +360,29 @@ impl<'tcx> ExprVisitor<'tcx> { // Note that this is only possible for explicit register // operands, which cannot be used in the asm string. if let Some(reg) = op.reg() { + // Some explicit registers cannot be used depending on the + // target. Reject those here. + if let InlineAsmRegOrRegClass::Reg(reg) = reg { + if let Err(msg) = reg.validate( + asm_arch, + self.tcx.sess.relocation_model(), + &target_features, + &self.tcx.sess.target, + op.is_clobber(), + ) { + let msg = format!("cannot use register `{}`: {}", reg.name(), msg); + self.tcx.sess.struct_span_err(*op_sp, &msg).emit(); + continue; + } + } + if !op.is_clobber() { let mut missing_required_features = vec![]; let reg_class = reg.reg_class(); - for &(_, feature) in reg_class.supported_types(self.tcx.sess.asm_arch.unwrap()) - { + for &(_, feature) in reg_class.supported_types(asm_arch) { match feature { Some(feature) => { - if self.tcx.sess.target_features.contains(&feature) - || attrs.target_features.contains(&feature) - { + if target_features.contains(&feature) { missing_required_features.clear(); break; } else { @@ -425,7 +438,7 @@ impl<'tcx> ExprVisitor<'tcx> { asm.template, true, None, - &attrs.target_features, + &target_features, ); } hir::InlineAsmOperand::Out { reg, late: _, ref expr } => { @@ -437,7 +450,7 @@ impl<'tcx> ExprVisitor<'tcx> { asm.template, false, None, - &attrs.target_features, + &target_features, ); } } @@ -449,7 +462,7 @@ impl<'tcx> ExprVisitor<'tcx> { asm.template, false, None, - &attrs.target_features, + &target_features, ); } hir::InlineAsmOperand::SplitInOut { reg, late: _, ref in_expr, ref out_expr } => { @@ -460,7 +473,7 @@ impl<'tcx> ExprVisitor<'tcx> { asm.template, true, None, - &attrs.target_features, + &target_features, ); if let Some(out_expr) = out_expr { self.check_asm_operand_type( @@ -470,7 +483,7 @@ impl<'tcx> ExprVisitor<'tcx> { asm.template, false, Some((in_expr, in_ty)), - &attrs.target_features, + &target_features, ); } } diff --git a/compiler/rustc_target/src/asm/aarch64.rs b/compiler/rustc_target/src/asm/aarch64.rs index b4a1b5e91430..7fb4dbdf2b18 100644 --- a/compiler/rustc_target/src/asm/aarch64.rs +++ b/compiler/rustc_target/src/asm/aarch64.rs @@ -1,5 +1,5 @@ use super::{InlineAsmArch, InlineAsmType}; -use crate::spec::{Target, RelocModel}; +use crate::spec::{RelocModel, Target}; use rustc_data_structures::stable_set::FxHashSet; use rustc_macros::HashStable_Generic; use rustc_span::Symbol; @@ -73,18 +73,18 @@ impl AArch64InlineAsmRegClass { } } -pub fn reserved_x18( +pub fn target_reserves_x18(target: &Target) -> bool { + target.os == "android" || target.is_like_fuchsia || target.is_like_osx || target.is_like_windows +} + +fn reserved_x18( _arch: InlineAsmArch, _reloc_model: RelocModel, _target_features: &FxHashSet, target: &Target, _is_clobber: bool, ) -> Result<(), &'static str> { - if target.os == "android" - || target.is_like_fuchsia - || target.is_like_osx - || target.is_like_windows - { + if target_reserves_x18(target) { Err("x18 is a reserved register on this target") } else { Ok(()) diff --git a/compiler/rustc_target/src/asm/mod.rs b/compiler/rustc_target/src/asm/mod.rs index 1bf4747a9705..5bc4b566daf6 100644 --- a/compiler/rustc_target/src/asm/mod.rs +++ b/compiler/rustc_target/src/asm/mod.rs @@ -25,7 +25,7 @@ macro_rules! def_reg_class { } } - pub fn parse(_arch: super::InlineAsmArch, name: rustc_span::Symbol) -> Result { + pub fn parse(name: rustc_span::Symbol) -> Result { match name { $( rustc_span::sym::$class => Ok(Self::$class), @@ -79,25 +79,38 @@ macro_rules! def_regs { } } - pub fn parse( + pub fn parse(name: &str) -> Result { + match name { + $( + $($alias)|* | $reg_name => Ok(Self::$reg), + )* + $( + $($bad_reg)|* => Err($error), + )* + _ => Err("unknown register"), + } + } + + pub fn validate(self, _arch: super::InlineAsmArch, _reloc_model: crate::spec::RelocModel, _target_features: &rustc_data_structures::fx::FxHashSet, _target: &crate::spec::Target, _is_clobber: bool, - name: &str, - ) -> Result { - match name { + ) -> Result<(), &'static str> { + match self { $( - $($alias)|* | $reg_name => { - $($filter(_arch, _reloc_model, _target_features, _target, _is_clobber)?;)? - Ok(Self::$reg) + Self::$reg => { + $($filter( + _arch, + _reloc_model, + _target_features, + _target, + _is_clobber + )?;)? + Ok(()) } )* - $( - $($bad_reg)|* => Err($error), - )* - _ => Err("unknown register"), } } } @@ -297,95 +310,60 @@ impl InlineAsmReg { } } - pub fn parse( - arch: InlineAsmArch, - reloc_model: RelocModel, - target_features: &FxHashSet, - target: &Target, - is_clobber: bool, - name: Symbol, - ) -> Result { + pub fn parse(arch: InlineAsmArch, name: Symbol) -> Result { // FIXME: use direct symbol comparison for register names // Use `Symbol::as_str` instead of `Symbol::with` here because `has_feature` may access `Symbol`. let name = name.as_str(); Ok(match arch { - InlineAsmArch::X86 | InlineAsmArch::X86_64 => { - Self::X86(X86InlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?) + InlineAsmArch::X86 | InlineAsmArch::X86_64 => Self::X86(X86InlineAsmReg::parse(name)?), + InlineAsmArch::Arm => Self::Arm(ArmInlineAsmReg::parse(name)?), + InlineAsmArch::AArch64 => Self::AArch64(AArch64InlineAsmReg::parse(name)?), + InlineAsmArch::RiscV32 | InlineAsmArch::RiscV64 => { + Self::RiscV(RiscVInlineAsmReg::parse(name)?) } - InlineAsmArch::Arm => { - Self::Arm(ArmInlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?) + InlineAsmArch::Nvptx64 => Self::Nvptx(NvptxInlineAsmReg::parse(name)?), + InlineAsmArch::PowerPC | InlineAsmArch::PowerPC64 => { + Self::PowerPC(PowerPCInlineAsmReg::parse(name)?) } - InlineAsmArch::AArch64 => Self::AArch64(AArch64InlineAsmReg::parse( - arch, - reloc_model,target_features, - target, - is_clobber, - name, - )?), - InlineAsmArch::RiscV32 | InlineAsmArch::RiscV64 => Self::RiscV( - RiscVInlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?, - ), - InlineAsmArch::Nvptx64 => Self::Nvptx(NvptxInlineAsmReg::parse( - arch, - reloc_model,target_features, - target, - is_clobber, - name, - )?), - InlineAsmArch::PowerPC | InlineAsmArch::PowerPC64 => Self::PowerPC( - PowerPCInlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?, - ), - InlineAsmArch::Hexagon => Self::Hexagon(HexagonInlineAsmReg::parse( - arch, - reloc_model,target_features, - target, - is_clobber, - name, - )?), - InlineAsmArch::Mips | InlineAsmArch::Mips64 => Self::Mips(MipsInlineAsmReg::parse( - arch, - reloc_model,target_features, - target, - is_clobber, - name, - )?), - InlineAsmArch::S390x => Self::S390x(S390xInlineAsmReg::parse( - arch, - reloc_model,target_features, - target, - is_clobber, - name, - )?), - InlineAsmArch::SpirV => Self::SpirV(SpirVInlineAsmReg::parse( - arch, - reloc_model,target_features, - target, - is_clobber, - name, - )?), - InlineAsmArch::Wasm32 | InlineAsmArch::Wasm64 => Self::Wasm(WasmInlineAsmReg::parse( - arch, - reloc_model,target_features, - target, - is_clobber, - name, - )?), - InlineAsmArch::Bpf => { - Self::Bpf(BpfInlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?) + InlineAsmArch::Hexagon => Self::Hexagon(HexagonInlineAsmReg::parse(name)?), + InlineAsmArch::Mips | InlineAsmArch::Mips64 => { + Self::Mips(MipsInlineAsmReg::parse(name)?) } - InlineAsmArch::Avr => { - Self::Avr(AvrInlineAsmReg::parse(arch, reloc_model,target_features, target, is_clobber, name)?) + InlineAsmArch::S390x => Self::S390x(S390xInlineAsmReg::parse(name)?), + InlineAsmArch::SpirV => Self::SpirV(SpirVInlineAsmReg::parse(name)?), + InlineAsmArch::Wasm32 | InlineAsmArch::Wasm64 => { + Self::Wasm(WasmInlineAsmReg::parse(name)?) } - InlineAsmArch::Msp430 => Self::Msp430(Msp430InlineAsmReg::parse( - arch, - reloc_model,target_features, - target, - is_clobber, - name, - )?), + InlineAsmArch::Bpf => Self::Bpf(BpfInlineAsmReg::parse(name)?), + InlineAsmArch::Avr => Self::Avr(AvrInlineAsmReg::parse(name)?), + InlineAsmArch::Msp430 => Self::Msp430(Msp430InlineAsmReg::parse(name)?), }) } + pub fn validate( + self, + arch: InlineAsmArch, + reloc_model: RelocModel, + target_features: &FxHashSet, + target: &Target, + is_clobber: bool, + ) -> Result<(), &'static str> { + match self { + Self::X86(r) => r.validate(arch, reloc_model, target_features, target, is_clobber), + Self::Arm(r) => r.validate(arch, reloc_model, target_features, target, is_clobber), + Self::AArch64(r) => r.validate(arch, reloc_model, target_features, target, is_clobber), + Self::RiscV(r) => r.validate(arch, reloc_model, target_features, target, is_clobber), + Self::PowerPC(r) => r.validate(arch, reloc_model, target_features, target, is_clobber), + Self::Hexagon(r) => r.validate(arch, reloc_model, target_features, target, is_clobber), + Self::Mips(r) => r.validate(arch, reloc_model, target_features, target, is_clobber), + Self::S390x(r) => r.validate(arch, reloc_model, target_features, target, is_clobber), + Self::Bpf(r) => r.validate(arch, reloc_model, target_features, target, is_clobber), + Self::Avr(r) => r.validate(arch, reloc_model, target_features, target, is_clobber), + Self::Msp430(r) => r.validate(arch, reloc_model, target_features, target, is_clobber), + Self::Err => unreachable!(), + } + } + // NOTE: This function isn't used at the moment, but is needed to support // falling back to an external assembler. pub fn emit( @@ -587,29 +565,29 @@ impl InlineAsmRegClass { pub fn parse(arch: InlineAsmArch, name: Symbol) -> Result { Ok(match arch { InlineAsmArch::X86 | InlineAsmArch::X86_64 => { - Self::X86(X86InlineAsmRegClass::parse(arch, name)?) + Self::X86(X86InlineAsmRegClass::parse(name)?) } - InlineAsmArch::Arm => Self::Arm(ArmInlineAsmRegClass::parse(arch, name)?), - InlineAsmArch::AArch64 => Self::AArch64(AArch64InlineAsmRegClass::parse(arch, name)?), + InlineAsmArch::Arm => Self::Arm(ArmInlineAsmRegClass::parse(name)?), + InlineAsmArch::AArch64 => Self::AArch64(AArch64InlineAsmRegClass::parse(name)?), InlineAsmArch::RiscV32 | InlineAsmArch::RiscV64 => { - Self::RiscV(RiscVInlineAsmRegClass::parse(arch, name)?) + Self::RiscV(RiscVInlineAsmRegClass::parse(name)?) } - InlineAsmArch::Nvptx64 => Self::Nvptx(NvptxInlineAsmRegClass::parse(arch, name)?), + InlineAsmArch::Nvptx64 => Self::Nvptx(NvptxInlineAsmRegClass::parse(name)?), InlineAsmArch::PowerPC | InlineAsmArch::PowerPC64 => { - Self::PowerPC(PowerPCInlineAsmRegClass::parse(arch, name)?) + Self::PowerPC(PowerPCInlineAsmRegClass::parse(name)?) } - InlineAsmArch::Hexagon => Self::Hexagon(HexagonInlineAsmRegClass::parse(arch, name)?), + InlineAsmArch::Hexagon => Self::Hexagon(HexagonInlineAsmRegClass::parse(name)?), InlineAsmArch::Mips | InlineAsmArch::Mips64 => { - Self::Mips(MipsInlineAsmRegClass::parse(arch, name)?) + Self::Mips(MipsInlineAsmRegClass::parse(name)?) } - InlineAsmArch::S390x => Self::S390x(S390xInlineAsmRegClass::parse(arch, name)?), - InlineAsmArch::SpirV => Self::SpirV(SpirVInlineAsmRegClass::parse(arch, name)?), + InlineAsmArch::S390x => Self::S390x(S390xInlineAsmRegClass::parse(name)?), + InlineAsmArch::SpirV => Self::SpirV(SpirVInlineAsmRegClass::parse(name)?), InlineAsmArch::Wasm32 | InlineAsmArch::Wasm64 => { - Self::Wasm(WasmInlineAsmRegClass::parse(arch, name)?) + Self::Wasm(WasmInlineAsmRegClass::parse(name)?) } - InlineAsmArch::Bpf => Self::Bpf(BpfInlineAsmRegClass::parse(arch, name)?), - InlineAsmArch::Avr => Self::Avr(AvrInlineAsmRegClass::parse(arch, name)?), - InlineAsmArch::Msp430 => Self::Msp430(Msp430InlineAsmRegClass::parse(arch, name)?), + InlineAsmArch::Bpf => Self::Bpf(BpfInlineAsmRegClass::parse(name)?), + InlineAsmArch::Avr => Self::Avr(AvrInlineAsmRegClass::parse(name)?), + InlineAsmArch::Msp430 => Self::Msp430(Msp430InlineAsmRegClass::parse(name)?), }) } @@ -857,8 +835,6 @@ impl InlineAsmClobberAbi { /// clobber ABIs for the target. pub fn parse( arch: InlineAsmArch, - reloc_model: RelocModel, - target_features: &FxHashSet, target: &Target, name: Symbol, ) -> Result { @@ -882,13 +858,11 @@ impl InlineAsmClobberAbi { _ => Err(&["C", "system", "efiapi", "aapcs"]), }, InlineAsmArch::AArch64 => match name { - "C" | "system" | "efiapi" => { - Ok(if aarch64::reserved_x18(arch, reloc_model, target_features, target, true).is_err() { - InlineAsmClobberAbi::AArch64NoX18 - } else { - InlineAsmClobberAbi::AArch64 - }) - } + "C" | "system" | "efiapi" => Ok(if aarch64::target_reserves_x18(target) { + InlineAsmClobberAbi::AArch64NoX18 + } else { + InlineAsmClobberAbi::AArch64 + }), _ => Err(&["C", "system", "efiapi"]), }, InlineAsmArch::RiscV32 | InlineAsmArch::RiscV64 => match name { diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 19b3d35566b2..c8bcc1d41d39 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -87,6 +87,7 @@ pub fn provide(providers: &mut Providers) { static_mutability, generator_kind, codegen_fn_attrs, + asm_target_features, collect_mod_item_types, should_inherit_track_caller, ..*providers @@ -3255,6 +3256,24 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { codegen_fn_attrs } +/// Computes the set of target features used in a function for the purposes of +/// inline assembly. +fn asm_target_features<'tcx>(tcx: TyCtxt<'tcx>, id: DefId) -> &'tcx FxHashSet { + let mut target_features = tcx.sess.target_features.clone(); + let attrs = tcx.codegen_fn_attrs(id); + target_features.extend(&attrs.target_features); + match attrs.instruction_set { + None => {} + Some(InstructionSetAttr::ArmA32) => { + target_features.remove(&sym::thumb_mode); + } + Some(InstructionSetAttr::ArmT32) => { + target_features.insert(sym::thumb_mode); + } + } + tcx.arena.alloc(target_features) +} + /// Checks if the provided DefId is a method in a trait impl for a trait which has track_caller /// applied to the method prototype. fn should_inherit_track_caller(tcx: TyCtxt<'_>, def_id: DefId) -> bool { diff --git a/src/test/ui/asm/x86_64/bad-reg.rs b/src/test/ui/asm/x86_64/bad-reg.rs index 257274b0bc31..4c4ce8b5e9e4 100644 --- a/src/test/ui/asm/x86_64/bad-reg.rs +++ b/src/test/ui/asm/x86_64/bad-reg.rs @@ -31,8 +31,6 @@ fn main() { //~^ ERROR invalid register `ip`: the instruction pointer cannot be used as an operand asm!("", in("k0") foo); //~^ ERROR invalid register `k0`: the k0 AVX mask register cannot be used as an operand - asm!("", in("ah") foo); - //~^ ERROR invalid register `ah`: high byte registers cannot be used as an operand asm!("", in("st(2)") foo); //~^ ERROR register class `x87_reg` can only be used as a clobber, not as an input or output diff --git a/src/test/ui/asm/x86_64/bad-reg.stderr b/src/test/ui/asm/x86_64/bad-reg.stderr index 3a89b2fdb74d..f8b024e1acd6 100644 --- a/src/test/ui/asm/x86_64/bad-reg.stderr +++ b/src/test/ui/asm/x86_64/bad-reg.stderr @@ -70,50 +70,44 @@ error: invalid register `k0`: the k0 AVX mask register cannot be used as an oper LL | asm!("", in("k0") foo); | ^^^^^^^^^^^^ -error: invalid register `ah`: high byte registers cannot be used as an operand on x86_64 - --> $DIR/bad-reg.rs:34:18 - | -LL | asm!("", in("ah") foo); - | ^^^^^^^^^^^^ - error: register class `x87_reg` can only be used as a clobber, not as an input or output - --> $DIR/bad-reg.rs:37:18 + --> $DIR/bad-reg.rs:35:18 | LL | asm!("", in("st(2)") foo); | ^^^^^^^^^^^^^^^ error: register class `mmx_reg` can only be used as a clobber, not as an input or output - --> $DIR/bad-reg.rs:39:18 + --> $DIR/bad-reg.rs:37:18 | LL | asm!("", in("mm0") foo); | ^^^^^^^^^^^^^ error: register class `x87_reg` can only be used as a clobber, not as an input or output - --> $DIR/bad-reg.rs:43:20 + --> $DIR/bad-reg.rs:41:20 | LL | asm!("{}", in(x87_reg) foo); | ^^^^^^^^^^^^^^^ error: register class `mmx_reg` can only be used as a clobber, not as an input or output - --> $DIR/bad-reg.rs:45:20 + --> $DIR/bad-reg.rs:43:20 | LL | asm!("{}", in(mmx_reg) foo); | ^^^^^^^^^^^^^^^ error: register class `x87_reg` can only be used as a clobber, not as an input or output - --> $DIR/bad-reg.rs:47:20 + --> $DIR/bad-reg.rs:45:20 | LL | asm!("{}", out(x87_reg) _); | ^^^^^^^^^^^^^^ error: register class `mmx_reg` can only be used as a clobber, not as an input or output - --> $DIR/bad-reg.rs:49:20 + --> $DIR/bad-reg.rs:47:20 | LL | asm!("{}", out(mmx_reg) _); | ^^^^^^^^^^^^^^ error: register `al` conflicts with register `ax` - --> $DIR/bad-reg.rs:55:33 + --> $DIR/bad-reg.rs:53:33 | LL | asm!("", in("eax") foo, in("al") bar); | ------------- ^^^^^^^^^^^^ register `al` @@ -121,7 +115,7 @@ LL | asm!("", in("eax") foo, in("al") bar); | register `ax` error: register `ax` conflicts with register `ax` - --> $DIR/bad-reg.rs:57:33 + --> $DIR/bad-reg.rs:55:33 | LL | asm!("", in("rax") foo, out("rax") bar); | ------------- ^^^^^^^^^^^^^^ register `ax` @@ -129,13 +123,13 @@ LL | asm!("", in("rax") foo, out("rax") bar); | register `ax` | help: use `lateout` instead of `out` to avoid conflict - --> $DIR/bad-reg.rs:57:18 + --> $DIR/bad-reg.rs:55:18 | LL | asm!("", in("rax") foo, out("rax") bar); | ^^^^^^^^^^^^^ error: register `ymm0` conflicts with register `xmm0` - --> $DIR/bad-reg.rs:60:34 + --> $DIR/bad-reg.rs:58:34 | LL | asm!("", in("xmm0") foo, in("ymm0") bar); | -------------- ^^^^^^^^^^^^^^ register `ymm0` @@ -143,7 +137,7 @@ LL | asm!("", in("xmm0") foo, in("ymm0") bar); | register `xmm0` error: register `ymm0` conflicts with register `xmm0` - --> $DIR/bad-reg.rs:62:34 + --> $DIR/bad-reg.rs:60:34 | LL | asm!("", in("xmm0") foo, out("ymm0") bar); | -------------- ^^^^^^^^^^^^^^^ register `ymm0` @@ -151,10 +145,10 @@ LL | asm!("", in("xmm0") foo, out("ymm0") bar); | register `xmm0` | help: use `lateout` instead of `out` to avoid conflict - --> $DIR/bad-reg.rs:62:18 + --> $DIR/bad-reg.rs:60:18 | LL | asm!("", in("xmm0") foo, out("ymm0") bar); | ^^^^^^^^^^^^^^ -error: aborting due to 21 previous errors +error: aborting due to 20 previous errors From fb5539b475586c31e6e51f252dae1559343b2be7 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Sat, 19 Feb 2022 21:46:49 +0000 Subject: [PATCH 09/12] Add tests --- compiler/rustc_target/src/asm/riscv.rs | 2 +- compiler/rustc_target/src/asm/x86.rs | 2 +- src/test/ui/asm/issue-85247.rs | 26 +++++++++++++++++++++ src/test/ui/asm/issue-85247.rwpi.stderr | 8 +++++++ src/test/ui/asm/issue-92378.rs | 30 +++++++++++++++++++++++++ 5 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/asm/issue-85247.rs create mode 100644 src/test/ui/asm/issue-85247.rwpi.stderr create mode 100644 src/test/ui/asm/issue-92378.rs diff --git a/compiler/rustc_target/src/asm/riscv.rs b/compiler/rustc_target/src/asm/riscv.rs index 65ce69cb5c0c..987bf9705293 100644 --- a/compiler/rustc_target/src/asm/riscv.rs +++ b/compiler/rustc_target/src/asm/riscv.rs @@ -1,5 +1,5 @@ use super::{InlineAsmArch, InlineAsmType}; -use crate::spec::{Target, RelocModel}; +use crate::spec::{RelocModel, Target}; use rustc_data_structures::stable_set::FxHashSet; use rustc_macros::HashStable_Generic; use rustc_span::{sym, Symbol}; diff --git a/compiler/rustc_target/src/asm/x86.rs b/compiler/rustc_target/src/asm/x86.rs index ac6f39f1c955..7c136a475486 100644 --- a/compiler/rustc_target/src/asm/x86.rs +++ b/compiler/rustc_target/src/asm/x86.rs @@ -1,5 +1,5 @@ use super::{InlineAsmArch, InlineAsmType}; -use crate::spec::{Target, RelocModel}; +use crate::spec::{RelocModel, Target}; use rustc_data_structures::stable_set::FxHashSet; use rustc_macros::HashStable_Generic; use rustc_span::Symbol; diff --git a/src/test/ui/asm/issue-85247.rs b/src/test/ui/asm/issue-85247.rs new file mode 100644 index 000000000000..e64f5e8af523 --- /dev/null +++ b/src/test/ui/asm/issue-85247.rs @@ -0,0 +1,26 @@ +// revisions: ropi rwpi + +// [ropi] compile-flags: --target armv7-unknown-linux-gnueabihf -C relocation-model=ropi +// [rwpi] compile-flags: --target armv7-unknown-linux-gnueabihf -C relocation-model=rwpi +// [ropi] needs-llvm-components: arm +// [rwpi] needs-llvm-components: arm +// [ropi] build-pass + +#![feature(no_core, lang_items, rustc_attrs)] +#![no_core] +#![crate_type = "rlib"] + +#[rustc_builtin_macro] +macro_rules! asm { + () => {}; +} +#[lang = "sized"] +trait Sized {} + +// R9 is reserved as the RWPI base register +fn main() { + unsafe { + asm!("", out("r9") _); + //[rwpi]~^ cannot use register `r9` + } +} diff --git a/src/test/ui/asm/issue-85247.rwpi.stderr b/src/test/ui/asm/issue-85247.rwpi.stderr new file mode 100644 index 000000000000..996b0933a341 --- /dev/null +++ b/src/test/ui/asm/issue-85247.rwpi.stderr @@ -0,0 +1,8 @@ +error: cannot use register `r9`: the RWPI static base register (r9) cannot be used as an operand for inline asm + --> $DIR/issue-85247.rs:23:18 + | +LL | asm!("", out("r9") _); + | ^^^^^^^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/asm/issue-92378.rs b/src/test/ui/asm/issue-92378.rs new file mode 100644 index 000000000000..d595e88ff80c --- /dev/null +++ b/src/test/ui/asm/issue-92378.rs @@ -0,0 +1,30 @@ +// compile-flags: --target armv5te-unknown-linux-gnueabi +// needs-llvm-components: arm +// build-pass + +#![feature(no_core, lang_items, rustc_attrs, isa_attribute)] +#![no_core] +#![crate_type = "rlib"] + +#[rustc_builtin_macro] +macro_rules! asm { + () => {}; +} +#[lang = "sized"] +trait Sized {} + +// ARM uses R11 for the frame pointer, make sure R7 is usable. +#[instruction_set(arm::a32)] +pub fn arm() { + unsafe { + asm!("", out("r7") _); + } +} + +// Thumb uses R7 for the frame pointer, make sure R11 is usable. +#[instruction_set(arm::t32)] +pub fn thumb() { + unsafe { + asm!("", out("r11") _); + } +} From a60b79137ca7829797111a63d8f480316af71aa8 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Mon, 21 Feb 2022 19:18:00 +0000 Subject: [PATCH 10/12] Add ignore-tidy-filelength --- compiler/rustc_typeck/src/collect.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index c8bcc1d41d39..392144ca7639 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -1,3 +1,4 @@ +// 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 From 0626919f21a82b9bbe8d20e3e5e98b9cfd76015e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 21 Feb 2022 16:25:59 -0800 Subject: [PATCH 11/12] Do not suggest wrapping an item if it has ambiguous un-imported methods --- .../rustc_typeck/src/check/method/suggest.rs | 37 ++++++++++++------- .../future-prelude-collision-shadow.stderr | 26 ------------- .../dont-wrap-ambiguous-receivers.rs | 21 +++++++++++ .../dont-wrap-ambiguous-receivers.stderr | 20 ++++++++++ 4 files changed, 65 insertions(+), 39 deletions(-) create mode 100644 src/test/ui/suggestions/dont-wrap-ambiguous-receivers.rs create mode 100644 src/test/ui/suggestions/dont-wrap-ambiguous-receivers.stderr diff --git a/compiler/rustc_typeck/src/check/method/suggest.rs b/compiler/rustc_typeck/src/check/method/suggest.rs index cbbb47ecaae1..3213148f6a3d 100644 --- a/compiler/rustc_typeck/src/check/method/suggest.rs +++ b/compiler/rustc_typeck/src/check/method/suggest.rs @@ -1485,27 +1485,38 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { (self.tcx.mk_mut_ref(self.tcx.lifetimes.re_erased, rcvr_ty), "&mut "), (self.tcx.mk_imm_ref(self.tcx.lifetimes.re_erased, rcvr_ty), "&"), ] { - if let Ok(pick) = self.lookup_probe( + match self.lookup_probe( span, item_name, *rcvr_ty, rcvr, crate::check::method::probe::ProbeScope::AllTraits, ) { - // If the method is defined for the receiver we have, it likely wasn't `use`d. - // We point at the method, but we just skip the rest of the check for arbitrary - // self types and rely on the suggestion to `use` the trait from - // `suggest_valid_traits`. - let did = Some(pick.item.container.id()); - let skip = skippable.contains(&did); - if pick.autoderefs == 0 && !skip { - err.span_label( - pick.item.ident(self.tcx).span, - &format!("the method is available for `{}` here", rcvr_ty), - ); + Ok(pick) => { + // If the method is defined for the receiver we have, it likely wasn't `use`d. + // We point at the method, but we just skip the rest of the check for arbitrary + // self types and rely on the suggestion to `use` the trait from + // `suggest_valid_traits`. + let did = Some(pick.item.container.id()); + let skip = skippable.contains(&did); + if pick.autoderefs == 0 && !skip { + err.span_label( + pick.item.ident(self.tcx).span, + &format!("the method is available for `{}` here", rcvr_ty), + ); + } + break; } - break; + Err(MethodError::Ambiguity(_)) => { + // If the method is defined (but ambiguous) for the receiver we have, it is also + // likely we haven't `use`d it. It may be possible that if we `Box`/`Pin`/etc. + // the receiver, then it might disambiguate this method, but I think these + // suggestions are generally misleading (see #94218). + break; + } + _ => {} } + for (rcvr_ty, pre) in &[ (self.tcx.mk_lang_item(*rcvr_ty, LangItem::OwnedBox), "Box::new"), (self.tcx.mk_lang_item(*rcvr_ty, LangItem::Pin), "Pin::new"), diff --git a/src/test/ui/rust-2021/future-prelude-collision-shadow.stderr b/src/test/ui/rust-2021/future-prelude-collision-shadow.stderr index d945b4c94ca2..3d21b735aea0 100644 --- a/src/test/ui/rust-2021/future-prelude-collision-shadow.stderr +++ b/src/test/ui/rust-2021/future-prelude-collision-shadow.stderr @@ -4,34 +4,8 @@ error[E0599]: no method named `try_into` found for type `u8` in the current scop LL | let _: u32 = 3u8.try_into().unwrap(); | ^^^^^^^^ method not found in `u8` | - ::: $SRC_DIR/core/src/convert/mod.rs:LL:COL - | -LL | fn try_into(self) -> Result; - | -------- - | | - | the method is available for `Box` here - | the method is available for `Pin` here - | the method is available for `Arc` here - | the method is available for `Rc` here - | = help: items from traits can only be used if the trait is in scope = note: 'std::convert::TryInto' is included in the prelude starting in Edition 2021 -help: consider wrapping the receiver expression with the appropriate type - | -LL | let _: u32 = Box::new(3u8).try_into().unwrap(); - | +++++++++ + -help: consider wrapping the receiver expression with the appropriate type - | -LL | let _: u32 = Pin::new(3u8).try_into().unwrap(); - | +++++++++ + -help: consider wrapping the receiver expression with the appropriate type - | -LL | let _: u32 = Arc::new(3u8).try_into().unwrap(); - | +++++++++ + -help: consider wrapping the receiver expression with the appropriate type - | -LL | let _: u32 = Rc::new(3u8).try_into().unwrap(); - | ++++++++ + help: the following traits are implemented but not in scope; perhaps add a `use` for one of them: | LL | use crate::m::TryIntoU32; diff --git a/src/test/ui/suggestions/dont-wrap-ambiguous-receivers.rs b/src/test/ui/suggestions/dont-wrap-ambiguous-receivers.rs new file mode 100644 index 000000000000..baa2128eb8e3 --- /dev/null +++ b/src/test/ui/suggestions/dont-wrap-ambiguous-receivers.rs @@ -0,0 +1,21 @@ +mod banana { + //~^ HELP the following traits are implemented but not in scope + pub struct Chaenomeles; + + pub trait Apple { + fn pick(&self) {} + } + impl Apple for Chaenomeles {} + + pub trait Peach { + fn pick(&self, a: &mut ()) {} + } + impl Peach for Box {} + impl Peach for Chaenomeles {} +} + +fn main() { + banana::Chaenomeles.pick() + //~^ ERROR no method named + //~| HELP items from traits can only be used if the trait is in scope +} diff --git a/src/test/ui/suggestions/dont-wrap-ambiguous-receivers.stderr b/src/test/ui/suggestions/dont-wrap-ambiguous-receivers.stderr new file mode 100644 index 000000000000..8fcadbf4c75f --- /dev/null +++ b/src/test/ui/suggestions/dont-wrap-ambiguous-receivers.stderr @@ -0,0 +1,20 @@ +error[E0599]: no method named `pick` found for struct `Chaenomeles` in the current scope + --> $DIR/dont-wrap-ambiguous-receivers.rs:18:25 + | +LL | pub struct Chaenomeles; + | ----------------------- method `pick` not found for this +... +LL | banana::Chaenomeles.pick() + | ^^^^ method not found in `Chaenomeles` + | + = help: items from traits can only be used if the trait is in scope +help: the following traits are implemented but not in scope; perhaps add a `use` for one of them: + | +LL | use banana::Apple; + | +LL | use banana::Peach; + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. From fb1ee8764f4d6ba78c1ad1803a34a0a2d2b0bf93 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 21 Feb 2022 21:46:51 -0500 Subject: [PATCH 12/12] ScalarMaybeUninit is explicitly hexadecimal in its formatting --- compiler/rustc_const_eval/src/interpret/operand.rs | 4 ++-- compiler/rustc_const_eval/src/interpret/validity.rs | 10 +++++----- compiler/rustc_middle/src/mir/interpret/value.rs | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/operand.rs b/compiler/rustc_const_eval/src/interpret/operand.rs index 60e915a7eee1..bc4dca4c146f 100644 --- a/compiler/rustc_const_eval/src/interpret/operand.rs +++ b/compiler/rustc_const_eval/src/interpret/operand.rs @@ -142,11 +142,11 @@ impl std::fmt::Display for ImmTy<'_, Tag> { p(cx, s, ty)?; return Ok(()); } - write!(f, "{}: {}", s, self.layout.ty) + write!(f, "{:x}: {}", s, self.layout.ty) } Immediate::ScalarPair(a, b) => { // FIXME(oli-obk): at least print tuples and slices nicely - write!(f, "({}, {}): {}", a, b, self.layout.ty,) + write!(f, "({:x}, {:x}): {}", a, b, self.layout.ty,) } } }) diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index 19c6449078d5..54e29299e6c9 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -503,7 +503,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' value.to_bool(), self.path, err_ub!(InvalidBool(..)) | err_ub!(InvalidUninitBytes(None)) => - { "{}", value } expected { "a boolean" }, + { "{:x}", value } expected { "a boolean" }, ); Ok(true) } @@ -513,7 +513,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' value.to_char(), self.path, err_ub!(InvalidChar(..)) | err_ub!(InvalidUninitBytes(None)) => - { "{}", value } expected { "a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)" }, + { "{:x}", value } expected { "a valid unicode scalar value (in `0..=0x10FFFF` but not in `0xD800..=0xDFFF`)" }, ); Ok(true) } @@ -526,7 +526,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' let is_bits = value.check_init().map_or(false, |v| v.try_to_int().is_ok()); if !is_bits { throw_validation_failure!(self.path, - { "{}", value } expected { "initialized plain (non-pointer) bytes" } + { "{:x}", value } expected { "initialized plain (non-pointer) bytes" } ) } } @@ -580,7 +580,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' err_ub!(DanglingIntPointer(..)) | err_ub!(InvalidFunctionPointer(..)) | err_ub!(InvalidUninitBytes(None)) => - { "{}", value } expected { "a function pointer" }, + { "{:x}", value } expected { "a function pointer" }, ); // FIXME: Check if the signature matches Ok(true) @@ -632,7 +632,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, ' let value = try_validation!( value.check_init(), self.path, - err_ub!(InvalidUninitBytes(None)) => { "{}", value } + err_ub!(InvalidUninitBytes(None)) => { "{:x}", value } expected { "something {}", wrapping_range_format(valid_range, max_value) }, ); let bits = match value.try_to_int() { diff --git a/compiler/rustc_middle/src/mir/interpret/value.rs b/compiler/rustc_middle/src/mir/interpret/value.rs index acf7847de541..c8ddc4edc8ad 100644 --- a/compiler/rustc_middle/src/mir/interpret/value.rs +++ b/compiler/rustc_middle/src/mir/interpret/value.rs @@ -498,7 +498,7 @@ impl fmt::Debug for ScalarMaybeUninit { } } -impl fmt::Display for ScalarMaybeUninit { +impl fmt::LowerHex for ScalarMaybeUninit { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { ScalarMaybeUninit::Uninit => write!(f, "uninitialized bytes"),