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 Extern Blocks #3439

Closed
wants to merge 1 commit into from
Closed

Unsafe Extern Blocks #3439

wants to merge 1 commit into from

Conversation

Lokathor
Copy link
Contributor

@Lokathor Lokathor commented May 29, 2023

@programmerjake
Copy link
Member

why limit unsafe extern to edition >=2024? it seems reasonable to allow unsafe extern blocks on all editions with the proposed syntax/semantics and only allow unsafe-less extern blocks in edition <=2021 (perhaps with a warning).


Within the block you can declare the exernal functions and statics that you want to make visible within the current scope.
Each function declaration gives only the function's signature, similar to how methods for traits are declared.
If calling a foreign function is `unsafe` then you must declare the function as `unsafe fn`, otherwise you can declare it as a normal `fn`.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it maybe make sense to always mark the declaration as unsafe or safe (i.e. you can never omit it) similar to how raw pointers always need const or mut? I feel like making safe the default here could cause people to accidentally not mark a function as unsafe when they should. In particular the transition from the current extern blocks to the new ones in a new edition would be error prone if people simply mark the extern blocks as unsafe and don't think about the fact that they also need to mark all / most of the functions as unsafe now too, simply because it suddenly starts compiling again and they move on.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cargo fix can automatically add "unsafe" to each fn listed in an extern block, but yes that's probably too error prone.

My initial thought was to default to safe so that the declarations would look as close to "normal" declarations elsewhere, but having a contextual "safe" keyword seems reasonable as well.

@ehuss ehuss added the T-lang Relevant to the language team, which will review and decide on the RFC. label May 29, 2023
@Lokathor
Copy link
Contributor Author

@programmerjake The best way to handle things in existing editions isn't super clear. However, with new editions people "expect" that a change might happen in some area. I'm not against a change to pre-2024 as well, if someone had a clear idea for that.

@programmerjake
Copy link
Member

programmerjake commented May 29, 2023

@programmerjake The best way to handle things in existing editions isn't super clear. However, with new editions people "expect" that a change might happen in some area. I'm not against a change to pre-2024 as well, if someone had a clear idea for that.

Well, I do have a clear idea: unsafe extern blocks work identically in all editions since that part is a pure syntax extension; extern blocks without unsafe work in editions <= 2021.

this is just like how let .. else { was added to all editions, or asm! was added to all editions, or dyn Trait.

@Lokathor
Copy link
Contributor Author

The problem I have with that is that just letting extern blocks without unsafe work indefinitely lets people have UB creep in without the program ever having said "unsafe" anywhere. I think whatever the fix for edition <= 2021 is, it needs to be something that moves old editions away from ever having UB without the greppable string unsafe. It's the same basic logic as the unsafe attributes RFC.

Until then, I would say that editions <= 2021 haven't been appropriately addressed.

@programmerjake
Copy link
Member

ok, then unsafe-less extern blocks can be deprecated and warn, like uninitialized() does.

in any case, my main point is that I think unsafe extern blocks should be available on all editions with identical functionality.

@Lokathor
Copy link
Contributor Author

I'd be fine with that. Actually it's probably a good idea even. I guess it comes down to how T-lang feels about actually deprecating a lot of old code.

Replace the *Functions* and *Statics* sections with the following:

### Functions
Functions within external blocks are declared in the same way as other Rust functions, with the exception that they must not have a body and are instead terminated by a semicolon. Patterns are not allowed in parameters, only IDENTIFIER or _ may be used. The function qualifiers `const`, `async`, and `extern` are not allowed. If the function is unsafe to call, then the function must use the `unsafe` qualifier.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the rationale for making extern blocks unsafe by default no longer apply? rust-lang/rust#2628 (implemented by rust-lang/rust#4599)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That issue is so old I'm not even entirely clear what it's referring to, but the declaration alone can be dangerous so that's what needs to be the unsafe part.

where `'l1`, ... `'lm` are its lifetime parameters, `A1`, ..., `An` are the declared types of its parameters and `R` is the declared return type.

### Statics
Statics within external blocks are declared in the same way as statics outside of external blocks, except that they do not have an expression initializing their value. It is unsafe to declare a static item in an extern block, whether or not it's mutable, because there is nothing guaranteeing that the bit pattern at the static's memory is valid for the type it is declared with.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, we're removing the ability for users to declare (immutable) statics that are unsafe to use. Would there ever be a use-cade for keeping those? E.g. having the ability to write pub unsafe static FOO: u8;, where the user discharges the safety obligations instead of the library author?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one use case where we might still want unsafe statics is when doing stuff that's relatively common in embedded C -- having special global variables that are just a way to get an address from the linker, they aren't real variables (you can't necessarily read/write them):

extern int bss_start;
// linker assigns the address to be right after the end of the memory allocated
// for the bss segment, you can't actually read/write this variable
extern int bss_end;
void _start() { // called by cpu reset, we need to initialize ram before we run main()
    // clear bss
    for(int *p = &bss_start; p != bss_end; p++)
        *p = 0;
    // initialize data
    for(int *p = &data_start, *src = &data_in_flash_start; p != data_end; p++, src++)
        *p = read_flash(src);
    main();
    while(true) {}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those can be emulated via static BSS_START: (); (or a named opaque type, I would have said static BSS_START: !;, but IIRC from discussion on rust-lang/rust#74840 that's not valid).

Is there any situation where reading the value is valid, but it's sometimes UB, which isn't covered by declaring it as static mut?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly related: #2937

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is resolved in the new text, unsafe external statics are kept.

@programmerjake
Copy link
Member

This RFC has been re-opened over at #3484. It was accidentally closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants