Fix some minor issues with stdin-loaded modules

[alexcrichton]

This commit addresses a few items I saw after reading #5342 such as:

• Module::from_trusted_file no longer panics if the file length is less than 4
• Module::from_trusted_file with an empty file now returns a syntax error for *.wat instead of a "failed to map" error.
• Usage of - for stdin no works on Windows instead of just Unix.
• The filename for - reported to the executable is <stdin> rather than stdin.

[jameysharp]

I like these changes for the most part, but I'd like to argue for one difference in philosophy here.

[jameysharp]

I don't want to re-introduce the ELF-magic check in the CLI for two reasons:

• We considered it a feature that --allow-precompiled doesn't work on pipes. The motivating use case for this feature was curl | wasmtime, where the provided code really shouldn't be trusted.
• I think the details of which formats are supported should be fully encapsulated in the wasmtime crate, not duplicated in the CLI. Somebody (I forget who) suggested that the CLI should serve as an example of how to use the library, and I like that as a guiding principle here.

I think it'd be reasonable to declare that --allow-precompiled is incompatible with reading from stdin. With #5342, if stdin happens to be mmap-able because e.g. it's redirected from a file on Unix, then --allow-precompiled works, but I think it's okay to lose that "feature".

So I'd prefer to reject that combination while validating arguments, and replace this block with:

Suggested change

	if self.allow_precompiled && module.starts_with(b"\x7fELF") {
	unsafe { Module::deserialize(linker.engine(), &module)? }
	} else {
	Module::new(linker.engine(), &module)?
	}
	Module::new(linker.engine(), &module)?

[alexcrichton]

Can you elaborate more on why it's considered a good thing to disallow --allow-precompiled and stdin? It's already opt-in behavior which I would imagine suitably covers any misuse issues. It surely would be bad to curl | wasmtime --allow-precompiled but I don't feel like it's our place to prevent that if someone really wanted to.

As for the ELF detection, I don't mind moving it into the crate one way or another.

[jameysharp]

Sorry, I lost track of this. I guess I don't have a strong argument for preferring to disallow --allow-precompiled when reading from stdin on general principle.

I do think the API design is a little fiddly if you want to allow that case though. My suggestion, which we adopted in #5342, had been to combine "is this safe to mmap" with "is this safe to blindly execute as native code", so the same Module::from_trusted_file API is used to mean both things. We could introduce, I dunno, Module::from_trusted_bytes or something to mean only "safe to execute"?

[alexcrichton]

Hm ok I've sort of lost context on this and don't feel strongly motivated for pushing it over the finish line, so I'll leave this to a future PR if it's necessary.

[github-actions]

Subscribe to Label Action

cc @peterhuene

Details

This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.