From 700b3ea61ba2a37876af0a5d2d2f991aba5c7000 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Mon, 13 May 2024 14:22:45 +0200 Subject: [PATCH 1/9] Panic if `PathBuf::set_extension` would add a path separator This is likely never intended and potentially a security vulnerability if it happens. I'd guess that it's mostly literal strings that are passed to this function in practice, so I'm guessing this doesn't break anyone. CC #125060 --- library/std/src/path.rs | 13 +++++++++++++ library/std/src/path/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/library/std/src/path.rs b/library/std/src/path.rs index 79d800ff0729b..acf23d976177b 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -1425,6 +1425,11 @@ impl PathBuf { /// If `extension` is the empty string, [`self.extension`] will be [`None`] /// afterwards, not `Some("")`. /// + /// # Panics + /// + /// Panics if the passed extension contains a path separator (see + /// [`is_separator`]). + /// /// # Caveats /// /// The new `extension` may contain dots and will be used in its entirety, @@ -1470,6 +1475,14 @@ impl PathBuf { } fn _set_extension(&mut self, extension: &OsStr) -> bool { + for &b in extension.as_encoded_bytes() { + if b < 128 { + if is_separator(b as char) { + panic!("extension cannot contain path separators: {:?}", extension); + } + } + } + let file_stem = match self.file_stem() { None => return false, Some(f) => f.as_encoded_bytes(), diff --git a/library/std/src/path/tests.rs b/library/std/src/path/tests.rs index fde6ed4f0c057..2d8e50d1f88e9 100644 --- a/library/std/src/path/tests.rs +++ b/library/std/src/path/tests.rs @@ -1803,6 +1803,29 @@ fn test_windows_absolute() { assert_eq!(absolute(r"COM1").unwrap().as_os_str(), Path::new(r"\\.\COM1").as_os_str()); } +#[test] +#[should_panic = "path separator"] +fn test_extension_path_sep() { + let mut path = PathBuf::from("path/to/file"); + path.set_extension("d/../../../../../etc/passwd"); +} + +#[test] +#[should_panic = "path separator"] +#[cfg(windows)] +fn test_extension_path_sep_alternate() { + let mut path = PathBuf::from("path/to/file"); + path.set_extension("d\\test"); +} + +#[test] +#[cfg(not(windows))] +fn test_extension_path_sep_alternate() { + let mut path = PathBuf::from("path/to/file"); + path.set_extension("d\\test"); + assert_eq!(path, Path::new("path/to/file.d\\test")); +} + #[bench] #[cfg_attr(miri, ignore)] // Miri isn't fast... fn bench_path_cmp_fast_path_buf_sort(b: &mut test::Bencher) { From c773debc87c99a97def1e2a3e0d83d0167cc9d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 21 May 2024 19:31:38 +0200 Subject: [PATCH 2/9] Create a triagebot ping group for Rust for Linux --- triagebot.toml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/triagebot.toml b/triagebot.toml index 2e45b257f8126..a0581a2987027 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -122,6 +122,17 @@ issues). """ label = "O-apple" +[ping.rust-for-linux] +alias = ["rfl"] +message = """\ +Hey Rust for Linux group! It looks like something broke the Rust for Linux integration. +Could you try to take a look? +In case it's useful, here are some [instructions] for tackling these sorts of issues. + +[instructions]: https://rustc-dev-guide.rust-lang.org/notification-groups/rust-for-linux.html +""" +label = "O-rfl" + [prioritize] label = "I-prioritize" From d0b45a951bddc4ed46cbbaff15818065795d0c2b Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sun, 19 May 2024 20:56:46 -0700 Subject: [PATCH 3/9] tidy: stop special-casing tests/ui entry limit --- src/tools/tidy/src/ui_tests.rs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index e1c6c9a2dacd1..37324639edfca 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -16,7 +16,6 @@ const ENTRY_LIMIT: usize = 900; // FIXME: The following limits should be reduced eventually. const ISSUES_ENTRY_LIMIT: usize = 1676; -const ROOT_ENTRY_LIMIT: usize = 757; const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[ "rs", // test source files @@ -63,14 +62,10 @@ fn check_entries(tests_path: &Path, bad: &mut bool) { } } - let (mut max, mut max_root, mut max_issues) = (0usize, 0usize, 0usize); + let (mut max, mut max_issues) = (0usize, 0usize); for (dir_path, count) in directories { - // Use special values for these dirs. - let is_root = tests_path.join("ui") == dir_path; let is_issues_dir = tests_path.join("ui/issues") == dir_path; - let (limit, maxcnt) = if is_root { - (ROOT_ENTRY_LIMIT, &mut max_root) - } else if is_issues_dir { + let (limit, maxcnt) = if is_issues_dir { (ISSUES_ENTRY_LIMIT, &mut max_issues) } else { (ENTRY_LIMIT, &mut max) @@ -87,12 +82,6 @@ fn check_entries(tests_path: &Path, bad: &mut bool) { ); } } - if ROOT_ENTRY_LIMIT > max_root { - tidy_error!( - bad, - "`ROOT_ENTRY_LIMIT` is too high (is {ROOT_ENTRY_LIMIT}, should be {max_root})" - ); - } if ISSUES_ENTRY_LIMIT > max_issues { tidy_error!( bad, From ab73bb25cfad9643c67db8b4a89f1aaaae8ddd21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Wed, 22 May 2024 13:04:14 +0200 Subject: [PATCH 4/9] Add a comment --- triagebot.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/triagebot.toml b/triagebot.toml index a0581a2987027..ec3061f69557b 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -122,6 +122,8 @@ issues). """ label = "O-apple" +# This ping group is meant for situations where a rustc/stdlib change breaks RfL. +# In that case, we want to notify the RfL group. [ping.rust-for-linux] alias = ["rfl"] message = """\ From 09c8e39adb5b19d4ff412e8f77ea42c569a8e02d Mon Sep 17 00:00:00 2001 From: surechen Date: Wed, 22 May 2024 18:25:26 +0800 Subject: [PATCH 5/9] A small diagnostic improvement for dropping_copy_types fixes #125189 --- compiler/rustc_lint/messages.ftl | 1 + .../rustc_lint/src/drop_forget_useless.rs | 21 +++++++++-- compiler/rustc_lint/src/lints.rs | 16 +++++++- .../defaults-unsound-62211-1.next.stderr | 6 ++- .../defaults-unsound-62211-2.next.stderr | 6 ++- ...g_copy_types-issue-125189-can-not-fixed.rs | 14 +++++++ ...py_types-issue-125189-can-not-fixed.stderr | 37 +++++++++++++++++++ .../dropping_copy_types-issue-125189.fixed | 10 +++++ .../lint/dropping_copy_types-issue-125189.rs | 10 +++++ .../dropping_copy_types-issue-125189.stderr | 35 ++++++++++++++++++ tests/ui/lint/dropping_copy_types.stderr | 24 ++++++++++-- 11 files changed, 169 insertions(+), 11 deletions(-) create mode 100644 tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.rs create mode 100644 tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.stderr create mode 100644 tests/ui/lint/dropping_copy_types-issue-125189.fixed create mode 100644 tests/ui/lint/dropping_copy_types-issue-125189.rs create mode 100644 tests/ui/lint/dropping_copy_types-issue-125189.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 82b90e1660a95..289005e000540 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -197,6 +197,7 @@ lint_drop_trait_constraints = lint_dropping_copy_types = calls to `std::mem::drop` with a value that implements `Copy` does nothing .label = argument has type `{$arg_ty}` .note = use `let _ = ...` to ignore the expression or result + .suggestion = use `let _ = ...` to ignore the expression or result lint_dropping_references = calls to `std::mem::drop` with a reference instead of an owned value does nothing .label = argument has type `{$arg_ty}` diff --git a/compiler/rustc_lint/src/drop_forget_useless.rs b/compiler/rustc_lint/src/drop_forget_useless.rs index 78ac7f9b2354d..5033d194a21ef 100644 --- a/compiler/rustc_lint/src/drop_forget_useless.rs +++ b/compiler/rustc_lint/src/drop_forget_useless.rs @@ -1,11 +1,11 @@ -use rustc_hir::{Arm, Expr, ExprKind, Node}; +use rustc_hir::{Arm, Expr, ExprKind, Node, StmtKind}; use rustc_middle::ty; use rustc_span::sym; use crate::{ lints::{ - DropCopyDiag, DropRefDiag, ForgetCopyDiag, ForgetRefDiag, UndroppedManuallyDropsDiag, - UndroppedManuallyDropsSuggestion, + DropCopyDiag, DropCopySuggestion, DropRefDiag, ForgetCopyDiag, ForgetRefDiag, + UndroppedManuallyDropsDiag, UndroppedManuallyDropsSuggestion, }, LateContext, LateLintPass, LintContext, }; @@ -163,10 +163,23 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetUseless { ); } sym::mem_drop if is_copy && !drop_is_single_call_in_arm => { + let sugg = if let Some((_, node)) = cx.tcx.hir().parent_iter(expr.hir_id).nth(0) + && let Node::Stmt(stmt) = node + && let StmtKind::Semi(e) = stmt.kind + && e.hir_id == expr.hir_id + { + DropCopySuggestion::Suggestion { + start_span: expr.span.shrink_to_lo().until(arg.span), + end_span: arg.span.shrink_to_hi().until(expr.span.shrink_to_hi()), + } + } else { + DropCopySuggestion::Note + }; + cx.emit_span_lint( DROPPING_COPY_TYPES, expr.span, - DropCopyDiag { arg_ty, label: arg.span }, + DropCopyDiag { arg_ty, label: arg.span, sugg }, ); } sym::mem_forget if is_copy => { diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index a034bebc85ec0..084e818a2758f 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -669,11 +669,25 @@ pub struct DropRefDiag<'a> { #[derive(LintDiagnostic)] #[diag(lint_dropping_copy_types)] -#[note] pub struct DropCopyDiag<'a> { pub arg_ty: Ty<'a>, #[label] pub label: Span, + #[subdiagnostic] + pub sugg: DropCopySuggestion, +} + +#[derive(Subdiagnostic)] +pub enum DropCopySuggestion { + #[note(lint_note)] + Note, + #[multipart_suggestion(lint_suggestion, style = "verbose", applicability = "maybe-incorrect")] + Suggestion { + #[suggestion_part(code = "let _ = ")] + start_span: Span, + #[suggestion_part(code = "")] + end_span: Span, + }, } #[derive(LintDiagnostic)] diff --git a/tests/ui/associated-types/defaults-unsound-62211-1.next.stderr b/tests/ui/associated-types/defaults-unsound-62211-1.next.stderr index 834ae00a8d85b..ffb02eccc77dc 100644 --- a/tests/ui/associated-types/defaults-unsound-62211-1.next.stderr +++ b/tests/ui/associated-types/defaults-unsound-62211-1.next.stderr @@ -6,8 +6,12 @@ LL | drop(origin); | | | argument has type `::Output` | - = note: use `let _ = ...` to ignore the expression or result = note: `#[warn(dropping_copy_types)]` on by default +help: use `let _ = ...` to ignore the expression or result + | +LL - drop(origin); +LL + let _ = origin; + | warning: 1 warning emitted diff --git a/tests/ui/associated-types/defaults-unsound-62211-2.next.stderr b/tests/ui/associated-types/defaults-unsound-62211-2.next.stderr index 0f944a18ed58b..e6ae8183e77dc 100644 --- a/tests/ui/associated-types/defaults-unsound-62211-2.next.stderr +++ b/tests/ui/associated-types/defaults-unsound-62211-2.next.stderr @@ -6,8 +6,12 @@ LL | drop(origin); | | | argument has type `::Output` | - = note: use `let _ = ...` to ignore the expression or result = note: `#[warn(dropping_copy_types)]` on by default +help: use `let _ = ...` to ignore the expression or result + | +LL - drop(origin); +LL + let _ = origin; + | warning: 1 warning emitted diff --git a/tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.rs b/tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.rs new file mode 100644 index 0000000000000..12c984c932acf --- /dev/null +++ b/tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.rs @@ -0,0 +1,14 @@ +//@ check-fail + +#![deny(dropping_copy_types)] + +fn main() { + let y = 1; + let z = 2; + match y { + 0 => drop(y), //~ ERROR calls to `std::mem::drop` + 1 => drop(z), //~ ERROR calls to `std::mem::drop` + 2 => drop(3), //~ ERROR calls to `std::mem::drop` + _ => {}, + } +} diff --git a/tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.stderr b/tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.stderr new file mode 100644 index 0000000000000..6c73e5ea90b47 --- /dev/null +++ b/tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.stderr @@ -0,0 +1,37 @@ +error: calls to `std::mem::drop` with a value that implements `Copy` does nothing + --> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:9:14 + | +LL | 0 => drop(y), + | ^^^^^-^ + | | + | argument has type `i32` + | + = note: use `let _ = ...` to ignore the expression or result +note: the lint level is defined here + --> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:3:9 + | +LL | #![deny(dropping_copy_types)] + | ^^^^^^^^^^^^^^^^^^^ + +error: calls to `std::mem::drop` with a value that implements `Copy` does nothing + --> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:10:14 + | +LL | 1 => drop(z), + | ^^^^^-^ + | | + | argument has type `i32` + | + = note: use `let _ = ...` to ignore the expression or result + +error: calls to `std::mem::drop` with a value that implements `Copy` does nothing + --> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:11:14 + | +LL | 2 => drop(3), + | ^^^^^-^ + | | + | argument has type `i32` + | + = note: use `let _ = ...` to ignore the expression or result + +error: aborting due to 3 previous errors + diff --git a/tests/ui/lint/dropping_copy_types-issue-125189.fixed b/tests/ui/lint/dropping_copy_types-issue-125189.fixed new file mode 100644 index 0000000000000..2dfd68d4cc16a --- /dev/null +++ b/tests/ui/lint/dropping_copy_types-issue-125189.fixed @@ -0,0 +1,10 @@ +//@ check-fail +//@ run-rustfix + +#![deny(dropping_copy_types)] + +fn main() { + let y = 1; + let _ = 3.2; //~ ERROR calls to `std::mem::drop` + let _ = y; //~ ERROR calls to `std::mem::drop` +} diff --git a/tests/ui/lint/dropping_copy_types-issue-125189.rs b/tests/ui/lint/dropping_copy_types-issue-125189.rs new file mode 100644 index 0000000000000..1f42efabba9ff --- /dev/null +++ b/tests/ui/lint/dropping_copy_types-issue-125189.rs @@ -0,0 +1,10 @@ +//@ check-fail +//@ run-rustfix + +#![deny(dropping_copy_types)] + +fn main() { + let y = 1; + drop(3.2); //~ ERROR calls to `std::mem::drop` + drop(y); //~ ERROR calls to `std::mem::drop` +} diff --git a/tests/ui/lint/dropping_copy_types-issue-125189.stderr b/tests/ui/lint/dropping_copy_types-issue-125189.stderr new file mode 100644 index 0000000000000..ba3e36f5b6ec4 --- /dev/null +++ b/tests/ui/lint/dropping_copy_types-issue-125189.stderr @@ -0,0 +1,35 @@ +error: calls to `std::mem::drop` with a value that implements `Copy` does nothing + --> $DIR/dropping_copy_types-issue-125189.rs:8:5 + | +LL | drop(3.2); + | ^^^^^---^ + | | + | argument has type `f64` + | +note: the lint level is defined here + --> $DIR/dropping_copy_types-issue-125189.rs:4:9 + | +LL | #![deny(dropping_copy_types)] + | ^^^^^^^^^^^^^^^^^^^ +help: use `let _ = ...` to ignore the expression or result + | +LL - drop(3.2); +LL + let _ = 3.2; + | + +error: calls to `std::mem::drop` with a value that implements `Copy` does nothing + --> $DIR/dropping_copy_types-issue-125189.rs:9:5 + | +LL | drop(y); + | ^^^^^-^ + | | + | argument has type `i32` + | +help: use `let _ = ...` to ignore the expression or result + | +LL - drop(y); +LL + let _ = y; + | + +error: aborting due to 2 previous errors + diff --git a/tests/ui/lint/dropping_copy_types.stderr b/tests/ui/lint/dropping_copy_types.stderr index b6291aa5ed634..bdeb0c290fe93 100644 --- a/tests/ui/lint/dropping_copy_types.stderr +++ b/tests/ui/lint/dropping_copy_types.stderr @@ -6,12 +6,16 @@ LL | drop(s1); | | | argument has type `SomeStruct` | - = note: use `let _ = ...` to ignore the expression or result note: the lint level is defined here --> $DIR/dropping_copy_types.rs:3:9 | LL | #![warn(dropping_copy_types)] | ^^^^^^^^^^^^^^^^^^^ +help: use `let _ = ...` to ignore the expression or result + | +LL - drop(s1); +LL + let _ = s1; + | warning: calls to `std::mem::drop` with a value that implements `Copy` does nothing --> $DIR/dropping_copy_types.rs:35:5 @@ -21,7 +25,11 @@ LL | drop(s2); | | | argument has type `SomeStruct` | - = note: use `let _ = ...` to ignore the expression or result +help: use `let _ = ...` to ignore the expression or result + | +LL - drop(s2); +LL + let _ = s2; + | warning: calls to `std::mem::drop` with a reference instead of an owned value does nothing --> $DIR/dropping_copy_types.rs:36:5 @@ -42,7 +50,11 @@ LL | drop(s4); | | | argument has type `SomeStruct` | - = note: use `let _ = ...` to ignore the expression or result +help: use `let _ = ...` to ignore the expression or result + | +LL - drop(s4); +LL + let _ = s4; + | warning: calls to `std::mem::drop` with a reference instead of an owned value does nothing --> $DIR/dropping_copy_types.rs:38:5 @@ -82,7 +94,11 @@ LL | drop(println_and(13)); | | | argument has type `i32` | - = note: use `let _ = ...` to ignore the expression or result +help: use `let _ = ...` to ignore the expression or result + | +LL - drop(println_and(13)); +LL + let _ = println_and(13); + | warning: calls to `std::mem::drop` with a value that implements `Copy` does nothing --> $DIR/dropping_copy_types.rs:74:14 From 24b5466892956b26b5e261ed418aee3c9c6e6d45 Mon Sep 17 00:00:00 2001 From: lcnr Date: Wed, 22 May 2024 16:56:10 +0000 Subject: [PATCH 6/9] drop region constraints for ambiguous goals --- .../src/solve/eval_ctxt/canonical.rs | 64 +++++++++++-------- .../leak-check-in-selection-5-ambig.rs | 28 ++++++++ ...eck-in-selection-6-ambig-unify.next.stderr | 22 +++++++ ...heck-in-selection-6-ambig-unify.old.stderr | 22 +++++++ .../leak-check-in-selection-6-ambig-unify.rs | 32 ++++++++++ .../ambig-goal-infer-in-type-oulives.rs | 29 +++++++++ 6 files changed, 172 insertions(+), 25 deletions(-) create mode 100644 tests/ui/higher-ranked/leak-check/leak-check-in-selection-5-ambig.rs create mode 100644 tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.next.stderr create mode 100644 tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.old.stderr create mode 100644 tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.rs create mode 100644 tests/ui/traits/next-solver/normalize/ambig-goal-infer-in-type-oulives.rs diff --git a/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs b/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs index 9590a82c0677f..f6ec654908450 100644 --- a/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs +++ b/compiler/rustc_trait_selection/src/solve/eval_ctxt/canonical.rs @@ -99,6 +99,13 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> { previous call to `try_evaluate_added_goals!`" ); + // We only check for leaks from universes which were entered inside + // of the query. + self.infcx.leak_check(self.max_input_universe, None).map_err(|e| { + trace!(?e, "failed the leak check"); + NoSolution + })?; + // When normalizing, we've replaced the expected term with an unconstrained // inference variable. This means that we dropped information which could // have been important. We handle this by instead returning the nested goals @@ -121,7 +128,7 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> { }; let external_constraints = - self.compute_external_query_constraints(normalization_nested_goals)?; + self.compute_external_query_constraints(certainty, normalization_nested_goals); let (var_values, mut external_constraints) = (self.var_values, external_constraints).fold_with(&mut EagerResolver::new(self.infcx)); // Remove any trivial region constraints once we've resolved regions @@ -170,30 +177,37 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> { #[instrument(level = "trace", skip(self), ret)] fn compute_external_query_constraints( &self, + certainty: Certainty, normalization_nested_goals: NestedNormalizationGoals<'tcx>, - ) -> Result, NoSolution> { - // We only check for leaks from universes which were entered inside - // of the query. - self.infcx.leak_check(self.max_input_universe, None).map_err(|e| { - trace!(?e, "failed the leak check"); - NoSolution - })?; - - // Cannot use `take_registered_region_obligations` as we may compute the response - // inside of a `probe` whenever we have multiple choices inside of the solver. - let region_obligations = self.infcx.inner.borrow().region_obligations().to_owned(); - let mut region_constraints = self.infcx.with_region_constraints(|region_constraints| { - make_query_region_constraints( - self.tcx(), - region_obligations - .iter() - .map(|r_o| (r_o.sup_type, r_o.sub_region, r_o.origin.to_constraint_category())), - region_constraints, - ) - }); - - let mut seen = FxHashSet::default(); - region_constraints.outlives.retain(|outlives| seen.insert(*outlives)); + ) -> ExternalConstraintsData<'tcx> { + // We only return region constraints once the certainty is `Yes`. This + // is necessary as we may drop nested goals on ambiguity, which may result + // in unconstrained inference variables in the region constraints. It also + // prevents us from emitting duplicate region constraints, avoiding some + // unnecessary work. This slightly weakens the leak check in case it uses + // region constraints from an ambiguous nested goal. This is tested in both + // `tests/ui/higher-ranked/leak-check/leak-check-in-selection-5-ambig.rs` and + // `tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.rs`. + let region_constraints = if certainty == Certainty::Yes { + // Cannot use `take_registered_region_obligations` as we may compute the response + // inside of a `probe` whenever we have multiple choices inside of the solver. + let region_obligations = self.infcx.inner.borrow().region_obligations().to_owned(); + let mut region_constraints = self.infcx.with_region_constraints(|region_constraints| { + make_query_region_constraints( + self.tcx(), + region_obligations.iter().map(|r_o| { + (r_o.sup_type, r_o.sub_region, r_o.origin.to_constraint_category()) + }), + region_constraints, + ) + }); + + let mut seen = FxHashSet::default(); + region_constraints.outlives.retain(|outlives| seen.insert(*outlives)); + region_constraints + } else { + Default::default() + }; let mut opaque_types = self.infcx.clone_opaque_types_for_query_response(); // Only return opaque type keys for newly-defined opaques @@ -201,7 +215,7 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> { self.predefined_opaques_in_body.opaque_types.iter().all(|(pa, _)| pa != a) }); - Ok(ExternalConstraintsData { region_constraints, opaque_types, normalization_nested_goals }) + ExternalConstraintsData { region_constraints, opaque_types, normalization_nested_goals } } /// After calling a canonical query, we apply the constraints returned diff --git a/tests/ui/higher-ranked/leak-check/leak-check-in-selection-5-ambig.rs b/tests/ui/higher-ranked/leak-check/leak-check-in-selection-5-ambig.rs new file mode 100644 index 0000000000000..beda719ac208a --- /dev/null +++ b/tests/ui/higher-ranked/leak-check/leak-check-in-selection-5-ambig.rs @@ -0,0 +1,28 @@ +//@ revisions: old next +//@[next] compile-flags: -Znext-solver +//@ check-pass + +// The new trait solver does not return region constraints if the goal +// is still ambiguous. This causes the following test to fail with ambiguity, +// even though `(): LeakCheckFailure<'!a, V>` would return `'!a: 'static` +// which would have caused a leak check failure. + +trait Ambig {} +impl Ambig for u32 {} +impl Ambig for u16 {} + +trait Id {} +impl Id for u32 {} +impl Id for u16 {} + + +trait LeakCheckFailure<'a, V: ?Sized> {} +impl LeakCheckFailure<'static, V> for () {} + +trait Trait {} +impl Trait for () where for<'a> (): LeakCheckFailure<'a, V> {} +impl Trait for () {} +fn impls_trait, U: Id, V>() {} +fn main() { + impls_trait::<(), _, _>() +} diff --git a/tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.next.stderr b/tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.next.stderr new file mode 100644 index 0000000000000..a12e3312230af --- /dev/null +++ b/tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.next.stderr @@ -0,0 +1,22 @@ +error[E0283]: type annotations needed + --> $DIR/leak-check-in-selection-6-ambig-unify.rs:30:5 + | +LL | impls_trait::<(), _, _>() + | ^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type of the type parameter `U` declared on the function `impls_trait` + | +note: multiple `impl`s satisfying `(): Trait<_, _>` found + --> $DIR/leak-check-in-selection-6-ambig-unify.rs:26:1 + | +LL | impl Trait for () where for<'b> (): LeakCheckFailure<'static, 'b, V> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | impl Trait for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: required by a bound in `impls_trait` + --> $DIR/leak-check-in-selection-6-ambig-unify.rs:28:19 + | +LL | fn impls_trait, U: Id, V>() {} + | ^^^^^^^^^^^ required by this bound in `impls_trait` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0283`. diff --git a/tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.old.stderr b/tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.old.stderr new file mode 100644 index 0000000000000..a12e3312230af --- /dev/null +++ b/tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.old.stderr @@ -0,0 +1,22 @@ +error[E0283]: type annotations needed + --> $DIR/leak-check-in-selection-6-ambig-unify.rs:30:5 + | +LL | impls_trait::<(), _, _>() + | ^^^^^^^^^^^^^^^^^^^^^^^ cannot infer type of the type parameter `U` declared on the function `impls_trait` + | +note: multiple `impl`s satisfying `(): Trait<_, _>` found + --> $DIR/leak-check-in-selection-6-ambig-unify.rs:26:1 + | +LL | impl Trait for () where for<'b> (): LeakCheckFailure<'static, 'b, V> {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | impl Trait for () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: required by a bound in `impls_trait` + --> $DIR/leak-check-in-selection-6-ambig-unify.rs:28:19 + | +LL | fn impls_trait, U: Id, V>() {} + | ^^^^^^^^^^^ required by this bound in `impls_trait` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0283`. diff --git a/tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.rs b/tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.rs new file mode 100644 index 0000000000000..592d581695e52 --- /dev/null +++ b/tests/ui/higher-ranked/leak-check/leak-check-in-selection-6-ambig-unify.rs @@ -0,0 +1,32 @@ +//@ revisions: old next +//@[next] compile-flags: -Znext-solver + +// The new trait solver does not return region constraints if the goal +// is still ambiguous. This should cause the following test to fail with +// ambiguity as even if `(): LeakCheckFailure<'static, '!b, V>` unifies +// `'!b` with `'static`, we erase all region constraints. +// +// However, we do still unify the var_value for `'b` with `'static`, +// causing us to return this requirement via the `var_values` even if +// we don't return any region constraints. This is a bit inconsistent +// but isn't something we should really worry about imo. +trait Ambig {} +impl Ambig for u32 {} +impl Ambig for u16 {} + +trait Id {} +impl Id for u32 {} +impl Id for u16 {} + + +trait LeakCheckFailure<'a, 'b, V: ?Sized> {} +impl<'a, 'b: 'a, V: ?Sized + Ambig> LeakCheckFailure<'a, 'b, V> for () {} + +trait Trait {} +impl Trait for () where for<'b> (): LeakCheckFailure<'static, 'b, V> {} +impl Trait for () {} +fn impls_trait, U: Id, V>() {} +fn main() { + impls_trait::<(), _, _>() + //~^ ERROR type annotations needed +} diff --git a/tests/ui/traits/next-solver/normalize/ambig-goal-infer-in-type-oulives.rs b/tests/ui/traits/next-solver/normalize/ambig-goal-infer-in-type-oulives.rs new file mode 100644 index 0000000000000..18dc34f7cc437 --- /dev/null +++ b/tests/ui/traits/next-solver/normalize/ambig-goal-infer-in-type-oulives.rs @@ -0,0 +1,29 @@ +//@ check-pass +//@ compile-flags: -Znext-solver +//@ ignore-compare-mode-next-solver (explicitly enabled) + +// Regression test for an ICE when trying to bootstrap rustc +// with #125343. An ambiguous goal returned a `TypeOutlives` +// constraint referencing an inference variable. This inference +// variable was created inside of the goal, causing it to be +// unconstrained in the caller. This then caused an ICE in MIR +// borrowck. + +struct Foo(T); +trait Extend { + fn extend>(iter: I); +} + +impl Extend for Foo { + fn extend>(_: I) { + todo!() + } +} + +impl<'a, T: 'a + Copy> Extend<&'a T> for Foo { + fn extend>(iter: I) { + >::extend(iter.into_iter().copied()) + } +} + +fn main() {} From 278212342ec2b2659c0e5b6e0038642fb6f37347 Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Sat, 25 May 2024 10:38:18 +0800 Subject: [PATCH 7/9] cleanup dependence of `ExtCtxt` in transcribe when macro expansion --- compiler/rustc_expand/src/mbe/macro_rules.rs | 3 +- compiler/rustc_expand/src/mbe/transcribe.rs | 60 ++++++++++---------- 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 8f18055f83817..8349727ade7fb 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -224,7 +224,8 @@ fn expand_macro<'cx>( let arm_span = rhses[i].span(); // rhs has holes ( `$id` and `$(...)` that need filled) - let tts = match transcribe(cx, &named_matches, rhs, rhs_span, transparency) { + let id = cx.current_expansion.id; + let tts = match transcribe(psess, &named_matches, rhs, rhs_span, transparency, id) { Ok(tts) => tts, Err(err) => { let guar = err.emit(); diff --git a/compiler/rustc_expand/src/mbe/transcribe.rs b/compiler/rustc_expand/src/mbe/transcribe.rs index 3901b82eb52ec..5c2b9198a89da 100644 --- a/compiler/rustc_expand/src/mbe/transcribe.rs +++ b/compiler/rustc_expand/src/mbe/transcribe.rs @@ -1,4 +1,3 @@ -use crate::base::ExtCtxt; use crate::errors::{ CountRepetitionMisplaced, MetaVarExprUnrecognizedVar, MetaVarsDifSeqMatchers, MustRepeatOnce, NoSyntaxVarsExprRepeat, VarStillRepeating, @@ -9,12 +8,13 @@ use rustc_ast::mut_visit::{self, MutVisitor}; use rustc_ast::token::{self, Delimiter, Token, TokenKind}; use rustc_ast::tokenstream::{DelimSpacing, DelimSpan, Spacing, TokenStream, TokenTree}; use rustc_data_structures::fx::FxHashMap; -use rustc_errors::{pluralize, Diag, PResult}; +use rustc_errors::{pluralize, Diag, DiagCtxt, PResult}; use rustc_parse::parser::ParseNtResult; use rustc_span::hygiene::{LocalExpnId, Transparency}; use rustc_span::symbol::{sym, Ident, MacroRulesNormalizedIdent}; use rustc_span::{with_metavar_spans, Span, SyntaxContext}; +use rustc_session::parse::ParseSess; use smallvec::{smallvec, SmallVec}; use std::mem; @@ -99,11 +99,12 @@ impl<'a> Iterator for Frame<'a> { /// /// Along the way, we do some additional error checking. pub(super) fn transcribe<'a>( - cx: &ExtCtxt<'a>, + psess: &'a ParseSess, interp: &FxHashMap, src: &mbe::Delimited, src_span: DelimSpan, transparency: Transparency, + expand_id: LocalExpnId, ) -> PResult<'a, TokenStream> { // Nothing for us to transcribe... if src.tts.is_empty() { @@ -137,8 +138,9 @@ pub(super) fn transcribe<'a>( // again, and we are done transcribing. let mut result: Vec = Vec::new(); let mut result_stack = Vec::new(); - let mut marker = Marker(cx.current_expansion.id, transparency, Default::default()); + let mut marker = Marker(expand_id, transparency, Default::default()); + let dcx = &psess.dcx; loop { // Look at the last frame on the stack. // If it still has a TokenTree we have not looked at yet, use that tree. @@ -201,9 +203,7 @@ pub(super) fn transcribe<'a>( seq @ mbe::TokenTree::Sequence(_, seq_rep) => { match lockstep_iter_size(seq, interp, &repeats) { LockstepIterSize::Unconstrained => { - return Err(cx - .dcx() - .create_err(NoSyntaxVarsExprRepeat { span: seq.span() })); + return Err(dcx.create_err(NoSyntaxVarsExprRepeat { span: seq.span() })); } LockstepIterSize::Contradiction(msg) => { @@ -211,9 +211,9 @@ pub(super) fn transcribe<'a>( // happens when two meta-variables are used in the same repetition in a // sequence, but they come from different sequence matchers and repeat // different amounts. - return Err(cx - .dcx() - .create_err(MetaVarsDifSeqMatchers { span: seq.span(), msg })); + return Err( + dcx.create_err(MetaVarsDifSeqMatchers { span: seq.span(), msg }) + ); } LockstepIterSize::Constraint(len, _) => { @@ -227,9 +227,7 @@ pub(super) fn transcribe<'a>( // FIXME: this really ought to be caught at macro definition // time... It happens when the Kleene operator in the matcher and // the body for the same meta-variable do not match. - return Err(cx - .dcx() - .create_err(MustRepeatOnce { span: sp.entire() })); + return Err(dcx.create_err(MustRepeatOnce { span: sp.entire() })); } } else { // 0 is the initial counter (we have done 0 repetitions so far). `len` @@ -259,7 +257,7 @@ pub(super) fn transcribe<'a>( MatchedSingle(ParseNtResult::Tt(tt)) => { // `tt`s are emitted into the output stream directly as "raw tokens", // without wrapping them into groups. - maybe_use_metavar_location(cx, &stack, sp, tt, &mut marker) + maybe_use_metavar_location(psess, &stack, sp, tt, &mut marker) } MatchedSingle(ParseNtResult::Ident(ident, is_raw)) => { marker.visit_span(&mut sp); @@ -280,7 +278,7 @@ pub(super) fn transcribe<'a>( } MatchedSeq(..) => { // We were unable to descend far enough. This is an error. - return Err(cx.dcx().create_err(VarStillRepeating { span: sp, ident })); + return Err(dcx.create_err(VarStillRepeating { span: sp, ident })); } }; result.push(tt) @@ -299,7 +297,7 @@ pub(super) fn transcribe<'a>( // Replace meta-variable expressions with the result of their expansion. mbe::TokenTree::MetaVarExpr(sp, expr) => { - transcribe_metavar_expr(cx, expr, interp, &mut marker, &repeats, &mut result, sp)?; + transcribe_metavar_expr(dcx, expr, interp, &mut marker, &repeats, &mut result, sp)?; } // If we are entering a new delimiter, we push its contents to the `stack` to be @@ -359,7 +357,7 @@ pub(super) fn transcribe<'a>( /// combine with each other and not with tokens outside of the sequence. /// - The metavariable span comes from a different crate, then we prefer the more local span. fn maybe_use_metavar_location( - cx: &ExtCtxt<'_>, + psess: &ParseSess, stack: &[Frame<'_>], mut metavar_span: Span, orig_tt: &TokenTree, @@ -397,7 +395,7 @@ fn maybe_use_metavar_location( && insert(mspans, dspan.entire(), metavar_span) }), }; - if no_collision || cx.source_map().is_imported(metavar_span) { + if no_collision || psess.source_map().is_imported(metavar_span) { return orig_tt.clone(); } @@ -558,7 +556,7 @@ fn lockstep_iter_size( /// * `[ $( ${count(foo, 1)} ),* ]` will return an error because `${count(foo, 1)}` is /// declared inside a single repetition and the index `1` implies two nested repetitions. fn count_repetitions<'a>( - cx: &ExtCtxt<'a>, + dcx: &'a DiagCtxt, depth_user: usize, mut matched: &NamedMatch, repeats: &[(usize, usize)], @@ -595,7 +593,7 @@ fn count_repetitions<'a>( .and_then(|el| el.checked_sub(repeats.len())) .unwrap_or_default(); if depth_user > depth_max { - return Err(out_of_bounds_err(cx, depth_max + 1, sp.entire(), "count")); + return Err(out_of_bounds_err(dcx, depth_max + 1, sp.entire(), "count")); } // `repeats` records all of the nested levels at which we are currently @@ -611,7 +609,7 @@ fn count_repetitions<'a>( } if let MatchedSingle(_) = matched { - return Err(cx.dcx().create_err(CountRepetitionMisplaced { span: sp.entire() })); + return Err(dcx.create_err(CountRepetitionMisplaced { span: sp.entire() })); } count(depth_user, depth_max, matched) @@ -619,7 +617,7 @@ fn count_repetitions<'a>( /// Returns a `NamedMatch` item declared on the LHS given an arbitrary [Ident] fn matched_from_ident<'ctx, 'interp, 'rslt>( - cx: &ExtCtxt<'ctx>, + dcx: &'ctx DiagCtxt, ident: Ident, interp: &'interp FxHashMap, ) -> PResult<'ctx, &'rslt NamedMatch> @@ -628,12 +626,12 @@ where { let span = ident.span; let key = MacroRulesNormalizedIdent::new(ident); - interp.get(&key).ok_or_else(|| cx.dcx().create_err(MetaVarExprUnrecognizedVar { span, key })) + interp.get(&key).ok_or_else(|| dcx.create_err(MetaVarExprUnrecognizedVar { span, key })) } /// Used by meta-variable expressions when an user input is out of the actual declared bounds. For /// example, index(999999) in an repetition of only three elements. -fn out_of_bounds_err<'a>(cx: &ExtCtxt<'a>, max: usize, span: Span, ty: &str) -> Diag<'a> { +fn out_of_bounds_err<'a>(dcx: &'a DiagCtxt, max: usize, span: Span, ty: &str) -> Diag<'a> { let msg = if max == 0 { format!( "meta-variable expression `{ty}` with depth parameter \ @@ -645,11 +643,11 @@ fn out_of_bounds_err<'a>(cx: &ExtCtxt<'a>, max: usize, span: Span, ty: &str) -> must be less than {max}" ) }; - cx.dcx().struct_span_err(span, msg) + dcx.struct_span_err(span, msg) } fn transcribe_metavar_expr<'a>( - cx: &ExtCtxt<'a>, + dcx: &'a DiagCtxt, expr: &MetaVarExpr, interp: &FxHashMap, marker: &mut Marker, @@ -664,8 +662,8 @@ fn transcribe_metavar_expr<'a>( }; match *expr { MetaVarExpr::Count(original_ident, depth) => { - let matched = matched_from_ident(cx, original_ident, interp)?; - let count = count_repetitions(cx, depth, matched, repeats, sp)?; + let matched = matched_from_ident(dcx, original_ident, interp)?; + let count = count_repetitions(dcx, depth, matched, repeats, sp)?; let tt = TokenTree::token_alone( TokenKind::lit(token::Integer, sym::integer(count), None), visited_span(), @@ -674,7 +672,7 @@ fn transcribe_metavar_expr<'a>( } MetaVarExpr::Ignore(original_ident) => { // Used to ensure that `original_ident` is present in the LHS - let _ = matched_from_ident(cx, original_ident, interp)?; + let _ = matched_from_ident(dcx, original_ident, interp)?; } MetaVarExpr::Index(depth) => match repeats.iter().nth_back(depth) { Some((index, _)) => { @@ -683,7 +681,7 @@ fn transcribe_metavar_expr<'a>( visited_span(), )); } - None => return Err(out_of_bounds_err(cx, repeats.len(), sp.entire(), "index")), + None => return Err(out_of_bounds_err(dcx, repeats.len(), sp.entire(), "index")), }, MetaVarExpr::Len(depth) => match repeats.iter().nth_back(depth) { Some((_, length)) => { @@ -692,7 +690,7 @@ fn transcribe_metavar_expr<'a>( visited_span(), )); } - None => return Err(out_of_bounds_err(cx, repeats.len(), sp.entire(), "len")), + None => return Err(out_of_bounds_err(dcx, repeats.len(), sp.entire(), "len")), }, } Ok(()) From d7248d7b71fd756bef62e98b52ee1110f8d3a3c4 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Fri, 24 May 2024 00:40:39 -0700 Subject: [PATCH 8/9] Stop SRoA'ing `DynMetadata` in MIR --- compiler/rustc_mir_transform/src/sroa.rs | 5 +++++ tests/ui/mir/dyn_metadata_sroa.rs | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 tests/ui/mir/dyn_metadata_sroa.rs diff --git a/compiler/rustc_mir_transform/src/sroa.rs b/compiler/rustc_mir_transform/src/sroa.rs index cdf3305b56096..f19c34cae7a70 100644 --- a/compiler/rustc_mir_transform/src/sroa.rs +++ b/compiler/rustc_mir_transform/src/sroa.rs @@ -70,6 +70,11 @@ fn escaping_locals<'tcx>( // Exclude #[repr(simd)] types so that they are not de-optimized into an array return true; } + if Some(def.did()) == tcx.lang_items().dyn_metadata() { + // codegen wants to see the `DynMetadata`, + // not the inner reference-to-opaque-type. + return true; + } // We already excluded unions and enums, so this ADT must have one variant let variant = def.variant(FIRST_VARIANT); if variant.fields.len() > 1 { diff --git a/tests/ui/mir/dyn_metadata_sroa.rs b/tests/ui/mir/dyn_metadata_sroa.rs new file mode 100644 index 0000000000000..1a00c0f0a3e34 --- /dev/null +++ b/tests/ui/mir/dyn_metadata_sroa.rs @@ -0,0 +1,19 @@ +//@ run-pass +//@ compile-flags: -Zmir-opt-level=5 -Zvalidate-mir + +#![feature(ptr_metadata)] + +// Regression for , +// which failed because of SRoA would project into `DynMetadata`. + +trait Foo {} + +struct Bar; + +impl Foo for Bar {} + +fn main() { + let a: *mut dyn Foo = &mut Bar; + + let _d = a.to_raw_parts().0 as usize; +} From bebcb4e4b85f0414da3aef50dfc5abded1ad9f55 Mon Sep 17 00:00:00 2001 From: Urgau Date: Sat, 25 May 2024 13:56:19 +0200 Subject: [PATCH 9/9] Also mention my-self for check-cfg docs changes --- triagebot.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/triagebot.toml b/triagebot.toml index 2e45b257f8126..303c2fccc0754 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -794,6 +794,9 @@ cc = ["@rust-lang/project-exploit-mitigations", "@rcvalle"] [mentions."src/doc/rustc/src/check-cfg.md"] cc = ["@Urgau"] +[mentions."src/doc/rustc/src/check-cfg"] +cc = ["@Urgau"] + [mentions."src/doc/rustc/src/platform-support"] cc = ["@Nilstrieb"]