From 79ad512ec0de1265c21cbd943f2273449f7e874d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 21 Nov 2023 10:54:47 +0100 Subject: [PATCH 01/15] warn against using intrinsics that leave the scope of our memory model --- library/core/src/intrinsics.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index f25ca9e2b18..214c8e49a5a 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -341,6 +341,9 @@ extern "rust-intrinsic" { /// [`Ordering::Relaxed`] as the `order`. For example, [`AtomicBool::load`]. #[rustc_nounwind] pub fn atomic_load_relaxed(src: *const T) -> T; + /// Do NOT use this intrinsic; "unordered" operations do not exist in our memory model! + /// In terms of the Rust Abstract Machine, this operation is equivalent to `src.read()`, + /// i.e., it performs a non-atomic read. #[rustc_nounwind] pub fn atomic_load_unordered(src: *const T) -> T; @@ -365,6 +368,9 @@ extern "rust-intrinsic" { /// [`Ordering::Relaxed`] as the `order`. For example, [`AtomicBool::store`]. #[rustc_nounwind] pub fn atomic_store_relaxed(dst: *mut T, val: T); + /// Do NOT use this intrinsic; "unordered" operations do not exist in our memory model! + /// In terms of the Rust Abstract Machine, this operation is equivalent to `dst.write(val)`, + /// i.e., it performs a non-atomic write. #[rustc_nounwind] pub fn atomic_store_unordered(dst: *mut T, val: T); @@ -2312,6 +2318,10 @@ extern "rust-intrinsic" { /// Emits a `!nontemporal` store according to LLVM (see their docs). /// Probably will never become stable. + /// + /// Do NOT use this intrinsic; "nontemporal" operations do not exist in our memory model! + /// It exists to support current stdarch, but the plan is to change stdarch and remove this intrinsic. + /// See for some more discussion. #[rustc_nounwind] pub fn nontemporal_store(ptr: *mut T, val: T); From 45efb8e6a68081462a70fa8cb40510d50ab848a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 19 Nov 2023 21:01:57 +0000 Subject: [PATCH 02/15] Provide structured suggestion for type mismatch in loop We currently provide only a `help` message, this PR introduces the last two structured suggestions instead: ``` error[E0308]: mismatched types --> $DIR/issue-98982.rs:2:5 | LL | fn foo() -> i32 { | --- expected `i32` because of return type LL | / for i in 0..0 { LL | | return i; LL | | } | |_____^ expected `i32`, found `()` | note: the function expects a value to always be returned, but loops might run zero times --> $DIR/issue-98982.rs:2:5 | LL | for i in 0..0 { | ^^^^^^^^^^^^^ this might have zero elements to iterate on LL | return i; | -------- if the loop doesn't execute, this value would never get returned help: return a value for the case when the loop has zero elements to iterate on | LL ~ } LL ~ /* `i32` value */ | help: otherwise consider changing the return type to account for that possibility | LL ~ fn foo() -> Option { LL | for i in 0..0 { LL ~ return Some(i); LL ~ } LL ~ None | ``` Fix #98982. --- compiler/rustc_hir_typeck/src/coercion.rs | 99 ++++++++++++++++--- .../break-while-condition.stderr | 2 +- tests/ui/typeck/issue-100285.rs | 8 +- tests/ui/typeck/issue-100285.stderr | 31 +++++- tests/ui/typeck/issue-98982.rs | 8 +- tests/ui/typeck/issue-98982.stderr | 16 ++- 6 files changed, 134 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 971f10631df..6a07966e13c 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -36,7 +36,9 @@ //! ``` use crate::FnCtxt; -use rustc_errors::{struct_span_err, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan}; +use rustc_errors::{ + struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, MultiSpan, +}; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{self, Visitor}; @@ -53,8 +55,7 @@ use rustc_middle::ty::adjustment::{ use rustc_middle::ty::error::TypeError; use rustc_middle::ty::relate::RelateResult; use rustc_middle::ty::visit::TypeVisitableExt; -use rustc_middle::ty::GenericArgsRef; -use rustc_middle::ty::{self, Ty, TypeAndMut}; +use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeAndMut}; use rustc_session::parse::feature_err; use rustc_span::symbol::sym; use rustc_span::{self, DesugaringKind}; @@ -1660,12 +1661,15 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { None, Some(coercion_error), ); - } - - if visitor.ret_exprs.len() > 0 - && let Some(expr) = expression - { - self.note_unreachable_loop_return(&mut err, expr, &visitor.ret_exprs); + if visitor.ret_exprs.len() > 0 { + self.note_unreachable_loop_return( + &mut err, + fcx.tcx, + &expr, + &visitor.ret_exprs, + expected, + ); + } } let reported = err.emit_unless(unsized_return); @@ -1678,8 +1682,10 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { fn note_unreachable_loop_return( &self, err: &mut Diagnostic, + tcx: TyCtxt<'tcx>, expr: &hir::Expr<'tcx>, ret_exprs: &Vec<&'tcx hir::Expr<'tcx>>, + ty: Ty<'tcx>, ) { let hir::ExprKind::Loop(_, _, _, loop_span) = expr.kind else { return; @@ -1704,10 +1710,77 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> { ret_exprs.len() - MAXITER )); } - err.help( - "return a value for the case when the loop has zero elements to iterate on, or \ - consider changing the return type to account for that possibility", - ); + let hir = tcx.hir(); + let item = hir.get_parent_item(expr.hir_id); + let ret_msg = "return a value for the case when the loop has zero elements to iterate on"; + let ret_ty_msg = + "otherwise consider changing the return type to account for that possibility"; + if let Some(node) = hir.find(item.into()) + && let Some(body_id) = node.body_id() + && let Some(sig) = node.fn_sig() + && let hir::ExprKind::Block(block, _) = hir.body(body_id).value.kind + && !ty.is_never() + { + let indentation = if let None = block.expr + && let [.., last] = &block.stmts[..] + { + tcx.sess.source_map().indentation_before(last.span).unwrap_or_else(String::new) + } else if let Some(expr) = block.expr { + tcx.sess.source_map().indentation_before(expr.span).unwrap_or_else(String::new) + } else { + String::new() + }; + if let None = block.expr + && let [.., last] = &block.stmts[..] + { + err.span_suggestion_verbose( + last.span.shrink_to_hi(), + ret_msg, + format!("\n{indentation}/* `{ty}` value */"), + Applicability::MaybeIncorrect, + ); + } else if let Some(expr) = block.expr { + err.span_suggestion_verbose( + expr.span.shrink_to_hi(), + ret_msg, + format!("\n{indentation}/* `{ty}` value */"), + Applicability::MaybeIncorrect, + ); + } + let mut sugg = match sig.decl.output { + hir::FnRetTy::DefaultReturn(span) => { + vec![(span, " -> Option<()>".to_string())] + } + hir::FnRetTy::Return(ty) => { + vec![ + (ty.span.shrink_to_lo(), "Option<".to_string()), + (ty.span.shrink_to_hi(), ">".to_string()), + ] + } + }; + for ret_expr in ret_exprs { + match ret_expr.kind { + hir::ExprKind::Ret(Some(expr)) => { + sugg.push((expr.span.shrink_to_lo(), "Some(".to_string())); + sugg.push((expr.span.shrink_to_hi(), ")".to_string())); + } + hir::ExprKind::Ret(None) => { + sugg.push((ret_expr.span.shrink_to_hi(), " Some(())".to_string())); + } + _ => {} + } + } + if let None = block.expr + && let [.., last] = &block.stmts[..] + { + sugg.push((last.span.shrink_to_hi(), format!("\n{indentation}None"))); + } else if let Some(expr) = block.expr { + sugg.push((expr.span.shrink_to_hi(), format!("\n{indentation}None"))); + } + err.multipart_suggestion(ret_ty_msg, sugg, Applicability::MaybeIncorrect); + } else { + err.help(format!("{ret_msg}, {ret_ty_msg}")); + } } fn report_return_mismatched_types<'a>( diff --git a/tests/ui/for-loop-while/break-while-condition.stderr b/tests/ui/for-loop-while/break-while-condition.stderr index e79f6a75fde..48b29f44fa1 100644 --- a/tests/ui/for-loop-while/break-while-condition.stderr +++ b/tests/ui/for-loop-while/break-while-condition.stderr @@ -38,7 +38,7 @@ LL | while false { | ^^^^^^^^^^^ this might have zero elements to iterate on LL | return | ------ if the loop doesn't execute, this value would never get returned - = help: return a value for the case when the loop has zero elements to iterate on, or consider changing the return type to account for that possibility + = help: return a value for the case when the loop has zero elements to iterate on, otherwise consider changing the return type to account for that possibility error: aborting due to 3 previous errors diff --git a/tests/ui/typeck/issue-100285.rs b/tests/ui/typeck/issue-100285.rs index e206469b85f..460e0457105 100644 --- a/tests/ui/typeck/issue-100285.rs +++ b/tests/ui/typeck/issue-100285.rs @@ -1,6 +1,5 @@ -fn foo(n: i32) -> i32 { - for i in 0..0 { - //~^ ERROR: mismatched types [E0308] +fn foo(n: i32) -> i32 { //~ HELP otherwise consider changing the return type to account for that possibility + for i in 0..0 { //~ ERROR mismatched types [E0308] if n < 0 { return i; } else if n < 10 { @@ -15,8 +14,7 @@ fn foo(n: i32) -> i32 { return 5; } - } - //~| help: return a value for the case when the loop has zero elements to iterate on, or consider changing the return type to account for that possibility + } //~ HELP return a value for the case when the loop has zero elements to iterate on } fn main() {} diff --git a/tests/ui/typeck/issue-100285.stderr b/tests/ui/typeck/issue-100285.stderr index 42c64b03918..383b9f2563e 100644 --- a/tests/ui/typeck/issue-100285.stderr +++ b/tests/ui/typeck/issue-100285.stderr @@ -4,9 +4,9 @@ error[E0308]: mismatched types LL | fn foo(n: i32) -> i32 { | --- expected `i32` because of return type LL | / for i in 0..0 { -LL | | LL | | if n < 0 { LL | | return i; +LL | | } else if n < 10 { ... | LL | | LL | | } @@ -17,7 +17,7 @@ note: the function expects a value to always be returned, but loops might run ze | LL | for i in 0..0 { | ^^^^^^^^^^^^^ this might have zero elements to iterate on -... +LL | if n < 0 { LL | return i; | -------- if the loop doesn't execute, this value would never get returned LL | } else if n < 10 { @@ -27,7 +27,32 @@ LL | } else if n < 20 { LL | return 2; | -------- if the loop doesn't execute, this value would never get returned = note: if the loop doesn't execute, 3 other values would never get returned - = help: return a value for the case when the loop has zero elements to iterate on, or consider changing the return type to account for that possibility +help: return a value for the case when the loop has zero elements to iterate on + | +LL ~ } +LL ~ /* `i32` value */ + | +help: otherwise consider changing the return type to account for that possibility + | +LL ~ fn foo(n: i32) -> Option { +LL | for i in 0..0 { +LL | if n < 0 { +LL ~ return Some(i); +LL | } else if n < 10 { +LL ~ return Some(1); +LL | } else if n < 20 { +LL ~ return Some(2); +LL | } else if n < 30 { +LL ~ return Some(3); +LL | } else if n < 40 { +LL ~ return Some(4); +LL | } else { +LL ~ return Some(5); +LL | } +LL | +LL ~ } +LL ~ None + | error: aborting due to previous error diff --git a/tests/ui/typeck/issue-98982.rs b/tests/ui/typeck/issue-98982.rs index 2553824bbfe..f875d20fa4c 100644 --- a/tests/ui/typeck/issue-98982.rs +++ b/tests/ui/typeck/issue-98982.rs @@ -1,9 +1,7 @@ -fn foo() -> i32 { - for i in 0..0 { - //~^ ERROR: mismatched types [E0308] +fn foo() -> i32 { //~ HELP otherwise consider changing the return type to account for that possibility + for i in 0..0 { //~ ERROR mismatched types [E0308] return i; - } - //~| help: return a value for the case when the loop has zero elements to iterate on, or consider changing the return type to account for that possibility + } //~ HELP return a value for the case when the loop has zero elements to iterate on } fn main() {} diff --git a/tests/ui/typeck/issue-98982.stderr b/tests/ui/typeck/issue-98982.stderr index 3c9806ac965..9c0ca4d8360 100644 --- a/tests/ui/typeck/issue-98982.stderr +++ b/tests/ui/typeck/issue-98982.stderr @@ -4,7 +4,6 @@ error[E0308]: mismatched types LL | fn foo() -> i32 { | --- expected `i32` because of return type LL | / for i in 0..0 { -LL | | LL | | return i; LL | | } | |_____^ expected `i32`, found `()` @@ -14,10 +13,21 @@ note: the function expects a value to always be returned, but loops might run ze | LL | for i in 0..0 { | ^^^^^^^^^^^^^ this might have zero elements to iterate on -LL | LL | return i; | -------- if the loop doesn't execute, this value would never get returned - = help: return a value for the case when the loop has zero elements to iterate on, or consider changing the return type to account for that possibility +help: return a value for the case when the loop has zero elements to iterate on + | +LL ~ } +LL ~ /* `i32` value */ + | +help: otherwise consider changing the return type to account for that possibility + | +LL ~ fn foo() -> Option { +LL | for i in 0..0 { +LL ~ return Some(i); +LL ~ } +LL ~ None + | error: aborting due to previous error From 45e634234690a7d061e133b8cf9d31af34b29a0b Mon Sep 17 00:00:00 2001 From: belovdv <70999565+belovdv@users.noreply.github.com> Date: Sat, 15 Jul 2023 18:48:09 +0300 Subject: [PATCH 03/15] jobserver: check file descriptors --- Cargo.lock | 4 +- compiler/rustc_codegen_ssa/Cargo.toml | 2 +- compiler/rustc_data_structures/Cargo.toml | 2 +- .../rustc_data_structures/src/jobserver.rs | 92 +++++++++++++------ compiler/rustc_session/src/session.rs | 17 +++- src/tools/miri/cargo-miri/src/phases.rs | 8 ++ tests/run-make/jobserver-error/Makefile | 12 ++- .../jobserver-error/cannot_open_fd.stderr | 6 ++ .../jobserver-error/not_a_pipe.stderr | 4 + ...{jobserver.stderr => poisoned_pipe.stderr} | 0 10 files changed, 112 insertions(+), 35 deletions(-) create mode 100644 tests/run-make/jobserver-error/cannot_open_fd.stderr create mode 100644 tests/run-make/jobserver-error/not_a_pipe.stderr rename tests/run-make/jobserver-error/{jobserver.stderr => poisoned_pipe.stderr} (100%) diff --git a/Cargo.lock b/Cargo.lock index 1296468b4e2..bbcc9f57b56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2085,9 +2085,9 @@ dependencies = [ [[package]] name = "jobserver" -version = "0.1.26" +version = "0.1.27" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "936cfd212a0155903bcbc060e316fb6cc7cbf2e1907329391ebadc1fe0ce77c2" +checksum = "8c37f63953c4c63420ed5fd3d6d398c719489b9f872b9fa683262f8edd363c7d" dependencies = [ "libc", ] diff --git a/compiler/rustc_codegen_ssa/Cargo.toml b/compiler/rustc_codegen_ssa/Cargo.toml index 0ca19e5fe4a..3f2ed257d08 100644 --- a/compiler/rustc_codegen_ssa/Cargo.toml +++ b/compiler/rustc_codegen_ssa/Cargo.toml @@ -9,7 +9,7 @@ ar_archive_writer = "0.1.5" bitflags = "1.2.1" cc = "1.0.69" itertools = "0.11" -jobserver = "0.1.22" +jobserver = "0.1.27" pathdiff = "0.2.0" regex = "1.4" rustc_arena = { path = "../rustc_arena" } diff --git a/compiler/rustc_data_structures/Cargo.toml b/compiler/rustc_data_structures/Cargo.toml index 77d9e491748..4732783a12d 100644 --- a/compiler/rustc_data_structures/Cargo.toml +++ b/compiler/rustc_data_structures/Cargo.toml @@ -11,7 +11,7 @@ elsa = "=1.7.1" ena = "0.14.2" indexmap = { version = "2.0.0" } itertools = "0.11" -jobserver_crate = { version = "0.1.13", package = "jobserver" } +jobserver_crate = { version = "0.1.27", package = "jobserver" } libc = "0.2" measureme = "10.0.0" rustc-hash = "1.1.0" diff --git a/compiler/rustc_data_structures/src/jobserver.rs b/compiler/rustc_data_structures/src/jobserver.rs index 09baa3095a4..b777bfd4d3c 100644 --- a/compiler/rustc_data_structures/src/jobserver.rs +++ b/compiler/rustc_data_structures/src/jobserver.rs @@ -1,40 +1,78 @@ pub use jobserver_crate::Client; -use std::sync::LazyLock; -// We can only call `from_env` once per process +use jobserver_crate::{FromEnv, FromEnvErrorKind}; -// Note that this is unsafe because it may misinterpret file descriptors -// on Unix as jobserver file descriptors. We hopefully execute this near -// the beginning of the process though to ensure we don't get false -// positives, or in other words we try to execute this before we open -// any file descriptors ourselves. -// -// Pick a "reasonable maximum" if we don't otherwise have -// a jobserver in our environment, capping out at 32 so we -// don't take everything down by hogging the process run queue. -// The fixed number is used to have deterministic compilation -// across machines. -// -// Also note that we stick this in a global because there could be -// multiple rustc instances in this process, and the jobserver is -// per-process. -static GLOBAL_CLIENT: LazyLock = LazyLock::new(|| unsafe { - Client::from_env().unwrap_or_else(|| { - let client = Client::new(32).expect("failed to create jobserver"); - // Acquire a token for the main thread which we can release later - client.acquire_raw().ok(); - client - }) +use std::sync::{LazyLock, OnceLock}; + +// We can only call `from_env_ext` once per process + +// We stick this in a global because there could be multiple rustc instances +// in this process, and the jobserver is per-process. +static GLOBAL_CLIENT: LazyLock> = LazyLock::new(|| { + // Note that this is unsafe because it may misinterpret file descriptors + // on Unix as jobserver file descriptors. We hopefully execute this near + // the beginning of the process though to ensure we don't get false + // positives, or in other words we try to execute this before we open + // any file descriptors ourselves. + let FromEnv { client, var } = unsafe { Client::from_env_ext(true) }; + + let error = match client { + Ok(client) => return Ok(client), + Err(e) => e, + }; + + if matches!( + error.kind(), + FromEnvErrorKind::NoEnvVar | FromEnvErrorKind::NoJobserver | FromEnvErrorKind::Unsupported + ) { + return Ok(default_client()); + } + + // Environment specifies jobserver, but it looks incorrect. + // Safety: `error.kind()` should be `NoEnvVar` if `var == None`. + let (name, value) = var.unwrap(); + Err(format!( + "failed to connect to jobserver from environment variable `{name}={:?}`: {error}", + value + )) }); +// Create a new jobserver if there's no inherited one. +fn default_client() -> Client { + // Pick a "reasonable maximum" capping out at 32 + // so we don't take everything down by hogging the process run queue. + // The fixed number is used to have deterministic compilation across machines. + let client = Client::new(32).expect("failed to create jobserver"); + + // Acquire a token for the main thread which we can release later + client.acquire_raw().ok(); + + client +} + +static GLOBAL_CLIENT_CHECKED: OnceLock = OnceLock::new(); + +pub fn check(report_warning: impl FnOnce(&'static str)) { + let client_checked = match &*GLOBAL_CLIENT { + Ok(client) => client.clone(), + Err(e) => { + report_warning(e); + default_client() + } + }; + GLOBAL_CLIENT_CHECKED.set(client_checked).ok(); +} + +const ACCESS_ERROR: &str = "jobserver check should have been called earlier"; + pub fn client() -> Client { - GLOBAL_CLIENT.clone() + GLOBAL_CLIENT_CHECKED.get().expect(ACCESS_ERROR).clone() } pub fn acquire_thread() { - GLOBAL_CLIENT.acquire_raw().ok(); + GLOBAL_CLIENT_CHECKED.get().expect(ACCESS_ERROR).acquire_raw().ok(); } pub fn release_thread() { - GLOBAL_CLIENT.release_raw().ok(); + GLOBAL_CLIENT_CHECKED.get().expect(ACCESS_ERROR).release_raw().ok(); } diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 20a67d6d036..515b412315f 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -26,7 +26,7 @@ use rustc_errors::registry::Registry; use rustc_errors::{ error_code, fallback_fluent_bundle, DiagnosticBuilder, DiagnosticId, DiagnosticMessage, ErrorGuaranteed, FluentBundle, Handler, IntoDiagnostic, LazyFallbackBundle, MultiSpan, Noted, - TerminalUrl, + SubdiagnosticMessage, TerminalUrl, }; use rustc_macros::HashStable_Generic; pub use rustc_span::def_id::StableCrateId; @@ -1479,6 +1479,11 @@ pub fn build_session( let asm_arch = if target_cfg.allow_asm { InlineAsmArch::from_str(&target_cfg.arch).ok() } else { None }; + // Check jobserver before getting `jobserver::client`. + jobserver::check(|err| { + handler.early_warn_with_note(err, "the build environment is likely misconfigured") + }); + let sess = Session { target: target_cfg, host, @@ -1762,6 +1767,16 @@ impl EarlyErrorHandler { pub fn early_warn(&self, msg: impl Into) { self.handler.struct_warn(msg).emit() } + + #[allow(rustc::untranslatable_diagnostic)] + #[allow(rustc::diagnostic_outside_of_impl)] + pub fn early_warn_with_note( + &self, + msg: impl Into, + note: impl Into, + ) { + self.handler.struct_warn(msg).note(note).emit() + } } fn mk_emitter(output: ErrorOutputType) -> Box { diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index d655df6d994..a7412f90c85 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -501,6 +501,14 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner // Set missing env vars. We prefer build-time env vars over run-time ones; see // for the kind of issue that fixes. for (name, val) in info.env { + // `CARGO_MAKEFLAGS` contains information about how to reach the + // jobserver, but by the time the program is being run, that jobserver + // no longer exists. Hence we shouldn't forward this. + // FIXME: Miri builds the final crate without a jobserver. + // This may be fixed with github.com/rust-lang/cargo/issues/12597. + if name == "CARGO_MAKEFLAGS" { + continue; + } if let Some(old_val) = env::var_os(&name) { if old_val == val { // This one did not actually change, no need to re-set it. diff --git a/tests/run-make/jobserver-error/Makefile b/tests/run-make/jobserver-error/Makefile index 4a1699cc740..a7601b86715 100644 --- a/tests/run-make/jobserver-error/Makefile +++ b/tests/run-make/jobserver-error/Makefile @@ -1,9 +1,15 @@ include ../tools.mk # only-linux -# ignore-test: This test randomly fails, see https://github.com/rust-lang/rust/issues/110321 +# ignore-cross-compile -# Test compiler behavior in case: `jobserver-auth` points to correct pipe which is not jobserver. +# Test compiler behavior in case environment specifies wrong jobserver. all: - bash -c 'echo "fn main() {}" | MAKEFLAGS="--jobserver-auth=3,3" $(RUSTC) - 3&1 | diff jobserver.stderr - + bash -c 'echo "fn main() {}" | MAKEFLAGS="--jobserver-auth=3,3" $(RUSTC)' 2>&1 | diff cannot_open_fd.stderr - + bash -c 'echo "fn main() {}" | MAKEFLAGS="--jobserver-auth=3,3" $(RUSTC) - 3&1 | diff not_a_pipe.stderr - + +# This test randomly fails, see https://github.com/rust-lang/rust/issues/110321 +disabled: + bash -c 'echo "fn main() {}" | MAKEFLAGS="--jobserver-auth=3,3" $(RUSTC) - 3< <(cat /dev/null)' 2>&1 | diff poisoned_pipe.stderr - + diff --git a/tests/run-make/jobserver-error/cannot_open_fd.stderr b/tests/run-make/jobserver-error/cannot_open_fd.stderr new file mode 100644 index 00000000000..343de5cd52c --- /dev/null +++ b/tests/run-make/jobserver-error/cannot_open_fd.stderr @@ -0,0 +1,6 @@ +warning: failed to connect to jobserver from environment variable `MAKEFLAGS="--jobserver-auth=3,3"`: cannot open file descriptor 3 from the jobserver environment variable value: Bad file descriptor (os error 9) + | + = note: the build environment is likely misconfigured + +error: no input filename given + diff --git a/tests/run-make/jobserver-error/not_a_pipe.stderr b/tests/run-make/jobserver-error/not_a_pipe.stderr new file mode 100644 index 00000000000..536c04576b9 --- /dev/null +++ b/tests/run-make/jobserver-error/not_a_pipe.stderr @@ -0,0 +1,4 @@ +warning: failed to connect to jobserver from environment variable `MAKEFLAGS="--jobserver-auth=3,3"`: file descriptor 3 from the jobserver environment variable value is not a pipe + | + = note: the build environment is likely misconfigured + diff --git a/tests/run-make/jobserver-error/jobserver.stderr b/tests/run-make/jobserver-error/poisoned_pipe.stderr similarity index 100% rename from tests/run-make/jobserver-error/jobserver.stderr rename to tests/run-make/jobserver-error/poisoned_pipe.stderr From dbea549d80674928c53fdbed3cffbc1052559330 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 30 Nov 2023 21:45:57 +0100 Subject: [PATCH 04/15] move exposed-provenance APIs into separate feature gate and explain the relationship of Exposed Provenance and Strict Provenance --- library/core/src/ptr/const_ptr.rs | 9 +- library/core/src/ptr/mod.rs | 86 +++++++++++-------- library/core/src/ptr/mut_ptr.rs | 11 +-- library/std/src/lib.rs | 1 + .../fail/provenance/ptr_int_unexposed.rs | 2 +- .../miri/tests/fail/provenance/ptr_invalid.rs | 2 +- .../fail/provenance/strict_provenance_cast.rs | 2 +- .../fail/stacked_borrows/exposed_only_ro.rs | 2 +- .../miri/tests/pass/ptr_int_from_exposed.rs | 2 +- .../tests/pass/stacked-borrows/int-to-ptr.rs | 2 +- .../pass/stacked-borrows/unknown-bottom-gc.rs | 2 +- 11 files changed, 68 insertions(+), 53 deletions(-) diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index c8a0eb4ffc2..8c9ffd18d99 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -219,7 +219,8 @@ impl *const T { /// later call [`from_exposed_addr`][] to reconstitute the original pointer including its /// provenance. (Reconstructing address space information, if required, is your responsibility.) /// - /// Using this method means that code is *not* following Strict Provenance rules. Supporting + /// Using this method means that code is *not* following [Strict + /// Provenance][../index.html#strict-provenance] rules. Supporting /// [`from_exposed_addr`][] complicates specification and reasoning and may not be supported by /// tools that help you to stay conformant with the Rust memory model, so it is recommended to /// use [`addr`][pointer::addr] wherever possible. @@ -230,13 +231,13 @@ impl *const T { /// side-effect which is required for [`from_exposed_addr`][] to work is typically not /// available. /// - /// This API and its claimed semantics are part of the Strict Provenance experiment, see the - /// [module documentation][crate::ptr] for details. + /// It is unclear whether this method can be given a satisfying unambiguous specification. This + /// API and its claimed semantics are part of [Exposed Provenance][../index.html#exposed-provenance]. /// /// [`from_exposed_addr`]: from_exposed_addr #[must_use] #[inline(always)] - #[unstable(feature = "strict_provenance", issue = "95228")] + #[unstable(feature = "exposed_provenance", issue = "95228")] pub fn expose_addr(self) -> usize { // FIXME(strict_provenance_magic): I am magic and should be a compiler intrinsic. self.cast::<()>() as usize diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index 2b21016c61d..50cf29227ca 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -312,22 +312,30 @@ //! For instance, ARM explicitly supports high-bit tagging, and so CHERI on ARM inherits //! that and should support it. //! -//! ## Pointer-usize-pointer roundtrips and 'exposed' provenance +//! ## Exposed Provenance //! -//! **This section is *non-normative* and is part of the [Strict Provenance] experiment.** +//! **This section is *non-normative* and is an extension to the [Strict Provenance] experiment.** //! //! As discussed above, pointer-usize-pointer roundtrips are not possible under [Strict Provenance]. -//! However, there exists legacy Rust code that is full of such roundtrips, and legacy platform APIs -//! regularly assume that `usize` can capture all the information that makes up a pointer. There -//! also might be code that cannot be ported to Strict Provenance (which is something we would [like -//! to hear about][Strict Provenance]). +//! This is by design: the goal of Strict Provenance is to provide a clear specification that we are +//! confident can be formalized unambiguously and can be subject to precise formal reasoning. //! -//! For situations like this, there is a fallback plan, a way to 'opt out' of Strict Provenance. -//! However, note that this makes your code a lot harder to specify, and the code will not work -//! (well) with tools like [Miri] and [CHERI]. +//! However, there exist situations where pointer-usize-pointer roundtrips cannot be avoided, or +//! where avoiding them would require major refactoring. Legacy platform APIs also regularly assume +//! that `usize` can capture all the information that makes up a pointer. The goal of Strict +//! Provenance is not to rule out such code; the goal is to put all the *other* pointer-manipulating +//! code onto a more solid foundation. Strict Provenance is about improving the situation where +//! possible (all the code that can be written with Strict Provenance) without making things worse +//! for situations where Strict Provenance is insufficient. //! -//! This fallback plan is provided by the [`expose_addr`] and [`from_exposed_addr`] methods (which -//! are equivalent to `as` casts between pointers and integers). [`expose_addr`] is a lot like +//! For these situations, there is a highly experimental extension to Strict Provenance called +//! *Exposed Provenance*. This extension permits pointer-usize-pointer roundtrips. However, its +//! semantics are on much less solid footing than Strict Provenance, and at this point it is not yet +//! clear where a satisfying unambiguous semantics can be defined for Exposed Provenance. +//! Furthermore, Exposed Provenance will not work (well) with tools like [Miri] and [CHERI]. +//! +//! Exposed Provenance is provided by the [`expose_addr`] and [`from_exposed_addr`] methods, which +//! are meant to replace `as` casts between pointers and integers. [`expose_addr`] is a lot like //! [`addr`], but additionally adds the provenance of the pointer to a global list of 'exposed' //! provenances. (This list is purely conceptual, it exists for the purpose of specifying Rust but //! is not materialized in actual executions, except in tools like [Miri].) [`from_exposed_addr`] @@ -341,10 +349,11 @@ //! there is *no* previously 'exposed' provenance that justifies the way the returned pointer will //! be used, the program has undefined behavior. //! -//! Using [`expose_addr`] or [`from_exposed_addr`] (or the equivalent `as` casts) means that code is +//! Using [`expose_addr`] or [`from_exposed_addr`] (or the `as` casts) means that code is //! *not* following Strict Provenance rules. The goal of the Strict Provenance experiment is to -//! determine whether it is possible to use Rust without [`expose_addr`] and [`from_exposed_addr`]. -//! If this is successful, it would be a major win for avoiding specification complexity and to +//! determine how far one can get in Rust without the use of [`expose_addr`] and +//! [`from_exposed_addr`], and to encourage code to be written with Strict Provenance APIs only. +//! Maximizing the amount of such code is a major win for avoiding specification complexity and to //! facilitate adoption of tools like [CHERI] and [Miri] that can be a big help in increasing the //! confidence in (unsafe) Rust code. //! @@ -619,12 +628,12 @@ pub const fn invalid_mut(addr: usize) -> *mut T { /// Convert an address back to a pointer, picking up a previously 'exposed' provenance. /// -/// This is equivalent to `addr as *const T`. The provenance of the returned pointer is that of *any* -/// pointer that was previously exposed by passing it to [`expose_addr`][pointer::expose_addr], -/// or a `ptr as usize` cast. In addition, memory which is outside the control of the Rust abstract -/// machine (MMIO registers, for example) is always considered to be exposed, so long as this memory -/// is disjoint from memory that will be used by the abstract machine such as the stack, heap, -/// and statics. +/// This is a more rigorously specified alternative to `addr as *const T`. The provenance of the +/// returned pointer is that of *any* pointer that was previously exposed by passing it to +/// [`expose_addr`][pointer::expose_addr], or a `ptr as usize` cast. In addition, memory which is +/// outside the control of the Rust abstract machine (MMIO registers, for example) is always +/// considered to be exposed, so long as this memory is disjoint from memory that will be used by +/// the abstract machine such as the stack, heap, and statics. /// /// If there is no 'exposed' provenance that justifies the way this pointer will be used, /// the program has undefined behavior. In particular, the aliasing rules still apply: pointers @@ -639,7 +648,8 @@ pub const fn invalid_mut(addr: usize) -> *mut T { /// On platforms with multiple address spaces, it is your responsibility to ensure that the /// address makes sense in the address space that this pointer will be used with. /// -/// Using this method means that code is *not* following strict provenance rules. "Guessing" a +/// Using this function means that code is *not* following [Strict +/// Provenance][../index.html#strict-provenance] rules. "Guessing" a /// suitable provenance complicates specification and reasoning and may not be supported by /// tools that help you to stay conformant with the Rust memory model, so it is recommended to /// use [`with_addr`][pointer::with_addr] wherever possible. @@ -649,13 +659,13 @@ pub const fn invalid_mut(addr: usize) -> *mut T { /// since it is generally not possible to actually *compute* which provenance the returned /// pointer has to pick up. /// -/// This API and its claimed semantics are part of the Strict Provenance experiment, see the -/// [module documentation][crate::ptr] for details. +/// It is unclear whether this function can be given a satisfying unambiguous specification. This +/// API and its claimed semantics are part of [Exposed Provenance][../index.html#exposed-provenance]. #[must_use] #[inline(always)] -#[unstable(feature = "strict_provenance", issue = "95228")] +#[unstable(feature = "exposed_provenance", issue = "95228")] #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces -#[allow(fuzzy_provenance_casts)] // this *is* the strict provenance API one should use instead +#[allow(fuzzy_provenance_casts)] // this *is* the explicit provenance API one should use instead pub fn from_exposed_addr(addr: usize) -> *const T where T: Sized, @@ -666,18 +676,20 @@ where /// Convert an address back to a mutable pointer, picking up a previously 'exposed' provenance. /// -/// This is equivalent to `addr as *mut T`. The provenance of the returned pointer is that of *any* -/// pointer that was previously passed to [`expose_addr`][pointer::expose_addr] or a `ptr as usize` -/// cast. If there is no previously 'exposed' provenance that justifies the way this pointer will be -/// used, the program has undefined behavior. Note that there is no algorithm that decides which -/// provenance will be used. You can think of this as "guessing" the right provenance, and the guess -/// will be "maximally in your favor", in the sense that if there is any way to avoid undefined -/// behavior, then that is the guess that will be taken. +/// This is a more rigorously specified alternative to `addr as *mut T`. The provenance of the +/// returned pointer is that of *any* pointer that was previously passed to +/// [`expose_addr`][pointer::expose_addr] or a `ptr as usize` cast. If there is no previously +/// 'exposed' provenance that justifies the way this pointer will be used, the program has undefined +/// behavior. Note that there is no algorithm that decides which provenance will be used. You can +/// think of this as "guessing" the right provenance, and the guess will be "maximally in your +/// favor", in the sense that if there is any way to avoid undefined behavior, then that is the +/// guess that will be taken. /// /// On platforms with multiple address spaces, it is your responsibility to ensure that the /// address makes sense in the address space that this pointer will be used with. /// -/// Using this method means that code is *not* following strict provenance rules. "Guessing" a +/// Using this function means that code is *not* following [Strict +/// Provenance][../index.html#strict-provenance] rules. "Guessing" a /// suitable provenance complicates specification and reasoning and may not be supported by /// tools that help you to stay conformant with the Rust memory model, so it is recommended to /// use [`with_addr`][pointer::with_addr] wherever possible. @@ -687,13 +699,13 @@ where /// since it is generally not possible to actually *compute* which provenance the returned /// pointer has to pick up. /// -/// This API and its claimed semantics are part of the Strict Provenance experiment, see the -/// [module documentation][crate::ptr] for details. +/// It is unclear whether this function can be given a satisfying unambiguous specification. This +/// API and its claimed semantics are part of [Exposed Provenance][../index.html#exposed-provenance]. #[must_use] #[inline(always)] -#[unstable(feature = "strict_provenance", issue = "95228")] +#[unstable(feature = "exposed_provenance", issue = "95228")] #[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces -#[allow(fuzzy_provenance_casts)] // this *is* the strict provenance API one should use instead +#[allow(fuzzy_provenance_casts)] // this *is* the explicit provenance API one should use instead pub fn from_exposed_addr_mut(addr: usize) -> *mut T where T: Sized, diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index ce0e6d6f297..194492a300b 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -226,7 +226,8 @@ impl *mut T { /// later call [`from_exposed_addr_mut`][] to reconstitute the original pointer including its /// provenance. (Reconstructing address space information, if required, is your responsibility.) /// - /// Using this method means that code is *not* following Strict Provenance rules. Supporting + /// Using this method means that code is *not* following [Strict + /// Provenance][../index.html#strict-provenance] rules. Supporting /// [`from_exposed_addr_mut`][] complicates specification and reasoning and may not be supported /// by tools that help you to stay conformant with the Rust memory model, so it is recommended /// to use [`addr`][pointer::addr] wherever possible. @@ -237,13 +238,13 @@ impl *mut T { /// side-effect which is required for [`from_exposed_addr_mut`][] to work is typically not /// available. /// - /// This API and its claimed semantics are part of the Strict Provenance experiment, see the - /// [module documentation][crate::ptr] for details. + /// It is unclear whether this method can be given a satisfying unambiguous specification. This + /// API and its claimed semantics are part of [Exposed Provenance][../index.html#exposed-provenance]. /// /// [`from_exposed_addr_mut`]: from_exposed_addr_mut #[must_use] #[inline(always)] - #[unstable(feature = "strict_provenance", issue = "95228")] + #[unstable(feature = "exposed_provenance", issue = "95228")] pub fn expose_addr(self) -> usize { // FIXME(strict_provenance_magic): I am magic and should be a compiler intrinsic. self.cast::<()>() as usize @@ -259,7 +260,7 @@ impl *mut T { /// This is equivalent to using [`wrapping_offset`][pointer::wrapping_offset] to offset /// `self` to the given address, and therefore has all the same capabilities and restrictions. /// - /// This API and its claimed semantics are part of the Strict Provenance experiment, + /// This API and its claimed semantics are an extension to the Strict Provenance experiment, /// see the [module documentation][crate::ptr] for details. #[must_use] #[inline] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 8dc5b07ce10..21ca5d12559 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -317,6 +317,7 @@ #![feature(error_iter)] #![feature(exact_size_is_empty)] #![feature(exclusive_wrapper)] +#![feature(exposed_provenance)] #![feature(extend_one)] #![feature(float_gamma)] #![feature(float_minimum_maximum)] diff --git a/src/tools/miri/tests/fail/provenance/ptr_int_unexposed.rs b/src/tools/miri/tests/fail/provenance/ptr_int_unexposed.rs index 07de41d10a0..20fd3306998 100644 --- a/src/tools/miri/tests/fail/provenance/ptr_int_unexposed.rs +++ b/src/tools/miri/tests/fail/provenance/ptr_int_unexposed.rs @@ -1,5 +1,5 @@ //@compile-flags: -Zmiri-permissive-provenance -#![feature(strict_provenance)] +#![feature(strict_provenance, exposed_provenance)] fn main() { let x: i32 = 3; diff --git a/src/tools/miri/tests/fail/provenance/ptr_invalid.rs b/src/tools/miri/tests/fail/provenance/ptr_invalid.rs index d7d32d83e07..5d44928d1d2 100644 --- a/src/tools/miri/tests/fail/provenance/ptr_invalid.rs +++ b/src/tools/miri/tests/fail/provenance/ptr_invalid.rs @@ -1,4 +1,4 @@ -#![feature(strict_provenance)] +#![feature(strict_provenance, exposed_provenance)] // Ensure that a `ptr::invalid` ptr is truly invalid. fn main() { diff --git a/src/tools/miri/tests/fail/provenance/strict_provenance_cast.rs b/src/tools/miri/tests/fail/provenance/strict_provenance_cast.rs index 04552d0c332..106cf4d804b 100644 --- a/src/tools/miri/tests/fail/provenance/strict_provenance_cast.rs +++ b/src/tools/miri/tests/fail/provenance/strict_provenance_cast.rs @@ -1,5 +1,5 @@ //@compile-flags: -Zmiri-strict-provenance -#![feature(strict_provenance)] +#![feature(exposed_provenance)] fn main() { let addr = &0 as *const i32 as usize; diff --git a/src/tools/miri/tests/fail/stacked_borrows/exposed_only_ro.rs b/src/tools/miri/tests/fail/stacked_borrows/exposed_only_ro.rs index 0b4fb0ccd33..b0e4cceb98f 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/exposed_only_ro.rs +++ b/src/tools/miri/tests/fail/stacked_borrows/exposed_only_ro.rs @@ -1,5 +1,5 @@ //@compile-flags: -Zmiri-permissive-provenance -#![feature(strict_provenance)] +#![feature(exposed_provenance)] // If we have only exposed read-only pointers, doing a write through a wildcard ptr should fail. diff --git a/src/tools/miri/tests/pass/ptr_int_from_exposed.rs b/src/tools/miri/tests/pass/ptr_int_from_exposed.rs index 35a52d0220b..d8d57679e6b 100644 --- a/src/tools/miri/tests/pass/ptr_int_from_exposed.rs +++ b/src/tools/miri/tests/pass/ptr_int_from_exposed.rs @@ -1,7 +1,7 @@ //@revisions: stack tree //@[tree]compile-flags: -Zmiri-tree-borrows //@compile-flags: -Zmiri-permissive-provenance -#![feature(strict_provenance)] +#![feature(strict_provenance, exposed_provenance)] use std::ptr; diff --git a/src/tools/miri/tests/pass/stacked-borrows/int-to-ptr.rs b/src/tools/miri/tests/pass/stacked-borrows/int-to-ptr.rs index 9e604f9abb8..e467356dd04 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/int-to-ptr.rs +++ b/src/tools/miri/tests/pass/stacked-borrows/int-to-ptr.rs @@ -1,5 +1,5 @@ //@compile-flags: -Zmiri-permissive-provenance -#![feature(strict_provenance)] +#![feature(exposed_provenance)] use std::ptr; // Just to make sure that casting a ref to raw, to int and back to raw diff --git a/src/tools/miri/tests/pass/stacked-borrows/unknown-bottom-gc.rs b/src/tools/miri/tests/pass/stacked-borrows/unknown-bottom-gc.rs index e62ee528686..5bb4e879c3e 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/unknown-bottom-gc.rs +++ b/src/tools/miri/tests/pass/stacked-borrows/unknown-bottom-gc.rs @@ -1,5 +1,5 @@ //@compile-flags: -Zmiri-permissive-provenance -#![feature(strict_provenance)] +#![feature(exposed_provenance)] use std::ptr; From f2dc18d0a1183bb7f9f8ff09f3c1abcd85ae851a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 1 Dec 2023 08:35:23 +0100 Subject: [PATCH 05/15] update addr docs --- library/core/src/ptr/const_ptr.rs | 8 ++++---- library/core/src/ptr/mut_ptr.rs | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 8c9ffd18d99..5b5284d8c4c 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -186,10 +186,10 @@ impl *const T { /// [`with_addr`][pointer::with_addr] or [`map_addr`][pointer::map_addr]. /// /// If using those APIs is not possible because there is no way to preserve a pointer with the - /// required provenance, use [`expose_addr`][pointer::expose_addr] and - /// [`from_exposed_addr`][from_exposed_addr] instead. However, note that this makes - /// your code less portable and less amenable to tools that check for compliance with the Rust - /// memory model. + /// required provenance, then Strict Provenance might not be for you. Use pointer-integer casts + /// or [`expose_addr`][pointer::expose_addr] and [`from_exposed_addr`][from_exposed_addr] + /// instead. However, note that this makes your code less portable and less amenable to tools + /// that check for compliance with the Rust memory model. /// /// On most platforms this will produce a value with the same bytes as the original /// pointer, because all the bytes are dedicated to describing the address. diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index 194492a300b..a9208698a29 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -193,10 +193,10 @@ impl *mut T { /// [`with_addr`][pointer::with_addr] or [`map_addr`][pointer::map_addr]. /// /// If using those APIs is not possible because there is no way to preserve a pointer with the - /// required provenance, use [`expose_addr`][pointer::expose_addr] and - /// [`from_exposed_addr_mut`][from_exposed_addr_mut] instead. However, note that this makes - /// your code less portable and less amenable to tools that check for compliance with the Rust - /// memory model. + /// required provenance, then Strict Provenance might not be for you. Use pointer-integer casts + /// or [`expose_addr`][pointer::expose_addr] and [`from_exposed_addr`][from_exposed_addr] + /// instead. However, note that this makes your code less portable and less amenable to tools + /// that check for compliance with the Rust memory model. /// /// On most platforms this will produce a value with the same bytes as the original /// pointer, because all the bytes are dedicated to describing the address. From 05bf5b764ab032c7173810dadc3111f6041492a7 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 13 Nov 2023 11:59:52 +0100 Subject: [PATCH 06/15] Add highlighting for comments in items declaration --- src/librustdoc/html/render/print_item.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index e4309c782f6..131b1d608e6 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -1509,7 +1509,7 @@ fn print_tuple_struct_fields<'a, 'cx: 'a>( matches!(*field.kind, clean::StrippedItem(box clean::StructFieldItem(..))) }) { - return f.write_str("/* private fields */"); + return f.write_str("/* private fields */"); } for (i, ty) in s.iter().enumerate() { @@ -1666,7 +1666,7 @@ fn render_enum_fields( } if variants_stripped && !is_non_exhaustive { - w.write_str(" // some variants omitted\n"); + w.write_str(" // some variants omitted\n"); } if toggle { toggle_close(&mut w); @@ -1811,7 +1811,8 @@ fn item_proc_macro( let name = it.name.expect("proc-macros always have names"); match m.kind { MacroKind::Bang => { - write!(buffer, "{name}!() {{ /* proc-macro */ }}").unwrap(); + write!(buffer, "{name}!() {{ /* proc-macro */ }}") + .unwrap(); } MacroKind::Attr => { write!(buffer, "#[{name}]").unwrap(); @@ -1819,7 +1820,12 @@ fn item_proc_macro( MacroKind::Derive => { write!(buffer, "#[derive({name})]").unwrap(); if !m.helpers.is_empty() { - buffer.write_str("\n{\n // Attributes available to this derive:\n").unwrap(); + buffer + .write_str( + "\n{\n \ + // Attributes available to this derive:\n", + ) + .unwrap(); for attr in &m.helpers { writeln!(buffer, " #[{attr}]").unwrap(); } @@ -2181,7 +2187,7 @@ fn render_union<'a, 'cx: 'a>( } if it.has_stripped_entries().unwrap() { - write!(f, " /* private fields */\n")?; + write!(f, " /* private fields */\n")?; } if toggle { toggle_close(&mut f); @@ -2267,11 +2273,11 @@ fn render_struct_fields( if has_visible_fields { if has_stripped_entries { - write!(w, "\n{tab} /* private fields */"); + write!(w, "\n{tab} /* private fields */"); } write!(w, "\n{tab}"); } else if has_stripped_entries { - write!(w, " /* private fields */ "); + write!(w, " /* private fields */ "); } if toggle { toggle_close(&mut w); @@ -2285,7 +2291,7 @@ fn render_struct_fields( matches!(*field.kind, clean::StrippedItem(box clean::StructFieldItem(..))) }) { - write!(w, "/* private fields */"); + write!(w, "/* private fields */"); } else { for (i, field) in fields.iter().enumerate() { if i > 0 { From 768a614380813a66c5b97ca4a02b721ea7916868 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 13 Nov 2023 12:00:17 +0100 Subject: [PATCH 07/15] Add GUI tests for comments highlighting in items declaration --- .../item-decl-comment-highlighting.goml | 73 +++++++++++++++++++ tests/rustdoc-gui/sidebar-source-code.goml | 2 +- .../src/proc_macro_test/Cargo.lock | 7 ++ .../src/proc_macro_test/Cargo.toml | 8 ++ tests/rustdoc-gui/src/proc_macro_test/lib.rs | 11 +++ tests/rustdoc-gui/src/test_docs/lib.rs | 18 +++++ 6 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 tests/rustdoc-gui/item-decl-comment-highlighting.goml create mode 100644 tests/rustdoc-gui/src/proc_macro_test/Cargo.lock create mode 100644 tests/rustdoc-gui/src/proc_macro_test/Cargo.toml create mode 100644 tests/rustdoc-gui/src/proc_macro_test/lib.rs diff --git a/tests/rustdoc-gui/item-decl-comment-highlighting.goml b/tests/rustdoc-gui/item-decl-comment-highlighting.goml new file mode 100644 index 00000000000..60772693d6c --- /dev/null +++ b/tests/rustdoc-gui/item-decl-comment-highlighting.goml @@ -0,0 +1,73 @@ +// This test checks that comments in item declarations are highlighted. +go-to: "file://" + |DOC_PATH| + "/test_docs/private/enum.Enum.html" +show-text: true + +define-function: ( + "check-item-decl-comment", + (theme, url, comment_color), + block { + go-to: |url| + set-local-storage: {"rustdoc-theme": |theme|, "rustdoc-use-system-theme": "false"} + reload: + assert-css: (".item-decl .comment", {"color": |comment_color|}, ALL) + } +) + +define-function: ( + "check-items-for-theme", + (theme, comment_color), + block { + call-function: ("check-item-decl-comment", { + "theme": |theme|, + "url": "file://" + |DOC_PATH| + "/test_docs/private/enum.Enum.html", + "comment_color": |comment_color|, + }) + call-function: ("check-item-decl-comment", { + "theme": |theme|, + "url": "file://" + |DOC_PATH| + "/test_docs/private/struct.Struct.html", + "comment_color": |comment_color|, + }) + call-function: ("check-item-decl-comment", { + "theme": |theme|, + "url": "file://" + |DOC_PATH| + "/test_docs/private/struct.Tuple.html", + "comment_color": |comment_color|, + }) + call-function: ("check-item-decl-comment", { + "theme": |theme|, + "url": "file://" + |DOC_PATH| + "/test_docs/private/union.Union.html", + "comment_color": |comment_color|, + }) + call-function: ("check-item-decl-comment", { + "theme": |theme|, + "url": "file://" + |DOC_PATH| + "/proc_macro_test/macro.make_answer.html", + "comment_color": |comment_color|, + }) + call-function: ("check-item-decl-comment", { + "theme": |theme|, + "url": "file://" + |DOC_PATH| + "/proc_macro_test/derive.HelperAttr.html", + "comment_color": |comment_color|, + }) + } +) + +call-function: ( + "check-items-for-theme", + { + "theme": "ayu", + "comment_color": "#788797", + } +) +call-function: ( + "check-items-for-theme", + { + "theme": "dark", + "comment_color": "#8d8d8b", + } +) +call-function: ( + "check-items-for-theme", + { + "theme": "light", + "comment_color": "#8e908c", + } +) diff --git a/tests/rustdoc-gui/sidebar-source-code.goml b/tests/rustdoc-gui/sidebar-source-code.goml index 92b9045b734..0d72e670cf4 100644 --- a/tests/rustdoc-gui/sidebar-source-code.goml +++ b/tests/rustdoc-gui/sidebar-source-code.goml @@ -73,7 +73,7 @@ assert: "//*[@class='dir-entry' and @open]/*[text()='sub_mod']" // Only "another_folder" should be "open" in "lib2". assert: "//*[@class='dir-entry' and not(@open)]/*[text()='another_mod']" // All other trees should be collapsed. -assert-count: ("//*[@id='src-sidebar']/details[not(text()='lib2') and not(@open)]", 10) +assert-count: ("//*[@id='src-sidebar']/details[not(text()='lib2') and not(@open)]", 11) // We now switch to mobile mode. set-window-size: (600, 600) diff --git a/tests/rustdoc-gui/src/proc_macro_test/Cargo.lock b/tests/rustdoc-gui/src/proc_macro_test/Cargo.lock new file mode 100644 index 00000000000..eae9d75367f --- /dev/null +++ b/tests/rustdoc-gui/src/proc_macro_test/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "proc_macro_test" +version = "0.1.0" diff --git a/tests/rustdoc-gui/src/proc_macro_test/Cargo.toml b/tests/rustdoc-gui/src/proc_macro_test/Cargo.toml new file mode 100644 index 00000000000..768ced65184 --- /dev/null +++ b/tests/rustdoc-gui/src/proc_macro_test/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "proc_macro_test" +version = "0.1.0" +edition = "2021" + +[lib] +path = "lib.rs" +proc-macro = true diff --git a/tests/rustdoc-gui/src/proc_macro_test/lib.rs b/tests/rustdoc-gui/src/proc_macro_test/lib.rs new file mode 100644 index 00000000000..8a6c62df87c --- /dev/null +++ b/tests/rustdoc-gui/src/proc_macro_test/lib.rs @@ -0,0 +1,11 @@ +use proc_macro::TokenStream; + +#[proc_macro] +pub fn make_answer(_item: TokenStream) -> TokenStream { + "fn answer() -> u32 { 42 }".parse().unwrap() +} + +#[proc_macro_derive(HelperAttr, attributes(helper))] +pub fn derive_helper_attr(_item: TokenStream) -> TokenStream { + TokenStream::new() +} diff --git a/tests/rustdoc-gui/src/test_docs/lib.rs b/tests/rustdoc-gui/src/test_docs/lib.rs index c7d115bdb98..0bc777230bf 100644 --- a/tests/rustdoc-gui/src/test_docs/lib.rs +++ b/tests/rustdoc-gui/src/test_docs/lib.rs @@ -593,3 +593,21 @@ pub mod foreign_impl_order { fn f(&mut self, fg: [u8; 3]) {} } } + +pub mod private { + pub struct Tuple(u32, u8); + pub struct Struct { + a: u8, + } + + pub union Union { + a: u8, + b: u16, + } + + pub enum Enum { + A, + #[doc(hidden)] + B, + } +} From 06695ea43614f7d8234f720d0f480cf5926ffffe Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 13 Nov 2023 13:57:05 +0100 Subject: [PATCH 08/15] Update snapshots of rustdoc tests to take into account the comment highlighting --- tests/rustdoc/where.SWhere_Simd_item-decl.html | 4 ++-- tests/rustdoc/where.alpha_trait_decl.html | 2 +- tests/rustdoc/whitespace-after-where-clause.union.html | 2 +- tests/rustdoc/whitespace-after-where-clause.union2.html | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/rustdoc/where.SWhere_Simd_item-decl.html b/tests/rustdoc/where.SWhere_Simd_item-decl.html index 46708b9e4e9..1987b1d59f5 100644 --- a/tests/rustdoc/where.SWhere_Simd_item-decl.html +++ b/tests/rustdoc/where.SWhere_Simd_item-decl.html @@ -1,3 +1,3 @@ -
pub struct Simd<T>(/* private fields */)
+
pub struct Simd<T>(/* private fields */)
 where
-    T: MyTrait;

+    T: MyTrait;
\ No newline at end of file diff --git a/tests/rustdoc/where.alpha_trait_decl.html b/tests/rustdoc/where.alpha_trait_decl.html index 0c0b2d1ceca..2c010ca7c2d 100644 --- a/tests/rustdoc/where.alpha_trait_decl.html +++ b/tests/rustdoc/where.alpha_trait_decl.html @@ -1,3 +1,3 @@ -pub struct Alpha<A>(/* private fields */) +pub struct Alpha<A>(/* private fields */) where A: MyTrait; \ No newline at end of file diff --git a/tests/rustdoc/whitespace-after-where-clause.union.html b/tests/rustdoc/whitespace-after-where-clause.union.html index 7e0d5f8717a..e63374760d9 100644 --- a/tests/rustdoc/whitespace-after-where-clause.union.html +++ b/tests/rustdoc/whitespace-after-where-clause.union.html @@ -1,4 +1,4 @@ 
pub union Union<'a, B>
where
     B: ToOwned<()> + ?Sized + 'a,
{
-    /* private fields */
+    /* private fields */
 }
\ No newline at end of file diff --git a/tests/rustdoc/whitespace-after-where-clause.union2.html b/tests/rustdoc/whitespace-after-where-clause.union2.html index 177a161b83a..da984343daa 100644 --- a/tests/rustdoc/whitespace-after-where-clause.union2.html +++ b/tests/rustdoc/whitespace-after-where-clause.union2.html @@ -1,3 +1,3 @@ 
pub union Union2<'a, B: ?Sized + ToOwned<()> + 'a> {
-    /* private fields */
+    /* private fields */
 }
\ No newline at end of file From 71c9ceb4da58bf792539879dbb1f32fba4569b71 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Dec 2023 19:16:09 +0100 Subject: [PATCH 09/15] update hashbrown --- Cargo.lock | 85 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 75c5f78e2b6..3b37f91878b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -37,13 +37,14 @@ dependencies = [ [[package]] name = "ahash" -version = "0.8.3" +version = "0.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c99f64d1e06488f620f932677e24bc6e2897582980441ae90a671415bd7ec2f" +checksum = "91429305e9f0a25f6205c5b8e0d2db09e0708a7a6df0f42212bb56c32c8ac97a" dependencies = [ "cfg-if", "once_cell", "version_check", + "zerocopy", ] [[package]] @@ -222,7 +223,7 @@ dependencies = [ "proc-macro2", "quote", "serde", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -525,7 +526,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -552,7 +553,7 @@ dependencies = [ "regex", "rustc_tools_util", "serde", - "syn 2.0.29", + "syn 2.0.32", "tempfile", "termize", "tester", @@ -964,7 +965,7 @@ dependencies = [ "proc-macro2", "quote", "strsim", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -986,7 +987,7 @@ checksum = "836a9bbc7ad63342d6d6e7b815ccab164bc77a2d95d84bc3117a8c0d5c98e2d5" dependencies = [ "darling_core 0.20.3", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -1001,7 +1002,7 @@ version = "0.1.76" dependencies = [ "itertools", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -1068,7 +1069,7 @@ dependencies = [ "darling 0.20.3", "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -1157,7 +1158,7 @@ checksum = "487585f4d0c6655fe74905e2504d8ad6908e4db67f744eb140876906c2f3175d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -1489,7 +1490,7 @@ checksum = "89ca545a94061b6365f2c7355b4b32bd20df3ff95f02da9329b34ccc3bd6ee72" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -1647,9 +1648,9 @@ dependencies = [ [[package]] name = "hashbrown" -version = "0.14.2" +version = "0.14.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f93e7192158dbcda357bdec5fb5788eebf8bbac027f3f33e719d29135ae84156" +checksum = "290f1a1d9242c78d09ce40a5e87e7554ee637af1351968159f4952f028f75604" dependencies = [ "ahash", "allocator-api2", @@ -1912,7 +1913,7 @@ checksum = "2060258edfcfe32ca7058849bf0f146cb5c59aadbedf480333c0d0002f97bc99" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -2675,7 +2676,7 @@ checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -2892,7 +2893,7 @@ dependencies = [ "pest_meta", "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -3868,7 +3869,7 @@ dependencies = [ "fluent-syntax", "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", "unic-langid", ] @@ -3999,7 +4000,7 @@ version = "0.0.0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", "synstructure", ] @@ -4145,7 +4146,7 @@ version = "0.0.0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", "synstructure", ] @@ -4729,7 +4730,7 @@ dependencies = [ "proc-macro2", "quote", "serde", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -4892,7 +4893,7 @@ checksum = "dc59dfdcbad1437773485e0367fea4b090a2e0a16d9ffc46af47764536a298ec" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -5183,9 +5184,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.29" +version = "2.0.32" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c324c494eba9d92503e6f1ef2e6df781e78f6a7705a0202d9801b198807d518a" +checksum = "239814284fd6f1a4ffe4ca893952cdd93c224b6a1571c9a9eadd670295c0c9e2" dependencies = [ "proc-macro2", "quote", @@ -5200,7 +5201,7 @@ checksum = "285ba80e733fac80aa4270fbcdf83772a79b80aa35c97075320abfee4a915b06" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", "unicode-xid", ] @@ -5379,7 +5380,7 @@ checksum = "6bb623b56e39ab7dcd4b1b98bb6c8f8d907ed255b18de254088016b27a8ee19b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -5600,7 +5601,7 @@ checksum = "5f4f31f56159e98206da9efd823404b79b6ef3143b4a7ab76e67b1751b25a4ab" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] @@ -5994,7 +5995,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", "wasm-bindgen-shared", ] @@ -6028,7 +6029,7 @@ checksum = "54681b18a46765f095758388f2d0cf16eb8d4169b639ab575a8f5693af210c7b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -6097,7 +6098,7 @@ checksum = "970efb0b6849eb8a87a898f586af7cc167567b070014c7434514c0bde0ca341c" dependencies = [ "proc-macro2", "rayon", - "syn 2.0.29", + "syn 2.0.32", "windows-metadata", ] @@ -6339,10 +6340,30 @@ checksum = "d5e19fb6ed40002bab5403ffa37e53e0e56f914a4450c8765f533018db1db35f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", "synstructure", ] +[[package]] +name = "zerocopy" +version = "0.7.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d6f15f7ade05d2a4935e34a457b936c23dc70a05cc1d97133dc99e7a3fe0f0e" +dependencies = [ + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.7.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbbad221e3f78500350ecbd7dfa4e63ef945c05f4c61cb7f4d3f84cd0bba649b" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.32", +] + [[package]] name = "zerofrom" version = "0.1.3" @@ -6360,7 +6381,7 @@ checksum = "e6a647510471d372f2e6c2e6b7219e44d8c574d24fdc11c610a61455782f18c3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", "synstructure", ] @@ -6383,7 +6404,7 @@ checksum = "acabf549809064225ff8878baedc4ce3732ac3b07e7c7ce6e5c2ccdbc485c324" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn 2.0.32", ] [[package]] From c0c0b5b5a6a390f0e59f109d07f9b257e75ce74c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 2 Dec 2023 19:36:08 +0100 Subject: [PATCH 10/15] allow zerocopy license --- src/tools/tidy/src/deps.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs index 1f8edd7937b..6baf2e21604 100644 --- a/src/tools/tidy/src/deps.rs +++ b/src/tools/tidy/src/deps.rs @@ -16,6 +16,7 @@ const LICENSES: &[&str] = &[ "Apache-2.0 OR MIT", "Apache-2.0 WITH LLVM-exception OR Apache-2.0 OR MIT", // wasi license "Apache-2.0/MIT", + "BSD-2-Clause OR Apache-2.0 OR MIT", // zerocopy "ISC", "MIT / Apache-2.0", "MIT OR Apache-2.0 OR LGPL-2.1-or-later", // r-efi, r-efi-alloc @@ -392,6 +393,8 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[ "yansi-term", // this is a false-positive: it's only used by rustfmt, but because it's enabled through a feature, tidy thinks it's used by rustc as well. "yoke", "yoke-derive", + "zerocopy", + "zerocopy-derive", "zerofrom", "zerofrom-derive", "zerovec", From a2171feb3384d5737eaf7ca24c5740a9615b2917 Mon Sep 17 00:00:00 2001 From: sjwang05 <63834813+sjwang05@users.noreply.github.com> Date: Fri, 1 Dec 2023 19:49:08 -0800 Subject: [PATCH 11/15] Fix ICE when suggesting closures for non-fn-like defs --- .../src/traits/error_reporting/suggestions.rs | 11 ++++++-- tests/ui/mismatched_types/issue-118510.rs | 10 +++++++ tests/ui/mismatched_types/issue-118510.stderr | 26 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 tests/ui/mismatched_types/issue-118510.rs create mode 100644 tests/ui/mismatched_types/issue-118510.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index d5e1efb9663..6b231a30ea7 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -2112,7 +2112,8 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { && !expected_inputs.is_empty() && expected_inputs.len() == found_inputs.len() && let Some(typeck) = &self.typeck_results - && let Res::Def(_, fn_def_id) = typeck.qpath_res(&path, *arg_hir_id) + && let Res::Def(res_kind, fn_def_id) = typeck.qpath_res(&path, *arg_hir_id) + && res_kind.is_fn_like() { let closure: Vec<_> = self .tcx @@ -2155,7 +2156,13 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { .map(|(name, ty)| { format!( "{name}{}", - if ty.has_infer_types() { String::new() } else { format!(": {ty}") } + if ty.has_infer_types() { + String::new() + } else if ty.references_error() { + ": /* type */".to_string() + } else { + format!(": {ty}") + } ) }) .collect(); diff --git a/tests/ui/mismatched_types/issue-118510.rs b/tests/ui/mismatched_types/issue-118510.rs new file mode 100644 index 00000000000..86b9d44a373 --- /dev/null +++ b/tests/ui/mismatched_types/issue-118510.rs @@ -0,0 +1,10 @@ +pub enum Sexpr<'a, S> { + Ident(&'a mut S), +} + +fn map T>(f: F) {} + +fn main() { + map(Sexpr::Ident); + //~^ ERROR type mismatch in function arguments +} diff --git a/tests/ui/mismatched_types/issue-118510.stderr b/tests/ui/mismatched_types/issue-118510.stderr new file mode 100644 index 00000000000..e8bf92d9047 --- /dev/null +++ b/tests/ui/mismatched_types/issue-118510.stderr @@ -0,0 +1,26 @@ +error[E0631]: type mismatch in function arguments + --> $DIR/issue-118510.rs:8:9 + | +LL | Ident(&'a mut S), + | ----- found signature defined here +... +LL | map(Sexpr::Ident); + | --- ^^^^^^^^^^^^ expected due to this + | | + | required by a bound introduced by this call + | + = note: expected function signature `for<'a> fn(&'a _) -> _` + found function signature `fn(&mut _) -> _` +note: required by a bound in `map` + --> $DIR/issue-118510.rs:5:19 + | +LL | fn map T>(f: F) {} + | ^^^^^^^^^^^^^^^^^ required by this bound in `map` +help: consider wrapping the function in a closure + | +LL | map(|arg0| Sexpr::Ident(&mut *arg0)); + | ++++++ ++++++++++++ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0631`. From eb2d4cb5414361d9148bf43388012dcbdce80abe Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 26 Nov 2023 22:20:16 +1100 Subject: [PATCH 12/15] coverage: Skip spans that can't be un-expanded back to the function body When we extract coverage spans from MIR, we try to "un-expand" them back to spans that are inside the function's body span. In cases where that doesn't succeed, the current code just swaps in the entire body span instead. But that tends to result in coverage spans that are completely unrelated to the control flow of the affected code, so it's better to just discard those spans. --- .../src/coverage/spans/from_mir.rs | 12 +++++++----- tests/coverage/async.cov-map | 12 ++++++------ tests/coverage/async2.cov-map | 12 ++++++------ tests/coverage/inline.cov-map | 4 ++-- tests/coverage/inline.coverage | 2 +- tests/coverage/unreachable.cov-map | 12 ++++++------ tests/coverage/unreachable.coverage | 4 ++-- 7 files changed, 30 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 6189e5379ea..e1531f2c239 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -63,14 +63,14 @@ fn bcb_to_initial_coverage_spans<'a, 'tcx>( let statement_spans = data.statements.iter().filter_map(move |statement| { let expn_span = filtered_statement_span(statement)?; - let span = function_source_span(expn_span, body_span); + let span = unexpand_into_body_span(expn_span, body_span)?; Some(CoverageSpan::new(span, expn_span, bcb, is_closure(statement))) }); let terminator_span = Some(data.terminator()).into_iter().filter_map(move |terminator| { let expn_span = filtered_terminator_span(terminator)?; - let span = function_source_span(expn_span, body_span); + let span = unexpand_into_body_span(expn_span, body_span)?; Some(CoverageSpan::new(span, expn_span, bcb, false)) }); @@ -180,14 +180,16 @@ fn filtered_terminator_span(terminator: &Terminator<'_>) -> Option { /// Returns an extrapolated span (pre-expansion[^1]) corresponding to a range /// within the function's body source. This span is guaranteed to be contained /// within, or equal to, the `body_span`. If the extrapolated span is not -/// contained within the `body_span`, the `body_span` is returned. +/// contained within the `body_span`, `None` is returned. /// /// [^1]Expansions result from Rust syntax including macros, syntactic sugar, /// etc.). #[inline] -fn function_source_span(span: Span, body_span: Span) -> Span { +fn unexpand_into_body_span(span: Span, body_span: Span) -> Option { use rustc_span::source_map::original_sp; + // FIXME(#118525): Consider switching from `original_sp` to `Span::find_ancestor_inside`, + // which is similar but gives slightly different results in some edge cases. let original_span = original_sp(span, body_span).with_ctxt(body_span.ctxt()); - if body_span.contains(original_span) { original_span } else { body_span } + body_span.contains(original_span).then_some(original_span) } diff --git a/tests/coverage/async.cov-map b/tests/coverage/async.cov-map index 857e0a536a7..e4354a1af87 100644 --- a/tests/coverage/async.cov-map +++ b/tests/coverage/async.cov-map @@ -74,28 +74,28 @@ Number of file 0 mappings: 6 = ((c0 + c1) - c1) Function name: async::executor::block_on::VTABLE::{closure#0} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 72, 11, 00, 33] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 72, 11, 00, 31] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 114, 17) to (start + 0, 51) +- Code(Counter(0)) at (prev + 114, 17) to (start + 0, 49) Function name: async::executor::block_on::VTABLE::{closure#1} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 73, 11, 00, 33] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 73, 11, 00, 31] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 115, 17) to (start + 0, 51) +- Code(Counter(0)) at (prev + 115, 17) to (start + 0, 49) Function name: async::executor::block_on::VTABLE::{closure#2} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 74, 11, 00, 33] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 74, 11, 00, 31] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 116, 17) to (start + 0, 51) +- Code(Counter(0)) at (prev + 116, 17) to (start + 0, 49) Function name: async::executor::block_on::VTABLE::{closure#3} Raw bytes (9): 0x[01, 01, 00, 01, 01, 75, 11, 00, 13] diff --git a/tests/coverage/async2.cov-map b/tests/coverage/async2.cov-map index cc7aed9aee3..23f26ee4e5f 100644 --- a/tests/coverage/async2.cov-map +++ b/tests/coverage/async2.cov-map @@ -78,28 +78,28 @@ Number of file 0 mappings: 6 = ((c0 + c1) - c1) Function name: async2::executor::block_on::VTABLE::{closure#0} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 2b, 11, 00, 33] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 2b, 11, 00, 31] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 43, 17) to (start + 0, 51) +- Code(Counter(0)) at (prev + 43, 17) to (start + 0, 49) Function name: async2::executor::block_on::VTABLE::{closure#1} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 2c, 11, 00, 33] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 2c, 11, 00, 31] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 44, 17) to (start + 0, 51) +- Code(Counter(0)) at (prev + 44, 17) to (start + 0, 49) Function name: async2::executor::block_on::VTABLE::{closure#2} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 2d, 11, 00, 33] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 2d, 11, 00, 31] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 45, 17) to (start + 0, 51) +- Code(Counter(0)) at (prev + 45, 17) to (start + 0, 49) Function name: async2::executor::block_on::VTABLE::{closure#3} Raw bytes (9): 0x[01, 01, 00, 01, 01, 2e, 11, 00, 13] diff --git a/tests/coverage/inline.cov-map b/tests/coverage/inline.cov-map index 72b10fd0cc2..001c333ae6d 100644 --- a/tests/coverage/inline.cov-map +++ b/tests/coverage/inline.cov-map @@ -15,12 +15,12 @@ Number of file 0 mappings: 5 = ((c0 + c1) - c1) Function name: inline::error -Raw bytes (9): 0x[01, 01, 00, 01, 01, 31, 01, 02, 02] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 31, 01, 01, 14] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 49, 1) to (start + 2, 2) +- Code(Counter(0)) at (prev + 49, 1) to (start + 1, 20) Function name: inline::length:: Raw bytes (9): 0x[01, 01, 00, 01, 01, 1e, 01, 02, 02] diff --git a/tests/coverage/inline.coverage b/tests/coverage/inline.coverage index 6efd9a0830b..68a2e408306 100644 --- a/tests/coverage/inline.coverage +++ b/tests/coverage/inline.coverage @@ -50,5 +50,5 @@ LL| |#[inline(always)] LL| 0|fn error() { LL| 0| panic!("error"); - LL| 0|} + LL| |} diff --git a/tests/coverage/unreachable.cov-map b/tests/coverage/unreachable.cov-map index 495419820c1..55d124a16f5 100644 --- a/tests/coverage/unreachable.cov-map +++ b/tests/coverage/unreachable.cov-map @@ -1,24 +1,24 @@ Function name: unreachable::UNREACHABLE_CLOSURE::{closure#0} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 0f, 27, 00, 49] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 0f, 27, 00, 47] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 15, 39) to (start + 0, 73) +- Code(Counter(0)) at (prev + 15, 39) to (start + 0, 71) Function name: unreachable::unreachable_function -Raw bytes (9): 0x[01, 01, 00, 01, 01, 11, 01, 02, 02] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 11, 01, 01, 25] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 17, 1) to (start + 2, 2) +- Code(Counter(0)) at (prev + 17, 1) to (start + 1, 37) Function name: unreachable::unreachable_intrinsic -Raw bytes (9): 0x[01, 01, 00, 01, 01, 16, 01, 02, 02] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 16, 01, 01, 2c] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 22, 1) to (start + 2, 2) +- Code(Counter(0)) at (prev + 22, 1) to (start + 1, 44) diff --git a/tests/coverage/unreachable.coverage b/tests/coverage/unreachable.coverage index fa0ac9ccfa1..7015bb90aa3 100644 --- a/tests/coverage/unreachable.coverage +++ b/tests/coverage/unreachable.coverage @@ -16,12 +16,12 @@ LL| | LL| 0|fn unreachable_function() { LL| 0| unsafe { unreachable_unchecked() } - LL| 0|} + LL| |} LL| | LL| |// Use an intrinsic to more reliably trigger unreachable-propagation. LL| 0|fn unreachable_intrinsic() { LL| 0| unsafe { std::intrinsics::unreachable() } - LL| 0|} + LL| |} LL| | LL| |#[coverage(off)] LL| |fn main() { From d87460a507dd4fa137c924b648c633b6d9e2b863 Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Sun, 3 Dec 2023 14:37:01 +0100 Subject: [PATCH 13/15] rustc_session: Address all `rustc::potential_query_instability` lints Instead of allowing `rustc::potential_query_instability` on the whole crate we go over each lint and allow it individually if it is safe to do. Turns out all instances were safe to allow in this crate. --- compiler/rustc_session/src/code_stats.rs | 4 ++++ compiler/rustc_session/src/lib.rs | 1 - compiler/rustc_session/src/parse.rs | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_session/src/code_stats.rs b/compiler/rustc_session/src/code_stats.rs index e1eb58fecc7..2553df33cc7 100644 --- a/compiler/rustc_session/src/code_stats.rs +++ b/compiler/rustc_session/src/code_stats.rs @@ -132,6 +132,8 @@ impl CodeStats { pub fn print_type_sizes(&self) { let type_sizes = self.type_sizes.borrow(); + // We will soon sort, so the initial order does not matter. + #[allow(rustc::potential_query_instability)] let mut sorted: Vec<_> = type_sizes.iter().collect(); // Primary sort: large-to-small. @@ -227,6 +229,8 @@ impl CodeStats { } pub fn print_vtable_sizes(&self, crate_name: Symbol) { + // We will soon sort, so the initial order does not matter. + #[allow(rustc::potential_query_instability)] let mut infos = std::mem::take(&mut *self.vtable_sizes.lock()).into_values().collect::>(); diff --git a/compiler/rustc_session/src/lib.rs b/compiler/rustc_session/src/lib.rs index 0b55af2f73b..805854bd5cf 100644 --- a/compiler/rustc_session/src/lib.rs +++ b/compiler/rustc_session/src/lib.rs @@ -6,7 +6,6 @@ #![feature(map_many_mut)] #![feature(iter_intersperse)] #![recursion_limit = "256"] -#![allow(rustc::potential_query_instability)] #![deny(rustc::untranslatable_diagnostic)] #![deny(rustc::diagnostic_outside_of_impl)] #![allow(internal_features)] diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index f7b33cb598b..881e1de6755 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -51,6 +51,9 @@ impl GatedSpans { /// Prepend the given set of `spans` onto the set in `self`. pub fn merge(&self, mut spans: FxHashMap>) { let mut inner = self.spans.borrow_mut(); + // The entries will be moved to another map so the drain order does not + // matter. + #[allow(rustc::potential_query_instability)] for (gate, mut gate_spans) in inner.drain() { spans.entry(gate).or_default().append(&mut gate_spans); } From bebba4f6e05fae60ce8dcddc5eb6b4e7686bd995 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 12 Nov 2023 16:06:50 +0100 Subject: [PATCH 14/15] miri: support 'promising' alignment for symbolic alignment check --- .../rustc_const_eval/src/interpret/machine.rs | 29 ++++----- .../rustc_const_eval/src/interpret/memory.rs | 9 ++- library/core/src/ptr/const_ptr.rs | 32 +++++++++- library/core/src/ptr/mut_ptr.rs | 32 +++++++++- library/core/src/slice/mod.rs | 27 ++++++++ src/tools/miri/src/machine.rs | 55 +++++++++++++++-- src/tools/miri/src/shims/foreign_items.rs | 37 ++++++++++- src/tools/miri/src/shims/mod.rs | 61 +------------------ ...romise_alignment.call_unaligned_ptr.stderr | 14 +++++ ...romise_alignment.read_unaligned_ptr.stderr | 15 +++++ .../unaligned_pointers/promise_alignment.rs | 54 ++++++++++++++++ .../miri/tests/pass/align_offset_symbolic.rs | 43 +++++-------- .../miri/tests/pass/panic/catch_panic.rs | 3 +- src/tools/miri/tests/utils/miri_extern.rs | 5 ++ 14 files changed, 298 insertions(+), 118 deletions(-) create mode 100644 src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.call_unaligned_ptr.stderr create mode 100644 src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.read_unaligned_ptr.stderr create mode 100644 src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index 6617f6f2ffb..221ff2b60e9 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -12,12 +12,12 @@ use rustc_middle::mir; use rustc_middle::ty::layout::TyAndLayout; use rustc_middle::ty::{self, TyCtxt}; use rustc_span::def_id::DefId; -use rustc_target::abi::Size; +use rustc_target::abi::{Align, Size}; use rustc_target::spec::abi::Abi as CallAbi; use super::{ - AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, InterpCx, - InterpResult, MPlaceTy, MemoryKind, OpTy, PlaceTy, Pointer, Provenance, + AllocBytes, AllocId, AllocKind, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, + InterpCx, InterpResult, MPlaceTy, MemoryKind, Misalignment, OpTy, PlaceTy, Pointer, Provenance, }; /// Data returned by Machine::stack_pop, @@ -143,11 +143,18 @@ pub trait Machine<'mir, 'tcx: 'mir>: Sized { /// Whether memory accesses should be alignment-checked. fn enforce_alignment(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool; - /// Whether, when checking alignment, we should look at the actual address and thus support - /// custom alignment logic based on whatever the integer address happens to be. - /// - /// If this returns true, Provenance::OFFSET_IS_ADDR must be true. - fn use_addr_for_alignment_check(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool; + /// Gives the machine a chance to detect more misalignment than the built-in checks would catch. + #[inline(always)] + fn alignment_check( + _ecx: &InterpCx<'mir, 'tcx, Self>, + _alloc_id: AllocId, + _alloc_align: Align, + _alloc_kind: AllocKind, + _offset: Size, + _align: Align, + ) -> Option { + None + } /// Whether to enforce the validity invariant for a specific layout. fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool; @@ -519,12 +526,6 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) { type FrameExtra = (); type Bytes = Box<[u8]>; - #[inline(always)] - fn use_addr_for_alignment_check(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool { - // We do not support `use_addr`. - false - } - #[inline(always)] fn ignore_optional_overflow_checks(_ecx: &InterpCx<$mir, $tcx, Self>) -> bool { false diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 1c1fd2a71ba..f865c0cc5fa 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -58,6 +58,7 @@ impl fmt::Display for MemoryKind { } /// The return value of `get_alloc_info` indicates the "kind" of the allocation. +#[derive(Copy, Clone, PartialEq, Debug)] pub enum AllocKind { /// A regular live data allocation. LiveData, @@ -473,8 +474,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { match self.ptr_try_get_alloc_id(ptr) { Err(addr) => offset_misalignment(addr, align), Ok((alloc_id, offset, _prov)) => { - let (_size, alloc_align, _kind) = self.get_alloc_info(alloc_id); - if M::use_addr_for_alignment_check(self) { + let (_size, alloc_align, kind) = self.get_alloc_info(alloc_id); + if let Some(misalign) = + M::alignment_check(self, alloc_id, alloc_align, kind, offset, align) + { + Some(misalign) + } else if M::Provenance::OFFSET_IS_ADDR { // `use_addr_for_alignment_check` can only be true if `OFFSET_IS_ADDR` is true. offset_misalignment(ptr.addr().bytes(), align) } else { diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 5b5284d8c4c..46e8676162a 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -1368,10 +1368,36 @@ impl *const T { panic!("align_offset: align is not a power-of-two"); } - { - // SAFETY: `align` has been checked to be a power of 2 above - unsafe { align_offset(self, align) } + // SAFETY: `align` has been checked to be a power of 2 above + let ret = unsafe { align_offset(self, align) }; + + // Inform Miri that we want to consider the resulting pointer to be suitably aligned. + #[cfg(miri)] + if ret != usize::MAX { + fn runtime(ptr: *const (), align: usize) { + extern "Rust" { + pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); + } + + // SAFETY: this call is always safe. + unsafe { + miri_promise_symbolic_alignment(ptr, align); + } + } + + const fn compiletime(_ptr: *const (), _align: usize) {} + + // SAFETY: the extra behavior at runtime is for UB checks only. + unsafe { + intrinsics::const_eval_select( + (self.wrapping_add(ret).cast(), align), + compiletime, + runtime, + ); + } } + + ret } /// Returns whether the pointer is properly aligned for `T`. diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index a9208698a29..3f44d78795b 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -1635,10 +1635,36 @@ impl *mut T { panic!("align_offset: align is not a power-of-two"); } - { - // SAFETY: `align` has been checked to be a power of 2 above - unsafe { align_offset(self, align) } + // SAFETY: `align` has been checked to be a power of 2 above + let ret = unsafe { align_offset(self, align) }; + + // Inform Miri that we want to consider the resulting pointer to be suitably aligned. + #[cfg(miri)] + if ret != usize::MAX { + fn runtime(ptr: *const (), align: usize) { + extern "Rust" { + pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); + } + + // SAFETY: this call is always safe. + unsafe { + miri_promise_symbolic_alignment(ptr, align); + } + } + + const fn compiletime(_ptr: *const (), _align: usize) {} + + // SAFETY: the extra behavior at runtime is for UB checks only. + unsafe { + intrinsics::const_eval_select( + (self.wrapping_add(ret).cast_const().cast(), align), + compiletime, + runtime, + ); + } } + + ret } /// Returns whether the pointer is properly aligned for `T`. diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index a7b36fe7d29..7d88f70efd1 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -3868,6 +3868,18 @@ impl [T] { } else { let (left, rest) = self.split_at(offset); let (us_len, ts_len) = rest.align_to_offsets::(); + // Inform Miri that we want to consider the "middle" pointer to be suitably aligned. + #[cfg(miri)] + { + extern "Rust" { + pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); + } + + // SAFETY: this call is always safe. + unsafe { + miri_promise_symbolic_alignment(rest.as_ptr().cast(), mem::align_of::()); + } + } // SAFETY: now `rest` is definitely aligned, so `from_raw_parts` below is okay, // since the caller guarantees that we can transmute `T` to `U` safely. unsafe { @@ -3938,6 +3950,21 @@ impl [T] { let (us_len, ts_len) = rest.align_to_offsets::(); let rest_len = rest.len(); let mut_ptr = rest.as_mut_ptr(); + // Inform Miri that we want to consider the "middle" pointer to be suitably aligned. + #[cfg(miri)] + { + extern "Rust" { + pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); + } + + // SAFETY: this call is always safe. + unsafe { + miri_promise_symbolic_alignment( + mut_ptr.cast() as *const (), + mem::align_of::(), + ); + } + } // We can't use `rest` again after this, that would invalidate its alias `mut_ptr`! // SAFETY: see comments for `align_to`. unsafe { diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 66d7dfcf3a1..0826034d75b 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -2,7 +2,7 @@ //! `Machine` trait. use std::borrow::Cow; -use std::cell::RefCell; +use std::cell::{Cell, RefCell}; use std::fmt; use std::path::Path; use std::process; @@ -309,11 +309,20 @@ pub struct AllocExtra<'tcx> { /// if this allocation is leakable. The backtrace is not /// pruned yet; that should be done before printing it. pub backtrace: Option>>, + /// An offset inside this allocation that was deemed aligned even for symbolic alignment checks. + /// Invariant: the promised alignment will never be less than the native alignment of this allocation. + pub symbolic_alignment: Cell>, } impl VisitProvenance for AllocExtra<'_> { fn visit_provenance(&self, visit: &mut VisitWith<'_>) { - let AllocExtra { borrow_tracker, data_race, weak_memory, backtrace: _ } = self; + let AllocExtra { + borrow_tracker, + data_race, + weak_memory, + backtrace: _, + symbolic_alignment: _, + } = self; borrow_tracker.visit_provenance(visit); data_race.visit_provenance(visit); @@ -907,8 +916,45 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { } #[inline(always)] - fn use_addr_for_alignment_check(ecx: &MiriInterpCx<'mir, 'tcx>) -> bool { - ecx.machine.check_alignment == AlignmentCheck::Int + fn alignment_check( + ecx: &MiriInterpCx<'mir, 'tcx>, + alloc_id: AllocId, + alloc_align: Align, + alloc_kind: AllocKind, + offset: Size, + align: Align, + ) -> Option { + if ecx.machine.check_alignment != AlignmentCheck::Symbolic { + // Just use the built-in check. + return None; + } + if alloc_kind != AllocKind::LiveData { + // Can't have any extra info here. + return None; + } + // Let's see which alignment we have been promised for this allocation. + let alloc_info = ecx.get_alloc_extra(alloc_id).unwrap(); // cannot fail since the allocation is live + let (promised_offset, promised_align) = + alloc_info.symbolic_alignment.get().unwrap_or((Size::ZERO, alloc_align)); + if promised_align < align { + // Definitely not enough. + Some(Misalignment { has: promised_align, required: align }) + } else { + // What's the offset between us and the promised alignment? + let distance = offset.bytes().wrapping_sub(promised_offset.bytes()); + // That must also be aligned. + if distance % align.bytes() == 0 { + // All looking good! + None + } else { + // The biggest power of two through which `distance` is divisible. + let distance_pow2 = 1 << distance.trailing_zeros(); + Some(Misalignment { + has: Align::from_bytes(distance_pow2).unwrap(), + required: align, + }) + } + } } #[inline(always)] @@ -1112,6 +1158,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { data_race: race_alloc, weak_memory: buffer_alloc, backtrace, + symbolic_alignment: Cell::new(None), }, |ptr| ecx.global_base_pointer(ptr), )?; diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index 0957e72ee91..3a0ff7a5567 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -467,7 +467,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let ptr = this.read_pointer(ptr)?; let (alloc_id, _, _) = this.ptr_get_alloc_id(ptr).map_err(|_e| { err_machine_stop!(TerminationInfo::Abort(format!( - "pointer passed to miri_get_alloc_id must not be dangling, got {ptr:?}" + "pointer passed to `miri_get_alloc_id` must not be dangling, got {ptr:?}" ))) })?; this.write_scalar(Scalar::from_u64(alloc_id.0.get()), dest)?; @@ -499,7 +499,7 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let (alloc_id, offset, _) = this.ptr_get_alloc_id(ptr)?; if offset != Size::ZERO { throw_unsup_format!( - "pointer passed to miri_static_root must point to beginning of an allocated block" + "pointer passed to `miri_static_root` must point to beginning of an allocated block" ); } this.machine.static_roots.push(alloc_id); @@ -556,6 +556,39 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { }; } + // Promises that a pointer has a given symbolic alignment. + "miri_promise_symbolic_alignment" => { + let [ptr, align] = this.check_shim(abi, Abi::Rust, link_name, args)?; + let ptr = this.read_pointer(ptr)?; + let align = this.read_target_usize(align)?; + let Ok(align) = Align::from_bytes(align) else { + throw_unsup_format!( + "`miri_promise_symbolic_alignment`: alignment must be a power of 2" + ); + }; + let (_, addr) = ptr.into_parts(); // we know the offset is absolute + if addr.bytes() % align.bytes() != 0 { + throw_unsup_format!( + "`miri_promise_symbolic_alignment`: pointer is not actually aligned" + ); + } + if let Ok((alloc_id, offset, ..)) = this.ptr_try_get_alloc_id(ptr) { + let (_size, alloc_align, _kind) = this.get_alloc_info(alloc_id); + // Not `get_alloc_extra_mut`, need to handle read-only allocations! + let alloc_extra = this.get_alloc_extra(alloc_id)?; + // If the newly promised alignment is bigger than the native alignment of this + // allocation, and bigger than the previously promised alignment, then set it. + if align > alloc_align + && !alloc_extra + .symbolic_alignment + .get() + .is_some_and(|(_, old_align)| align <= old_align) + { + alloc_extra.symbolic_alignment.set(Some((offset, align))); + } + } + } + // Standard C allocation "malloc" => { let [size] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; diff --git a/src/tools/miri/src/shims/mod.rs b/src/tools/miri/src/shims/mod.rs index a031a2a25c9..1e9d927e1a9 100644 --- a/src/tools/miri/src/shims/mod.rs +++ b/src/tools/miri/src/shims/mod.rs @@ -23,7 +23,6 @@ use rustc_middle::{mir, ty}; use rustc_target::spec::abi::Abi; use crate::*; -use helpers::check_arg_count; impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { @@ -39,16 +38,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let this = self.eval_context_mut(); trace!("eval_fn_call: {:#?}, {:?}", instance, dest); - // There are some more lang items we want to hook that CTFE does not hook (yet). - if this.tcx.lang_items().align_offset_fn() == Some(instance.def.def_id()) { - let args = this.copy_fn_args(args)?; - let [ptr, align] = check_arg_count(&args)?; - if this.align_offset(ptr, align, dest, ret, unwind)? { - return Ok(None); - } - } - - // Try to see if we can do something about foreign items. + // For foreign items, try to see if we can emulate them. if this.tcx.is_foreign_item(instance.def_id()) { // An external function call that does not have a MIR body. We either find MIR elsewhere // or emulate its effect. @@ -64,53 +54,4 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Otherwise, load the MIR. Ok(Some((this.load_mir(instance.def, None)?, instance))) } - - /// Returns `true` if the computation was performed, and `false` if we should just evaluate - /// the actual MIR of `align_offset`. - fn align_offset( - &mut self, - ptr_op: &OpTy<'tcx, Provenance>, - align_op: &OpTy<'tcx, Provenance>, - dest: &PlaceTy<'tcx, Provenance>, - ret: Option, - unwind: mir::UnwindAction, - ) -> InterpResult<'tcx, bool> { - let this = self.eval_context_mut(); - let ret = ret.unwrap(); - - if this.machine.check_alignment != AlignmentCheck::Symbolic { - // Just use actual implementation. - return Ok(false); - } - - let req_align = this.read_target_usize(align_op)?; - - // Stop if the alignment is not a power of two. - if !req_align.is_power_of_two() { - this.start_panic("align_offset: align is not a power-of-two", unwind)?; - return Ok(true); // nothing left to do - } - - let ptr = this.read_pointer(ptr_op)?; - // If this carries no provenance, treat it like an integer. - if ptr.provenance.is_none() { - // Use actual implementation. - return Ok(false); - } - - if let Ok((alloc_id, _offset, _)) = this.ptr_try_get_alloc_id(ptr) { - // Only do anything if we can identify the allocation this goes to. - let (_size, cur_align, _kind) = this.get_alloc_info(alloc_id); - if cur_align.bytes() >= req_align { - // If the allocation alignment is at least the required alignment we use the - // real implementation. - return Ok(false); - } - } - - // Return error result (usize::MAX), and jump to caller. - this.write_scalar(Scalar::from_target_usize(this.target_usize_max(), this), dest)?; - this.go_to_block(ret); - Ok(true) - } } diff --git a/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.call_unaligned_ptr.stderr b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.call_unaligned_ptr.stderr new file mode 100644 index 00000000000..e23ac5ac2fc --- /dev/null +++ b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.call_unaligned_ptr.stderr @@ -0,0 +1,14 @@ +error: unsupported operation: `miri_promise_symbolic_alignment`: pointer is not actually aligned + --> $DIR/promise_alignment.rs:LL:CC + | +LL | unsafe { utils::miri_promise_symbolic_alignment(align8.add(1).cast(), 8) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `miri_promise_symbolic_alignment`: pointer is not actually aligned + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support + = note: BACKTRACE: + = note: inside `main` at $DIR/promise_alignment.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.read_unaligned_ptr.stderr b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.read_unaligned_ptr.stderr new file mode 100644 index 00000000000..0842ccd6d5b --- /dev/null +++ b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.read_unaligned_ptr.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: accessing memory based on pointer with alignment ALIGN, but alignment ALIGN is required + --> $DIR/promise_alignment.rs:LL:CC + | +LL | let _val = unsafe { align8.cast::().read() }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ accessing memory based on pointer with alignment ALIGN, but alignment ALIGN is required + | + = help: this usually indicates that your program performed an invalid operation and caused Undefined Behavior + = help: but due to `-Zmiri-symbolic-alignment-check`, alignment errors can also be false positives + = note: BACKTRACE: + = note: inside `main` at $DIR/promise_alignment.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs new file mode 100644 index 00000000000..d2d49c604af --- /dev/null +++ b/src/tools/miri/tests/fail/unaligned_pointers/promise_alignment.rs @@ -0,0 +1,54 @@ +//@compile-flags: -Zmiri-symbolic-alignment-check +//@revisions: call_unaligned_ptr read_unaligned_ptr +#![feature(strict_provenance)] + +#[path = "../../utils/mod.rs"] +mod utils; + +#[repr(align(8))] +#[derive(Copy, Clone)] +struct Align8(u64); + +fn main() { + let buffer = [0u32; 128]; // get some 4-aligned memory + let buffer = buffer.as_ptr(); + // "Promising" the alignment down to 1 must not hurt. + unsafe { utils::miri_promise_symbolic_alignment(buffer.cast(), 1) }; + let _val = unsafe { buffer.read() }; + + // Let's find a place to promise alignment 8. + let align8 = if buffer.addr() % 8 == 0 { + buffer + } else { + buffer.wrapping_add(1) + }; + assert!(align8.addr() % 8 == 0); + unsafe { utils::miri_promise_symbolic_alignment(align8.cast(), 8) }; + // Promising the alignment down to 1 *again* still must not hurt. + unsafe { utils::miri_promise_symbolic_alignment(buffer.cast(), 1) }; + // Now we can do 8-aligned reads here. + let _val = unsafe { align8.cast::().read() }; + + // Make sure we error if the pointer is not actually aligned. + if cfg!(call_unaligned_ptr) { + unsafe { utils::miri_promise_symbolic_alignment(align8.add(1).cast(), 8) }; + //~[call_unaligned_ptr]^ ERROR: pointer is not actually aligned + } + + // Also don't accept even higher-aligned reads. + if cfg!(read_unaligned_ptr) { + #[repr(align(16))] + #[derive(Copy, Clone)] + struct Align16(u128); + + let align16 = if align8.addr() % 16 == 0 { + align8 + } else { + align8.wrapping_add(2) + }; + assert!(align16.addr() % 16 == 0); + + let _val = unsafe { align8.cast::().read() }; + //~[read_unaligned_ptr]^ ERROR: accessing memory based on pointer with alignment 8, but alignment 16 is required + } +} diff --git a/src/tools/miri/tests/pass/align_offset_symbolic.rs b/src/tools/miri/tests/pass/align_offset_symbolic.rs index 4e1584b8387..292858ebc2e 100644 --- a/src/tools/miri/tests/pass/align_offset_symbolic.rs +++ b/src/tools/miri/tests/pass/align_offset_symbolic.rs @@ -1,29 +1,6 @@ //@compile-flags: -Zmiri-symbolic-alignment-check #![feature(strict_provenance)] -use std::ptr; - -fn test_align_offset() { - let d = Box::new([0u32; 4]); - // Get u8 pointer to base - let raw = d.as_ptr() as *const u8; - - assert_eq!(raw.align_offset(2), 0); - assert_eq!(raw.align_offset(4), 0); - assert_eq!(raw.align_offset(8), usize::MAX); // requested alignment higher than allocation alignment - - assert_eq!(raw.wrapping_offset(1).align_offset(2), 1); - assert_eq!(raw.wrapping_offset(1).align_offset(4), 3); - assert_eq!(raw.wrapping_offset(1).align_offset(8), usize::MAX); // requested alignment higher than allocation alignment - - assert_eq!(raw.wrapping_offset(2).align_offset(2), 0); - assert_eq!(raw.wrapping_offset(2).align_offset(4), 2); - assert_eq!(raw.wrapping_offset(2).align_offset(8), usize::MAX); // requested alignment higher than allocation alignment - - let p = ptr::invalid::<()>(1); - assert_eq!(p.align_offset(1), 0); -} - fn test_align_to() { const N: usize = 4; let d = Box::new([0u32; N]); @@ -31,6 +8,8 @@ fn test_align_to() { let s = unsafe { std::slice::from_raw_parts(d.as_ptr() as *const u8, 4 * N) }; let raw = s.as_ptr(); + // Cases where we get the expected "middle" part without any fuzz, since the allocation is + // 4-aligned. { let (l, m, r) = unsafe { s.align_to::() }; assert_eq!(l.len(), 0); @@ -63,18 +42,21 @@ fn test_align_to() { assert_eq!(raw.wrapping_offset(4), m.as_ptr() as *const u8); } + // Cases where we request more alignment than the allocation has. { #[repr(align(8))] + #[derive(Copy, Clone)] struct Align8(u64); - let (l, m, r) = unsafe { s.align_to::() }; // requested alignment higher than allocation alignment - assert_eq!(l.len(), 4 * N); - assert_eq!(r.len(), 0); - assert_eq!(m.len(), 0); + let (_l, m, _r) = unsafe { s.align_to::() }; + assert!(m.len() > 0); + // Ensure the symbolic alignment check has been informed that this is okay now. + let _val = m[0]; } } fn test_from_utf8() { + // uses `align_offset` internally const N: usize = 10; let vec = vec![0x4141414141414141u64; N]; let content = unsafe { std::slice::from_raw_parts(vec.as_ptr() as *const u8, 8 * N) }; @@ -103,9 +85,14 @@ fn test_u64_array() { example(&Data::default()); } +fn test_cstr() { + // uses `align_offset` internally + std::ffi::CStr::from_bytes_with_nul(b"this is a test that is longer than 16 bytes\0").unwrap(); +} + fn main() { - test_align_offset(); test_align_to(); test_from_utf8(); test_u64_array(); + test_cstr(); } diff --git a/src/tools/miri/tests/pass/panic/catch_panic.rs b/src/tools/miri/tests/pass/panic/catch_panic.rs index f5b4eaf685d..b83902a8b19 100644 --- a/src/tools/miri/tests/pass/panic/catch_panic.rs +++ b/src/tools/miri/tests/pass/panic/catch_panic.rs @@ -1,5 +1,3 @@ -// We test the `align_offset` panic below, make sure we test the interpreter impl and not the "real" one. -//@compile-flags: -Zmiri-symbolic-alignment-check #![feature(never_type)] #![allow(unconditional_panic, non_fmt_panics)] @@ -70,6 +68,7 @@ fn main() { process::abort() }); + // Panic somewhere in the standard library. test(Some("align_offset: align is not a power-of-two"), |_old_val| { let _ = std::ptr::null::().align_offset(3); process::abort() diff --git a/src/tools/miri/tests/utils/miri_extern.rs b/src/tools/miri/tests/utils/miri_extern.rs index 7363c189840..ff7990561f2 100644 --- a/src/tools/miri/tests/utils/miri_extern.rs +++ b/src/tools/miri/tests/utils/miri_extern.rs @@ -142,4 +142,9 @@ extern "Rust" { /// but in tests we want to for sure run it at certain points to check /// that it doesn't break anything. pub fn miri_run_provenance_gc(); + + /// Miri-provided extern function to promise that a given pointer is properly aligned for + /// "symbolic" alignment checks. Will fail if the pointer is not actually aligned or `align` is + /// not a power of two. Has no effect when alignment checks are concrete (which is the default). + pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); } From 2a3fcc0a57d620f09207ab76671709eba8af8559 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 20 Nov 2023 08:22:56 +0100 Subject: [PATCH 15/15] move calling miri_promise_symbolic_alignment to a shared helper --- library/core/src/intrinsics.rs | 25 +++++++++++++++++++++++++ library/core/src/ptr/const_ptr.rs | 22 +--------------------- library/core/src/ptr/mut_ptr.rs | 25 ++++--------------------- library/core/src/slice/mod.rs | 31 ++++++++----------------------- 4 files changed, 38 insertions(+), 65 deletions(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index 214c8e49a5a..ab86e4e39ea 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -2859,3 +2859,28 @@ pub const unsafe fn write_bytes(dst: *mut T, val: u8, count: usize) { write_bytes(dst, val, count) } } + +/// Inform Miri that a given pointer definitely has a certain alignment. +#[cfg(miri)] +pub(crate) const fn miri_promise_symbolic_alignment(ptr: *const (), align: usize) { + extern "Rust" { + /// Miri-provided extern function to promise that a given pointer is properly aligned for + /// "symbolic" alignment checks. Will fail if the pointer is not actually aligned or `align` is + /// not a power of two. Has no effect when alignment checks are concrete (which is the default). + fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); + } + + fn runtime(ptr: *const (), align: usize) { + // SAFETY: this call is always safe. + unsafe { + miri_promise_symbolic_alignment(ptr, align); + } + } + + const fn compiletime(_ptr: *const (), _align: usize) {} + + // SAFETY: the extra behavior at runtime is for UB checks only. + unsafe { + const_eval_select((ptr, align), compiletime, runtime); + } +} diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index 46e8676162a..2f47ca29ec5 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -1374,27 +1374,7 @@ impl *const T { // Inform Miri that we want to consider the resulting pointer to be suitably aligned. #[cfg(miri)] if ret != usize::MAX { - fn runtime(ptr: *const (), align: usize) { - extern "Rust" { - pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); - } - - // SAFETY: this call is always safe. - unsafe { - miri_promise_symbolic_alignment(ptr, align); - } - } - - const fn compiletime(_ptr: *const (), _align: usize) {} - - // SAFETY: the extra behavior at runtime is for UB checks only. - unsafe { - intrinsics::const_eval_select( - (self.wrapping_add(ret).cast(), align), - compiletime, - runtime, - ); - } + intrinsics::miri_promise_symbolic_alignment(self.wrapping_add(ret).cast(), align); } ret diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index 3f44d78795b..3aaae679a6f 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -1641,27 +1641,10 @@ impl *mut T { // Inform Miri that we want to consider the resulting pointer to be suitably aligned. #[cfg(miri)] if ret != usize::MAX { - fn runtime(ptr: *const (), align: usize) { - extern "Rust" { - pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); - } - - // SAFETY: this call is always safe. - unsafe { - miri_promise_symbolic_alignment(ptr, align); - } - } - - const fn compiletime(_ptr: *const (), _align: usize) {} - - // SAFETY: the extra behavior at runtime is for UB checks only. - unsafe { - intrinsics::const_eval_select( - (self.wrapping_add(ret).cast_const().cast(), align), - compiletime, - runtime, - ); - } + intrinsics::miri_promise_symbolic_alignment( + self.wrapping_add(ret).cast_const().cast(), + align, + ); } ret diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 7d88f70efd1..5957f9fd443 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -3870,16 +3870,10 @@ impl [T] { let (us_len, ts_len) = rest.align_to_offsets::(); // Inform Miri that we want to consider the "middle" pointer to be suitably aligned. #[cfg(miri)] - { - extern "Rust" { - pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); - } - - // SAFETY: this call is always safe. - unsafe { - miri_promise_symbolic_alignment(rest.as_ptr().cast(), mem::align_of::()); - } - } + crate::intrinsics::miri_promise_symbolic_alignment( + rest.as_ptr().cast(), + mem::align_of::(), + ); // SAFETY: now `rest` is definitely aligned, so `from_raw_parts` below is okay, // since the caller guarantees that we can transmute `T` to `U` safely. unsafe { @@ -3952,19 +3946,10 @@ impl [T] { let mut_ptr = rest.as_mut_ptr(); // Inform Miri that we want to consider the "middle" pointer to be suitably aligned. #[cfg(miri)] - { - extern "Rust" { - pub fn miri_promise_symbolic_alignment(ptr: *const (), align: usize); - } - - // SAFETY: this call is always safe. - unsafe { - miri_promise_symbolic_alignment( - mut_ptr.cast() as *const (), - mem::align_of::(), - ); - } - } + crate::intrinsics::miri_promise_symbolic_alignment( + mut_ptr.cast() as *const (), + mem::align_of::(), + ); // We can't use `rest` again after this, that would invalidate its alias `mut_ptr`! // SAFETY: see comments for `align_to`. unsafe {