RFC: unsafe global_asm

[inkreasing]

Require unsafe to use the global_asm macro, because it can overwrite symbols, which can lead to UB.

unsafe global_asm!("");

Important

Since RFCs involve many conversations at once that can be difficult to follow, please use review comment threads on the text changes instead of direct comments on the RFC.

If you don't have a particular section of the RFC to comment on, you can click on the "Comment on this file" button on the top-right corner of the diff, to the right of the "Viewed" checkbox. This will create a separate thread even if others have commented on the file too.

Rendered

[ds84182]

This could be an attribute #[unsafe] applied to the macro invocation, similar to #[unsafe(no_mangle)] on fns. It wouldn't require changes to the grammar as attributes can already precede macro items. https://doc.rust-lang.org/reference/grammar.html#railroad-summary-Item

EDIT: Missed the bullet point in the alternatives section. Could you expand on why this is more preferable than the attribute?

View changes since the review

[ChayimFriedman2]

This was mentioned in the Zulip thread which discussed a lot of threads. I think it will be good to link that thread in the RFC.

[inkreasing]

I don't feel like an #[unsafe] attribute would be similar to #[unsafe(no_mangle)]. Wouldn't it also have been possible to require something like:

#[unsafe]
#[no_mangle]
fn my_write() {}

which i feel like is actually the equivalent to requiring an unsafe attribute here.

In general i don't have that strong opinions on the syntax, so i found it hard to really argue against the others in the RFC, but i have some arguments that may be helpful and i can try to include the most convincing in the rfc.

• I think "native" syntax is in general preferable to attributes.
• An unsafe attribute would "imply much generality". why can't it be used instead of the unsafe keyword when implementing an unsafe trait, why can't it be used above a block to make it an unsafe block.
• If unsafe macros are added the attribute solution is probably not enough anymore and then another change needs to be done.

I think it will be good to link that thread in the RFC.

Do you think so? i think in the syntax alternatives i mentioned every proposed alternative and i would favor summarizing arguments in favor or against syntax alternatives rather than linking to them.

[ChayimFriedman2]

Please read the Zulip thread (and link it). This was already discussed there.

[inkreasing]

I have read (and participated in) the zulip thread. I added an argument against #[unsafe]. I am not convinced that linking the zulip thread in the RFC is superior to summarizing arguments. If i have missed one let me know and i can add it.

[ChayimFriedman2]

Linking previous discussions (in "Prior Art") is important in RFCs even if you summarize them perfectly.

I have not compared the thread and the RFC closely, but you do for example not mention the argument @ds84182 mentioned here, nor the response (#[unsafe(...)] was for things that are already attributes).

[tgross35]

It's not really unsoundness, just less greppable soundness assertions. Similar to #[no_mangle] before unsafe, unsafe code lints already fire if you're looking to prevent more author-written unsoundness:

#![forbid(unsafe_code)]
std::arch::global_asm!("");

error: usage of `core::arch::global_asm`
 --> src/lib.rs:2:1
  |
2 | std::arch::global_asm!("");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: using this macro is unsafe even though it does not need an `unsafe` block
note: the lint level is defined here
 --> src/lib.rs:1:11
  |
1 | #![forbid(unsafe_code)]
  |           ^^^^^^^^^^^

View changes since the review

[inkreasing]

In #3325 as far as i understand no_mangle and similar where also already rejected by the unsafe_code lint. It was still called an unsoundness.

There it was argued that this is not enough, i agree with this, but i can reference or copy the relevant section. edit: done

[tgross35]

Drawback: it doesn't really change anything other than making soundness risks more obvious, debatably. Making everyone add a keyword isn't likely to make more or fewer people use global_asm, or change what's written in the macro.

View changes since the review

[fmease]

This section is a bit light on reference-level details IMO. Could you add an updated grammar snippet that's possibly based on MacroInvocationSemi)? or possibly on MacroItem instead depending on which syntax you actually propose?

View changes since the review

[inkreasing]

MacroItem ->
      `unsafe`? MacroInvocationSemi
    | MacroRulesDefinition

I have never written reference grammar, does this seem good to you? The `unsafe`? could of course also be inlined into MacroInvocationSemi, but i don't think it makes a difference?

I attempted to implement this two weeks ago (rust-lang/rust@main...inkreasing:rust:push-lowtqqruokpq) and i think the implementation follows the grammar i proposed.

[fmease]

Could you define what "invoked by using the unsafe keyword" means? You should assume that the reader skipped the guide-level explanation and went straight to the reference-level one.

View changes since the review