ICE when accessing externed DST

[JustAPerson]

I realize it's kinda nonsensical, but here:

fn main() {
    extern {
        static symbol: [usize];
    }
    println!("{}", symbol[0]);
}

thread 'rustc' panicked at 'assertion failed: self.llextra != ptr::null_mut()', ../src/librustc_trans/mir/lvalue.rs:64
stack backtrace:
   1:     0x7f8aedf3eb13 - std::sys::backtrace::tracing::imp::write::h46f28e67d38b4637
   2:     0x7f8aedf4f40d - std::panicking::default_hook::{{closure}}::h1d3243f546573ff4
   3:     0x7f8aedf4d95d - std::panicking::default_hook::h96c288d728df3ebf
   4:     0x7f8aedf4e058 - std::panicking::rust_panic_with_hook::hb1322e5f2588b4db
   5:     0x7f8aed023f9f - std::panicking::begin_panic::h0914615a412ba184
   6:     0x7f8aed0fd12b - rustc_trans::mir::lvalue::LvalueRef::len::h7704b2160ae8eef8
   7:     0x7f8aed10216f - rustc_trans::mir::rvalue::<impl rustc_trans::mir::MirContext<'bcx, 'tcx>>::trans_rvalue_operand::hc6bf66aa2cc81d96
   8:     0x7f8aed0ed8e4 - rustc_trans::mir::block::<impl rustc_trans::mir::MirContext<'bcx, 'tcx>>::trans_block::he2f6252e2877e76b
   9:     0x7f8aed0eb438 - rustc_trans::mir::trans_mir::hc6963a43e5e7752b
  10:     0x
7f8aed08aeae - rustc_trans::base::trans_closure::h2174ffc103f3d309
  11:     0x7f8aed10de82 - rustc_trans::trans_item::TransItem::define::h159a1f0a8708f3d3
  12:     0x7f8aed08e3b3 - rustc_trans::base::trans_crate::hf067cf9d0a8bcbf2
  13:     0x7f8aee308962 - rustc_driver::driver::phase_4_translate_to_llvm::he9d5d0022988d46e
  14:     0x7f8aee3439a0 - rustc_driver::driver::compile_input::{{closure}}::h13e3af2358852a64
  15:     0x7f8aee340450 - rustc_driver::driver::phase_3_run_analysis_passes::{{closure}}::h63e83c14f91c1ee5
  16:     0x7f8aee286a5f - rustc::ty::context::TyCtxt::create_and_enter::h6d5dc8f8bc2765a0
  17:     0x7f8aee2f7f64 - rustc_driver::driver::compile_input::h7dacd98cd2fd7d2b
  18:     0x7f8aee31e05d - rustc_driver::run_compiler::h37c4294ab73436f7
  19:     0x7f8aee25fbb3 - std::panicking::try::do_call::h4d040997e2efdaf3
  20:     0x7f8aedf5d626 - __rust_maybe_catch_panic
  21:     0x7f8aee27cf19 - <F as alloc::boxed::FnBox<A>>::call_box::hefd1c85fab3e6c31
  22:     0x7f8aedf4be90 - std::sys::thread::Thread::new::thread_start::h5b631f48cd23f128
  23:     0x7f8ae63b06f9 - start_thread
  24:     0x7f8aedb8fb5c - clone
  25:                0x0 - <unknown>

rustc 1.13.0-nightly (acd3f796d 2016-08-28)
binary: rustc
commit-hash: acd3f796d26e9295db1eba1ef16e0d4cc3b96dd5
commit-date: 2016-08-28
host: x86_64-unknown-linux-gnu
release: 1.13.0-nightly

[eddyb]

@nikomatsakis @arielb1 Why didn't WF catch this?

[arielb1]

That's because e85adff only checks statics/constants.

[istankovic]

Still reproducible with rustc 1.26.0-nightly (adf2135ad 2018-03-17).

[nikomatsakis]

This ought to be a mostly easy fix. The problem is that the well formedness check, which ought to be rejected this code, is ignoring things in an extern:

rust/src/librustc_typeck/check/wfcheck.rs

Line 86 in a04b88d

fn check_item_well_formed(&mut self, item: &hir::Item) {

They fall through to this _ case:

rust/src/librustc_typeck/check/wfcheck.rs

Line 160 in a04b88d

_ => {}

We would need to add support for the ForeignMod HIR item:

rust/src/librustc/hir/mod.rs

Lines 2034 to 2035 in a04b88d

	/// An external module
	ItemForeignMod(ForeignMod),

which in turn has a number of items:

rust/src/librustc/hir/mod.rs

Lines 1832 to 1836 in a04b88d

	#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
	pub struct ForeignMod {
	pub abi: Abi,
	pub items: HirVec<ForeignItem>,
	}

which are defined here:

rust/src/librustc/hir/mod.rs

Lines 2143 to 2163 in a04b88d

	#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
	pub struct ForeignItem {
	pub name: Name,
	pub attrs: HirVec<Attribute>,
	pub node: ForeignItem_,
	pub id: NodeId,
	pub span: Span,
	pub vis: Visibility,
	}
	/// An item within an `extern` block
	#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
	pub enum ForeignItem_ {
	/// A foreign function
	ForeignItemFn(P<FnDecl>, HirVec<Spanned<Name>>, Generics),
	/// A foreign static item (`static ext: u8`), with optional mutability
	/// (the boolean is true when mutable)
	ForeignItemStatic(P<Ty>, bool),
	/// A foreign type
	ForeignItemType,
	}

presumably we want the to check that the types of ForeignItemStatic are well-formed, roughly analogous to how we current check normal static items:

rust/src/librustc_typeck/check/wfcheck.rs

Lines 130 to 132 in a04b88d

	hir::ItemStatic(..) => {
	self.check_item_type(item);
	}

(We probably want similar checks on foreign item fns, though?)

One tricky part might be compatibility, we may need to turn this into a compatibility lint in case there is lots of broken code in the wild.

[krk]

I am trying to fix this bug, and need help moving forward.

wfcheck::check_item_type function is modified to accept a hir::Item and a hir::ForeignItem, yet it doesn't report any additional errors and the ICE prevails.

Test file:

fn main() {
    extern "C" {
        static symbol: [usize];
    }

    unsafe {
        println!("{}", symbol[0]);
    }
}

Commit: krk@aaa0ce7

@nikomatsakis Should I create a pull request or is it too early?

[eddyb]

@krk Looks like @nikomatsakis' suggestions are incomplete (check_item_type does not require Sized). Also, I left a comment on your commit about how you can avoid adding IdAndSpan.

I suspect the only reason regular statics are required to be Sized is they have an initializer (and that, not the type, requires Sized), i.e. for static FOO: [u32] = [0, 1, 2]; we have these errors:

error[E0308]: mismatched types
 --> src/main.rs:1:21
  |
1 | static FOO: [u32] = [0, 1, 2];
  |                     ^^^^^^^^^ expected slice, found array of 3 elements
  |
  = note: expected type `[u32]`
             found type `[u32; 3]`

error[E0277]: the trait bound `[u32]: std::marker::Sized` is not satisfied
 --> src/main.rs:1:21
  |
1 | static FOO: [u32] = [0, 1, 2];
  |                     ^^^^^^^^^ `[u32]` does not have a constant size known at compile-time
  |
  = help: the trait `std::marker::Sized` is not implemented for `[u32]`
  = note: constant expressions must have a statically known size

[pnkfelix]

Oddly, the PR that were previously posted to fix this (PR #51867, following on PR #50126) was subsequently closed as inactive ... but the bug itself seems like it may have been resolved in the meantime?

[pnkfelix]

I'm going to assume this was fixed. Tagging as E-needstest, but if someone wants to take the time, it'd be even better to bisect to where this was fixed, and see if a regression test for this was added at that time.

[krk]

Compiler no longer panics, tested with rustc 1.34.0 (91856ed52 2019-04-10)