Internal error when building code with nested static items

[podhrmic]

Steps to reproduce:

1. git clone git@github.com:chronotope/chrono.git
2. cd chrono
3. cargo build (that proceeds normally)
4. cargo crux-test --lib
   The relevant error output:

thread 'rustc' panicked at compiler/rustc_middle/src/hir/map/mod.rs:952:32:
internal error: entered unreachable code
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: could not compile `chrono` (lib test)

Caused by:
  process didn't exit successfully: `/Users/mpodhradsky/Applications/mir-json-macos-14-ARM64/bin/mir-json-rustc-wrapper rustc --crate-name chrono --edition=2021 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --diagnostic-width=240 --emit=dep-info,link -C embed-bitcode=no -C debuginfo=2 -C split-debuginfo=unpacked --test --cfg 'feature="alloc"' --cfg 'feature="android-tzdata"' --cfg 'feature="clock"' --cfg 'feature="default"' --cfg 'feature="iana-time-zone"' --cfg 'feature="js-sys"' --cfg 'feature="now"' --cfg 'feature="oldtime"' --cfg 'feature="std"' --cfg 'feature="wasm-bindgen"' --cfg 'feature="wasmbind"' --cfg 'feature="winapi"' --cfg 'feature="windows-link"' --check-cfg 'cfg(docsrs,test)' --check-cfg 'cfg(feature, values("__internal_bench", "alloc", "android-tzdata", "arbitrary", "clock", "default", "iana-time-zone", "js-sys", "libc", "now", "oldtime", "pure-rust-locales", "rkyv", "rkyv-16", "rkyv-32", "rkyv-64", "rkyv-validation", "serde", "std", "unstable-locales", "wasm-bindgen", "wasmbind", "winapi", "windows-link"))' -C metadata=eb2fbdf882aca9fa -C extra-filename=-99415242a23a6b83 --out-dir /private/tmp/chrono/target/aarch64-apple-darwin/debug/deps --target aarch64-apple-darwin -C incremental=/private/tmp/chrono/target/aarch64-apple-darwin/debug/incremental -L dependency=/private/tmp/chrono/target/aarch64-apple-darwin/debug/deps -L dependency=/private/tmp/chrono/target/debug/deps --extern bincode=/private/tmp/chrono/target/aarch64-apple-darwin/debug/deps/libbincode-91582fba06090d95.rlib --extern iana_time_zone=/private/tmp/chrono/target/aarch64-apple-darwin/debug/deps/libiana_time_zone-c04a6f76e0a7b408.rlib --extern num_traits=/private/tmp/chrono/target/aarch64-apple-darwin/debug/deps/libnum_traits-3121c6ccd68721f9.rlib --extern serde_derive=/private/tmp/chrono/target/debug/deps/libserde_derive-a1f7dd05e40f13ad.dylib --extern serde_json=/private/tmp/chrono/target/aarch64-apple-darwin/debug/deps/libserde_json-192e34f499792e9d.rlib --extern similar_asserts=/private/tmp/chrono/target/aarch64-apple-darwin/debug/deps/libsimilar_asserts-670972fa35ec9ddb.rlib` (exit status: 101)

And with RUST_BACKTRACE=1 cargo crux-test --lib:

thread 'rustc' panicked at compiler/rustc_middle/src/hir/map/mod.rs:952:32:
internal error: entered unreachable code
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::panic
   3: <rustc_middle::hir::map::Map>::span_with_body
   4: rustc_hir_typeck::typeck_with_inspect::{closure#0}
   5: rustc_hir_typeck::typeck
      [... omitted 1 frame ...]
   6: rustc_middle::query::plumbing::query_get_at::<rustc_data_structures::vec_cache::VecCache<rustc_span::def_id::LocalDefId, rustc_middle::query::erase::Erased<[u8; 8]>, rustc_query_system::dep_graph::graph::DepNodeIndex>>
   7: rustc_mir_build::thir::pattern::check_match::check_match
      [... omitted 1 frame ...]
   8: rustc_middle::query::plumbing::query_get_at::<rustc_data_structures::vec_cache::VecCache<rustc_span::def_id::LocalDefId, rustc_middle::query::erase::Erased<[u8; 1]>, rustc_query_system::dep_graph::graph::DepNodeIndex>>
   9: rustc_mir_build::builder::build_mir
  10: rustc_mir_transform::mir_built
      [... omitted 1 frame ...]
  11: rustc_middle::query::plumbing::query_get_at::<rustc_data_structures::vec_cache::VecCache<rustc_span::def_id::LocalDefId, rustc_middle::query::erase::Erased<[u8; 8]>, rustc_query_system::dep_graph::graph::DepNodeIndex>>
  12: rustc_mir_transform::mir_const_qualif
      [... omitted 1 frame ...]
  13: rustc_middle::query::plumbing::query_get_at::<rustc_query_system::query::caches::DefIdCache<rustc_middle::query::erase::Erased<[u8; 4]>>>
  14: rustc_mir_transform::mir_promoted
      [... omitted 1 frame ...]
  15: rustc_middle::query::plumbing::query_get_at::<rustc_data_structures::vec_cache::VecCache<rustc_span::def_id::LocalDefId, rustc_middle::query::erase::Erased<[u8; 16]>, rustc_query_system::dep_graph::graph::DepNodeIndex>>
  16: rustc_borrowck::mir_borrowck
      [... omitted 1 frame ...]
  17: rustc_middle::query::plumbing::query_get_at::<rustc_data_structures::vec_cache::VecCache<rustc_span::def_id::LocalDefId, rustc_middle::query::erase::Erased<[u8; 8]>, rustc_query_system::dep_graph::graph::DepNodeIndex>>
  18: rustc_mir_transform::mir_drops_elaborated_and_const_checked
      [... omitted 1 frame ...]
  19: rustc_middle::query::plumbing::query_get_at::<rustc_data_structures::vec_cache::VecCache<rustc_span::def_id::LocalDefId, rustc_middle::query::erase::Erased<[u8; 8]>, rustc_query_system::dep_graph::graph::DepNodeIndex>>
  20: rustc_mir_transform::mir_for_ctfe
      [... omitted 1 frame ...]
  21: rustc_middle::query::plumbing::query_get_at
  22: mir_json::analyz::analyze_inner
  23: mir_json::analyz::analyze_streaming
  24: <mir_json_rustc_wrapper_real::MirJsonCallbacks as rustc_driver_impl::Callbacks>::after_analysis
  25: rustc_interface::passes::create_and_enter_global_ctxt::<core::option::Option<rustc_interface::queries::Linker>, rustc_driver_impl::run_compiler::{closure#0}::{closure#2}>
  26: rustc_interface::interface::run_compiler::<(), rustc_driver_impl::run_compiler::{closure#0}>::{closure#1}
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
error: could not compile `chrono` (lib test)

I am using the latest crux-mir:

% crux-mir --version
[Crux] version: 0.7.2.0.99, crux-mir version: 0.10.0.0.99

And rustc 1.87

[RyanGlScott]

Minimal reproducer (adapted from rust-lang/miri#3389, which was fixed by rust-lang/rust#122719, the patch that introduced Node::Synthetic):

// test.rs
const OID_RSA_ENCRYPTION: &[u64] = &[1, 2, 840, 113549, 1, 1, 1];

pub struct SignatureAlgorithm {
    pub oids: &'static [&'static [u64]],
}

pub static PKCS_RSA_SHA256: SignatureAlgorithm = SignatureAlgorithm {
    // Does not repro with this constant inlined.
    oids: &[&OID_RSA_ENCRYPTION],
};

$ saw-rustc test.rs
test build - extract output path - ["rustc", "test.rs", "--test", "--target", "aarch64-apple-darwin", "--cfg", "crux", "-Z", "ub-checks=false", "-L", "/Users/rscott/Documents/Hacking/Rust/mir-json/rlibs", "--sysroot", "/Users/rscott/Documents/Hacking/Rust/mir-json/rlibs"]
test build - ["rustc", "test.rs", "--target", "aarch64-apple-darwin", "--cfg", "crux", "-Z", "ub-checks=false", "-L", "/Users/rscott/Documents/Hacking/Rust/mir-json/rlibs", "--sysroot", "/Users/rscott/Documents/Hacking/Rust/mir-json/rlibs", "--cfg", "crux_top_level", "--crate-type", "rlib"]
note: Emitting MIR for test/2bca7747::PKCS_RSA_SHA256

note: Emitting MIR for test/2bca7747::PKCS_RSA_SHA256::{{promoted}}[0]


thread 'rustc' panicked at compiler/rustc_middle/src/hir/map/mod.rs:952:32:
internal error: entered unreachable code
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The unreachable code comes from here in rustc_middle:

            Node::Synthetic => unreachable!(),

[RyanGlScott]

The part of mir-json that leads to the unreachable code is here in emit_static:

mir-json/src/analyz/mod.rs

Lines 784 to 785 in 80802b1

	// let mir = tcx.optimized_mir(def_id);
	let mir = tcx.mir_for_ctfe(def_id);

Debug printing reveals that the def_id right before panicking is test[b10b]::PKCS_RSA_SHA256::{constant#0}.

[RyanGlScott]

Slightly minimized example:

// test.rs
const X: &[u64] = &[42];
pub static Y: &[&[u64]] = &[&X];

pub fn f() -> &'static [&'static [u64]] {
    &Y
}

[RyanGlScott]

I am starting to suspect that the root issue involves nested statics, a rustc featurette where anonymous static definitions are emitted for complex static items. For instance, in the example above, a nested static is emitted for the &X part of pub static Y: &[&[u64]] = &[&X]. You can determine whether a static item is nested or not by looking at the nested field of DefKind::Static.

rust-lang/rust#121644 (the PR which introduced DefKind::Static's nested field) introduces a lot of special-casing for nested statics in the compiler, along with several comments of the form "Nested statics don't have a type", so perhaps you're not meant to be able to call functions like mir_for_ctfe() on a nested static. If that is the case, then we may need to special-case nested statics in mir-json somehow as well.

I wonder if it would suffice to skip emitting JSON for nested statics altogether?

[spernsteiner]

I wonder if it would suffice to skip emitting JSON for nested statics altogether?

Maybe. Looking at the MIR for your previous example (playground link), I see that it has a MIR body for Y, but it also has a fully-evaluated alloc for Y as well, and the alloc is what it uses in f(). I wonder if we should try to switch mir-json to emit try_render_opty-style initializers for all statics, and get rid of the bodies entirely.

[RyanGlScott]

It is certainly difficult to convince myself that dropping nested statics is the right thing to do in general. It does appear to work in the previous example, but that may only be because rustc is inlining the definition of the nested static in the right places such that there are no longer any dangling references to the nested static (which would be a problem for crucible-mir's translation).

If I understand your proposal correctly, you want to const-eval the bodies of each static item and use the results as initializers, thereby eliminating any need for the nested statics in the first place? That sounds like a much more solid basis for dropping nested statics, although I suppose it does mean we would have to change the MIR JSON schema for statics to accommodate this change.

[spernsteiner]

we would have to change the MIR JSON schema for statics to accommodate this change

We already emit "rendered constants" for some statics. In --print-mir output, some statics have an expr: STATIC_NAME : Ty = expr. Others have just STATIC_NAME : Ty and a MIR body for STATIC_NAME in the FNs section. If we can migrate all of them to have rendered constants instead of bodies, then we can get rid of some of the static initializer logic in crucible-mir.

[RyanGlScott]

For reference, here is the part of the code which emits statics without a rendered constant (just a "body"):

mir-json/src/analyz/mod.rs

Lines 847 to 863 in ae81cec

	/// Add a new static declaration to `out.statics`.
	fn emit_static_decl<'tcx>(
	ms: &mut MirState<'_, 'tcx>,
	out: &mut impl JsonOutput,
	name: &str,
	ty: ty::Ty<'tcx>,
	mutable: bool,
	) -> io::Result<()> {
	let j = json!({
	"name": name,
	"ty": ty.to_json(ms),
	"mutable": mutable,
	"kind": "body",
	});
	out.emit(EntryKind::Static, j)?;
	emit_new_defs(ms, out)
	}

If we wanted to modify this code to emit a rendered constant using something like try_render_opty, then it sounds as if we'd need to turn the Body of a static declaration into something like a ConstVal/OpTy/MPlaceTy. It's not entirely obvious to me how to do that, however...

[spernsteiner]

it sounds as if we'd need to turn the Body of a static declaration into something like a ConstVal/OpTy/MPlaceTy. It's not entirely obvious to me how to do that, however...

TyCtxt has methods like const_eval_instance that sound like they should be able to take a DefId and evaluate it down to a ConstValue

[RyanGlScott]

Ah, const_eval_poly looks like it would work... at least, for static items that have DefIds. The tricky part is that not all static items have DefIds, such as promoteds. As such, it's unclear what DefId I would pass const_eval_poly when emitting the static declaration arising from a promoted in emit_promoted.

For static declarations, we can avoid this problem by observing that if we const-eval the body of a static item, then the resulting value won't contain any references to promoted names, thereby avoiding the need to call emit_promoted in the first place. However, we also call emit_promoted from here in emit_instance:

mir-json/src/analyz/mod.rs

Lines 1080 to 1090 in dbdc8b7

	if let ty::InstanceKind::Item(def_id) = ty_inst.def {
	for (idx, mir) in tcx.promoted_mir(def_id).iter_enumerated() {
	let mir = ty_inst.instantiate_mir_and_normalize_erasing_regions(
	tcx,
	ty::TypingEnv::fully_monomorphized(),
	ty::EarlyBinder::bind(mir.clone()),
	);
	let mir = tcx.arena.alloc(mir);
	emit_promoted(ms, out, &name, idx, mir)?;
	}
	}

This gives me pause, because the FnInst very well might not be const-eval'd, which means that it could reference promoted things. If it does, then I'm genuinely not sure what DefId we would use here.

[spernsteiner]

The tricky part is that not all static items have DefIds, such as promoteds

const_eval_global_id and const_eval_resolve both accept an Option<Promoted> index

[RyanGlScott]

With much effort, I was able to rework emit_static_decl to remove the call to mir_for_ctfe and replace it with a call to const_eval_global_id. I've pushed my progress to the rgs/T129-nested-statics branch if you'd like to check my work.

Unfortunately, even after all that, rustc still fails when trying to compile the example from #129 (comment):

$ saw-rustc test.rs
test build - extract output path - ["rustc", "test.rs", "--test", "--target", "aarch64-apple-darwin", "--cfg", "crux", "-Z", "ub-checks=false", "-L", "/Users/rscott/Documents/Hacking/Rust/mir-json/rlibs", "--sysroot", "/Users/rscott/Documents/Hacking/Rust/mir-json/rlibs"]
test build - ["rustc", "test.rs", "--target", "aarch64-apple-darwin", "--cfg", "crux", "-Z", "ub-checks=false", "-L", "/Users/rscott/Documents/Hacking/Rust/mir-json/rlibs", "--sysroot", "/Users/rscott/Documents/Hacking/Rust/mir-json/rlibs", "--cfg", "crux_top_level", "--crate-type", "rlib"]

thread 'rustc' panicked at compiler/rustc_const_eval/src/const_eval/eval_queries.rs:340:5:
assertion failed: key.value.promoted.is_some() || !tcx.is_static(key.value.instance.def_id())
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Where the failing assertion comes from here:

pub fn eval_to_allocation_raw_provider<'tcx>(
    tcx: TyCtxt<'tcx>,
    key: ty::PseudoCanonicalInput<'tcx, GlobalId<'tcx>>,
) -> ::rustc_middle::mir::interpret::EvalToAllocationRawResult<'tcx> {
    // This shouldn't be used for statics, since statics are conceptually places,
    // not values -- so what we do here could break pointer identity.
    assert!(key.value.promoted.is_some() || !tcx.is_static(key.value.instance.def_id()));

That comment suggests that this simply won't work for statics. Any ideas on what to try next?