From 624f9d3c78ab3f11e52a7071c07e8230c9889f23 Mon Sep 17 00:00:00 2001 From: Xiaobo Liu Date: Sat, 2 Mar 2024 16:03:23 +0800 Subject: [PATCH 1/3] style library/core/src/error.rs --- library/core/src/error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/core/src/error.rs b/library/core/src/error.rs index ded17e69bd9c6..a3f2b767054e1 100644 --- a/library/core/src/error.rs +++ b/library/core/src/error.rs @@ -183,6 +183,7 @@ pub trait Error: Debug + Display { #[allow(unused_variables)] fn provide<'a>(&'a self, request: &mut Request<'a>) {} } + mod private { // This is a hack to prevent `type_id` from being overridden by `Error` // implementations, since that can enable unsound downcasting. From ec5e2dc241718b14dcd784eb75f17fd891d5f9b8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 28 Feb 2024 19:12:36 +0100 Subject: [PATCH 2/3] attempt to further clarify addr_of docs --- library/core/src/ptr/mod.rs | 78 ++++++++++++++++++++++++++++++------- 1 file changed, 63 insertions(+), 15 deletions(-) diff --git a/library/core/src/ptr/mod.rs b/library/core/src/ptr/mod.rs index fc5b08c9801a8..018efd4b9b34b 100644 --- a/library/core/src/ptr/mod.rs +++ b/library/core/src/ptr/mod.rs @@ -2071,11 +2071,16 @@ impl fmt::Debug for F { /// as all other references. This macro can create a raw pointer *without* creating /// a reference first. /// -/// The `expr` in `addr_of!(expr)` is evaluated as a place expression, but never loads -/// from the place or requires the place to be dereferenceable. This means that -/// `addr_of!(*ptr)` is defined behavior even if `ptr` is null, dangling, or misaligned. -/// Note however that `addr_of!((*ptr).field)` still requires the projection to -/// `field` to be in-bounds, using the same rules as [`offset`]. +/// See [`addr_of_mut`] for how to create a pointer to uninitialized data. +/// Doing that with `addr_of` would not make much sense since one could only +/// read the data, and that would be Undefined Behavior. +/// +/// # Safety +/// +/// The `expr` in `addr_of!(expr)` is evaluated as a place expression, but never loads from the +/// place or requires the place to be dereferenceable. This means that `addr_of!((*ptr).field)` +/// still requires the projection to `field` to be in-bounds, using the same rules as [`offset`]. +/// However, `addr_of!(*ptr)` is defined behavior even if `ptr` is null, dangling, or misaligned. /// /// Note that `Deref`/`Index` coercions (and their mutable counterparts) are applied inside /// `addr_of!` like everywhere else, in which case a reference is created to call `Deref::deref` or @@ -2086,6 +2091,8 @@ impl fmt::Debug for F { /// /// # Example /// +/// **Correct usage: Creating a pointer to unaligned data** +/// /// ``` /// use std::ptr; /// @@ -2101,9 +2108,27 @@ impl fmt::Debug for F { /// assert_eq!(unsafe { raw_f2.read_unaligned() }, 2); /// ``` /// -/// See [`addr_of_mut`] for how to create a pointer to uninitialized data. -/// Doing that with `addr_of` would not make much sense since one could only -/// read the data, and that would be Undefined Behavior. +/// **Incorrect usage: Out-of-bounds fields projection** +/// +/// ```rust,no_run +/// use std::ptr; +/// +/// #[repr(C)] +/// struct MyStruct { +/// field1: i32, +/// field2: i32, +/// } +/// +/// let ptr: *const MyStruct = ptr::null(); +/// let fieldptr = unsafe { ptr::addr_of!((*ptr).field2) }; // Undefined Behavior ⚠️ +/// ``` +/// +/// The field projection `.field2` would offset the pointer by 4 bytes, +/// but the pointer is not in-bounds of an allocation for 4 bytes, +/// so this offset is Undefined Behavior. +/// See the [`offset`] docs for a full list of requirements for inbounds pointer arithmetic; the +/// same requirements apply to field projections, even inside `addr_of!`. (In particular, it makes +/// no difference whether the pointer is null or dangling.) #[stable(feature = "raw_ref_macros", since = "1.51.0")] #[rustc_macro_transparency = "semitransparent"] #[allow_internal_unstable(raw_ref_op)] @@ -2120,11 +2145,12 @@ pub macro addr_of($place:expr) { /// as all other references. This macro can create a raw pointer *without* creating /// a reference first. /// -/// The `expr` in `addr_of_mut!(expr)` is evaluated as a place expression, but never loads -/// from the place or requires the place to be dereferenceable. This means that -/// `addr_of_mut!(*ptr)` is defined behavior even if `ptr` is null, dangling, or misaligned. -/// Note however that `addr_of_mut!((*ptr).field)` still requires the projection to -/// `field` to be in-bounds, using the same rules as [`offset`]. +/// # Safety +/// +/// The `expr` in `addr_of_mut!(expr)` is evaluated as a place expression, but never loads from the +/// place or requires the place to be dereferenceable. This means that `addr_of_mut!((*ptr).field)` +/// still requires the projection to `field` to be in-bounds, using the same rules as [`offset`]. +/// However, `addr_of_mut!(*ptr)` is defined behavior even if `ptr` is null, dangling, or misaligned. /// /// Note that `Deref`/`Index` coercions (and their mutable counterparts) are applied inside /// `addr_of_mut!` like everywhere else, in which case a reference is created to call `Deref::deref` @@ -2135,7 +2161,7 @@ pub macro addr_of($place:expr) { /// /// # Examples /// -/// **Creating a pointer to unaligned data:** +/// **Correct usage: Creating a pointer to unaligned data** /// /// ``` /// use std::ptr; @@ -2153,7 +2179,7 @@ pub macro addr_of($place:expr) { /// assert_eq!({packed.f2}, 42); // `{...}` forces copying the field instead of creating a reference. /// ``` /// -/// **Creating a pointer to uninitialized data:** +/// **Correct usage: Creating a pointer to uninitialized data** /// /// ```rust /// use std::{ptr, mem::MaybeUninit}; @@ -2169,6 +2195,28 @@ pub macro addr_of($place:expr) { /// unsafe { f1_ptr.write(true); } /// let init = unsafe { uninit.assume_init() }; /// ``` +/// +/// **Incorrect usage: Out-of-bounds fields projection** +/// +/// ```rust,no_run +/// use std::ptr; +/// +/// #[repr(C)] +/// struct MyStruct { +/// field1: i32, +/// field2: i32, +/// } +/// +/// let ptr: *mut MyStruct = ptr::null_mut(); +/// let fieldptr = unsafe { ptr::addr_of_mut!((*ptr).field2) }; // Undefined Behavior ⚠️ +/// ``` +/// +/// The field projection `.field2` would offset the pointer by 4 bytes, +/// but the pointer is not in-bounds of an allocation for 4 bytes, +/// so this offset is Undefined Behavior. +/// See the [`offset`] docs for a full list of requirements for inbounds pointer arithmetic; the +/// same requirements apply to field projections, even inside `addr_of_mut!`. (In particular, it +/// makes no difference whether the pointer is null or dangling.) #[stable(feature = "raw_ref_macros", since = "1.51.0")] #[rustc_macro_transparency = "semitransparent"] #[allow_internal_unstable(raw_ref_op)] From 972d8daf47d8a61e00304e1e362b32ca380ce947 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 2 Mar 2024 17:45:38 +1100 Subject: [PATCH 3/3] The ordinary lowering of `thir::ExprKind::Let` is unreachable After desugaring, `let` expressions should only appear inside `if` expressions or `match` guards, possibly nested within a let-chain. In both cases they are specifically handled by the lowerings of those expressions, so this case is currently unreachable. --- .../rustc_mir_build/src/build/expr/into.rs | 38 +++---------------- 1 file changed, 6 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 2978491d646e8..f2cf6fe613c33 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -109,38 +109,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.cfg.goto(else_blk, source_info, join_block); join_block.unit() } - ExprKind::Let { expr, ref pat } => { - let scope = this.local_scope(); - let (true_block, false_block) = this.in_if_then_scope(scope, expr_span, |this| { - this.lower_let_expr(block, expr, pat, scope, None, expr_span, true) - }); - - this.cfg.push_assign_constant( - true_block, - source_info, - destination, - ConstOperand { - span: expr_span, - user_ty: None, - const_: Const::from_bool(this.tcx, true), - }, - ); - - this.cfg.push_assign_constant( - false_block, - source_info, - destination, - ConstOperand { - span: expr_span, - user_ty: None, - const_: Const::from_bool(this.tcx, false), - }, - ); - - let join_block = this.cfg.start_new_block(); - this.cfg.goto(true_block, source_info, join_block); - this.cfg.goto(false_block, source_info, join_block); - join_block.unit() + ExprKind::Let { .. } => { + // After desugaring, `let` expressions should only appear inside `if` + // expressions or `match` guards, possibly nested within a let-chain. + // In both cases they are specifically handled by the lowerings of + // those expressions, so this case is currently unreachable. + span_bug!(expr_span, "unexpected let expression outside of if or match-guard"); } ExprKind::NeverToAny { source } => { let source_expr = &this.thir[source];