Add version header to -Zno-link .rlink files

[bjorn3]

This allows giving a better error when attempting to use the original source file with -Zlink-only rather than crashing and allows rejecting rlink files produced by a different rustc version.

[nnethercote]

To clarify: here's what I thought you had to do:

rustc +nightly -Zno-link z.rs
rustc +nightly -Zlink-only z.rs

But that gives an inscrutable error like this:

thread 'rustc' panicked at 'index out of bounds: the len is 43 but the index is 112', /home/njn/dev/rust0/compiler/rustc_serialize/src/opaque.rs:655:24

The correct way to invoke is actually this

rustc +nightly -Zno-link z.rs
rustc +nightly -Zlink-only z.rlink      # z.rlink instead of z.rs!

If the .rlink file has a header, the code that reads it can check for that header. If it's not present (e.g. because somebody passed a .rs file by mistake) then it can give a nice error message like "this doesn't look like a .rlink file".

[Kobzol]

@nnethercote I tried to look at this. CodegenResults is a struct that is serialized to .rlink files using the Encodable/Decodable machinery. I tried to create a wrapper struct that would serialize and deserialize a header to provide nicer messages, but it's not really easy to provide a custom message if decoding of the header fails currently, since you removed the Results from Decoder methods 😅

I see several solutions to this:

1. Catch panic that could be caused by invalid decoding of the header and provide a nicer message in that case (seems dubious).
2. Implement serialization and deserialization of the header using manual byte writes/reads, going around the Encodable/Decodable traits.
3. Return Result sto Decoder methods (probably not worth it because of this).

What do you suggest, solution 2)?

[bjorn3]

The crate metadata encodes and decodes the header separately from the contents. For crate metadata the header is of the form "rust" + 32bit be version number (currently 5 I believe) + 32bit offset of the crate root + rustc version string. During decoding the header is checked for "rust", the right version number and then the right rustc version. Only after that is a Decoder created to decode the contents.

rust/compiler/rustc_metadata/src/rmeta/encoder.rs

Lines 2173 to 2177 in 07a461a

	let mut encoder = opaque::Encoder::new(vec![]);
	encoder.emit_raw_bytes(METADATA_HEADER).unwrap();
	// Will be filled with the root position after encoding everything.
	encoder.emit_raw_bytes(&[0, 0, 0, 0]).unwrap();

rust/compiler/rustc_metadata/src/rmeta/encoder.rs

Lines 2201 to 2202 in 07a461a

	// Encode the rustc version string in a predictable location.
	rustc_version().encode(&mut ecx).unwrap();

The rlink header could work the same way except omitting the offset of the crate root and using say "rsld" (rust ld) or "rustlink" as start instead of "rust" like the crate metadata.

[Kobzol]

Thanks for the hint. I checked rmeta decoding and it basically decodes the magic header byte slice with starts_with, which should be easy, but then it uses Lazy::<String>::decode for decoding the rustc version, which will panic if the version is missing.

So with this approach we could have a nice error if header magic is missing, nice error if rust version is wrong, but weird error if rust version if missing. That is basically the same that we get for rmeta parsing, so I suppose that's ok?

[bjorn3]

So with this approach we could have a nice error if header magic is missing, nice error if rust version is wrong, but weird error if rust version if missing.

Yeah, the header magic is supposed to be changed every time the rustc version needs to be decoded in a different way.

That is basically the same that we get for rmeta parsing, so I suppose that's ok?

IMO it is ok.