Detect std by checking if the crate defines #[lang = "start"] rather than string comparison

[ghost]

I also considered to compare the crate name with sym::std, but it's easy to name any crate std by using --crate-name std, so I don't think that is robust enough.

Note that this only checks the crate, it does not check whether the call is in sys::unix or sys::windows, unlike the previous implementation, but I think it's already robust enough.

Fixes #1821.

[bjorn3]

There may not be a #[lang = "start"] when using #![no_std] in combination with #[start], so please don't unwrap.

[ghost]

There is already an unwrap() in create_ecx() 🤔

miri/src/eval.rs

Line 113 in ef99830

let start_id = tcx.lang_items().start_fn().unwrap();

[RalfJung]

using #![no_std] in combination with #[start]

That sounds like a test case we should have, then. ;)

[ghost]

using #![no_std] in combination with #[start]

Note that it's currently broken even before the unwrap():

error: internal compiler error: src/tools/miri/src/eval.rs:110:9: main function must not take any arguments

thread 'rustc' panicked at 'Box<Any>', compiler/rustc_errors/src/lib.rs:1007:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.54.0-nightly (c79419af0 2021-06-04) running on x86_64-unknown-linux-gnu

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=b867b78dd787c573bdfb44c88fa856dc

miri/src/eval.rs

Line 110 in ef99830

bug!("main function must not take any arguments");

[RalfJung]

Hm, the start item in Rust has no arguments so I am not sure if it makes sense to support other start items that do.

But this sounds like a separate discussion then, could you open an issue?

[RalfJung]

Note that the check does not have to be extremely robust. If someone wants to go out of their way to use these incomplete Miri shims, I'm fine with that, but they don't get to complain about any bugs they encounter. ;)

So I'm in favor of your code changes, but I am worried about the test size blowup.^^ tests/run-pass/std_only_foreign_function.rs is not a supported use-case, so we shouldn't test it IMO. For the compile-fail test, I think having just one is enough -- just to make sure that in_std does not always return true. So I'm in favor of also removing tests/compile-fail/unsupported_get_process_heap.rs.

[ghost]

tests/run-pass/std_only_foreign_function.rs is not a supported use-case, so we shouldn't test it IMO.

(I have removed it now, but) I added it to test that in_std() does not always return false, which is not explicitly tested in other tests.

[RalfJung]

Thanks @hyd-dev :)
@bors r+

[bors]

📌 Commit 0ece55d has been approved by RalfJung

[bors]

⌛ Testing commit 0ece55d with merge 28fbf81...

[RalfJung]

(I have removed it now, but) I added it to test that in_std() does not always return false, which is not explicitly tested in other tests.

Fair -- however, it is implicitly tested in every single test, which IMO is good enough.

[bors]

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 28fbf81 to master...