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

Unsafe macros and #![forbid(unsafe_code)] #2

Closed
vi opened this issue Aug 15, 2018 · 8 comments · Fixed by #24
Closed

Unsafe macros and #![forbid(unsafe_code)] #2

vi opened this issue Aug 15, 2018 · 8 comments · Fixed by #24

Comments

@vi
Copy link

vi commented Aug 15, 2018

There are macros like unsafe_unpinned!, which are documented as unsafe and creates a safe function, but with an unsafe block inside.

Does it mean that a crate under #![forbid(unsafe_code)] rule may circumvent the safety by linking to pin_utils? Shall those macros somehow require unsafe{} block or define an unsafe function?

Or should entire "pin_utils" be an "unsafe crate", not directly linkable from #![forbid(unsafe_code)] crates?

@MajorBreakfast
Copy link
Contributor

MajorBreakfast commented Aug 16, 2018

I just tested this and I can confirm that #![forbid(unsafe_code)] doesn't forbid using the projection macros. IMO it should forbid them.

@cramertj @Nemo157 Any ideas?

@vi The pin_mut! macro is safe to use

@vi
Copy link
Author

vi commented Aug 16, 2018

What if just make the macro insert #[allow(unsafe_code)], which may fail with error[E0453]: allow(unsafe_code) overruled by outer forbid(unsafe_code)? Will it fail in in forbid-unsafe-code mode?

On the other hand, it may contradict with user intentions who use deny(unsafe_code) instead of forbid(unsafe_code) to make almost-no-unsafe environment except of just one block, specifically marked with allow(unsafe_code) permit.

@Nemo157
Copy link
Member

Nemo157 commented Aug 17, 2018

At first glance this seems like a rustc bug, why does #[allow] apply to code generated in macros but #[forbid] doesn’t?

@vi
Copy link
Author

vi commented Aug 17, 2018

I mean unsafe{} is explicitly allowed to be bringed in by macros (it is stated in the docs), but it is not stated that macros can override forbids with allows.

@Nemo157
Copy link
Member

Nemo157 commented Aug 17, 2018

I mean unsafe{} is explicitly allowed to be bringed in by macros (it is stated in the docs)

Which docs are you referring to?


Given a crate like

#[macro_export]
macro_rules! foo {
    () => { loop { break; let _ = 5; } }
}

#[macro_export]
macro_rules! bar {
    () => { unsafe { let _ = ::std::mem::transmute::<i32, u32>(5); } }
}

and using it in another crate like

use temp::{foo, bar};

pub fn baz() {
    foo!();
    bar!();
}

#[allow(unreachable_code)]
#[forbid(unsafe_code)]
pub fn qux() {
    foo!();
    bar!();
}

the output is:

warning: unreachable statement
 --> src/lib.rs:5:5
  |
5 |     foo!();
  |     ^^^^^^^
  |
  = note: #[warn(unreachable_code)] on by default
  = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

I would expect either both the allow and forbid to apply to external macros, or neither, this output can only happen if only the allow applies, and the forbid(unsafe_code) is ignored when the unsafe code is coming from an external macro.

If this is intended behaviour, then I don't know what pin-utils can really do, it seems like the unsafe_code lint is just not strong enough to get the restrictions some people want.

@Nemo157
Copy link
Member

Nemo157 commented Aug 17, 2018

Ok, testing adding allow(unsafe_code) to the bar! macro does give an error that it's overruled by an outer forbid(unsafe_code). But that doesn't seem to fix the issue because it would instead likely cause issues with people using deny(unsafe_code).

forbid(unsafe_code) forbidding using allow(unsafe_code) while still just allowing unsafe blocks really seems like a bug to me.

@Diggsey
Copy link

Diggsey commented Jul 7, 2019

How is this different from linking to a crate that uses unsafe internally? Surely as long as the macro is sound, and it's presenting a safe interface, then nothing is being bypassed?

@vi
Copy link
Author

vi commented Jul 7, 2019

@Diggsey, Unsafe-wielding dependency crates may be reviewed and locked. pin-utils looks like a core crate created by Rust developers and may be in whilelist of allowed unsafe-using crates. It is like a part of Rust language itself.

The macro's name and documentation looks like a one of an unsafe function, yet it can be used from safe code.

For example, if #[repr(packed)] is attached to struct Foo in the documentation page's example, it still compiles, even with #![forbid(unsafe_code)] at the top.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants