Repository navigation
Conversation
…of crashing
An enum whose variants are all uninhabited and contain only zero-sized
fields (e.g. `enum E { A(!) }`) gets the `Variants::Empty` layout of `!`,
so its goto struct has no fields. The Downcast projection passed the
expression through unchanged, and the following Field projection panicked
in `Expr::member`, aborting codegen of the whole crate (and the whole
autoharness run).
Return an `UnimplementedData` error for this case instead, so the place is
codegened as an unsupported construct that only fails verification if it
is reachable.
Resolves model-checking#3596
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open issues
What changed in this PR
This PR prevents kani autoharness runs from aborting when MIR performs a by-reference Downcast into an enum whose layout is Variants::Empty (e.g., enum E { A(!) }), by reporting it as an unsupported construct instead of crashing.
Changes:
- Handle
Variants::Emptyincodegen_projection’sDowncastarm by returningUnimplementedData(instead of passing through and later crashing on a missing field). - Add regression tests covering both expected-test and script-based autoharness scenarios to ensure the run completes and reports a targeted failure.
| File | Description |
|---|---|
| kani-compiler/src/codegen_cprover_gotoc/codegen/place.rs | Converts the Variants::Empty downcast case from a crash into a structured unsupported-construct error. |
| tests/expected/uninhabited-enum-downcast/main.rs | Adds a reproducer exercising by-reference downcast via let and match. |
| tests/expected/uninhabited-enum-downcast/expected | Locks in the expected “unsupported construct” failure message instead of a compiler panic. |
| tests/script-based-pre/autoharness_uninhabited_enum_downcast/uninhabited_enum_downcast.rs | Adds an autoharness crate containing both a triggering function and a normal function to verify the run continues. |
| tests/script-based-pre/autoharness_uninhabited_enum_downcast/run.sh | Runs autoharness over the reproducer crate to ensure the whole run doesn’t abort. |
| tests/script-based-pre/autoharness_uninhabited_enum_downcast/expected | Asserts the run completes, with ok_fn verified and trigger failing. |
| tests/script-based-pre/autoharness_uninhabited_enum_downcast/config.yml | Wires the new script-based test into the harness. |
Review effort: Lite
| // An enum whose variants are all uninhabited and contain only zero-sized fields | ||
| // (e.g. `enum E { A(!) }`) gets the layout of `!`, so its goto struct has no | ||
| // field to project into. | ||
| Variants::Empty => { | ||
| return Err(Box::new(UnimplementedData::new( | ||
| "Downcast into a variant of an uninhabited enum", | ||
| "https://github.com/model-checking/kani/issues/3596", | ||
| self.codegen_ty_stable(ty), | ||
| *before.goto_expr.location(), | ||
| ))); | ||
| } |
| // An enum whose variants are all uninhabited and contain only zero-sized fields | ||
| // (e.g. `enum E { A(!) }`) gets the layout of `!`, so its goto struct has no | ||
| // field to project into. |
feliperodri
left a comment
There was a problem hiding this comment.
The fix is minimal and safe: an uninhabited downcast becomes an unsupported-construct check, which only fails when reachable. Both tests fail without the change, which is good evidence.
Before merge:
- This is a crash fix, so please add
[C] Bugand[F] Crash. We can discussT-High Priorityseparately, since it stops whole autoharness runs. - The issue URL is hardcoded here. Please centralize it, as Copilot suggests.
- The comment suggests only zero-sized fields trigger this. The arm matches
Variants::Emptyalone, so please reword it. - Six jobs are cancelled. Please re-run them.


This fixes #3596, an issue in the Autoharness milestone.
Priority: Critical (C3, stops a whole run; P3, 11 changed lines outside tests with tests that fail without the fix). Until this is fixed, users of
kani autoharnesscannot get any verdicts for a crate that contains a by-reference downcast into an enum likeenum E { A(!) }: the compiler panics and the whole run aborts. It affects place codegen (the Downcast arm ofcodegen_projection) and fixes the crate-wide abort of #3596.Root cause
rustc gives an enum whose variants are all uninhabited and contain only zero-sized fields (e.g.
enum E { A(!) }) theVariants::Emptylayout of!, so Kani generates a goto struct with no fields for it. MIR still projectsDowncast(A)thenField(0). The Downcast arm treatedEmptylikeSingleand passed the expression through, so the Field projection called.member("0")on an empty struct and hitunreachable!inExpr::member(cprover_bindings/src/goto_program/expr.rs:914).Codegen runs for the whole crate before any harness is checked, so under
autoharnessone such function aborts the entire run and every other function in the crate gets no result.Fix
The Downcast arm now returns an
UnimplementedDataerror forVariants::Empty. The existing flow turns it into an unsupported-construct check that only fails verification if it is reachable. No program that can fail is reported as successful.Testing
tests/expected/uninhabited-enum-downcast: the issue's reproducer (let E::A(ref _a) = ...) and amatchby-reference variant. Both now fail with the unsupported-construct check instead of crashing.tests/script-based-pre/autoharness_uninhabited_enum_downcast: an autoharness run over a crate with the crashing function plus an ordinaryok_fn.ok_fnis verified and the run completes with a summary.expr.rs:914) with the compiler change reverted, and pass with it.let E::A(a) = ...,match ... { E::A(a) => a }) never reached the Downcast, since rustc lowers them tounreachable, and behave as before.Notes for reviewers
unable to find field 0 for type StructTag(never_type) #3596, which this PR closes. Happy to point it at a different issue if you prefer.intrinsic.rs) rather than as unsupported, but since the opsem question about reference validity is still open, this PR keeps to the conservative fix.min_lengthof a constant index into an array #4974 also touchcodegen_projection, in different arms (subslice and constant index). They are independent of this change.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.