Skip to content

Commit

Permalink
Auto merge of rust-lang#128629 - matthiaskrgr:rollup-dde55vp, r=matth…
Browse files Browse the repository at this point in the history
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#128362 (add test for symbol visibility of `#[naked]` functions)
 - rust-lang#128526 (time.rs: remove "Basic usage text")
 - rust-lang#128531 (Miri: add a flag to do recursive validity checking)
 - rust-lang#128578 (rustdoc: Cleanup `CacheBuilder` code for building search index)
 - rust-lang#128589 (allow setting `link-shared` and `static-libstdcpp` with CI LLVM)
 - rust-lang#128615 (rustdoc: make the hover trail for doc anchors a bit bigger)
 - rust-lang#128620 (Update rinja version to 0.3.0)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Aug 4, 2024
2 parents b389b0a + 08f3693 commit c3d944a
Show file tree
Hide file tree
Showing 23 changed files with 548 additions and 285 deletions.
15 changes: 9 additions & 6 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3282,20 +3282,22 @@ dependencies = [

[[package]]
name = "rinja"
version = "0.2.0"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d2d47a46d7729e891c8accf260e9daa02ae6d570aa2a94fb1fb27eb5364a2323"
checksum = "6d3762e3740cdbf2fd2be465cc2c26d643ad17353cc2e0223d211c1b096118bd"
dependencies = [
"itoa",
"rinja_derive",
]

[[package]]
name = "rinja_derive"
version = "0.2.0"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "44dae9afe59d58ed8d988d67d1945f3638125d2fd2104058399382e11bd3ea2a"
checksum = "fd01fd8e15e7d19c8b8052c1d428325131e02ff1633cdcf695190c2e56ab682c"
dependencies = [
"basic-toml",
"memchr",
"mime",
"mime_guess",
"once_map",
Expand All @@ -3308,10 +3310,11 @@ dependencies = [

[[package]]
name = "rinja_parser"
version = "0.2.0"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1b1771c78cd5d3b1646ef8d8f2ed100db936e8b291d3cc06e92a339ff346858c"
checksum = "a2f6bf7cef118c6de21206edf0b3f19f5ede60006be674a58ca21b6e003a1b57"
dependencies = [
"memchr",
"nom",
]

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ fn const_validate_mplace<'tcx>(
let alloc_id = mplace.ptr().provenance.unwrap().alloc_id();
let mut ref_tracking = RefTracking::new(mplace.clone());
let mut inner = false;
while let Some((mplace, path)) = ref_tracking.todo.pop() {
while let Some((mplace, path)) = ref_tracking.next() {
let mode = match ecx.tcx.static_mutability(cid.instance.def_id()) {
_ if cid.promoted.is_some() => CtfeValidationMode::Promoted,
Some(mutbl) => CtfeValidationMode::Static { mutbl }, // a `static`
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_const_eval/src/interpret/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,13 @@ pub trait Machine<'tcx>: Sized {

/// Whether to enforce the validity invariant for a specific layout.
fn enforce_validity(ecx: &InterpCx<'tcx, Self>, layout: TyAndLayout<'tcx>) -> bool;
/// Whether to enforce the validity invariant *recursively*.
fn enforce_validity_recursively(
_ecx: &InterpCx<'tcx, Self>,
_layout: TyAndLayout<'tcx>,
) -> bool {
false
}

/// Whether function calls should be [ABI](CallAbi)-checked.
fn enforce_abi(_ecx: &InterpCx<'tcx, Self>) -> bool {
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1006,8 +1006,11 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
})
}

/// Runs the close in "validation" mode, which means the machine's memory read hooks will be
/// Runs the closure in "validation" mode, which means the machine's memory read hooks will be
/// suppressed. Needless to say, this must only be set with great care! Cannot be nested.
///
/// We do this so Miri's allocation access tracking does not show the validation
/// reads as spurious accesses.
pub(super) fn run_for_validation<R>(&self, f: impl FnOnce() -> R) -> R {
// This deliberately uses `==` on `bool` to follow the pattern
// `assert!(val.replace(new) == old)`.
Expand Down
15 changes: 12 additions & 3 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,10 @@ where

if M::enforce_validity(self, dest.layout()) {
// Data got changed, better make sure it matches the type!
self.validate_operand(&dest.to_op(self)?)?;
self.validate_operand(
&dest.to_op(self)?,
M::enforce_validity_recursively(self, dest.layout()),
)?;
}

Ok(())
Expand Down Expand Up @@ -811,15 +814,21 @@ where
// Generally for transmutation, data must be valid both at the old and new type.
// But if the types are the same, the 2nd validation below suffices.
if src.layout().ty != dest.layout().ty && M::enforce_validity(self, src.layout()) {
self.validate_operand(&src.to_op(self)?)?;
self.validate_operand(
&src.to_op(self)?,
M::enforce_validity_recursively(self, src.layout()),
)?;
}

// Do the actual copy.
self.copy_op_no_validate(src, dest, allow_transmute)?;

if validate_dest && M::enforce_validity(self, dest.layout()) {
// Data got changed, better make sure it matches the type!
self.validate_operand(&dest.to_op(self)?)?;
self.validate_operand(
&dest.to_op(self)?,
M::enforce_validity_recursively(self, dest.layout()),
)?;
}

Ok(())
Expand Down
176 changes: 104 additions & 72 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@ impl CtfeValidationMode {

/// State for tracking recursive validation of references
pub struct RefTracking<T, PATH = ()> {
pub seen: FxHashSet<T>,
pub todo: Vec<(T, PATH)>,
seen: FxHashSet<T>,
todo: Vec<(T, PATH)>,
}

impl<T: Clone + Eq + Hash + std::fmt::Debug, PATH: Default> RefTracking<T, PATH> {
Expand All @@ -169,8 +169,11 @@ impl<T: Clone + Eq + Hash + std::fmt::Debug, PATH: Default> RefTracking<T, PATH>
ref_tracking_for_consts.seen.insert(op);
ref_tracking_for_consts
}
pub fn next(&mut self) -> Option<(T, PATH)> {
self.todo.pop()
}

pub fn track(&mut self, op: T, path: impl FnOnce() -> PATH) {
fn track(&mut self, op: T, path: impl FnOnce() -> PATH) {
if self.seen.insert(op.clone()) {
trace!("Recursing below ptr {:#?}", op);
let path = path();
Expand Down Expand Up @@ -435,95 +438,112 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
if self.ecx.scalar_may_be_null(Scalar::from_maybe_pointer(place.ptr(), self.ecx))? {
throw_validation_failure!(self.path, NullPtr { ptr_kind })
}
// Do not allow pointers to uninhabited types.
// Do not allow references to uninhabited types.
if place.layout.abi.is_uninhabited() {
let ty = place.layout.ty;
throw_validation_failure!(self.path, PtrToUninhabited { ptr_kind, ty })
}
// Recursive checking
if let Some(ref_tracking) = self.ref_tracking.as_deref_mut() {
// Determine whether this pointer expects to be pointing to something mutable.
let ptr_expected_mutbl = match ptr_kind {
PointerKind::Box => Mutability::Mut,
PointerKind::Ref(mutbl) => {
// We do not take into account interior mutability here since we cannot know if
// there really is an `UnsafeCell` inside `Option<UnsafeCell>` -- so we check
// that in the recursive descent behind this reference (controlled by
// `allow_immutable_unsafe_cell`).
mutbl
}
};
// Proceed recursively even for ZST, no reason to skip them!
// `!` is a ZST and we want to validate it.
if let Ok((alloc_id, _offset, _prov)) = self.ecx.ptr_try_get_alloc_id(place.ptr(), 0) {
if let Some(ctfe_mode) = self.ctfe_mode {
let mut skip_recursive_check = false;
if let Some(GlobalAlloc::Static(did)) = self.ecx.tcx.try_get_global_alloc(alloc_id)
// CTFE imposes restrictions on what references can point to.
if let Ok((alloc_id, _offset, _prov)) =
self.ecx.ptr_try_get_alloc_id(place.ptr(), 0)
{
let DefKind::Static { nested, .. } = self.ecx.tcx.def_kind(did) else { bug!() };
// Special handling for pointers to statics (irrespective of their type).
assert!(!self.ecx.tcx.is_thread_local_static(did));
assert!(self.ecx.tcx.is_static(did));
// Mode-specific checks
match self.ctfe_mode {
Some(
CtfeValidationMode::Static { .. } | CtfeValidationMode::Promoted { .. },
) => {
// We skip recursively checking other statics. These statics must be sound by
// themselves, and the only way to get broken statics here is by using
// unsafe code.
// The reasons we don't check other statics is twofold. For one, in all
// sound cases, the static was already validated on its own, and second, we
// trigger cycle errors if we try to compute the value of the other static
// and that static refers back to us (potentially through a promoted).
// This could miss some UB, but that's fine.
// We still walk nested allocations, as they are fundamentally part of this validation run.
// This means we will also recurse into nested statics of *other*
// statics, even though we do not recurse into other statics directly.
// That's somewhat inconsistent but harmless.
skip_recursive_check = !nested;
}
Some(CtfeValidationMode::Const { .. }) => {
// We can't recursively validate `extern static`, so we better reject them.
if self.ecx.tcx.is_foreign_item(did) {
throw_validation_failure!(self.path, ConstRefToExtern);
if let Some(GlobalAlloc::Static(did)) =
self.ecx.tcx.try_get_global_alloc(alloc_id)
{
let DefKind::Static { nested, .. } = self.ecx.tcx.def_kind(did) else {
bug!()
};
// Special handling for pointers to statics (irrespective of their type).
assert!(!self.ecx.tcx.is_thread_local_static(did));
assert!(self.ecx.tcx.is_static(did));
// Mode-specific checks
match ctfe_mode {
CtfeValidationMode::Static { .. }
| CtfeValidationMode::Promoted { .. } => {
// We skip recursively checking other statics. These statics must be sound by
// themselves, and the only way to get broken statics here is by using
// unsafe code.
// The reasons we don't check other statics is twofold. For one, in all
// sound cases, the static was already validated on its own, and second, we
// trigger cycle errors if we try to compute the value of the other static
// and that static refers back to us (potentially through a promoted).
// This could miss some UB, but that's fine.
// We still walk nested allocations, as they are fundamentally part of this validation run.
// This means we will also recurse into nested statics of *other*
// statics, even though we do not recurse into other statics directly.
// That's somewhat inconsistent but harmless.
skip_recursive_check = !nested;
}
CtfeValidationMode::Const { .. } => {
// We can't recursively validate `extern static`, so we better reject them.
if self.ecx.tcx.is_foreign_item(did) {
throw_validation_failure!(self.path, ConstRefToExtern);
}
}
}
None => {}
}
}

// Dangling and Mutability check.
let (size, _align, alloc_kind) = self.ecx.get_alloc_info(alloc_id);
if alloc_kind == AllocKind::Dead {
// This can happen for zero-sized references. We can't have *any* references to non-existing
// allocations though, interning rejects them all as the rest of rustc isn't happy with them...
// so we throw an error, even though this isn't really UB.
// A potential future alternative would be to resurrect this as a zero-sized allocation
// (which codegen will then compile to an aligned dummy pointer anyway).
throw_validation_failure!(self.path, DanglingPtrUseAfterFree { ptr_kind });
}
// If this allocation has size zero, there is no actual mutability here.
if size != Size::ZERO {
let alloc_actual_mutbl = mutability(self.ecx, alloc_id);
// Mutable pointer to immutable memory is no good.
if ptr_expected_mutbl == Mutability::Mut
&& alloc_actual_mutbl == Mutability::Not
{
throw_validation_failure!(self.path, MutableRefToImmutable);
// Dangling and Mutability check.
let (size, _align, alloc_kind) = self.ecx.get_alloc_info(alloc_id);
if alloc_kind == AllocKind::Dead {
// This can happen for zero-sized references. We can't have *any* references to
// non-existing allocations in const-eval though, interning rejects them all as
// the rest of rustc isn't happy with them... so we throw an error, even though
// this isn't really UB.
// A potential future alternative would be to resurrect this as a zero-sized allocation
// (which codegen will then compile to an aligned dummy pointer anyway).
throw_validation_failure!(self.path, DanglingPtrUseAfterFree { ptr_kind });
}
// In a const, everything must be completely immutable.
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. })) {
// If this allocation has size zero, there is no actual mutability here.
if size != Size::ZERO {
// Determine whether this pointer expects to be pointing to something mutable.
let ptr_expected_mutbl = match ptr_kind {
PointerKind::Box => Mutability::Mut,
PointerKind::Ref(mutbl) => {
// We do not take into account interior mutability here since we cannot know if
// there really is an `UnsafeCell` inside `Option<UnsafeCell>` -- so we check
// that in the recursive descent behind this reference (controlled by
// `allow_immutable_unsafe_cell`).
mutbl
}
};
// Determine what it actually points to.
let alloc_actual_mutbl = mutability(self.ecx, alloc_id);
// Mutable pointer to immutable memory is no good.
if ptr_expected_mutbl == Mutability::Mut
|| alloc_actual_mutbl == Mutability::Mut
&& alloc_actual_mutbl == Mutability::Not
{
throw_validation_failure!(self.path, ConstRefToMutable);
throw_validation_failure!(self.path, MutableRefToImmutable);
}
// In a const, everything must be completely immutable.
if matches!(self.ctfe_mode, Some(CtfeValidationMode::Const { .. })) {
if ptr_expected_mutbl == Mutability::Mut
|| alloc_actual_mutbl == Mutability::Mut
{
throw_validation_failure!(self.path, ConstRefToMutable);
}
}
}
}
// Potentially skip recursive check.
if skip_recursive_check {
return Ok(());
}
} else {
// This is not CTFE, so it's Miri with recursive checking.
// FIXME: we do *not* check behind boxes, since creating a new box first creates it uninitialized
// and then puts the value in there, so briefly we have a box with uninit contents.
// FIXME: should we also skip `UnsafeCell` behind shared references? Currently that is not
// needed since validation reads bypass Stacked Borrows and data race checks.
if matches!(ptr_kind, PointerKind::Box) {
return Ok(());
}
}
let path = &self.path;
ref_tracking.track(place, || {
Expand Down Expand Up @@ -1072,11 +1092,23 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
/// `op` is assumed to cover valid memory if it is an indirect operand.
/// It will error if the bits at the destination do not match the ones described by the layout.
#[inline(always)]
pub fn validate_operand(&self, op: &OpTy<'tcx, M::Provenance>) -> InterpResult<'tcx> {
pub fn validate_operand(
&self,
op: &OpTy<'tcx, M::Provenance>,
recursive: bool,
) -> InterpResult<'tcx> {
// Note that we *could* actually be in CTFE here with `-Zextra-const-ub-checks`, but it's
// still correct to not use `ctfe_mode`: that mode is for validation of the final constant
// value, it rules out things like `UnsafeCell` in awkward places. It also can make checking
// recurse through references which, for now, we don't want here, either.
self.validate_operand_internal(op, vec![], None, None)
// value, it rules out things like `UnsafeCell` in awkward places.
if !recursive {
return self.validate_operand_internal(op, vec![], None, None);
}
// Do a recursive check.
let mut ref_tracking = RefTracking::empty();
self.validate_operand_internal(op, vec![], Some(&mut ref_tracking), None)?;
while let Some((mplace, path)) = ref_tracking.todo.pop() {
self.validate_operand_internal(&mplace.into(), path, Some(&mut ref_tracking), None)?;
}
Ok(())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ fn might_permit_raw_init_strict<'tcx>(
// This does *not* actually check that references are dereferenceable, but since all types that
// require dereferenceability also require non-null, we don't actually get any false negatives
// due to this.
Ok(cx.validate_operand(&ot).is_ok())
Ok(cx.validate_operand(&ot, /*recursive*/ false).is_ok())
}

/// Implements the 'lax' (default) version of the `might_permit_raw_init` checks; see that function for
Expand Down
10 changes: 0 additions & 10 deletions library/core/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,8 +617,6 @@ impl Duration {
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// use std::time::Duration;
///
Expand All @@ -640,8 +638,6 @@ impl Duration {
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// use std::time::Duration;
///
Expand Down Expand Up @@ -700,8 +696,6 @@ impl Duration {
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// use std::time::Duration;
///
Expand Down Expand Up @@ -758,8 +752,6 @@ impl Duration {
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// use std::time::Duration;
///
Expand Down Expand Up @@ -814,8 +806,6 @@ impl Duration {
///
/// # Examples
///
/// Basic usage:
///
/// ```
/// use std::time::Duration;
///
Expand Down
Loading

0 comments on commit c3d944a

Please sign in to comment.