|
| 1 | +- Feature Name: `unsafe_attributes` |
| 2 | +- Start Date: 2022-10-11 |
| 3 | +- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) |
| 4 | +- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) |
| 5 | + |
| 6 | +# Summary |
| 7 | +[summary]: #summary |
| 8 | + |
| 9 | +Consider some attributes 'unsafe', so that they must only be used like this: |
| 10 | + |
| 11 | +```rust |
| 12 | +#[unsafe(no_mangle)] |
| 13 | +``` |
| 14 | + |
| 15 | +# Motivation |
| 16 | +[motivation]: #motivation |
| 17 | + |
| 18 | +Some of our attributes, such as `no_mangle`, can be used to |
| 19 | +[cause Undefined Behavior without any `unsafe` block](https://github.com/rust-lang/rust/issues/28179). |
| 20 | +If this was regular code we would require them to be placed in an `unsafe {}` |
| 21 | +block, but since they are attributes that makes less sense. Hence we need a |
| 22 | +concept of 'unsafe attributes' and accompanying syntax to declare that one is |
| 23 | +aware of the UB risks here (and it might be good to add a SAFETY comment |
| 24 | +explaining why this use of the attribute is fine). |
| 25 | + |
| 26 | +# Guide-level explanation |
| 27 | +[guide-level-explanation]: #guide-level-explanation |
| 28 | + |
| 29 | +*Example explanation for `no_mangle`; the other attributes need something similar.* |
| 30 | + |
| 31 | +When declaring a function like this |
| 32 | + |
| 33 | +```rust |
| 34 | +#[no_mangle] |
| 35 | +pub fn write(...) { ... } |
| 36 | +``` |
| 37 | + |
| 38 | +this will cause Rust to generate a globally visible function with the |
| 39 | +linker/export name `write`. As consequence of that, other code that wants to |
| 40 | +call the |
| 41 | +[C `write` function](https://man7.org/linux/man-pages/man2/write.2.html) might |
| 42 | +end up calling this other `write` instead. This can easily lead to Undefined |
| 43 | +Behavior: |
| 44 | +- The other `write` might have the wrong signature, so arguments are passed |
| 45 | + incorrectly. |
| 46 | +- The other `write` might not have the expected behavior of |
| 47 | + [write](https://man7.org/linux/man-pages/man2/write.2.html), causing code |
| 48 | + relying on this behavior to misbehave. |
| 49 | + |
| 50 | +To avoid this, when declaring a function `no_mangle`, it is important that the |
| 51 | +name of the function does not clash with other globally named functions. Similar |
| 52 | +to how `unsafe { ... }` blocks are used to acknowledge that this code is |
| 53 | +dangerous and needs manual checking, `unsafe(no_mangle)` acknowledges that |
| 54 | +`no_mangle` is dangerous and needs to be manually checked for correctness: |
| 55 | + |
| 56 | +```rust |
| 57 | +// SAFETY: there is no other global function of this name |
| 58 | +#[unsafe(no_mangle)] |
| 59 | +pub fn my_own_write(...) { ... } |
| 60 | +``` |
| 61 | + |
| 62 | +# Reference-level explanation |
| 63 | +[reference-level-explanation]: #reference-level-explanation |
| 64 | + |
| 65 | +Some attributes (e.g. `no_mangle`, `export_name`, `link_section` -- see |
| 66 | +[here](https://github.com/rust-lang/rust/issues/82499) for a more complete list) |
| 67 | +are considered "unsafe" attributes. An unsafe attribute must only be used inside |
| 68 | +`unsafe(...)` in the attribute declaration, like |
| 69 | + |
| 70 | +```rust |
| 71 | +#[unsafe(no_mangle)] |
| 72 | +``` |
| 73 | + |
| 74 | +For backwards compatibility reasons, using these attributes outside of |
| 75 | +`unsafe(...)` is just a lint, not a hard error. Initially, this lint will be |
| 76 | +allow-by-default. Unsafe attributes that are added in the future can |
| 77 | +hard-require `unsafe` from the start since the backwards compatibility concern |
| 78 | +does not apply to them. |
| 79 | + |
| 80 | +Syntactically, for each unsafe attribute `attr`, we now also accept |
| 81 | +`unsafe(attr)` anywhere that `attr` can be used. `unsafe` cannot be nested, |
| 82 | +cannot contain `cfg_attr`, and cannot contain any other (non-unsafe) attributes. |
| 83 | + |
| 84 | +The `deny(unsafe_code)` lint denies the use of unsafe attributes both inside and |
| 85 | +outside of `unsafe(...)` blocks. (That lint currently has special handling to |
| 86 | +deny these attributes. Once there is a general notion of 'unsafe attributes' as |
| 87 | +proposed by this RFC, that special handling should no longer be needed.) |
| 88 | + |
| 89 | +# Drawbacks |
| 90 | +[drawbacks]: #drawbacks |
| 91 | + |
| 92 | +I think if we had thought of this around Rust 1.0, then this would be rather |
| 93 | +uncontroversial. As things stand now, this proposal will cause a lot of churn |
| 94 | +since all existing uses of these unsafe attributes need to be adjusted. The |
| 95 | +warning for using unsafe attributes outside `unsafe(...)` should probably have |
| 96 | +an auto-fix available to help ease the transition here. |
| 97 | + |
| 98 | +# Rationale and alternatives |
| 99 | +[rationale-and-alternatives]: #rationale-and-alternatives |
| 100 | + |
| 101 | +- **Nothing.** We could do nothing at all, and live with the status quo. However |
| 102 | + then we will not be able to fix issues like |
| 103 | + [`no_mangle` being unsound](https://github.com/rust-lang/rust/issues/28179), |
| 104 | + which is one of the oldest open soundness issues. |
| 105 | +- **Rename.** We could just rename the attributes to `unsafe_no_mangle` etc. |
| 106 | + However that is inconsistent with how we approach `unsafe` on expressions, and |
| 107 | + feels much less systematic and much more ad-hoc. |
| 108 | +- **`deny(unsafe_code)`.** We already |
| 109 | + [started the process](https://github.com/rust-lang/rust/issues/82499) of |
| 110 | + rejecting these attributes when `deny(unsafe_code)` is used. We could say that |
| 111 | + is enough. However the RFC authors thinks that is insufficient, since only few |
| 112 | + crates use that lint, and since it is the wrong default for Rust (users have |
| 113 | + to opt-in to a soundness-critical diagnostic -- that's totally against the |
| 114 | + "safety by default" goal of Rust). This RFC says that yes, `deny(unsafe_code)` |
| 115 | + should deny those attributes, but we should go further and require an explicit |
| 116 | + `unsafe(...)` attribute block for them to be used at all. |
| 117 | +- **Item-level unsafe blocks.** We could find some way to have 'unsafe blocks' |
| 118 | + around entire functions or modules. However, those would go against the usual |
| 119 | + goal of keeping `unsafe` blocks small. Big `unsafe` blocks risk accidentally |
| 120 | + calling an unsafe operation in there without even realizing it. |
| 121 | +- **Other syntax.** Obviously we could pick a different syntax for the same |
| 122 | + concept, but this seems like the most natural marriage of the idea of unsafe |
| 123 | + blocks from regular code, and the existing attributes syntax. |
| 124 | + |
| 125 | +# Prior art |
| 126 | +[prior-art]: #prior-art |
| 127 | + |
| 128 | +We have `unsafe` blocks; this is basically the same thing for the "attributes |
| 129 | +DSL". |
| 130 | + |
| 131 | +In the attribute DSL, we already have a "nesting" construct: `cfg_attr`. That |
| 132 | +allows terms like |
| 133 | +`#[cfg_attr(debug_assertions, deny(unsafe_code), allow(unused))]`, so there is |
| 134 | +precedent for having a list of attributes inside a single attribute. |
| 135 | + |
| 136 | +I don't know of other languages that would distinguish safe and unsafe |
| 137 | +attributes. |
| 138 | + |
| 139 | +# Unresolved questions |
| 140 | +[unresolved-questions]: #unresolved-questions |
| 141 | + |
| 142 | +- **Different lint staging.** The lint on using existing unsafe attributes like |
| 143 | + `no_mangle` outside `unsafe(...)` could be staged in various ways: it could be |
| 144 | + warn-by-default to start or we wait a while before to do that, it could be |
| 145 | + edition-dependent, it might eventually be deny-by-default or even a hard error |
| 146 | + on some editions -- there are lots of details here, which can be determined |
| 147 | + later during the process. |
| 148 | + |
| 149 | +# Future possibilities |
| 150 | +[future-possibilities]: #future-possibilities |
| 151 | + |
| 152 | +- **Unsafe attribute proc macros.** We could imagine something like |
| 153 | + ``` |
| 154 | + #[proc_macro_attribute(require_unsafe)] |
| 155 | + fn spoopy(args: TokenStream, input: TokenStream) -> TokenStream {…} |
| 156 | + ``` |
| 157 | + to declare that an attribute proc macro is unsafe to use, and must only |
| 158 | + occur as an unsafe macro. Such an unsafe-to-use attribute proc macro must |
| 159 | + declare in a comment what its safety requirements are. (This is the `unsafe` |
| 160 | + from `unsafe fn`, whereas the rest of the RFC is using the `unsafe` from |
| 161 | + `unsafe { ... }`.) |
| 162 | +- **Unsafe derive.** We could use `#[unsafe(derive(Trait))]` to derive an |
| 163 | + `unsafe impl` where the deriving macro itself cannot check all required safety |
| 164 | + conditions. |
| 165 | +- **Unsafe tool attributes.** Same as above, but for tool attributes. |
| 166 | +- **Unsafe attributes on statements.** For now, the only unsafe attributes we |
| 167 | + have don't make sense on the statement level. Once we do have unsafe statement |
| 168 | + attributes, we need to figure out whether inside `unsafe {}` blocks one still |
| 169 | + needs to also write `unsafe(...)`. |
| 170 | +- **Lists and nesting.** We could specify that `unsafe(...)` may contain a list |
| 171 | + of arbitrary attributes (including safe ones), may be nested, and may contain |
| 172 | + `cfg_attr` that gets expanded appropriately. However that could make it tricky |
| 173 | + to consistently support non-builtin unsafe attributes in the future, so the |
| 174 | + RFC proposes to not do that yet. The current approach is forward-compatible |
| 175 | + with allowing lists and nesting in the future. |
0 commit comments