Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

This crate is very unsound #1

Closed
RalfJung opened this issue Sep 27, 2022 · 2 comments
Closed

This crate is very unsound #1

RalfJung opened this issue Sep 27, 2022 · 2 comments

Comments

@RalfJung
Copy link

RalfJung commented Sep 27, 2022

The inconceivable! macro can be used from safe code, which makes this crate unsound -- a fatal bug for any crate.

The concept of having an 'unreachable' macro with a debug assertion makes a lot of sense, but the macro absolutely needs to be unsafe to call.

Indeed, as the crate description says

This crate is created purely to inject undefined behavior into stable, safe rust.

This is the antithesis of Rust. (It makes me wonder whether this is even serious or just trying to prove some kind of point.)

@valarauca
Copy link
Owner

The inconceivable! macro can be used from safe code, which makes this crate unsound -- a fatal bug for any crate.

So can unsafe blocks.

The concept of having an 'unreachable' macro with a debug assertion makes a lot of sense

Using unreachable! has some trade-offs, see last paragraph of this section

It makes me wonder whether this is even serious or just trying to prove some kind of point

IDK there are some well documented use cases

This is the antithesis of Rust

Short of trolling, why would you open an issue to say this?

Could you have said it in a more constructive manner?

@valarauca
Copy link
Owner

It makes me wonder whether this is even serious or just trying to prove some kind of point

Oh no, it is serious. Please review the doc comment.

The goal is to permit the LLVM to be able to more aggressively prune branches that are not provable at compile time, but impossible due to program structure. Philosophical objections aside concerning safety & purity aside this is useful and greatly impacts micro-benchmarks.

The general assumption that predictable branches are free is in fact very untrue (intel errata link). By carefully doing pref analysis of a sample of code you can note that front end stalls are occurring for maligned branches and their state is not being cached. While the LLVM has shipped some features to address (link) this feature is not consistently enabled and passing arguments to the LLVM through cargo -> rustc -> llvm is extremely difficult/annoying. Rustc even has an open issue about this.

This is a long way of saying that pruning unnecessary branches is useful.

While you undoubtedly have objections to this crate, it does serve a useful purpose. Until static analysis significantly improves being able to configure a build where I can in no uncertain terms tell the compiler "simply treat all these conditions as impossible" is extremely useful.


All this said the crate is only UB if whatever is importing this crate enables it via cfg options.

The crate is safe by default as outlined in the readme. Unsafe behavior is only available if somebody opts-into it.

Repository owner locked as spam and limited conversation to collaborators Sep 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants