interpret: properly check for inhabitedness of nested references by RalfJung · Pull Request #156977 · rust-lang/rust

RalfJung · 2026-05-26T15:41:06Z

This implements the opsem from the ongoing FCP in rust-lang/unsafe-code-guidelines#413. The bit we were previously missing is that transmuting a &&! into existence was not caught as being immediate UB -- only the &! case behaved as expected.

I did not adjust the layout computation because when we compute the layout of &T, we cannot know the layout of T (as that might be recursive).

r? @oli-obk

rustbot · 2026-05-26T15:41:10Z

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the CTFE machinery

cc @oli-obk, @lcnr

RalfJung · 2026-05-26T15:42:23Z

+        ty::Coroutine(..) => {
+            true // FIXME should these really be trivially inhabited?
+        }
+        ty::CoroutineClosure(..) => {
+            true // FIXME should these really be trivially inhabited?
+        }


I wasn't sure how to recurse into these. Are coroutines always inhabited via a trivial start state, or can they be uninhabited due to capturing ! as an "upvar"? How do coroutine closures work?

View changes since the review

Looks like yes they can be uninhabited

Coroutine(DefId(0:13 ~ diverges[9ce3]::async_let::{closure#0}), [(), std::future::ResumeTy, (), !, (Void,)]) is ABI-uninhabited but not opsem-uninhabited?

I now made them all check the upvar_tys, but I am not entirely sure if that is enough.

For coroutines, checking upvars is enough. The coroutine state is pretty much struct { upvars, enum { Unresumed, Returned, Panicked, Suspend0 { .. }, Suspent1 { .. }, .. } }

Note that #135527 proposes to change this to enum { Unresumed { upvars }, Returned, Panicked, Suspend0 { .. }, Suspent1 { .. }, .. }. Starting from that PR, the enum state will be inhabited in all cases.

Coroutine closures work exactly like closures.

Thanks for confirming!

RalfJung · 2026-05-26T15:42:47Z

+        }
+        ty::UnsafeBinder(..) => {
+            true // FIXME should these really be trivially inhabited?
+        }


I have no idea what the UnsafeBinder implementation status is but their opsem has not even been discussed yet AFAIK.

View changes since the review

in limbo, but so far I think it was "except for lifetimes, they behave the same as their bound value", and this is how we check them in validation

How do I get the bound value out?

this is how we check them in validation

I just see

ty::UnsafeBinder(_) => todo!("FIXME(unsafe_binder)"),

Thanks, I added the recursion for that.

RalfJung · 2026-05-26T15:43:37Z

+            len.try_to_target_usize(tcx).unwrap() == 0
+                || is_opsem_inhabited_recursor(elem, tcx, root, adt_handler)
+        }
+        ty::Pat(inner, _pat) => is_opsem_inhabited_recursor(inner, tcx, root, adt_handler),


I guess in theory the pattern could make a type uninhabited... so technically if we ever want to use that for the opsem we have to add it has a check here before pattern types get stabilized.

View changes since the review

none of the current patterns can, but yes, that may be possible in the future

RalfJung · 2026-05-26T15:44:57Z

+///
+/// When we git an ADT, we call `adt_handler`, giving it as its last argument a closure that it
+/// can invoke to continue the recursion.
+fn is_opsem_inhabited_recursor<'tcx>(


Do we need to do something to bound this recursion? We already stop when encountering the same ADT again, so recursion is bounded by the depth of ADT field types until it comes back to the original type.

Do we need to do some stack growing magic?

View changes since the review

it's possible this can't be hit due to other stack growth protections, but theoretically you can create a very nested tuple, or just &&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&T with a lot of repetitions of the reference. I'd honestly just wait for an issue

oli-obk · 2026-05-27T10:30:57Z

+///
+/// When we git an ADT, we call `adt_handler`, giving it as its last argument a closure that it
+/// can invoke to continue the recursion.
+fn is_opsem_inhabited_recursor<'tcx>(


it's possible this can't be hit due to other stack growth protections, but theoretically you can create a very nested tuple, or just &&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&&T with a lot of repetitions of the reference. I'd honestly just wait for an issue

oli-obk · 2026-05-27T10:36:52Z

+        | 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


so slices of uninhabited types must have zero length? Is this properly checked?

View changes since the review

so slices of uninhabited types must have zero length?

Yes.

Is this properly checked?

What do you mean by this? I can add a test that transmutes &[()] as &[()] into &[!] if that's what you mean.

I guess I'm just confused about which things this open check treats as inhabited while it can't actually guarantee they are.

Feels like it should be a three-state enum, but that doesn't bring any value to the call sites

This is meant to be an upper bound to anything we could ever consider uninhabited on the ABI/Layout level. Eventually it will have to be stably documented in the Reference.

oli-obk · 2026-05-27T10:37:38Z

+            len.try_to_target_usize(tcx).unwrap() == 0
+                || is_opsem_inhabited_recursor(elem, tcx, root, adt_handler)
+        }
+        ty::Pat(inner, _pat) => is_opsem_inhabited_recursor(inner, tcx, root, adt_handler),


none of the current patterns can, but yes, that may be possible in the future

oli-obk · 2026-05-27T10:39:02Z

+        }
+        ty::UnsafeBinder(..) => {
+            true // FIXME should these really be trivially inhabited?
+        }


in limbo, but so far I think it was "except for lifetimes, they behave the same as their bound value", and this is how we check them in validation

RalfJung · 2026-05-27T14:30:49Z

+            // Bailing out here is unfortuante as it means that the recursion limit affects the
+            // operational semantics... but what else could we do?
+            return true;
+        }


I pushed a proper recursion check including a recursion limit check.

But... I am not sure it's sound. This means that depending on the recursion limit, different crates might consider the same type inhabited or not.

OTOH if we just hard-code e.g. 256 here, then with a sufficiently high query recursion limit one can write a 257-level-nested type (Wrap<Wrap<...<Wrap<!>>...>>) that layout will still consider uninhabited but this check will give up.

View changes since the review

Why exactly is this check soundness-critical? I don't quite follow...

As far as I can tell, it seems like this is just a courtesy best-effort check for UB in consteval, and a sanity check after computing layout?

This is also used by Miri so it is supposed to define what is and isn't UB.

Also our query system relies on cross-crate-consistent results, doesn't it?

@camsteffen proposed to do something more like inhabited_predicate_adt. I must admit that I do not understand how that query works -- I can see that it produces a predicate and ships it to the trait solver, but how is that helpful for recursive types?

Sorry, that was kind of a red herring.

More to the point is check_representability. That query uses query cycle detection to detect an infinite sized type and abort compilation.

So I think we could have a check_opsem_inhabited query which also has query cycle detection, but instead of aborting, return OpsemInhabited::False or something like that for the query result.

check_representability uses params_in_repr, so check_opsem_inhabited could use something similar like params_in_repr_or_ref to include references.

Another idea, that lcnr proposed, is that we can error on overflow. I.e. when computing inhabitedness of a type, if we reach the recursion limit, emit a hard error.

Since an error prevents the code from being compiled, there is no problem with linking two different crates with different recursion limits -- they either both get the same result, or one of them doesn't compile.

We can still support (assume inhabited) types which refer to themselves directly like

struct A(&'static A);

And error only if the same type is mentioned with different generic parameters and overflows:

struct B<T: 'static>(&'static B<(T, T)>);

This is technically a breaking change, but it seems fine not to support types which infinitely grow like this (or am I missing something?).

But this seems fine, considering how much of an edge case this is?

Given,

struct Wrap<T>(T);

I believe that the algorithm you proposed would say that &Wrap<Wrap<!>> is inhabited, which doesn't seem like a particularly strange edge case. @WaffleLapkin

This is technically a breaking change, but it seems fine not to support types which infinitely grow like this (or am I missing something?).

Given that #138599 fixed multiple issues, it seems that there exist some use cases where people want such strange recursive types. We can't know how much that is before we run crater though.

I believe that the algorithm you proposed would say that &Wrap<Wrap<!>> is inhabited, which doesn't seem like a particularly strange edge case. @WaffleLapkin

I personally think that it's "fine", especially given that this improves over the status quo. But I suppose we could also allow recursing through the same definition a set number of times (i.e. setting a recursion limit higher than 1)...

@WaffleLapkin I like that first version, and I think I can combine it with an idea I had to avoid the problem @theemathas mentioned. This avoids any dependency on the recursion limit while also still satisfying the desired implication "layout inhabited => opsem inhabited". I pushed that now.

interpret: properly check for inhabitedness of nested references

rust-bors · 2026-06-02T23:35:24Z

☀️ Try build successful (CI)
Build commit: 68787dc (68787dcef6397e54dbe06ab624836141335401e7, parent: 20e19eeac0c548e31f84d3914e0ce17cde618119)

rust-timer · 2026-06-03T00:40:38Z

Finished benchmarking commit (68787dc): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -3.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-5.8%	[-6.9%, -3.8%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.9%	[-6.9%, 1.7%]	4

Cycles

Results (secondary -4.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.6%	[-5.4%, -3.8%]	4
All ❌✅ (primary)	-	-	0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 511.673s -> 534.284s (4.42%)
Artifact size: 400.63 MiB -> 400.67 MiB (0.01%)

RalfJung · 2026-06-03T06:10:57Z

Hm, somehow cranelift-assembler-x64 is still blowing the stack. Is my recursion check broken or does that crate have a ridiculously nested but not recursive type?

oli-obk · 2026-06-03T06:32:00Z

Do we need to crater it now that we are introducing a new error?

RalfJung · 2026-06-03T06:38:12Z

Yeah once this works it needs cratering. But previously cranelift failed to build somehow. I did a small change to the logic which might fix that if cranelift hit a really weird edge case... if not I think I'll have to sprinkle ensure_sufficient_stack somewhere.

RalfJung · 2026-06-03T06:50:08Z

Well, now docker.io is down...

theemathas · 2026-06-03T10:34:51Z

+        is_opsem_inhabited_recursor(self, tcx, &mut (), /* stop_at_ref */ false, &|ty, _, _| {
+            tcx.is_opsem_inhabited_raw(typing_env.as_query_input(ty))
+        })


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

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.

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

RalfJung · 2026-06-03T15:18:02Z

@bors try

interpret: properly check for inhabitedness of nested references

rust-bors · 2026-06-03T17:29:48Z

☀️ Try build successful (CI)
Build commit: 05c0158 (05c0158cca4d7ab383df6c11b9cc128ebf3730be, parent: 042c759f774872cf2f94c6685ce87e24c046c722)

RalfJung · 2026-06-03T17:35:39Z

Okay, this should be ready for review now while the crater run is ongoing.
@craterbot check

That said, technically all code this breaks already had UB.

craterbot · 2026-06-03T17:36:06Z

👌 Experiment pr-156977 created and queued.
🤖 Automatically detected try build 05c0158
⚠️ Try build based on commit 5f63b50, but latest commit is d1a8249. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

RalfJung · 2026-06-03T17:36:12Z

@rustbot ready

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 26, 2026

rustbot assigned oli-obk May 26, 2026

RalfJung commented May 26, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

RalfJung force-pushed the interpret-opsem-inhabited branch from 4e90bd5 to e6d1440 Compare May 26, 2026 15:51

This comment has been minimized.

Sign in to view

RalfJung force-pushed the interpret-opsem-inhabited branch from e6d1440 to 33dc892 Compare May 26, 2026 17:12

This comment has been minimized.

Sign in to view

RalfJung force-pushed the interpret-opsem-inhabited branch from 33dc892 to 846fa4f Compare May 26, 2026 18:07

RalfJung mentioned this pull request May 26, 2026

Can references to uninhabited types ever be valid? rust-lang/unsafe-code-guidelines#413

Open

This comment has been minimized.

Sign in to view

RalfJung force-pushed the interpret-opsem-inhabited branch from 846fa4f to 8d272fd Compare May 26, 2026 20:45

This comment has been minimized.

Sign in to view

RalfJung force-pushed the interpret-opsem-inhabited branch from 8d272fd to 06e6db9 Compare May 26, 2026 21:42

This comment has been minimized.

Sign in to view

RalfJung force-pushed the interpret-opsem-inhabited branch from 06e6db9 to f328265 Compare May 27, 2026 06:27

oli-obk requested changes May 27, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 27, 2026

theemathas reviewed May 27, 2026

View reviewed changes

Comment thread compiler/rustc_middle/src/ty/inhabitedness/mod.rs Outdated

RalfJung force-pushed the interpret-opsem-inhabited branch from f328265 to bffa3cd Compare May 27, 2026 14:28

RalfJung commented May 27, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

RalfJung force-pushed the interpret-opsem-inhabited branch from bffa3cd to 2552ff6 Compare May 27, 2026 14:56

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 2, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Jun 2, 2026

Auto merge of #156977 - RalfJung:interpret-opsem-inhabited, r=<try>

68787dc

interpret: properly check for inhabitedness of nested references

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 3, 2026

RalfJung force-pushed the interpret-opsem-inhabited branch from b60b5f7 to 6a1ac87 Compare June 3, 2026 06:14

RalfJung force-pushed the interpret-opsem-inhabited branch from 6a1ac87 to 5f63b50 Compare June 3, 2026 06:37

This comment has been minimized.

Sign in to view

theemathas reviewed Jun 3, 2026

View reviewed changes

rust-bors Bot pushed a commit that referenced this pull request Jun 3, 2026

Auto merge of #156977 - RalfJung:interpret-opsem-inhabited, r=<try>

05c0158

interpret: properly check for inhabitedness of nested references

This comment has been minimized.

Sign in to view

RalfJung force-pushed the interpret-opsem-inhabited branch from 5f63b50 to 72bf43a Compare June 3, 2026 15:26

interpret: properly check for inhabitedness of nested references

d1a8249

RalfJung force-pushed the interpret-opsem-inhabited branch from 72bf43a to d1a8249 Compare June 3, 2026 15:28

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 3, 2026

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 3, 2026

Uh oh!

Conversation

RalfJung commented May 26, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented May 26, 2026

Uh oh!

RalfJung May 26, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung May 26, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung May 26, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung May 26, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oli-obk May 27, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

RalfJung May 27, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theemathas May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RalfJung commented May 26, 2026 •

edited by rustbot

Loading

RalfJung May 26, 2026 •

edited by rustbot

Loading

RalfJung May 26, 2026 •

edited by rustbot

Loading

RalfJung May 26, 2026 •

edited by rustbot

Loading

RalfJung May 26, 2026 •

edited by rustbot

Loading

oli-obk May 27, 2026 •

edited by rustbot

Loading

RalfJung May 27, 2026 •

edited by rustbot

Loading

theemathas May 27, 2026 •

edited

Loading

RalfJung May 27, 2026 •

edited

Loading

theemathas Jun 1, 2026 •

edited

Loading

theemathas Jun 1, 2026 •

edited

Loading