You_can::turn_off_the_borrow_checker should not be allowed without declaring unsafe

Attribute Macro you_can::turn_off_the_borrow_checker should not be allowed without declaring unsafe.

Speaking of which why is Function unbounded::reference even allowed without declaring unsafe?

Discussion: Reddit

Repo: you-can, unbounded

The “unbounded::reference” function does use unsafe but doesn’t offer a sound safe API. The fact that it’s not declared as an unsafe fn is making it unsound, i.e. violating the usual principles you are expected to follow when using unsafe code. It self-documents as unsound though; maybe its purpose is to be used in that macro you linked without the macro needing to insert any “unsafe” blocks. The macro also self-documents as “allow[ing] unsound [code]”.

There is no way for the rust compiler to prevent unsound functions. The reason why you can use unsafe {} blocks in a non-unsafe function is because the whole purpose of unsafe {} blocks is to be able to call unsafe functions internally, hidden behind a (usually sound) safe API. So yes, a function like this is “allowed” by the compiler in that it doesn’t run into any compilation errors, and it is not allowed in the sense that you’re not supposed to write unsound code like this. The reddit discussion you linked also has multiple people pointing out that the crate lacks any part that requires users of it to write the word “unsafe” when using - either the macro, or that function.

In any case, note that’s just shenanigans that some random crate is pulling. What’s happening here is in no way related to the compiler or the standard library, hence it’s off-topic to this forum :wink: ; unless you’re asking that the compiler should change its rules here. In that case, you could have also started by asking somewhere about why it is allowed in the first place; e.g. in that reddit discussion or on users.rust-lang.org. It’s often advisable to try and learn more about the subject at hand first before proposing compiler changes. Still, since you’ve posted here now, I hope my explanation above helps you learn more. To learn more in-depth context around the distinction between “unsafe” code and the concept of “unsound” code, see also e.g. this blog post or this post it links to; or this glossary entry in the UCG reference, this section in the reference, or the Rustonomicon.

20 Likes

FWIW the latest release of unbounded marks unbounded::reference as unsafe:

This is unsound and unsafe . Invoking this functions risks incurring the wrath of the optimizer. Rust references are not pointers. For edutainment purposes only.

It is just that one can use turn_off_the_borrow_checker macro without declaring unsafe even when it uses unsafe functions underneath. That's the main issue.

Rust is built on writing safe abstractions that use unsafe underneath. A simple "Hello world" program uses a lot of unsafe, but the caller does not have to worry about it because the standard library has ensured that it is not possible to use the function unsoundly. The way that crate uses unsafe is unsound (as a joke), but that isn't a Rust problem, it's a you_can::turn_off_the_borrow_checker problem.

8 Likes

I do think there's one meaningful issue here. you_can::turn_off_the_borrow_checker is clearly unsafe, so it should be marked as unsafe, just like unbounded::reference is - but it can't be, because it's an attribute macro. I don't know that there's enough need to add a syntax to declare and use unsafe attribute macros, but at least theoretically, that's a gap in what Rust can express.

2 Likes

An attribute macro can require an unsafe token as an argument. You’d call it as

#[you_can::turn_off_the_borrow_checker(unsafe)]
fn foo() {

    // ...
}

This is not covered by #![forbid(unsafe_code)], but the approach to “grep for unsafe tokens” covers this at least. I just double-checked, #![forbid(unsafe_code)] apparently doesn’t properly catch uses of unsafe anyways. You can easily bypass it using a macro_rules macro from a different (also #![forbid(unsafe_code)]) crate to compose the unsafe with a block.

1 Like

You can probably come up with some minor adjustments to you_can that would make it safer for cargo geiger, e.g. require some sort of unsafe token in the attribute itself or require that the function itself is an unsafe one, but at the end of the day we're discussing a joke crate here. I feel like even the discussion about the API of unbounded::reference is overblown in that context.

This is related to RUSTSEC-2020-0011 is not a security vulnerability. · Issue #275 · rustsec/advisory-db · GitHub. I find it problematic that existing tooling cannot catch such crates in some automated way (even if it means maintaining a list of questionable crates somewhere), but IMO there's nothing that can be changed in the language to make this better, and fixing this particular crate to be "nicer to audit" won't solve a real-world concern of auditing underhanded code.

Indeed, for Macros Rust does not have a built-in notion of them being "unsafe". It is up to the macro author to somehow indicate that a macro is unsafe to use, e.g. by including unsafe in its name.

2 Likes

For what it's worth: I was failing to pass along a span when generating the unsafe { ... } blocks. Now that I've fixed that (v0.0.7), #[warn(unsafe_code]) is appropriately triggered by use of this macro:

Tangentially: custom diagnostic warnings on nightly are very neat:

7 Likes

Oh, that’s cool! I’m not too much into proc macros, I just tested what happened to an unsafe token passed to a macro that ignores it, and noted that this doesn’t trigger the lint. Whether or not warn(unsafe_code) catches its usage is an interesting aspect in the discussion of soundness of macros, besides approaches like

I suppose, in a perfect world, an unsafe macro would usually try to do both? However, arguably a macro that spells “turn_off_the_borrow_checker” doesn’t really need any additional remarks on whether or not such an operation is unsafe.

(And beyond that, if that wasn’t enough, the “DANGER DANGER DANGER” message during the build makes things quite clear as-well :slight_smile: )

5 Likes

While the macro system itself doesn't have any safety notation, it's practial in many cases to enforce the macro is used in usafe context. For example unsafe macros that produces statements/expressions should not wrap its result with unsafe block by itself. A serious implementation of #[you_can::turn_off_the_borrow_checker] would assert/make the function unsafe fn.

Indeed, i documented the current behavior with a test at Add a strange test for `unsafe_code` lint. by crlf0710 · Pull Request #89821 · rust-lang/rust · GitHub

It gets even weirder if the macro generates #[allow(unsafe_code)] unsafe { ... } since the forbid will forbid setting the allow still.

1 Like