From d8e9db0dcf0fc5cd6585a86145bfed5ea3c55031 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 16 Sep 2021 13:05:14 -0700 Subject: [PATCH 1/9] feat(rustc_parse): recover from pre-RFC-2000 const generics syntax Fixes #89013 --- compiler/rustc_parse/src/parser/path.rs | 20 ++++++++- .../issue-89013-no-assoc.rs | 16 ++++++++ .../issue-89013-no-assoc.stderr | 8 ++++ .../issue-89013-no-kw.rs | 18 ++++++++ .../issue-89013-no-kw.stderr | 35 ++++++++++++++++ .../parser-error-recovery/issue-89013-type.rs | 17 ++++++++ .../issue-89013-type.stderr | 24 +++++++++++ .../parser-error-recovery/issue-89013.rs | 19 +++++++++ .../parser-error-recovery/issue-89013.stderr | 41 +++++++++++++++++++ 9 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.rs create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013.rs create mode 100644 src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr diff --git a/compiler/rustc_parse/src/parser/path.rs b/compiler/rustc_parse/src/parser/path.rs index 953c6915068af..4e4130dfc2306 100644 --- a/compiler/rustc_parse/src/parser/path.rs +++ b/compiler/rustc_parse/src/parser/path.rs @@ -495,11 +495,16 @@ impl<'a> Parser<'a> { None => { let after_eq = eq.shrink_to_hi(); let before_next = self.token.span.shrink_to_lo(); + let the_type_placeholder = if matches!(self.token.kind, token::Comma | token::Gt) { + " TheType" + } else { + " TheType " + }; self.struct_span_err(after_eq.to(before_next), "missing type to the right of `=`") .span_suggestion( self.sess.source_map().next_point(eq).to(before_next), "to constrain the associated type, add a type after `=`", - " TheType".to_string(), + the_type_placeholder.to_string(), Applicability::HasPlaceholders, ) .span_suggestion( @@ -572,6 +577,19 @@ impl<'a> Parser<'a> { return self.recover_const_arg(start, err).map(Some); } } + } else if self.eat_keyword_noexpect(kw::Const) { + // Detect and recover from the old, pre-RFC2000 syntax for const generics. + let mut err = self.struct_span_err( + start, + "expected lifetime, type, or constant, found keyword `const`", + ); + if self.check_const_arg() { + err.emit(); + GenericArg::Const(self.parse_const_arg()?) + } else { + let after_kw_const = self.token.span; + return self.recover_const_arg(after_kw_const, err).map(Some); + } } else { return Ok(None); }; diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.rs new file mode 100644 index 0000000000000..99d8e9dea910d --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.rs @@ -0,0 +1,16 @@ +trait Foo { + fn do_x(&self) -> [u8; N]; +} + +struct Bar; + +const T: usize = 42; + +impl Foo for Bar { +//~^ERROR expected lifetime, type, or constant, found keyword `const` + fn do_x(&self) -> [u8; 3] { + [0u8; 3] + } +} + +fn main() {} diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr new file mode 100644 index 0000000000000..533d381fa3136 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr @@ -0,0 +1,8 @@ +error: expected lifetime, type, or constant, found keyword `const` + --> $DIR/issue-89013-no-assoc.rs:9:10 + | +LL | impl Foo for Bar { + | ^^^^^ + +error: aborting due to previous error + diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs new file mode 100644 index 0000000000000..b73b3e2910453 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs @@ -0,0 +1,18 @@ +trait Foo { + fn do_x(&self) -> [u8; N]; +} + +struct Bar; + +const T: usize = 42; + +impl Foo for Bar { +//~^ERROR cannot constrain an associated constant to a value +//~^^ERROR this trait takes 1 generic argument but 0 generic arguments +//~^^^ERROR associated type bindings are not allowed here + fn do_x(&self) -> [u8; 3] { + [0u8; 3] + } +} + +fn main() {} diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr new file mode 100644 index 0000000000000..7682815557671 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr @@ -0,0 +1,35 @@ +error: cannot constrain an associated constant to a value + --> $DIR/issue-89013-no-kw.rs:9:10 + | +LL | impl Foo for Bar { + | -^^^- + | | | + | | ...cannot be constrained to this value + | this associated constant... + +error[E0107]: this trait takes 1 generic argument but 0 generic arguments were supplied + --> $DIR/issue-89013-no-kw.rs:9:6 + | +LL | impl Foo for Bar { + | ^^^ expected 1 generic argument + | +note: trait defined here, with 1 generic parameter: `N` + --> $DIR/issue-89013-no-kw.rs:1:7 + | +LL | trait Foo { + | ^^^ - +help: add missing generic argument + | +LL | impl Foo for Bar { + | ++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/issue-89013-no-kw.rs:9:10 + | +LL | impl Foo for Bar { + | ^^^^^ associated type not allowed here + +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0107, E0229. +For more information about an error, try `rustc --explain E0107`. diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs new file mode 100644 index 0000000000000..c34936d197620 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs @@ -0,0 +1,17 @@ +trait Foo { + fn do_x(&self) -> [u8; N]; +} + +struct Bar; + +const T: usize = 42; + +impl Foo for Bar { +//~^ERROR missing type to the right of `=` +//~^^ERROR found keyword `type` + fn do_x(&self) -> [u8; 3] { + [0u8; 3] + } +} + +fn main() {} diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr new file mode 100644 index 0000000000000..0f33f4b0df60c --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr @@ -0,0 +1,24 @@ +error: missing type to the right of `=` + --> $DIR/issue-89013-type.rs:9:13 + | +LL | impl Foo for Bar { + | ^ + | +help: to constrain the associated type, add a type after `=` + | +LL | impl Foo for Bar { + | +++++++ +help: remove the `=` if `N` is a type + | +LL - impl Foo for Bar { +LL + impl Foo for Bar { + | + +error: expected one of `,`, `>`, a const expression, lifetime, or type, found keyword `type` + --> $DIR/issue-89013-type.rs:9:14 + | +LL | impl Foo for Bar { + | ^^^^ expected one of `,`, `>`, a const expression, lifetime, or type + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs new file mode 100644 index 0000000000000..d5ded44188a02 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs @@ -0,0 +1,19 @@ +trait Foo { + fn do_x(&self) -> [u8; N]; +} + +struct Bar; + +const T: usize = 42; + +impl Foo for Bar { +//~^ERROR expected lifetime, type, or constant, found keyword `const` +//~^^ERROR cannot constrain an associated constant to a value +//~^^^ERROR this trait takes 1 generic argument but 0 generic arguments +//~^^^^ERROR associated type bindings are not allowed here + fn do_x(&self) -> [u8; 3] { + [0u8; 3] + } +} + +fn main() {} diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr new file mode 100644 index 0000000000000..3df459ce16285 --- /dev/null +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr @@ -0,0 +1,41 @@ +error: expected lifetime, type, or constant, found keyword `const` + --> $DIR/issue-89013.rs:9:14 + | +LL | impl Foo for Bar { + | ^^^^^ + +error: cannot constrain an associated constant to a value + --> $DIR/issue-89013.rs:9:10 + | +LL | impl Foo for Bar { + | -^^^^^^^^^- + | | | + | | ...cannot be constrained to this value + | this associated constant... + +error[E0107]: this trait takes 1 generic argument but 0 generic arguments were supplied + --> $DIR/issue-89013.rs:9:6 + | +LL | impl Foo for Bar { + | ^^^ expected 1 generic argument + | +note: trait defined here, with 1 generic parameter: `N` + --> $DIR/issue-89013.rs:1:7 + | +LL | trait Foo { + | ^^^ - +help: add missing generic argument + | +LL | impl Foo for Bar { + | ++ + +error[E0229]: associated type bindings are not allowed here + --> $DIR/issue-89013.rs:9:10 + | +LL | impl Foo for Bar { + | ^^^^^^^^^^^ associated type not allowed here + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0107, E0229. +For more information about an error, try `rustc --explain E0107`. From 42e9dfd75d293e5051c0204eacd773e1453e3365 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Mon, 30 Aug 2021 14:36:24 +0200 Subject: [PATCH 2/9] Reapply "Remove optimization_fuel_crate from Session" --- compiler/rustc_session/src/session.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 27215556045be..b8b9c03091f0e 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -169,9 +169,6 @@ pub struct Session { /// Data about code being compiled, gathered during compilation. pub code_stats: CodeStats, - /// If `-zfuel=crate=n` is specified, `Some(crate)`. - optimization_fuel_crate: Option, - /// Tracks fuel info if `-zfuel=crate=n` is specified. optimization_fuel: Lock, @@ -896,7 +893,7 @@ impl Session { /// This expends fuel if applicable, and records fuel if applicable. pub fn consider_optimizing String>(&self, crate_name: &str, msg: T) -> bool { let mut ret = true; - if let Some(ref c) = self.optimization_fuel_crate { + if let Some((ref c, _)) = self.opts.debugging_opts.fuel { if c == crate_name { assert_eq!(self.threads(), 1); let mut fuel = self.optimization_fuel.lock(); @@ -1274,7 +1271,6 @@ pub fn build_session( let local_crate_source_file = local_crate_source_file.map(|path| file_path_mapping.map_prefix(path).0); - let optimization_fuel_crate = sopts.debugging_opts.fuel.as_ref().map(|i| i.0.clone()); let optimization_fuel = Lock::new(OptimizationFuel { remaining: sopts.debugging_opts.fuel.as_ref().map_or(0, |i| i.1), out_of_fuel: false, @@ -1326,7 +1322,6 @@ pub fn build_session( normalize_projection_ty: AtomicUsize::new(0), }, code_stats: Default::default(), - optimization_fuel_crate, optimization_fuel, print_fuel, jobserver: jobserver::client(), From 8a7c1306b43895a97c0675c6d854afa468a22aad Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 28 Sep 2021 10:57:34 -0700 Subject: [PATCH 3/9] Use less verbose syntax for error annotations Co-authored-by: Esteban Kuber --- .../const-generics/parser-error-recovery/issue-89013.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs index d5ded44188a02..99f3549d9ac34 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs @@ -7,10 +7,10 @@ struct Bar; const T: usize = 42; impl Foo for Bar { -//~^ERROR expected lifetime, type, or constant, found keyword `const` -//~^^ERROR cannot constrain an associated constant to a value -//~^^^ERROR this trait takes 1 generic argument but 0 generic arguments -//~^^^^ERROR associated type bindings are not allowed here +//~^ ERROR expected lifetime, type, or constant, found keyword `const` +//~| ERROR cannot constrain an associated constant to a value +//~| ERROR this trait takes 1 generic argument but 0 generic arguments +//~| ERROR associated type bindings are not allowed here fn do_x(&self) -> [u8; 3] { [0u8; 3] } From befdfb5c7113b3d82d973b1acfdcc2b0d9c14ab1 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 28 Sep 2021 12:48:58 -0700 Subject: [PATCH 4/9] Improve error messages for bad type constraints Co-authored-by: Esteban Kuber --- compiler/rustc_parse/src/parser/path.rs | 31 ++++++++++++------- .../issue-89013-no-assoc.stderr | 6 ++++ .../parser-error-recovery/issue-89013-type.rs | 1 - .../issue-89013-type.stderr | 20 ++---------- .../parser-error-recovery/issue-89013.stderr | 6 ++++ 5 files changed, 34 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_parse/src/parser/path.rs b/compiler/rustc_parse/src/parser/path.rs index 4e4130dfc2306..c7d080a80fe6f 100644 --- a/compiler/rustc_parse/src/parser/path.rs +++ b/compiler/rustc_parse/src/parser/path.rs @@ -495,25 +495,28 @@ impl<'a> Parser<'a> { None => { let after_eq = eq.shrink_to_hi(); let before_next = self.token.span.shrink_to_lo(); - let the_type_placeholder = if matches!(self.token.kind, token::Comma | token::Gt) { - " TheType" - } else { - " TheType " - }; - self.struct_span_err(after_eq.to(before_next), "missing type to the right of `=`") - .span_suggestion( + let mut err = self + .struct_span_err(after_eq.to(before_next), "missing type to the right of `=`"); + if matches!(self.token.kind, token::Comma | token::Gt) { + err.span_suggestion( self.sess.source_map().next_point(eq).to(before_next), "to constrain the associated type, add a type after `=`", - the_type_placeholder.to_string(), + " TheType".to_string(), Applicability::HasPlaceholders, - ) - .span_suggestion( + ); + err.span_suggestion( eq.to(before_next), &format!("remove the `=` if `{}` is a type", ident), String::new(), Applicability::MaybeIncorrect, ) - .emit(); + } else { + err.span_label( + self.token.span, + &format!("expected type, found {}", super::token_descr(&self.token)), + ) + }; + return Err(err); } } Ok(self.mk_ty(span, ast::TyKind::Err)) @@ -584,6 +587,12 @@ impl<'a> Parser<'a> { "expected lifetime, type, or constant, found keyword `const`", ); if self.check_const_arg() { + err.span_suggestion_verbose( + start.until(self.token.span), + "the `const` keyword is only needed in the definition of the type", + String::new(), + Applicability::MaybeIncorrect, + ); err.emit(); GenericArg::Const(self.parse_const_arg()?) } else { diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr index 533d381fa3136..ddddd86ab9c06 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-assoc.stderr @@ -3,6 +3,12 @@ error: expected lifetime, type, or constant, found keyword `const` | LL | impl Foo for Bar { | ^^^^^ + | +help: the `const` keyword is only needed in the definition of the type + | +LL - impl Foo for Bar { +LL + impl Foo<3> for Bar { + | error: aborting due to previous error diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs index c34936d197620..0ec6762b6e282 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.rs @@ -8,7 +8,6 @@ const T: usize = 42; impl Foo for Bar { //~^ERROR missing type to the right of `=` -//~^^ERROR found keyword `type` fn do_x(&self) -> [u8; 3] { [0u8; 3] } diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr index 0f33f4b0df60c..f0d0d90c774f8 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-type.stderr @@ -2,23 +2,7 @@ error: missing type to the right of `=` --> $DIR/issue-89013-type.rs:9:13 | LL | impl Foo for Bar { - | ^ - | -help: to constrain the associated type, add a type after `=` - | -LL | impl Foo for Bar { - | +++++++ -help: remove the `=` if `N` is a type - | -LL - impl Foo for Bar { -LL + impl Foo for Bar { - | - -error: expected one of `,`, `>`, a const expression, lifetime, or type, found keyword `type` - --> $DIR/issue-89013-type.rs:9:14 - | -LL | impl Foo for Bar { - | ^^^^ expected one of `,`, `>`, a const expression, lifetime, or type + | ^---- expected type, found keyword `type` -error: aborting due to 2 previous errors +error: aborting due to previous error diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr index 3df459ce16285..a60e17aef589a 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr @@ -3,6 +3,12 @@ error: expected lifetime, type, or constant, found keyword `const` | LL | impl Foo for Bar { | ^^^^^ + | +help: the `const` keyword is only needed in the definition of the type + | +LL - impl Foo for Bar { +LL + impl Foo for Bar { + | error: cannot constrain an associated constant to a value --> $DIR/issue-89013.rs:9:10 From 105b60f2501fac061284ea7f5fbb8fb3f8134a92 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Tue, 28 Sep 2021 15:56:45 -0700 Subject: [PATCH 5/9] feat(rustc_typeck): avoid erroring with "wrong number of generics" if there's other problems As shown in the two test requirements that got updated, if there's other problems, then those other problems are probably the root cause of the incorrect generics count. --- compiler/rustc_hir/src/hir.rs | 10 +++++++++ compiler/rustc_typeck/src/astconv/generics.rs | 2 +- .../issue-89013-no-kw.rs | 5 ++--- .../issue-89013-no-kw.stderr | 21 ++----------------- .../parser-error-recovery/issue-89013.rs | 1 - .../parser-error-recovery/issue-89013.stderr | 21 ++----------------- 6 files changed, 17 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 5655c4c0e973e..d952d5a544d7b 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -384,6 +384,16 @@ impl GenericArgs<'_> { self.args.iter().any(|arg| matches!(arg, GenericArg::Type(_))) } + pub fn has_err(&self) -> bool { + self.args.iter().any(|arg| match arg { + GenericArg::Type(ty) => matches!(ty.kind, TyKind::Err), + _ => false, + }) || self.bindings.iter().any(|arg| match arg.kind { + TypeBindingKind::Equality { ty } => matches!(ty.kind, TyKind::Err), + _ => false, + }) + } + #[inline] pub fn num_type_params(&self) -> usize { self.args.iter().filter(|arg| matches!(arg, GenericArg::Type(_))).count() diff --git a/compiler/rustc_typeck/src/astconv/generics.rs b/compiler/rustc_typeck/src/astconv/generics.rs index fd0544a47bb7c..92fc18854ee77 100644 --- a/compiler/rustc_typeck/src/astconv/generics.rs +++ b/compiler/rustc_typeck/src/astconv/generics.rs @@ -601,7 +601,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { def_id, ) .diagnostic() - .emit(); + .emit_unless(gen_args.has_err()); false }; diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs index b73b3e2910453..19e0f38d320c4 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.rs @@ -7,9 +7,8 @@ struct Bar; const T: usize = 42; impl Foo for Bar { -//~^ERROR cannot constrain an associated constant to a value -//~^^ERROR this trait takes 1 generic argument but 0 generic arguments -//~^^^ERROR associated type bindings are not allowed here +//~^ ERROR cannot constrain an associated constant to a value +//~| ERROR associated type bindings are not allowed here fn do_x(&self) -> [u8; 3] { [0u8; 3] } diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr index 7682815557671..bbca92ad63a91 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013-no-kw.stderr @@ -7,29 +7,12 @@ LL | impl Foo for Bar { | | ...cannot be constrained to this value | this associated constant... -error[E0107]: this trait takes 1 generic argument but 0 generic arguments were supplied - --> $DIR/issue-89013-no-kw.rs:9:6 - | -LL | impl Foo for Bar { - | ^^^ expected 1 generic argument - | -note: trait defined here, with 1 generic parameter: `N` - --> $DIR/issue-89013-no-kw.rs:1:7 - | -LL | trait Foo { - | ^^^ - -help: add missing generic argument - | -LL | impl Foo for Bar { - | ++ - error[E0229]: associated type bindings are not allowed here --> $DIR/issue-89013-no-kw.rs:9:10 | LL | impl Foo for Bar { | ^^^^^ associated type not allowed here -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors -Some errors have detailed explanations: E0107, E0229. -For more information about an error, try `rustc --explain E0107`. +For more information about this error, try `rustc --explain E0229`. diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs index 99f3549d9ac34..ca1158a2f6d16 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.rs @@ -9,7 +9,6 @@ const T: usize = 42; impl Foo for Bar { //~^ ERROR expected lifetime, type, or constant, found keyword `const` //~| ERROR cannot constrain an associated constant to a value -//~| ERROR this trait takes 1 generic argument but 0 generic arguments //~| ERROR associated type bindings are not allowed here fn do_x(&self) -> [u8; 3] { [0u8; 3] diff --git a/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr index a60e17aef589a..85379d3f06e4b 100644 --- a/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr +++ b/src/test/ui/const-generics/parser-error-recovery/issue-89013.stderr @@ -19,29 +19,12 @@ LL | impl Foo for Bar { | | ...cannot be constrained to this value | this associated constant... -error[E0107]: this trait takes 1 generic argument but 0 generic arguments were supplied - --> $DIR/issue-89013.rs:9:6 - | -LL | impl Foo for Bar { - | ^^^ expected 1 generic argument - | -note: trait defined here, with 1 generic parameter: `N` - --> $DIR/issue-89013.rs:1:7 - | -LL | trait Foo { - | ^^^ - -help: add missing generic argument - | -LL | impl Foo for Bar { - | ++ - error[E0229]: associated type bindings are not allowed here --> $DIR/issue-89013.rs:9:10 | LL | impl Foo for Bar { | ^^^^^^^^^^^ associated type not allowed here -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors -Some errors have detailed explanations: E0107, E0229. -For more information about an error, try `rustc --explain E0107`. +For more information about this error, try `rustc --explain E0229`. From 6490ed30e1025fbb85b3a2dd4849a249c072fb30 Mon Sep 17 00:00:00 2001 From: Fabian Wolff Date: Wed, 29 Sep 2021 00:11:55 +0200 Subject: [PATCH 6/9] Improve error message for `printf`-style format strings --- compiler/rustc_builtin_macros/src/format.rs | 26 ++++++-- .../src/format_foreign.rs | 65 ++++++++++++++----- .../src/format_foreign/printf/tests.rs | 4 +- .../src/format_foreign/shell/tests.rs | 4 +- src/test/ui/fmt/issue-89173.rs | 14 ++++ src/test/ui/fmt/issue-89173.stderr | 18 +++++ 6 files changed, 105 insertions(+), 26 deletions(-) create mode 100644 src/test/ui/fmt/issue-89173.rs create mode 100644 src/test/ui/fmt/issue-89173.stderr diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 1c9c6834c100c..f0056cb79766a 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -1154,11 +1154,12 @@ pub fn expand_preparsed_format_args( // account for `"` and account for raw strings `r#` let padding = str_style.map(|i| i + 2).unwrap_or(1); for sub in foreign::$kind::iter_subs(fmt_str, padding) { - let trn = match sub.translate() { - Some(trn) => trn, + let (trn, success) = match sub.translate() { + Ok(trn) => (trn, true), + Err(Some(msg)) => (msg, false), // If it has no translation, don't call it out specifically. - None => continue, + _ => continue, }; let pos = sub.position(); @@ -1175,9 +1176,24 @@ pub fn expand_preparsed_format_args( if let Some(inner_sp) = pos { let sp = fmt_sp.from_inner(inner_sp); - suggestions.push((sp, trn)); + + if success { + suggestions.push((sp, trn)); + } else { + diag.span_note( + sp, + &format!("format specifiers use curly braces, and {}", trn), + ); + } } else { - diag.help(&format!("`{}` should be written as `{}`", sub, trn)); + if success { + diag.help(&format!("`{}` should be written as `{}`", sub, trn)); + } else { + diag.note(&format!( + "`{}` should use curly braces, and {}", + sub, trn + )); + } } } diff --git a/compiler/rustc_builtin_macros/src/format_foreign.rs b/compiler/rustc_builtin_macros/src/format_foreign.rs index 0cc520e5bd1f0..bfddd7073ff2c 100644 --- a/compiler/rustc_builtin_macros/src/format_foreign.rs +++ b/compiler/rustc_builtin_macros/src/format_foreign.rs @@ -1,4 +1,4 @@ -pub mod printf { +pub(crate) mod printf { use super::strcursor::StrCursor as Cur; use rustc_span::InnerSpan; @@ -36,10 +36,10 @@ pub mod printf { /// /// This ignores cases where the substitution does not have an exact equivalent, or where /// the substitution would be unnecessary. - pub fn translate(&self) -> Option { + pub fn translate(&self) -> Result> { match *self { Substitution::Format(ref fmt) => fmt.translate(), - Substitution::Escape => None, + Substitution::Escape => Err(None), } } } @@ -68,9 +68,9 @@ pub mod printf { impl Format<'_> { /// Translate this directive into an equivalent Rust formatting directive. /// - /// Returns `None` in cases where the `printf` directive does not have an exact Rust + /// Returns `Err` in cases where the `printf` directive does not have an exact Rust /// equivalent, rather than guessing. - pub fn translate(&self) -> Option { + pub fn translate(&self) -> Result> { use std::fmt::Write; let (c_alt, c_zero, c_left, c_plus) = { @@ -84,7 +84,12 @@ pub mod printf { '0' => c_zero = true, '-' => c_left = true, '+' => c_plus = true, - _ => return None, + _ => { + return Err(Some(format!( + "the flag `{}` is unknown or unsupported", + c + ))); + } } } (c_alt, c_zero, c_left, c_plus) @@ -104,7 +109,9 @@ pub mod printf { let width = match self.width { Some(Num::Next) => { // NOTE: Rust doesn't support this. - return None; + return Err(Some( + "you have to use a positional or named parameter for the width".to_string(), + )); } w @ Some(Num::Arg(_)) => w, w @ Some(Num::Num(_)) => w, @@ -125,13 +132,21 @@ pub mod printf { "p" => (Some(self.type_), false, true), "g" => (Some("e"), true, false), "G" => (Some("E"), true, false), - _ => return None, + _ => { + return Err(Some(format!( + "the conversion specifier `{}` is unknown or unsupported", + self.type_ + ))); + } }; let (fill, width, precision) = match (is_int, width, precision) { (true, Some(_), Some(_)) => { // Rust can't duplicate this insanity. - return None; + return Err(Some( + "width and precision cannot both be specified for integer conversions" + .to_string(), + )); } (true, None, Some(p)) => (Some("0"), Some(p), None), (true, w, None) => (fill, w, None), @@ -169,7 +184,17 @@ pub mod printf { s.push('{'); if let Some(arg) = self.parameter { - write!(s, "{}", arg.checked_sub(1)?).ok()?; + match write!( + s, + "{}", + match arg.checked_sub(1) { + Some(a) => a, + None => return Err(None), + } + ) { + Err(_) => return Err(None), + _ => {} + } } if has_options { @@ -199,12 +224,18 @@ pub mod printf { } if let Some(width) = width { - width.translate(&mut s).ok()?; + match width.translate(&mut s) { + Err(_) => return Err(None), + _ => {} + } } if let Some(precision) = precision { s.push('.'); - precision.translate(&mut s).ok()?; + match precision.translate(&mut s) { + Err(_) => return Err(None), + _ => {} + } } if let Some(type_) = type_ { @@ -213,7 +244,7 @@ pub mod printf { } s.push('}'); - Some(s) + Ok(s) } } @@ -623,11 +654,11 @@ pub mod shell { } } - pub fn translate(&self) -> Option { + pub fn translate(&self) -> Result> { match *self { - Substitution::Ordinal(n, _) => Some(format!("{{{}}}", n)), - Substitution::Name(n, _) => Some(format!("{{{}}}", n)), - Substitution::Escape(_) => None, + Substitution::Ordinal(n, _) => Ok(format!("{{{}}}", n)), + Substitution::Name(n, _) => Ok(format!("{{{}}}", n)), + Substitution::Escape(_) => Err(None), } } } diff --git a/compiler/rustc_builtin_macros/src/format_foreign/printf/tests.rs b/compiler/rustc_builtin_macros/src/format_foreign/printf/tests.rs index 33c54c9cee001..1336aab731674 100644 --- a/compiler/rustc_builtin_macros/src/format_foreign/printf/tests.rs +++ b/compiler/rustc_builtin_macros/src/format_foreign/printf/tests.rs @@ -3,7 +3,7 @@ use super::{iter_subs, parse_next_substitution as pns, Format as F, Num as N, Su macro_rules! assert_eq_pnsat { ($lhs:expr, $rhs:expr) => { assert_eq!( - pns($lhs).and_then(|(s, _)| s.translate()), + pns($lhs).and_then(|(s, _)| s.translate().ok()), $rhs.map(>::from) ) }; @@ -98,7 +98,7 @@ fn test_parse() { #[test] fn test_iter() { let s = "The %d'th word %% is: `%.*s` %!\n"; - let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate()).collect(); + let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate().ok()).collect(); assert_eq!( subs.iter().map(|ms| ms.as_ref().map(|s| &s[..])).collect::>(), vec![Some("{}"), None, Some("{:.*}"), None] diff --git a/compiler/rustc_builtin_macros/src/format_foreign/shell/tests.rs b/compiler/rustc_builtin_macros/src/format_foreign/shell/tests.rs index ed8fe81dfcdd8..f5f82732f2034 100644 --- a/compiler/rustc_builtin_macros/src/format_foreign/shell/tests.rs +++ b/compiler/rustc_builtin_macros/src/format_foreign/shell/tests.rs @@ -3,7 +3,7 @@ use super::{parse_next_substitution as pns, Substitution as S}; macro_rules! assert_eq_pnsat { ($lhs:expr, $rhs:expr) => { assert_eq!( - pns($lhs).and_then(|(f, _)| f.translate()), + pns($lhs).and_then(|(f, _)| f.translate().ok()), $rhs.map(>::from) ) }; @@ -37,7 +37,7 @@ fn test_parse() { fn test_iter() { use super::iter_subs; let s = "The $0'th word $$ is: `$WORD` $!\n"; - let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate()).collect(); + let subs: Vec<_> = iter_subs(s, 0).map(|sub| sub.translate().ok()).collect(); assert_eq!( subs.iter().map(|ms| ms.as_ref().map(|s| &s[..])).collect::>(), vec![Some("{0}"), None, Some("{WORD}")] diff --git a/src/test/ui/fmt/issue-89173.rs b/src/test/ui/fmt/issue-89173.rs new file mode 100644 index 0000000000000..96277d4d0d9d7 --- /dev/null +++ b/src/test/ui/fmt/issue-89173.rs @@ -0,0 +1,14 @@ +// Regression test for #89173: Make sure a helpful note is issued for +// printf-style format strings using `*` to specify the width. + +fn main() { + let num = 0x0abcde; + let width = 6; + print!("%0*x", width, num); + //~^ ERROR: multiple unused formatting arguments + //~| NOTE: multiple missing formatting specifiers + //~| NOTE: argument never used + //~| NOTE: argument never used + //~| NOTE: format specifiers use curly braces, and you have to use a positional or named parameter for the width + //~| NOTE: printf formatting not supported +} diff --git a/src/test/ui/fmt/issue-89173.stderr b/src/test/ui/fmt/issue-89173.stderr new file mode 100644 index 0000000000000..7b21e0a4fc896 --- /dev/null +++ b/src/test/ui/fmt/issue-89173.stderr @@ -0,0 +1,18 @@ +error: multiple unused formatting arguments + --> $DIR/issue-89173.rs:7:20 + | +LL | print!("%0*x", width, num); + | ------ ^^^^^ ^^^ argument never used + | | | + | | argument never used + | multiple missing formatting specifiers + | +note: format specifiers use curly braces, and you have to use a positional or named parameter for the width + --> $DIR/issue-89173.rs:7:13 + | +LL | print!("%0*x", width, num); + | ^^^^ + = note: printf formatting not supported; see the documentation for `std::fmt` + +error: aborting due to previous error + From 09f1542418fb6312e2252b4dead8a16ad0817e30 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Sun, 29 Aug 2021 08:55:29 -0700 Subject: [PATCH 7/9] Implemented -Z randomize-layout --- Cargo.lock | 21 ++++++++--- compiler/rustc_middle/Cargo.toml | 2 ++ compiler/rustc_middle/src/ty/layout.rs | 48 +++++++++++++++++++------- compiler/rustc_middle/src/ty/mod.rs | 38 +++++++++++++++++++- compiler/rustc_session/src/options.rs | 2 ++ src/tools/tidy/src/deps.rs | 1 + 6 files changed, 93 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6b3a714920bb4..9eff4dd8f6b14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1660,7 +1660,7 @@ checksum = "3ca8957e71f04a205cb162508f9326aea04676c8dfd0711220190d6b83664f3f" dependencies = [ "bitmaps", "rand_core 0.5.1", - "rand_xoshiro", + "rand_xoshiro 0.4.0", "sized-chunks", "typenum", "version_check", @@ -2256,7 +2256,7 @@ dependencies = [ "libc", "log", "measureme", - "rand 0.8.3", + "rand 0.8.4", "rustc-workspace-hack", "rustc_version", "shell-escape", @@ -2852,9 +2852,9 @@ dependencies = [ [[package]] name = "rand" -version = "0.8.3" +version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ef9e7e66b4468674bfcb0c81af8b7fa0bb154fa9f28eb840da5c447baeb8d7e" +checksum = "2e7573632e6454cf6b99d7aac4ccca54be06da05aca2ef7423d22d27d4d4bcd8" dependencies = [ "libc", "rand_chacha 0.3.0", @@ -2945,6 +2945,15 @@ dependencies = [ "rand_core 0.5.1", ] +[[package]] +name = "rand_xoshiro" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f97cdb2a36ed4183de61b2f824cc45c9f1037f28afe0a322e9fff4c108b5aaa" +dependencies = [ + "rand_core 0.6.2", +] + [[package]] name = "rayon" version = "1.3.1" @@ -4086,6 +4095,8 @@ dependencies = [ "either", "gsgdt", "polonius-engine", + "rand 0.8.4", + "rand_xoshiro 0.6.0", "rustc-rayon-core", "rustc_apfloat", "rustc_arena", @@ -5096,7 +5107,7 @@ checksum = "dac1c663cfc93810f88aed9b8941d48cabf856a1b111c29a40439018d870eb22" dependencies = [ "cfg-if 1.0.0", "libc", - "rand 0.8.3", + "rand 0.8.4", "redox_syscall", "remove_dir_all", "winapi", diff --git a/compiler/rustc_middle/Cargo.toml b/compiler/rustc_middle/Cargo.toml index b1fcc34bee1d7..d06c593d39481 100644 --- a/compiler/rustc_middle/Cargo.toml +++ b/compiler/rustc_middle/Cargo.toml @@ -32,3 +32,5 @@ chalk-ir = "0.55.0" smallvec = { version = "1.6.1", features = ["union", "may_dangle"] } rustc_session = { path = "../rustc_session" } rustc_type_ir = { path = "../rustc_type_ir" } +rand = "0.8.4" +rand_xoshiro = "0.6.0" diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index cfbbec374a172..b6aeb9122c3d3 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -24,6 +24,9 @@ use std::iter; use std::num::NonZeroUsize; use std::ops::Bound; +use rand::{seq::SliceRandom, SeedableRng}; +use rand_xoshiro::Xoshiro128StarStar; + pub fn provide(providers: &mut ty::query::Providers) { *providers = ty::query::Providers { layout_of, fn_abi_of_fn_ptr, fn_abi_of_instance, ..*providers }; @@ -324,6 +327,10 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let mut inverse_memory_index: Vec = (0..fields.len() as u32).collect(); + // `ReprOptions.layout_seed` is a deterministic seed that we can use to + // randomize field ordering with + let mut rng = Xoshiro128StarStar::seed_from_u64(repr.field_shuffle_seed); + let optimize = !repr.inhibit_struct_field_reordering_opt(); if optimize { let end = @@ -332,20 +339,35 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> { let field_align = |f: &TyAndLayout<'_>| { if let Some(pack) = pack { f.align.abi.min(pack) } else { f.align.abi } }; - match kind { - StructKind::AlwaysSized | StructKind::MaybeUnsized => { - optimizing.sort_by_key(|&x| { - // Place ZSTs first to avoid "interesting offsets", - // especially with only one or two non-ZST fields. - let f = &fields[x as usize]; - (!f.is_zst(), cmp::Reverse(field_align(f))) - }); - } - StructKind::Prefixed(..) => { - // Sort in ascending alignment so that the layout stay optimal - // regardless of the prefix - optimizing.sort_by_key(|&x| field_align(&fields[x as usize])); + + // If `-Z randomize-layout` was enabled for the type definition we can shuffle + // the field ordering to try and catch some code making assumptions about layouts + // we don't guarantee + if repr.can_randomize_type_layout() { + // Shuffle the ordering of the fields + optimizing.shuffle(&mut rng); + + // Otherwise we just leave things alone and actually optimize the type's fields + } else { + match kind { + StructKind::AlwaysSized | StructKind::MaybeUnsized => { + optimizing.sort_by_key(|&x| { + // Place ZSTs first to avoid "interesting offsets", + // especially with only one or two non-ZST fields. + let f = &fields[x as usize]; + (!f.is_zst(), cmp::Reverse(field_align(f))) + }); + } + + StructKind::Prefixed(..) => { + // Sort in ascending alignment so that the layout stays optimal + // regardless of the prefix + optimizing.sort_by_key(|&x| field_align(&fields[x as usize])); + } } + + // FIXME(Kixiron): We can always shuffle fields within a given alignment class + // regardless of the status of `-Z randomize-layout` } } diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index b3ae76d987167..d04d1565fead7 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -1491,6 +1491,9 @@ bitflags! { const IS_LINEAR = 1 << 3; // If true, don't expose any niche to type's context. const HIDE_NICHE = 1 << 4; + // If true, the type's layout can be randomized using + // the seed stored in `ReprOptions.layout_seed` + const RANDOMIZE_LAYOUT = 1 << 5; // Any of these flags being set prevent field reordering optimisation. const IS_UNOPTIMISABLE = ReprFlags::IS_C.bits | ReprFlags::IS_SIMD.bits | @@ -1505,6 +1508,14 @@ pub struct ReprOptions { pub align: Option, pub pack: Option, pub flags: ReprFlags, + /// The seed to be used for randomizing a type's layout + /// + /// Note: This could technically be a `[u8; 16]` (a `u128`) which would + /// be the "most accurate" hash as it'd encompass the item and crate + /// hash without loss, but it does pay the price of being larger. + /// Everything's a tradeoff, a `u64` seed should be sufficient for our + /// purposes (primarily `-Z randomize-layout`) + pub field_shuffle_seed: u64, } impl ReprOptions { @@ -1513,6 +1524,11 @@ impl ReprOptions { let mut size = None; let mut max_align: Option = None; let mut min_pack: Option = None; + + // Generate a deterministically-derived seed from the item's path hash + // to allow for cross-crate compilation to actually work + let field_shuffle_seed = tcx.def_path_hash(did).0.to_smaller_hash(); + for attr in tcx.get_attrs(did).iter() { for r in attr::find_repr_attrs(&tcx.sess, attr) { flags.insert(match r { @@ -1541,33 +1557,45 @@ impl ReprOptions { } } + // If `-Z randomize-layout` was enabled for the type definition then we can + // consider performing layout randomization + if tcx.sess.opts.debugging_opts.randomize_layout { + flags.insert(ReprFlags::RANDOMIZE_LAYOUT); + } + // This is here instead of layout because the choice must make it into metadata. if !tcx.consider_optimizing(|| format!("Reorder fields of {:?}", tcx.def_path_str(did))) { flags.insert(ReprFlags::IS_LINEAR); } - ReprOptions { int: size, align: max_align, pack: min_pack, flags } + + Self { int: size, align: max_align, pack: min_pack, flags, field_shuffle_seed } } #[inline] pub fn simd(&self) -> bool { self.flags.contains(ReprFlags::IS_SIMD) } + #[inline] pub fn c(&self) -> bool { self.flags.contains(ReprFlags::IS_C) } + #[inline] pub fn packed(&self) -> bool { self.pack.is_some() } + #[inline] pub fn transparent(&self) -> bool { self.flags.contains(ReprFlags::IS_TRANSPARENT) } + #[inline] pub fn linear(&self) -> bool { self.flags.contains(ReprFlags::IS_LINEAR) } + #[inline] pub fn hide_niche(&self) -> bool { self.flags.contains(ReprFlags::HIDE_NICHE) @@ -1594,9 +1622,17 @@ impl ReprOptions { return true; } } + self.flags.intersects(ReprFlags::IS_UNOPTIMISABLE) || self.int.is_some() } + /// Returns `true` if this type is valid for reordering and `-Z randomize-layout` + /// was enabled for its declaration crate + pub fn can_randomize_type_layout(&self) -> bool { + !self.inhibit_struct_field_reordering_opt() + && self.flags.contains(ReprFlags::RANDOMIZE_LAYOUT) + } + /// Returns `true` if this `#[repr()]` should inhibit union ABI optimisations. pub fn inhibit_union_abi_opt(&self) -> bool { self.c() diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 8110afe75fa92..8ecb7a031ad81 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1246,6 +1246,8 @@ options! { "enable queries of the dependency graph for regression testing (default: no)"), query_stats: bool = (false, parse_bool, [UNTRACKED], "print some statistics about the query system (default: no)"), + randomize_layout: bool = (false, parse_bool, [TRACKED], + "randomize the layout of types (default: no)"), relax_elf_relocations: Option = (None, parse_opt_bool, [TRACKED], "whether ELF relocations can be relaxed"), relro_level: Option = (None, parse_relro_level, [TRACKED], diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 60703384e9e85..c1719a9ffe80c 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -167,6 +167,7 @@ const PERMITTED_DEPENDENCIES: &[&str] = &[ "rand_hc", "rand_pcg", "rand_xorshift", + "rand_xoshiro", "redox_syscall", "regex", "regex-automata", From 3565e8a088f0ade7fbd17348bbf113470fb40181 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 30 Sep 2021 21:25:00 -0400 Subject: [PATCH 8/9] Correct caller/callsite confusion in inliner message --- compiler/rustc_mir_transform/src/inline.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index d43528a1cf098..ee4e91ecb62b0 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -147,7 +147,7 @@ impl Inliner<'tcx> { self.check_mir_body(callsite, callee_body, callee_attrs)?; if !self.tcx.consider_optimizing(|| { - format!("Inline {:?} into {}", callee_body.span, callsite.callee) + format!("Inline {:?} into {:?}", callsite.callee, caller_body.source) }) { return Err("optimization fuel exhausted"); } From 198d90786b9fb429928d70c423bad5d65374a532 Mon Sep 17 00:00:00 2001 From: Marcel Hlopko Date: Fri, 10 Sep 2021 15:11:56 +0200 Subject: [PATCH 9/9] Add `pie` as another `relocation-model` value --- compiler/rustc_codegen_llvm/src/back/write.rs | 3 +- compiler/rustc_codegen_llvm/src/context.rs | 7 +++- compiler/rustc_codegen_llvm/src/lib.rs | 13 +++++-- compiler/rustc_codegen_llvm/src/mono_item.rs | 6 +++ compiler/rustc_codegen_ssa/src/back/link.rs | 8 +++- compiler/rustc_target/src/spec/mod.rs | 3 ++ src/doc/rustc/src/codegen-options/index.md | 4 ++ src/test/assembly/pic-relocation-model.rs | 35 +++++++++++++++++ src/test/assembly/pie-relocation-model.rs | 38 +++++++++++++++++++ src/test/codegen/pic-relocation-model.rs | 16 ++++++++ src/test/codegen/pie-relocation-model.rs | 22 +++++++++++ 11 files changed, 147 insertions(+), 8 deletions(-) create mode 100644 src/test/assembly/pic-relocation-model.rs create mode 100644 src/test/assembly/pie-relocation-model.rs create mode 100644 src/test/codegen/pic-relocation-model.rs create mode 100644 src/test/codegen/pie-relocation-model.rs diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index 791604a18273d..e8484c24df9db 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -129,7 +129,8 @@ fn to_pass_builder_opt_level(cfg: config::OptLevel) -> llvm::PassBuilderOptLevel fn to_llvm_relocation_model(relocation_model: RelocModel) -> llvm::RelocModel { match relocation_model { RelocModel::Static => llvm::RelocModel::Static, - RelocModel::Pic => llvm::RelocModel::PIC, + // LLVM doesn't have a PIE relocation model, it represents PIE as PIC with an extra attribute. + RelocModel::Pic | RelocModel::Pie => llvm::RelocModel::PIC, RelocModel::DynamicNoPic => llvm::RelocModel::DynamicNoPic, RelocModel::Ropi => llvm::RelocModel::ROPI, RelocModel::Rwpi => llvm::RelocModel::RWPI, diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 2d397dc583534..e69d42c948285 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -190,11 +190,14 @@ pub unsafe fn create_module( let llvm_target = SmallCStr::new(&sess.target.llvm_target); llvm::LLVMRustSetNormalizedTarget(llmod, llvm_target.as_ptr()); - if sess.relocation_model() == RelocModel::Pic { + let reloc_model = sess.relocation_model(); + if matches!(reloc_model, RelocModel::Pic | RelocModel::Pie) { llvm::LLVMRustSetModulePICLevel(llmod); // PIE is potentially more effective than PIC, but can only be used in executables. // If all our outputs are executables, then we can relax PIC to PIE. - if sess.crate_types().iter().all(|ty| *ty == CrateType::Executable) { + if reloc_model == RelocModel::Pie + || sess.crate_types().iter().all(|ty| *ty == CrateType::Executable) + { llvm::LLVMRustSetModulePIELevel(llmod); } } diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 1da14344b1d26..0b06fe8bdcf9c 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -211,9 +211,16 @@ impl CodegenBackend for LlvmCodegenBackend { match req { PrintRequest::RelocationModels => { println!("Available relocation models:"); - for name in - &["static", "pic", "dynamic-no-pic", "ropi", "rwpi", "ropi-rwpi", "default"] - { + for name in &[ + "static", + "pic", + "pie", + "dynamic-no-pic", + "ropi", + "rwpi", + "ropi-rwpi", + "default", + ] { println!(" {}", name); } println!(); diff --git a/compiler/rustc_codegen_llvm/src/mono_item.rs b/compiler/rustc_codegen_llvm/src/mono_item.rs index 8ba3e870fbb71..17bf55bf512fe 100644 --- a/compiler/rustc_codegen_llvm/src/mono_item.rs +++ b/compiler/rustc_codegen_llvm/src/mono_item.rs @@ -144,6 +144,12 @@ impl CodegenCx<'ll, 'tcx> { return true; } + // With pie relocation model calls of functions defined in the translation + // unit can use copy relocations. + if self.tcx.sess.relocation_model() == RelocModel::Pie && !is_declaration { + return true; + } + return false; } } diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 4fb51ecc1d347..0dcddb693de75 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1490,9 +1490,13 @@ fn exec_linker( fn link_output_kind(sess: &Session, crate_type: CrateType) -> LinkOutputKind { let kind = match (crate_type, sess.crt_static(Some(crate_type)), sess.relocation_model()) { (CrateType::Executable, _, _) if sess.is_wasi_reactor() => LinkOutputKind::WasiReactorExe, - (CrateType::Executable, false, RelocModel::Pic) => LinkOutputKind::DynamicPicExe, + (CrateType::Executable, false, RelocModel::Pic | RelocModel::Pie) => { + LinkOutputKind::DynamicPicExe + } (CrateType::Executable, false, _) => LinkOutputKind::DynamicNoPicExe, - (CrateType::Executable, true, RelocModel::Pic) => LinkOutputKind::StaticPicExe, + (CrateType::Executable, true, RelocModel::Pic | RelocModel::Pie) => { + LinkOutputKind::StaticPicExe + } (CrateType::Executable, true, _) => LinkOutputKind::StaticNoPicExe, (_, true, _) => LinkOutputKind::StaticDylib, (_, false, _) => LinkOutputKind::DynamicDylib, diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 273221360b8b5..44eafde300181 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -287,6 +287,7 @@ impl ToJson for MergeFunctions { pub enum RelocModel { Static, Pic, + Pie, DynamicNoPic, Ropi, Rwpi, @@ -300,6 +301,7 @@ impl FromStr for RelocModel { Ok(match s { "static" => RelocModel::Static, "pic" => RelocModel::Pic, + "pie" => RelocModel::Pie, "dynamic-no-pic" => RelocModel::DynamicNoPic, "ropi" => RelocModel::Ropi, "rwpi" => RelocModel::Rwpi, @@ -314,6 +316,7 @@ impl ToJson for RelocModel { match *self { RelocModel::Static => "static", RelocModel::Pic => "pic", + RelocModel::Pie => "pie", RelocModel::DynamicNoPic => "dynamic-no-pic", RelocModel::Ropi => "ropi", RelocModel::Rwpi => "rwpi", diff --git a/src/doc/rustc/src/codegen-options/index.md b/src/doc/rustc/src/codegen-options/index.md index 05384117ac175..4f8c4c66f8891 100644 --- a/src/doc/rustc/src/codegen-options/index.md +++ b/src/doc/rustc/src/codegen-options/index.md @@ -435,6 +435,10 @@ Equivalent to the "uppercase" `-fPIC` or `-fPIE` options in other compilers, depending on the produced crate types. \ This is the default model for majority of supported targets. +- `pie` - position independent executable, relocatable code but without support for symbol +interpositioning (replacing symbols by name using `LD_PRELOAD` and similar). Equivalent to the "uppercase" `-fPIE` option in other compilers. `pie` +code cannot be linked into shared libraries (you'll get a linking error on attempt to do this). + #### Special relocation models - `dynamic-no-pic` - relocatable external references, non-relocatable code. \ diff --git a/src/test/assembly/pic-relocation-model.rs b/src/test/assembly/pic-relocation-model.rs new file mode 100644 index 0000000000000..72471ffcdb0cb --- /dev/null +++ b/src/test/assembly/pic-relocation-model.rs @@ -0,0 +1,35 @@ +// revisions: x64 +// assembly-output: emit-asm +// [x64] compile-flags: --target x86_64-unknown-linux-gnu -Crelocation-model=pic +// [x64] needs-llvm-components: x86 + + +#![feature(no_core, lang_items)] +#![no_core] +#![crate_type="rlib"] + +#[lang = "sized"] +trait Sized {} +#[lang = "copy"] +trait Copy {} + +// CHECK-LABEL: call_other_fn: +// CHECK: {{(jmpq|callq)}} *other_fn@GOTPCREL(%rip) +#[no_mangle] +pub fn call_other_fn() -> u8 { + unsafe { + other_fn() + } +} + +// CHECK-LABEL: other_fn: +// CHECK: callq *foreign_fn@GOTPCREL(%rip) +#[no_mangle] +#[inline(never)] +pub fn other_fn() -> u8 { + unsafe { + foreign_fn() + } +} + +extern "C" {fn foreign_fn() -> u8;} diff --git a/src/test/assembly/pie-relocation-model.rs b/src/test/assembly/pie-relocation-model.rs new file mode 100644 index 0000000000000..e40797e038d4b --- /dev/null +++ b/src/test/assembly/pie-relocation-model.rs @@ -0,0 +1,38 @@ +// revisions: x64 +// assembly-output: emit-asm +// [x64] compile-flags: --target x86_64-unknown-linux-gnu -Crelocation-model=pie +// [x64] needs-llvm-components: x86 + + +#![feature(no_core, lang_items)] +#![no_core] +#![crate_type="rlib"] + +#[lang = "sized"] +trait Sized {} +#[lang = "copy"] +trait Copy {} + +// CHECK-LABEL: call_other_fn: +// With PIE local functions are called "directly". +// CHECK: {{(jmp|callq)}} other_fn +#[no_mangle] +pub fn call_other_fn() -> u8 { + unsafe { + other_fn() + } +} + +// CHECK-LABEL: other_fn: +// External functions are still called through GOT, since we don't know if the symbol +// is defined in the binary or in the shared library. +// CHECK: callq *foreign_fn@GOTPCREL(%rip) +#[no_mangle] +#[inline(never)] +pub fn other_fn() -> u8 { + unsafe { + foreign_fn() + } +} + +extern "C" {fn foreign_fn() -> u8;} diff --git a/src/test/codegen/pic-relocation-model.rs b/src/test/codegen/pic-relocation-model.rs new file mode 100644 index 0000000000000..6e1d5a6c3f271 --- /dev/null +++ b/src/test/codegen/pic-relocation-model.rs @@ -0,0 +1,16 @@ +// compile-flags: -C relocation-model=pic + +#![crate_type = "rlib"] + +// CHECK: define i8 @call_foreign_fn() +#[no_mangle] +pub fn call_foreign_fn() -> u8 { + unsafe { + foreign_fn() + } +} + +// CHECK: declare zeroext i8 @foreign_fn() +extern "C" {fn foreign_fn() -> u8;} + +// CHECK: !{i32 7, !"PIC Level", i32 2} diff --git a/src/test/codegen/pie-relocation-model.rs b/src/test/codegen/pie-relocation-model.rs new file mode 100644 index 0000000000000..a843202a94f82 --- /dev/null +++ b/src/test/codegen/pie-relocation-model.rs @@ -0,0 +1,22 @@ +// compile-flags: -C relocation-model=pie +// only-x86_64-unknown-linux-gnu + +#![crate_type = "rlib"] + +// With PIE we know local functions cannot be interpositioned, we can mark them +// as dso_local. +// CHECK: define dso_local i8 @call_foreign_fn() +#[no_mangle] +pub fn call_foreign_fn() -> u8 { + unsafe { + foreign_fn() + } +} + +// External functions are still marked as non-dso_local, since we don't know if the symbol +// is defined in the binary or in the shared library. +// CHECK: declare zeroext i8 @foreign_fn() +extern "C" {fn foreign_fn() -> u8;} + +// CHECK: !{i32 7, !"PIC Level", i32 2} +// CHECK: !{i32 7, !"PIE Level", i32 2}