Skip to content

interpret: properly check for inhabitedness of nested references #156977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
RalfJung wants to merge 2 commits into
base: main
Choose a base branch
from
+306 −34
Open

interpret: properly check for inhabitedness of nested references #156977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09db86e
interpret: properly check for inhabitedness of nested references
RalfJung May 26, 2026
8781463
address some review feedback
RalfJung Jun 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions compiler/rustc_const_eval/src/interpret/validity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use super::{
format_interp_error,
};
use crate::enter_trace_span;
use crate::interpret::ensure_monomorphic_enough;

// for the validation errors
#[rustfmt::skip]
Expand Down Expand Up @@ -734,11 +735,11 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValidityVisitor<'rt, 'tcx, M> {
)
}
// Do not allow references to uninhabited types.
if place.layout.is_uninhabited() {
if !place.layout.ty.is_opsem_inhabited(*self.ecx.tcx, self.ecx.typing_env) {
let ty = place.layout.ty;
throw_validation_failure!(
self.path,
format!("encountered a {ptr_kind} pointing to uninhabited type {ty}")
format!("encountered a {ptr_kind} pointing to uninhabited type `{ty}`")
)
}

Expand Down Expand Up @@ -1568,8 +1569,9 @@ impl<'rt, 'tcx, M: Machine<'tcx>> ValueVisitor<'tcx, M> for ValidityVisitor<'rt,
}

// Assert that we checked everything there is to check about this type.
// `is_opsem_inhabited` implies that the layout is inhabited (checked by layout invariants).
assert!(
!val.layout.is_uninhabited(),
val.layout.ty.is_opsem_inhabited(*self.ecx.tcx, self.ecx.typing_env),
"a value of type `{}` passed validation but that type is uninhabited",
val.layout.ty
);
Expand Down Expand Up @@ -1627,6 +1629,9 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
) -> InterpResult<'tcx> {
trace!("validate_operand_internal: {:?}, {:?}", *val, val.layout.ty);

// We can't check validity if there are any generics left.
ensure_monomorphic_enough(*self.tcx, val.layout.ty)?;

// Run the visitor.
self.run_for_validation_mut(|ecx| {
let reset_padding = reset_provenance_and_padding && {
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/queries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2145,6 +2145,11 @@ rustc_queries! {
desc { "computing the uninhabited predicate of `{}`", key }
}

/// Do not call this query directly: invoke `Ty::is_opsem_inhabited` instead.
query is_opsem_inhabited_raw(env: ty::PseudoCanonicalInput<'tcx, Ty<'tcx>>) -> bool {
desc { "computing whether `{}` is inhabited on the opsem level", env.value }
}

query crate_dep_kind(_: CrateNum) -> CrateDepKind {
eval_always
desc { "fetching what a dependency looks like" }
Expand Down
184 changes: 182 additions & 2 deletions compiler/rustc_middle/src/ty/inhabitedness/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@
//! This code should only compile in modules where the uninhabitedness of `Foo`
//! is visible.

use std::assert_matches;

use rustc_data_structures::fx::FxHashSet;
use rustc_type_ir::TyKind::*;
use tracing::instrument;

Expand All @@ -54,7 +57,12 @@ pub mod inhabited_predicate;
pub use inhabited_predicate::InhabitedPredicate;

pub(crate) fn provide(providers: &mut Providers) {
*providers = Providers { inhabited_predicate_adt, inhabited_predicate_type, ..*providers };
*providers = Providers {
inhabited_predicate_adt,
inhabited_predicate_type,
is_opsem_inhabited_raw,
..*providers
};
}

/// Returns an `InhabitedPredicate` that is generic over type parameters and
Expand Down Expand Up @@ -186,14 +194,33 @@ impl<'tcx> Ty<'tcx> {
self.inhabited_predicate(tcx).apply(tcx, typing_env, module)
}

/// Returns true if the type is uninhabited without regard to visibility
/// Returns true if the type is uninhabited without regard to visibility.
///
/// This is still conservative; for instance, a `#[non_exhaustive]` enum *in another crate*
/// is always considered inhabited.
pub fn is_privately_uninhabited(
self,
tcx: TyCtxt<'tcx>,
typing_env: ty::TypingEnv<'tcx>,
) -> bool {
!self.inhabited_predicate(tcx).apply_ignore_module(tcx, typing_env)
}

/// Returns whether `self` is considered inhabited on the opsem level, i.e., its validity
/// invariant might be satisfiable. `self` is expected to be monomorphic and normalized.
///
/// Key constraints are:
/// - if a type's validity invariant is satisfiable, it must be opsem-inhabited.
/// - if a type's layout is marked uninhabited, it must be opsem-uninhabited.
///
/// Beyond that, the value returned by this function is not a stable guarantee.
pub fn is_opsem_inhabited(self, tcx: TyCtxt<'tcx>, typing_env: ty::TypingEnv<'tcx>) -> bool {
// Handle simple cases directly, use the query with its cache for the rest.
is_opsem_inhabited_recursor(self, tcx, &mut (), /* stop_at_ref */ false, &|ty, _, _| {
// ADT handler: stop recursing, invoke the query.
tcx.is_opsem_inhabited_raw(typing_env.as_query_input(ty))
})
Comment on lines +219 to +222

@theemathas theemathas Jun 3, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the fact that this closure discards the seen argument mean that we keep creating a new HashSet when we shouldn't?

(Sorry, but I'm very confused by the recursive code 😅)

View changes since the review

@RalfJung RalfJung Jun 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the recursion is quite gnarly. Happy to hear suggestions for improving it. This is where Rust isn't quite as much a functional language as I'd like it to be. ;)

We're only calling HashSet::default() once so I don't see how we could possibly create new HashSet where we don't want that.

@RalfJung RalfJung Jun 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this here is the non-recursive case. That why the ADT case is a fallback:

  • when invoked from Ty::is_opsem_inhabited, when we encounter an ADT, we invoke the query (as then we do want the cache)
  • when invoked in the query, when we encounter an ADT, we want to recurse

}
}

/// N.B. this query should only be called through `Ty::inhabited_predicate`
Expand All @@ -216,3 +243,156 @@ fn inhabited_predicate_type<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> InhabitedP
_ => bug!("unexpected TyKind, use `Ty::inhabited_predicate`"),
}
}

/// Recurse over a type to determine whether it is inhabited on the opsem level.
/// See `is_opsem_inhabited` above for the spec of what we compute.
///
/// When we encounter an ADT, we call `adt_handler`, giving it as its last argument a closure that
/// it can invoke to continue the recursion. This lets us share the logic for "simple" cases
/// (i.e., everything except for ADTs) between `Ty::is_opsem_inhabited` and the query.
///
/// `seen` is used to detect infinite recursion: the set contains all ADTs that we encountered
/// on our path to the current type.
/// If `stop_at_ref` is true, we stop recursing at the next reference we encounter.
fn is_opsem_inhabited_recursor<'tcx, SEEN>(
ty: Ty<'tcx>,
tcx: TyCtxt<'tcx>,
seen: &mut SEEN,
stop_at_ref: bool,
adt_handler: &impl Fn(
Ty<'tcx>,
&mut SEEN,
&dyn Fn(Ty<'tcx>, &mut SEEN, /* stop_at_ref */ bool) -> bool,
) -> bool,
) -> bool {
match *ty.kind() {
// Trivially (un)inhabited types
ty::Int(_)
| ty::Uint(_)
| ty::Float(_)
| ty::Bool
| ty::Char
| ty::Str
| ty::Foreign(..)
| ty::RawPtr(..)
| ty::FnPtr(..)
| ty::FnDef(..) => true,
ty::Dynamic(..) => true, // We can't reason about traits, assume they are inhabited
ty::Slice(..) => true, // Slices can always be empty
Comment thread
WaffleLapkin marked this conversation as resolved.
ty::Never => false,

// Types where we recurse
ty::Ref(_, pointee, _) => {
if stop_at_ref {
// Bailing out here is safe as the layout code always considers references
// inhabited, so the implication ("layout uninhabited => opsem uninhabited")
// is upheld.
return true;
}
is_opsem_inhabited_recursor(pointee, tcx, seen, stop_at_ref, adt_handler)
}
ty::Tuple(tys) => tys
.iter()
.all(|ty| is_opsem_inhabited_recursor(ty, tcx, seen, stop_at_ref, adt_handler)),
ty::Array(elem, len) => {
len.try_to_target_usize(tcx).unwrap() == 0
|| is_opsem_inhabited_recursor(elem, tcx, seen, stop_at_ref, adt_handler)
}
ty::Pat(inner, _pat) => {
is_opsem_inhabited_recursor(inner, tcx, seen, stop_at_ref, adt_handler)
}
Comment on lines +301 to +303

@WaffleLapkin WaffleLapkin Jun 22, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When pattern types start supporting enums, we'll need to decide if type X here is inhabited or not:

#![feature(pattern_types)]
#![feature(pattern_type_macro)]
#![feature(never_type)]
enum E {
    A,
    B(!),
}

type X = pattern_type!(E is E::B(_));

View changes since the review

@RalfJung RalfJung Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have to make final decisions -- the exact value of "opsem inhabited" is an implementation detail (similar to the exact size and alignment of a type), subject to two constraints:

  • user-facing: if a type has a satisfiable validity invariant, then it is opsem-inhabited
  • internal: if a type is opsem-inhabited, then it is layout-inhabited

ty::Closure(_def, args) => {
let args = args.as_closure();
args.upvar_tys()
.iter()
.all(|ty| is_opsem_inhabited_recursor(ty, tcx, seen, stop_at_ref, adt_handler))
}
ty::Coroutine(_def, args) => {
let args = args.as_coroutine();
args.upvar_tys()
.iter()
.all(|ty| is_opsem_inhabited_recursor(ty, tcx, seen, stop_at_ref, adt_handler))
}
ty::CoroutineClosure(_def, args) => {
let args = args.as_coroutine_closure();
args.upvar_tys()
.iter()
.all(|ty| is_opsem_inhabited_recursor(ty, tcx, seen, stop_at_ref, adt_handler))
}
ty::UnsafeBinder(base) => {
let base = tcx.instantiate_bound_regions_with_erased((*base).into());
is_opsem_inhabited_recursor(base, tcx, seen, stop_at_ref, adt_handler)
}
Comment thread
WaffleLapkin marked this conversation as resolved.
ty::Adt(..) => {
// ADTs need a special handler to avoid infinite recursion. That handler is meant to
// call back into the recursor. Ideally it'd just call `is_opsem_inhabited_recursor` but
// then it would have to pass itself as the adt_handler argument which is not possible
// in Rust... so we provide the handler with a callback that it can use to continue the
// recursion with the same `adt_handler`.
adt_handler(ty, seen, &|ty, seen, stop_at_ref| {
is_opsem_inhabited_recursor(ty, tcx, seen, stop_at_ref, adt_handler)
})
}

ty::Error(_)
| ty::Infer(..)
| ty::Placeholder(..)
| ty::Bound(..)
| ty::Param(..)
| ty::Alias(..)
| ty::CoroutineWitness(..) => {
bug!("non-normalized type in `is_opsem_uninhabited`: `{ty}`")
}
}
}

fn is_opsem_inhabited_raw<'tcx>(

@WaffleLapkin WaffleLapkin Jun 22, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the code is fairly confusing with the closures & mutual recursion.

This query is only called for ADTs. I think if you change it to be is_adt_opsem_inhabited(tcx, adt_def, adt_args, seen) the code will become a lot clearer.

Then you can have

  • is_opsem_inhabited calls is_opsem_inhabited_recursor
  • is_opsem_inhabited_recursor calls is_adt_opsem_inhabited for adts
  • is_adt_opsem_inhabited calls is_opsem_inhabited_recursor directly

I think this should work & make the code a lot easier to follow.

View changes since the review

@RalfJung RalfJung Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree the query can be changed to specifically take an ADT but I don't see how this simplifies anything regarding the handling of the recursion?

tcx: TyCtxt<'tcx>,
env: ty::PseudoCanonicalInput<'tcx, Ty<'tcx>>,
) -> bool {
let (ty, typing_env) = (env.value, env.typing_env);
assert_matches!(
ty.kind(),
ty::Adt(..),
"the query should only be invoked by `Ty::is_opsem_inhabited`"
);

is_opsem_inhabited_recursor(
ty,
tcx,
&mut FxHashSet::<DefId>::default(),
/* stop_at_ref */ false,
&|ty, seen, rec| {
let ty::Adt(adt_def, adt_args) = *ty.kind() else {
unreachable! {}
};
if adt_def.is_union() {
// Unions are always inhabited.
return true;
}

let new_adt = seen.insert(adt_def.did());
// If we have seen this ADT before, stop at the next reference to avoid infinite
// recursion. We can't stop here since we have to ensure that "layout uninhabited"
// implies "opsem uninhabited". References are always layout-inhabited so the
// implication is vacuously true.
let stop_at_ref = !new_adt;

@WaffleLapkin WaffleLapkin Jun 22, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: wasn't the thing that we want "layout uninhabited" => "opsem uninhabited"?

View changes since the review

@RalfJung RalfJung Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops yes indeed


// We are inhabited if in some variant all fields are inhabited.
let inhabited = adt_def.variants().iter().any(|variant| {
variant.fields.iter().all(|field| {
let ty = field.ty(tcx, adt_args);
let ty = tcx.normalize_erasing_regions(typing_env, ty);
rec(ty, seen, stop_at_ref)
})
});

// Remove the type again so that we allow it to appear on other branches.
if new_adt {
seen.remove(&adt_def.did());
}

inhabited
},
)
}
10 changes: 10 additions & 0 deletions compiler/rustc_ty_utils/src/layout/invariant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::assert_matches;

use rustc_abi::{BackendRepr, FieldsShape, Scalar, Size, TagEncoding, Variants};
use rustc_middle::ty::TypeVisitableExt;
use rustc_middle::ty::layout::{HasTyCtxt, LayoutCx, TyAndLayout};
use rustc_middle::{bug, ty};

Expand Down Expand Up @@ -34,6 +35,15 @@ pub(super) fn layout_sanity_check<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayou
layout.ty
);
}
// ABI uninhabitedness should imply opsem uninhabitedness. However, we can only check that if
// the type is really monomorphic (while we can compute a layout for some generic types).
if layout.is_uninhabited() && !layout.ty.has_param() {
assert!(
!layout.ty.is_opsem_inhabited(tcx, cx.typing_env),
"{:?} is ABI-uninhabited but not opsem-uninhabited?",
layout.ty
);
}

/// Yields non-ZST fields of the type
fn non_zst_fields<'tcx, 'a>(
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/fail/validity/ref_to_uninhabited1.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::mem::{forget, transmute};

fn main() {
unsafe {
let x: Box<!> = transmute(&mut 42); //~ERROR: encountered a box pointing to uninhabited type !
let x: Box<!> = transmute(&mut 42); //~ERROR: encountered a box pointing to uninhabited type `!`

@WaffleLapkin WaffleLapkin Jun 22, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Do we need to special case Box in is_opsem_inhabited?

View changes since the review

@RalfJung RalfJung Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have to... currently we consider Box always opsem-inhabited, which is okay because it is also always layout-inhabited.

I guess it'd make sense to apply the reference rules also to Box. But we can do that in a future PR. Custom allocators make boxes more annoying to deal with than references...

forget(x);
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/tests/fail/validity/ref_to_uninhabited1.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: Undefined Behavior: constructing invalid value of type std::boxed::Box<!>: encountered a box pointing to uninhabited type !
error: Undefined Behavior: constructing invalid value of type std::boxed::Box<!>: encountered a box pointing to uninhabited type `!`
--> tests/fail/validity/ref_to_uninhabited1.rs:LL:CC
|
LL | let x: Box<!> = transmute(&mut 42);
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/tests/fail/validity/ref_to_uninhabited2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ enum Void {}

fn main() {
unsafe {
let _x: &(i32, Void) = transmute(&42); //~ERROR: encountered a reference pointing to uninhabited type (i32, Void)
let _x: &&(i32, Void) = transmute(&&42); //~ERROR: encountered a reference pointing to uninhabited type `&(i32, Void)`

@WaffleLapkin WaffleLapkin Jun 22, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: why change this?

View changes since the review

@RalfJung RalfJung Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to ensure we handle the nested case properly

}
}
6 changes: 3 additions & 3 deletions src/tools/miri/tests/fail/validity/ref_to_uninhabited2.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Undefined Behavior: constructing invalid value of type &(i32, Void): encountered a reference pointing to uninhabited type (i32, Void)
error: Undefined Behavior: constructing invalid value of type &&(i32, Void): encountered a reference pointing to uninhabited type `&(i32, Void)`
--> tests/fail/validity/ref_to_uninhabited2.rs:LL:CC
|
LL | let _x: &(i32, Void) = transmute(&42);
| ^^^^^^^^^^^^^^ Undefined Behavior occurred here
LL | let _x: &&(i32, Void) = transmute(&&42);
| ^^^^^^^^^^^^^^^ Undefined Behavior occurred here
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/consts/const-eval/raw-bytes.32bit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ LL | const DATA_FN_PTR: fn() = unsafe { mem::transmute(&13) };
╾ALLOC_ID╼ │ ╾──╼
}

error[E0080]: constructing invalid value of type &Bar: encountered a reference pointing to uninhabited type Bar
error[E0080]: constructing invalid value of type &Bar: encountered a reference pointing to uninhabited type `Bar`
--> $DIR/raw-bytes.rs:110:1
|
LL | const BAD_BAD_REF: &Bar = unsafe { mem::transmute(1usize) };
Expand Down Expand Up @@ -458,7 +458,7 @@ LL | const RAW_TRAIT_OBJ_VTABLE_INVALID: *const dyn Trait = unsafe { mem::transm
╾ALLOC_ID╼ ╾ALLOC_ID╼ │ ╾──╼╾──╼
}

error[E0080]: constructing invalid value of type &[!; 1]: encountered a reference pointing to uninhabited type [!; 1]
error[E0080]: constructing invalid value of type &[!; 1]: encountered a reference pointing to uninhabited type `[!; 1]`
--> $DIR/raw-bytes.rs:188:1
|
LL | const _: &[!; 1] = unsafe { &*(1_usize as *const [!; 1]) };
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/consts/const-eval/raw-bytes.64bit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ LL | const DATA_FN_PTR: fn() = unsafe { mem::transmute(&13) };
╾ALLOC_ID╼ │ ╾──────╼
}

error[E0080]: constructing invalid value of type &Bar: encountered a reference pointing to uninhabited type Bar
error[E0080]: constructing invalid value of type &Bar: encountered a reference pointing to uninhabited type `Bar`
--> $DIR/raw-bytes.rs:110:1
|
LL | const BAD_BAD_REF: &Bar = unsafe { mem::transmute(1usize) };
Expand Down Expand Up @@ -458,7 +458,7 @@ LL | const RAW_TRAIT_OBJ_VTABLE_INVALID: *const dyn Trait = unsafe { mem::transm
╾ALLOC_ID╼ ╾ALLOC_ID╼ │ ╾──────╼╾──────╼
}

error[E0080]: constructing invalid value of type &[!; 1]: encountered a reference pointing to uninhabited type [!; 1]
error[E0080]: constructing invalid value of type &[!; 1]: encountered a reference pointing to uninhabited type `[!; 1]`
--> $DIR/raw-bytes.rs:188:1
|
LL | const _: &[!; 1] = unsafe { &*(1_usize as *const [!; 1]) };
Expand Down
Loading
Loading