From d6a1b36cb41539be32498d1188eb9d70cd4249fb Mon Sep 17 00:00:00 2001 From: Pavel Grigorenko Date: Thu, 14 Mar 2024 16:01:29 +0300 Subject: [PATCH 01/10] Implement ptr_as_ref_unchecked (#122034) --- library/core/src/ptr/const_ptr.rs | 48 ++++++++++++++ library/core/src/ptr/mut_ptr.rs | 103 ++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+) diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index a0c04d3f65dfd..a8af4c1b29138 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -362,6 +362,54 @@ impl *const T { if self.is_null() { None } else { unsafe { Some(&*self) } } } + /// Returns a shared reference to the value behind the pointer. + /// If the pointer may be null or the value may be uninitialized, [`as_uninit_ref`] must be used instead. + /// If the pointer may be null, but the value is known to have been initialized, [`as_ref`] must be used instead. + /// + /// [`as_ref`]: #method.as_ref + /// [`as_uninit_ref`]: #method.as_uninit_ref + /// + /// # Safety + /// + /// When calling this method, you have to ensure that all of the following is true: + /// + /// * The pointer must be properly aligned. + /// + /// * It must be "dereferenceable" in the sense defined in [the module documentation]. + /// + /// * The pointer must point to an initialized instance of `T`. + /// + /// * You must enforce Rust's aliasing rules, since the returned lifetime `'a` is + /// arbitrarily chosen and does not necessarily reflect the actual lifetime of the data. + /// In particular, while this reference exists, the memory the pointer points to must + /// not get mutated (except inside `UnsafeCell`). + /// + /// This applies even if the result of this method is unused! + /// (The part about being initialized is not yet fully decided, but until + /// it is, the only safe approach is to ensure that they are indeed initialized.) + /// + /// [the module documentation]: crate::ptr#safety + /// + /// # Examples + /// + /// ``` + /// #![feature(ptr_as_ref_unchecked)] + /// let ptr: *const u8 = &10u8 as *const u8; + /// + /// unsafe { + /// println!("We got back the value: {}!", ptr.as_ref_unchecked()); + /// } + /// ``` + // FIXME: mention it in the docs for `as_ref` and `as_uninit_ref` once stabilized. + #[unstable(feature = "ptr_as_ref_unchecked", issue = "122034")] + #[rustc_const_unstable(feature = "const_ptr_as_ref", issue = "91822")] + #[inline] + #[must_use] + pub const unsafe fn as_ref_unchecked<'a>(self) -> &'a T { + // SAFETY: the caller must guarantee that `self` is valid for a reference + unsafe { &*self } + } + /// Returns `None` if the pointer is null, or else returns a shared reference to /// the value wrapped in `Some`. In contrast to [`as_ref`], this does not require /// that the value has to be initialized. diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index 6a9033a144deb..4a6c9e8b8e0e3 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -372,6 +372,57 @@ impl *mut T { if self.is_null() { None } else { unsafe { Some(&*self) } } } + /// Returns a shared reference to the value behind the pointer. + /// If the pointer may be null or the value may be uninitialized, [`as_uninit_ref`] must be used instead. + /// If the pointer may be null, but the value is known to have been initialized, [`as_ref`] must be used instead. + /// + /// For the mutable counterpart see [`as_mut_unchecked`]. + /// + /// [`as_ref`]: #method.as_ref + /// [`as_uninit_ref`]: #method.as_uninit_ref + /// [`as_mut_unchecked`]: #method.as_mut_unchecked + /// + /// # Safety + /// + /// When calling this method, you have to ensure that all of the following is true: + /// + /// * The pointer must be properly aligned. + /// + /// * It must be "dereferenceable" in the sense defined in [the module documentation]. + /// + /// * The pointer must point to an initialized instance of `T`. + /// + /// * You must enforce Rust's aliasing rules, since the returned lifetime `'a` is + /// arbitrarily chosen and does not necessarily reflect the actual lifetime of the data. + /// In particular, while this reference exists, the memory the pointer points to must + /// not get mutated (except inside `UnsafeCell`). + /// + /// This applies even if the result of this method is unused! + /// (The part about being initialized is not yet fully decided, but until + /// it is, the only safe approach is to ensure that they are indeed initialized.) + /// + /// [the module documentation]: crate::ptr#safety + /// + /// # Examples + /// + /// ``` + /// #![feature(ptr_as_ref_unchecked)] + /// let ptr: *mut u8 = &mut 10u8 as *mut u8; + /// + /// unsafe { + /// println!("We got back the value: {}!", ptr.as_ref_unchecked()); + /// } + /// ``` + // FIXME: mention it in the docs for `as_ref` and `as_uninit_ref` once stabilized. + #[unstable(feature = "ptr_as_ref_unchecked", issue = "122034")] + #[rustc_const_unstable(feature = "const_ptr_as_ref", issue = "91822")] + #[inline] + #[must_use] + pub const unsafe fn as_ref_unchecked<'a>(self) -> &'a T { + // SAFETY: the caller must guarantee that `self` is valid for a reference + unsafe { &*self } + } + /// Returns `None` if the pointer is null, or else returns a shared reference to /// the value wrapped in `Some`. In contrast to [`as_ref`], this does not require /// that the value has to be initialized. @@ -693,6 +744,58 @@ impl *mut T { if self.is_null() { None } else { unsafe { Some(&mut *self) } } } + /// Returns a unique reference to the value behind the pointer. + /// If the pointer may be null or the value may be uninitialized, [`as_uninit_mut`] must be used instead. + /// If the pointer may be null, but the value is known to have been initialized, [`as_mut`] must be used instead. + /// + /// For the shared counterpart see [`as_ref_unchecked`]. + /// + /// [`as_mut`]: #method.as_mut + /// [`as_uninit_mut`]: #method.as_uninit_mut + /// [`as_ref_unchecked`]: #method.as_mut_unchecked + /// + /// # Safety + /// + /// When calling this method, you have to ensure that all of the following is true: + /// + /// * The pointer must be properly aligned. + /// + /// * It must be "dereferenceable" in the sense defined in [the module documentation]. + /// + /// * The pointer must point to an initialized instance of `T`. + /// + /// * You must enforce Rust's aliasing rules, since the returned lifetime `'a` is + /// arbitrarily chosen and does not necessarily reflect the actual lifetime of the data. + /// In particular, while this reference exists, the memory the pointer points to must + /// not get mutated (except inside `UnsafeCell`). + /// + /// This applies even if the result of this method is unused! + /// (The part about being initialized is not yet fully decided, but until + /// it is, the only safe approach is to ensure that they are indeed initialized.) + /// + /// [the module documentation]: crate::ptr#safety + /// + /// # Examples + /// + /// ``` + /// #![feature(ptr_as_ref_unchecked)] + /// let mut s = [1, 2, 3]; + /// let ptr: *mut u32 = s.as_mut_ptr(); + /// let first_value = unsafe { ptr.as_mut_unchecked() }; + /// *first_value = 4; + /// # assert_eq!(s, [4, 2, 3]); + /// println!("{s:?}"); // It'll print: "[4, 2, 3]". + /// ``` + // FIXME: mention it in the docs for `as_mut` and `as_uninit_mut` once stabilized. + #[unstable(feature = "ptr_as_ref_unchecked", issue = "122034")] + #[rustc_const_unstable(feature = "const_ptr_as_ref", issue = "91822")] + #[inline] + #[must_use] + pub const unsafe fn as_mut_unchecked<'a>(self) -> &'a mut T { + // SAFETY: the caller must guarantee that `self` is valid for a reference + unsafe { &mut *self } + } + /// Returns `None` if the pointer is null, or else returns a unique reference to /// the value wrapped in `Some`. In contrast to [`as_mut`], this does not require /// that the value has to be initialized. From 3ad0618076af29d23c8d0ca386e4da0b1f271d18 Mon Sep 17 00:00:00 2001 From: Jacob Trueb Date: Thu, 11 Apr 2024 13:24:32 -0500 Subject: [PATCH 02/10] Fix cannot usage in time.rs --- library/core/src/time.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/time.rs b/library/core/src/time.rs index 78494b866b108..72f6a3b773bb5 100644 --- a/library/core/src/time.rs +++ b/library/core/src/time.rs @@ -1437,10 +1437,10 @@ impl TryFromFloatSecsError { const fn description(&self) -> &'static str { match self.kind { TryFromFloatSecsErrorKind::Negative => { - "can not convert float seconds to Duration: value is negative" + "cannot convert float seconds to Duration: value is negative" } TryFromFloatSecsErrorKind::OverflowOrNan => { - "can not convert float seconds to Duration: value is either too big or NaN" + "cannot convert float seconds to Duration: value is either too big or NaN" } } } From 589c2fe24d1f9e7bde8a50b07bb53eb4fbee6d8e Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Mon, 29 Apr 2024 14:59:39 +0800 Subject: [PATCH 03/10] Add raw identifier in a typo suggestion --- compiler/rustc_resolve/src/diagnostics.rs | 2 +- tests/ui/span/suggestion-raw-68962.rs | 11 +++++++++++ tests/ui/span/suggestion-raw-68962.stderr | 18 ++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 tests/ui/span/suggestion-raw-68962.rs create mode 100644 tests/ui/span/suggestion-raw-68962.stderr diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 12484462f82f4..01e279b6d04fa 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -1617,7 +1617,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let post = format!(", consider renaming `{}` into `{snippet}`", suggestion.candidate); (span, snippet, post) } else { - (span, suggestion.candidate.to_string(), String::new()) + (span, suggestion.candidate.to_ident_string(), String::new()) }; let msg = match suggestion.target { SuggestionTarget::SimilarlyNamed => format!( diff --git a/tests/ui/span/suggestion-raw-68962.rs b/tests/ui/span/suggestion-raw-68962.rs new file mode 100644 index 0000000000000..0b581308f6628 --- /dev/null +++ b/tests/ui/span/suggestion-raw-68962.rs @@ -0,0 +1,11 @@ +fn r#fn() {} + +fn main() { + let r#final = 1; + + // Should correctly suggest variable defined using raw identifier. + fina; //~ ERROR cannot find value + + // Should correctly suggest function defined using raw identifier. + f(); //~ ERROR cannot find function +} diff --git a/tests/ui/span/suggestion-raw-68962.stderr b/tests/ui/span/suggestion-raw-68962.stderr new file mode 100644 index 0000000000000..2e25f5cbdf58d --- /dev/null +++ b/tests/ui/span/suggestion-raw-68962.stderr @@ -0,0 +1,18 @@ +error[E0425]: cannot find value `fina` in this scope + --> $DIR/suggestion-raw-68962.rs:7:5 + | +LL | fina; + | ^^^^ help: a local variable with a similar name exists: `r#final` + +error[E0425]: cannot find function `f` in this scope + --> $DIR/suggestion-raw-68962.rs:10:5 + | +LL | fn r#fn() {} + | --------- similarly named function `r#fn` defined here +... +LL | f(); + | ^ help: a function with a similar name exists: `r#fn` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0425`. From 52d608b5607d17baa398c45674e84b218d5aa072 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 30 Apr 2024 17:18:59 +1000 Subject: [PATCH 04/10] coverage: Eagerly do start-of-function codegen for coverage --- .../src/coverageinfo/mod.rs | 54 +++++++++---------- compiler/rustc_codegen_ssa/src/mir/mod.rs | 4 ++ .../src/traits/coverageinfo.rs | 5 ++ 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 679c6e1a2ff83..551d923444c7e 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -13,7 +13,7 @@ use rustc_codegen_ssa::traits::{ use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_llvm::RustString; use rustc_middle::bug; -use rustc_middle::mir::coverage::{CoverageKind, FunctionCoverageInfo}; +use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::ty::layout::HasTyCtxt; use rustc_middle::ty::Instance; use rustc_target::abi::Align; @@ -91,6 +91,32 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { } impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { + fn init_coverage(&mut self, instance: Instance<'tcx>) { + let Some(function_coverage_info) = + self.tcx.instance_mir(instance.def).function_coverage_info.as_deref() + else { + return; + }; + + // If there are no MC/DC bitmaps to set up, return immediately. + if function_coverage_info.mcdc_bitmap_bytes == 0 { + return; + } + + let fn_name = self.get_pgo_func_name_var(instance); + let hash = self.const_u64(function_coverage_info.function_source_hash); + let bitmap_bytes = self.const_u32(function_coverage_info.mcdc_bitmap_bytes); + let max_decision_depth = function_coverage_info.mcdc_max_decision_depth; + let cond_bitmap = + self.mcdc_parameters(fn_name, hash, bitmap_bytes, max_decision_depth as u32); + + self.coverage_context() + .expect("always present when coverage is enabled") + .mcdc_condition_bitmap_map + .borrow_mut() + .insert(instance, cond_bitmap); + } + #[instrument(level = "debug", skip(self))] fn add_coverage(&mut self, instance: Instance<'tcx>, kind: &CoverageKind) { // Our caller should have already taken care of inlining subtleties, @@ -109,10 +135,6 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { return; }; - if function_coverage_info.mcdc_bitmap_bytes > 0 { - ensure_mcdc_parameters(bx, instance, function_coverage_info); - } - let Some(coverage_context) = bx.coverage_context() else { return }; let mut coverage_map = coverage_context.function_coverage_map.borrow_mut(); let func_coverage = coverage_map @@ -193,28 +215,6 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { } } -fn ensure_mcdc_parameters<'ll, 'tcx>( - bx: &mut Builder<'_, 'll, 'tcx>, - instance: Instance<'tcx>, - function_coverage_info: &FunctionCoverageInfo, -) { - let Some(cx) = bx.coverage_context() else { return }; - if cx.mcdc_condition_bitmap_map.borrow().contains_key(&instance) { - return; - } - - let fn_name = bx.get_pgo_func_name_var(instance); - let hash = bx.const_u64(function_coverage_info.function_source_hash); - let bitmap_bytes = bx.const_u32(function_coverage_info.mcdc_bitmap_bytes); - let max_decision_depth = function_coverage_info.mcdc_max_decision_depth; - let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes, max_decision_depth as u32); - bx.coverage_context() - .expect("already checked above") - .mcdc_condition_bitmap_map - .borrow_mut() - .insert(instance, cond_bitmap); -} - /// Calls llvm::createPGOFuncNameVar() with the given function instance's /// mangled function name. The LLVM API returns an llvm::GlobalVariable /// containing the function name, with the specific variable name and linkage diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 0064c16f5d9f6..cf6e2e8d14c6c 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -259,6 +259,10 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( // Apply debuginfo to the newly allocated locals. fx.debug_introduce_locals(&mut start_bx); + // If the backend supports coverage, and coverage is enabled for this function, + // do any necessary start-of-function codegen (e.g. locals for MC/DC bitmaps). + start_bx.init_coverage(instance); + // The builders will be created separately for each basic block at `codegen_block`. // So drop the builder of `start_llbb` to avoid having two at the same time. drop(start_bx); diff --git a/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs b/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs index d1d813bd38922..906d8b87d3bc9 100644 --- a/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs +++ b/compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs @@ -3,6 +3,11 @@ use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::ty::Instance; pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes { + /// Performs any start-of-function codegen needed for coverage instrumentation. + /// + /// Can be a no-op in backends that don't support coverage instrumentation. + fn init_coverage(&mut self, _instance: Instance<'tcx>) {} + /// Handle the MIR coverage info in a backend-specific way. /// /// This can potentially be a no-op in backends that don't support From 0b3a47900e995cbaaa71b951d7fd5244cf966c08 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 30 Apr 2024 17:45:40 +1000 Subject: [PATCH 05/10] coverage: Set up MC/DC bitmaps without additional unsafe code Because this now always takes place at the start of the function, we can just use the normal `alloca` method and then initialize each bitmap immediately. This patch also moves bitmap setup out of the `mcdc_parameters` method, because there is no longer any particular reason for it to be there. --- compiler/rustc_abi/src/lib.rs | 2 ++ compiler/rustc_abi/src/tests.rs | 7 ++++ compiler/rustc_codegen_llvm/src/builder.rs | 36 +++++++------------ .../src/coverageinfo/mod.rs | 20 ++++++++--- 4 files changed, 36 insertions(+), 29 deletions(-) create mode 100644 compiler/rustc_abi/src/tests.rs diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index a46148242d5cc..31b4ef0b1ea61 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -21,6 +21,8 @@ use rustc_macros::{Decodable_Generic, Encodable_Generic}; use std::iter::Step; mod layout; +#[cfg(test)] +mod tests; pub use layout::LayoutCalculator; diff --git a/compiler/rustc_abi/src/tests.rs b/compiler/rustc_abi/src/tests.rs new file mode 100644 index 0000000000000..d993012378c81 --- /dev/null +++ b/compiler/rustc_abi/src/tests.rs @@ -0,0 +1,7 @@ +use super::*; + +#[test] +fn align_constants() { + assert_eq!(Align::ONE, Align::from_bytes(1).unwrap()); + assert_eq!(Align::EIGHT, Align::from_bytes(8).unwrap()); +} diff --git a/compiler/rustc_codegen_llvm/src/builder.rs b/compiler/rustc_codegen_llvm/src/builder.rs index 5c8f358d03a1f..1a1b4ae383133 100644 --- a/compiler/rustc_codegen_llvm/src/builder.rs +++ b/compiler/rustc_codegen_llvm/src/builder.rs @@ -17,7 +17,7 @@ use rustc_data_structures::small_c_str::SmallCStr; use rustc_hir::def_id::DefId; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs; use rustc_middle::ty::layout::{ - FnAbiError, FnAbiOfHelpers, FnAbiRequest, HasTyCtxt, LayoutError, LayoutOfHelpers, TyAndLayout, + FnAbiError, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOfHelpers, TyAndLayout, }; use rustc_middle::ty::{self, Instance, Ty, TyCtxt}; use rustc_sanitizers::{cfi, kcfi}; @@ -27,7 +27,6 @@ use rustc_target::abi::{self, call::FnAbi, Align, Size, WrappingRange}; use rustc_target::spec::{HasTargetSpec, SanitizerSet, Target}; use smallvec::SmallVec; use std::borrow::Cow; -use std::ffi::CString; use std::iter; use std::ops::Deref; use std::ptr; @@ -1705,13 +1704,21 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { kcfi_bundle } + /// Emits a call to `llvm.instrprof.mcdc.parameters`. + /// + /// This doesn't produce any code directly, but is used as input by + /// the LLVM pass that handles coverage instrumentation. + /// + /// (See clang's [`CodeGenPGO::emitMCDCParameters`] for comparison.) + /// + /// [`CodeGenPGO::emitMCDCParameters`]: + /// https://github.com/rust-lang/llvm-project/blob/5399a24/clang/lib/CodeGen/CodeGenPGO.cpp#L1124 pub(crate) fn mcdc_parameters( &mut self, fn_name: &'ll Value, hash: &'ll Value, bitmap_bytes: &'ll Value, - max_decision_depth: u32, - ) -> Vec<&'ll Value> { + ) { debug!("mcdc_parameters() with args ({:?}, {:?}, {:?})", fn_name, hash, bitmap_bytes); assert!(llvm_util::get_version() >= (18, 0, 0), "MCDC intrinsics require LLVM 18 or later"); @@ -1724,8 +1731,6 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { let args = &[fn_name, hash, bitmap_bytes]; let args = self.check_call("call", llty, llfn, args); - let mut cond_bitmaps = vec![]; - unsafe { let _ = llvm::LLVMRustBuildCall( self.llbuilder, @@ -1736,23 +1741,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { [].as_ptr(), 0 as c_uint, ); - // Create condition bitmap named `mcdc.addr`. - for i in 0..=max_decision_depth { - let mut bx = Builder::with_cx(self.cx); - bx.position_at_start(llvm::LLVMGetFirstBasicBlock(self.llfn())); - - let name = CString::new(format!("mcdc.addr.{i}")).unwrap(); - let cond_bitmap = { - let alloca = - llvm::LLVMBuildAlloca(bx.llbuilder, bx.cx.type_i32(), name.as_ptr()); - llvm::LLVMSetAlignment(alloca, 4); - alloca - }; - bx.store(self.const_i32(0), cond_bitmap, self.tcx().data_layout.i32_align.abi); - cond_bitmaps.push(cond_bitmap); - } } - cond_bitmaps } pub(crate) fn mcdc_tvbitmap_update( @@ -1794,8 +1783,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> { 0 as c_uint, ); } - let i32_align = self.tcx().data_layout.i32_align.abi; - self.store(self.const_i32(0), mcdc_temp, i32_align); + self.store(self.const_i32(0), mcdc_temp, self.tcx.data_layout.i32_align.abi); } pub(crate) fn mcdc_condbitmap_update( diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 551d923444c7e..ac5ab365875cd 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -16,7 +16,7 @@ use rustc_middle::bug; use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::ty::layout::HasTyCtxt; use rustc_middle::ty::Instance; -use rustc_target::abi::Align; +use rustc_target::abi::{Align, Size}; use std::cell::RefCell; @@ -106,15 +106,25 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { let fn_name = self.get_pgo_func_name_var(instance); let hash = self.const_u64(function_coverage_info.function_source_hash); let bitmap_bytes = self.const_u32(function_coverage_info.mcdc_bitmap_bytes); - let max_decision_depth = function_coverage_info.mcdc_max_decision_depth; - let cond_bitmap = - self.mcdc_parameters(fn_name, hash, bitmap_bytes, max_decision_depth as u32); + self.mcdc_parameters(fn_name, hash, bitmap_bytes); + + // Create pointers named `mcdc.addr.{i}` to stack-allocated condition bitmaps. + let mut cond_bitmaps = vec![]; + for i in 0..=function_coverage_info.mcdc_max_decision_depth { + // MC/DC intrinsics will perform loads/stores that use the ABI default + // alignment for i32, so our variable declaration should match. + let align = self.tcx.data_layout.i32_align.abi; + let cond_bitmap = self.alloca(Size::from_bytes(4), align); + llvm::set_value_name(cond_bitmap, format!("mcdc.addr.{i}").as_bytes()); + self.store(self.const_i32(0), cond_bitmap, align); + cond_bitmaps.push(cond_bitmap); + } self.coverage_context() .expect("always present when coverage is enabled") .mcdc_condition_bitmap_map .borrow_mut() - .insert(instance, cond_bitmap); + .insert(instance, cond_bitmaps); } #[instrument(level = "debug", skip(self))] From de972b7321a90b85cef953f659b8d6ec5a5a865f Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 30 Apr 2024 17:54:06 +1000 Subject: [PATCH 06/10] coverage: Replace `max_decision_depth` with `num_condition_bitmaps` This clearly distinguishes individual decision-depth indices from the total number of condition bitmaps to allocate. --- compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs | 2 +- compiler/rustc_middle/src/mir/coverage.rs | 2 +- compiler/rustc_mir_transform/src/coverage/mod.rs | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index ac5ab365875cd..c51a7744a30a3 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -110,7 +110,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { // Create pointers named `mcdc.addr.{i}` to stack-allocated condition bitmaps. let mut cond_bitmaps = vec![]; - for i in 0..=function_coverage_info.mcdc_max_decision_depth { + for i in 0..function_coverage_info.mcdc_num_condition_bitmaps { // MC/DC intrinsics will perform loads/stores that use the ABI default // alignment for i32, so our variable declaration should match. let align = self.tcx.data_layout.i32_align.abi; diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 9d9ca22247ad6..477303e2434f4 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -277,7 +277,7 @@ pub struct FunctionCoverageInfo { pub mappings: Vec, /// The depth of the deepest decision is used to know how many /// temp condbitmaps should be allocated for the function. - pub mcdc_max_decision_depth: u16, + pub mcdc_num_condition_bitmaps: usize, } /// Branch information recorded during THIR-to-MIR lowering, and stored in MIR. diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 9edde6662469e..ffe61e761c53d 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -102,7 +102,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: inject_mcdc_statements(mir_body, &basic_coverage_blocks, &coverage_spans); - let mcdc_max_decision_depth = coverage_spans + let mcdc_num_condition_bitmaps = coverage_spans .mappings .iter() .filter_map(|bcb_mapping| match bcb_mapping.kind { @@ -110,7 +110,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: _ => None, }) .max() - .unwrap_or(0); + .map_or(0, |max| usize::from(max) + 1); mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo { function_source_hash: hir_info.function_source_hash, @@ -118,7 +118,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir: mcdc_bitmap_bytes: coverage_spans.test_vector_bitmap_bytes(), expressions: coverage_counters.into_expressions(), mappings, - mcdc_max_decision_depth, + mcdc_num_condition_bitmaps, })); } From e610a52a6259cae4c9f3ebdb6016e68b507878e4 Mon Sep 17 00:00:00 2001 From: George Bateman Date: Wed, 1 May 2024 19:28:45 +0100 Subject: [PATCH 07/10] Describe and use CStr literals in CStr and CString docs --- library/alloc/src/ffi/c_str.rs | 14 +++++--------- library/core/src/ffi/c_str.rs | 35 ++++++++++++++++++++++------------ 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/library/alloc/src/ffi/c_str.rs b/library/alloc/src/ffi/c_str.rs index 6a64eaf576bb9..f143e5578717f 100644 --- a/library/alloc/src/ffi/c_str.rs +++ b/library/alloc/src/ffi/c_str.rs @@ -41,6 +41,7 @@ use crate::sync::Arc; /// or anything that implements [Into]<[Vec]<[u8]>> (for /// example, you can build a `CString` straight out of a [`String`] or /// a &[str], since both implement that trait). +/// You can create a `CString` from a literal with `CString::from(c"Text")`. /// /// The [`CString::new`] method will actually check that the provided &[[u8]] /// does not have 0 bytes in the middle, and return an error if it @@ -1069,27 +1070,22 @@ impl CStr { /// /// # Examples /// - /// Calling `to_string_lossy` on a `CStr` containing valid UTF-8: + /// Calling `to_string_lossy` on a `CStr` containing valid UTF-8. The leading + /// `c` on the string literal denotes a `CStr`. /// /// ``` /// use std::borrow::Cow; - /// use std::ffi::CStr; /// - /// let cstr = CStr::from_bytes_with_nul(b"Hello World\0") - /// .expect("CStr::from_bytes_with_nul failed"); - /// assert_eq!(cstr.to_string_lossy(), Cow::Borrowed("Hello World")); + /// assert_eq!(c"Hello World".to_string_lossy(), Cow::Borrowed("Hello World")); /// ``` /// /// Calling `to_string_lossy` on a `CStr` containing invalid UTF-8: /// /// ``` /// use std::borrow::Cow; - /// use std::ffi::CStr; /// - /// let cstr = CStr::from_bytes_with_nul(b"Hello \xF0\x90\x80World\0") - /// .expect("CStr::from_bytes_with_nul failed"); /// assert_eq!( - /// cstr.to_string_lossy(), + /// c"Hello \xF0\x90\x80World".to_string_lossy(), /// Cow::Owned(String::from("Hello �World")) as Cow<'_, str> /// ); /// ``` diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index f4f33f8584bd0..9d914d798eb22 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -23,28 +23,32 @@ use crate::str; /// /// This type represents a borrowed reference to a nul-terminated /// array of bytes. It can be constructed safely from a &[[u8]] -/// slice, or unsafely from a raw `*const c_char`. It can then be -/// converted to a Rust &[str] by performing UTF-8 validation, or -/// into an owned `CString`. +/// slice, or unsafely from a raw `*const c_char`. It can be expressed as a +/// literal in the form `c"Hello world"`. +/// +/// The `CStr` can then be converted to a Rust &[str] by performing +/// UTF-8 validation, or into an owned `CString`. /// /// `&CStr` is to `CString` as &[str] is to `String`: the former /// in each pair are borrowed references; the latter are owned /// strings. /// /// Note that this structure does **not** have a guaranteed layout (the `repr(transparent)` -/// notwithstanding) and is not recommended to be placed in the signatures of FFI functions. -/// Instead, safe wrappers of FFI functions may leverage the unsafe [`CStr::from_ptr`] constructor -/// to provide a safe interface to other consumers. +/// notwithstanding) and should not be placed in the signatures of FFI functions. +/// Instead, safe wrappers of FFI functions may leverage [`CStr::as_ptr`] and the unsafe +/// [`CStr::from_ptr`] constructor to provide a safe interface to other consumers. /// /// # Examples /// /// Inspecting a foreign C string: /// -/// ```ignore (extern-declaration) +/// ``` /// use std::ffi::CStr; /// use std::os::raw::c_char; /// +/// # #[cfg(any())] // Extern functions are awkward in doc comments - fake it instead /// extern "C" { fn my_string() -> *const c_char; } +/// # unsafe extern "C" fn my_string() -> *const c_char { c"hello".as_ptr() } /// /// unsafe { /// let slice = CStr::from_ptr(my_string()); @@ -54,12 +58,14 @@ use crate::str; /// /// Passing a Rust-originating C string: /// -/// ```ignore (extern-declaration) +/// ``` /// use std::ffi::{CString, CStr}; /// use std::os::raw::c_char; /// /// fn work(data: &CStr) { +/// # #[cfg(any())] // Extern functions are awkward in doc comments - fake it instead /// extern "C" { fn work_with(data: *const c_char); } +/// # unsafe extern "C" fn work_with(s: *const c_char) {} /// /// unsafe { work_with(data.as_ptr()) } /// } @@ -70,11 +76,13 @@ use crate::str; /// /// Converting a foreign C string into a Rust `String`: /// -/// ```ignore (extern-declaration) +/// ``` /// use std::ffi::CStr; /// use std::os::raw::c_char; /// +/// # #[cfg(any())] // Extern functions are awkward in doc comments - fake it instead /// extern "C" { fn my_string() -> *const c_char; } +/// # unsafe extern "C" fn my_string() -> *const c_char { c"hello".as_ptr() } /// /// fn my_string_safe() -> String { /// let cstr = unsafe { CStr::from_ptr(my_string()) }; @@ -241,11 +249,11 @@ impl CStr { /// /// # Examples /// - /// ```ignore (extern-declaration) + /// ``` /// use std::ffi::{c_char, CStr}; /// - /// extern "C" { - /// fn my_string() -> *const c_char; + /// fn my_string() -> *const c_char { + /// c"hello".as_ptr() /// } /// /// unsafe { @@ -264,6 +272,8 @@ impl CStr { /// BYTES.as_ptr().cast() /// }; /// const HELLO: &CStr = unsafe { CStr::from_ptr(HELLO_PTR) }; + /// + /// assert_eq!(c"Hello, world!", HELLO); /// ``` /// /// [valid]: core::ptr#safety @@ -549,6 +559,7 @@ impl CStr { /// /// let empty_cstr = CStr::from_bytes_with_nul(b"\0")?; /// assert!(empty_cstr.is_empty()); + /// assert!(c"".is_empty()); /// # Ok(()) /// # } /// ``` From 22456309eae112e23f382b88729d00ac815a2c85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 2 May 2024 21:23:52 +0200 Subject: [PATCH 08/10] CI: remove `env-x86_64-apple-tests` YAML anchor --- src/ci/github-actions/jobs.yml | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/ci/github-actions/jobs.yml b/src/ci/github-actions/jobs.yml index 1fabf889e38a1..42749f806530e 100644 --- a/src/ci/github-actions/jobs.yml +++ b/src/ci/github-actions/jobs.yml @@ -36,6 +36,17 @@ runners: os: [ self-hosted, ARM64, linux ] envs: + env-x86_64-apple-tests: &env-x86_64-apple-tests + SCRIPT: ./x.py --stage 2 test --skip tests/ui --skip tests/rustdoc --skip tests/run-make-fulldeps + RUST_CONFIGURE_ARGS: --build=x86_64-apple-darwin --enable-sanitizers --enable-profiler --set rust.jemalloc + RUSTC_RETRY_LINKER_ON_SEGFAULT: 1 + MACOSX_DEPLOYMENT_TARGET: 10.12 + MACOSX_STD_DEPLOYMENT_TARGET: 10.12 + SELECT_XCODE: /Applications/Xcode_14.3.1.app + NO_LLVM_ASSERTIONS: 1 + NO_DEBUG_ASSERTIONS: 1 + NO_OVERFLOW_CHECKS: 1 + production: &production DEPLOY_BUCKET: rust-lang-ci2 @@ -272,16 +283,8 @@ auto: <<: *job-macos-xl - image: x86_64-apple-1 - env: &env-x86_64-apple-tests - SCRIPT: ./x.py --stage 2 test --skip tests/ui --skip tests/rustdoc --skip tests/run-make-fulldeps - RUST_CONFIGURE_ARGS: --build=x86_64-apple-darwin --enable-sanitizers --enable-profiler --set rust.jemalloc - RUSTC_RETRY_LINKER_ON_SEGFAULT: 1 - MACOSX_DEPLOYMENT_TARGET: 10.12 - MACOSX_STD_DEPLOYMENT_TARGET: 10.12 - SELECT_XCODE: /Applications/Xcode_14.3.1.app - NO_LLVM_ASSERTIONS: 1 - NO_DEBUG_ASSERTIONS: 1 - NO_OVERFLOW_CHECKS: 1 + env: + <<: *env-x86_64-apple-tests <<: *job-macos-xl - image: x86_64-apple-2 From 0e0042f16ec74bd9b219f4df65c5589636cacd15 Mon Sep 17 00:00:00 2001 From: George Bateman Date: Thu, 2 May 2024 21:29:13 +0100 Subject: [PATCH 09/10] Update based on review --- library/core/src/ffi/c_str.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index 9d914d798eb22..57cc0e4d612b8 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -46,9 +46,9 @@ use crate::str; /// use std::ffi::CStr; /// use std::os::raw::c_char; /// -/// # #[cfg(any())] // Extern functions are awkward in doc comments - fake it instead +/// # /* Extern functions are awkward in doc comments - fake it instead /// extern "C" { fn my_string() -> *const c_char; } -/// # unsafe extern "C" fn my_string() -> *const c_char { c"hello".as_ptr() } +/// # */ unsafe extern "C" fn my_string() -> *const c_char { c"hello".as_ptr() } /// /// unsafe { /// let slice = CStr::from_ptr(my_string()); @@ -63,9 +63,9 @@ use crate::str; /// use std::os::raw::c_char; /// /// fn work(data: &CStr) { -/// # #[cfg(any())] // Extern functions are awkward in doc comments - fake it instead +/// # /* Extern functions are awkward in doc comments - fake it instead /// extern "C" { fn work_with(data: *const c_char); } -/// # unsafe extern "C" fn work_with(s: *const c_char) {} +/// # */ unsafe extern "C" fn work_with(s: *const c_char) {} /// /// unsafe { work_with(data.as_ptr()) } /// } @@ -80,9 +80,9 @@ use crate::str; /// use std::ffi::CStr; /// use std::os::raw::c_char; /// -/// # #[cfg(any())] // Extern functions are awkward in doc comments - fake it instead +/// # /* Extern functions are awkward in doc comments - fake it instead /// extern "C" { fn my_string() -> *const c_char; } -/// # unsafe extern "C" fn my_string() -> *const c_char { c"hello".as_ptr() } +/// # */ unsafe extern "C" fn my_string() -> *const c_char { c"hello".as_ptr() } /// /// fn my_string_safe() -> String { /// let cstr = unsafe { CStr::from_ptr(my_string()) }; @@ -258,7 +258,7 @@ impl CStr { /// /// unsafe { /// let slice = CStr::from_ptr(my_string()); - /// println!("string returned: {}", slice.to_str().unwrap()); + /// assert_eq!(slice.to_str().unwrap(), "hello"); /// } /// ``` /// From 3f6703bbd8537e4d8957a5cb458b8e53a8e0e9f0 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 17 Apr 2024 10:06:06 +0200 Subject: [PATCH 10/10] default_alloc_error_hook: explain difference to default __rdl_oom in alloc --- library/std/src/alloc.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/library/std/src/alloc.rs b/library/std/src/alloc.rs index dc0e302a81088..b98fbbf762fa2 100644 --- a/library/std/src/alloc.rs +++ b/library/std/src/alloc.rs @@ -353,6 +353,12 @@ fn default_alloc_error_hook(layout: Layout) { if unsafe { __rust_alloc_error_handler_should_panic != 0 } { panic!("memory allocation of {} bytes failed", layout.size()); } else { + // This is the default path taken on OOM, and the only path taken on stable with std. + // Crucially, it does *not* call any user-defined code, and therefore users do not have to + // worry about allocation failure causing reentrancy issues. That makes it different from + // the default `__rdl_oom` defined in alloc (i.e., the default alloc error handler that is + // called when there is no `#[alloc_error_handler]`), which triggers a regular panic and + // thus can invoke a user-defined panic hook, executing arbitrary user-defined code. rtprintpanic!("memory allocation of {} bytes failed\n", layout.size()); } }