Evaluating macro expression in unsafe block

[chertl]

Requesting a Clippy lint for evaluating macro expressions in an unsafe block, such as the following:

macro_rules! is_1 {
    ($a:expr) => (
        unsafe {
            $a == 1
        }
    );
}

This is bad code hygiene because although the above may not look dangerous on its own, it can be used to hide unsafe code in another function. For example, the following program produces an out of bounds read, despite not appearing to contain any unsafe blocks:

fn main() {
    let a: [u8; 1] = [0; 1];
    
    let x = is_1!(*a.get_unchecked(100));
    
    println!("{}", x);
}

[oli-obk]

Oh wow. That's not even a footgun that's a whole footnuke.

Unfortunately I think we can only detect the problem in expansions of the macro, not in the macro definition itself.

Impl instructions for a lint detecting the issue in an expansion:

1. implement the check_block method
2. check for unsafety and in_macro (abort if not in a macro or not an unsafe block)
3. build a visitor that visits the block body
4. lint any expression that is not in_macro

A better fix would probably be making the compiler to undo the unsafety when leaving the current macro. So the $a would not know about the unsafe around it.

[chertl]

Thanks for your reply!

After some more thought, I agree that it would be preferable for the compiler itself to prevent this behavior from occurring, so I've created a post on the Rust internals forum to encourage discussion there.

If a Clippy lint is produced for this issue, one false positive to watch out for would be if the expression is used as a compile time constant, for example the following evaluation of size expression in exported macro uninitialized_array in libcore cannot be abused by a Rust program to hide any unsafe functionality:

libcore/macros.rs:

macro_rules! uninitialized_array {
    // This `into_initialized` is safe because an array of `MaybeUninit` does not
    // require initialization.
    // FIXME(#49147): Could be replaced by an array initializer, once those can
    // be any const expression.
    ($t:ty; $size:expr) => (unsafe {
        MaybeUninit::<[MaybeUninit<$t>; $size]>::uninitialized().into_initialized()
    });
}