Skip to content

add a warning about using safe on extern c-variadic functions #1839

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

Merged

Conversation

folkertdev
Copy link
Contributor

@folkertdev folkertdev commented Jun 3, 2025

edit: the lang team found an edge case.

An extern c-variadic function can be marked as safe when it does not look at its arguments at all. That is not very useful, but technically allowed.

So, I've now added a warning with this information to the section on extern c-variadic functions.

@rustbot rustbot added the S-waiting-on-review Status: The marked PR is awaiting review from a maintainer label Jun 3, 2025
@rustbot

This comment has been minimized.

@folkertdev folkertdev force-pushed the c-variadic-unsafe-at-any-speed branch 2 times, most recently from 1ba64f4 to 07d289f Compare June 3, 2025 21:36
@ehuss ehuss force-pushed the c-variadic-unsafe-at-any-speed branch from 07d289f to 5d90855 Compare June 3, 2025 21:41
@ehuss ehuss added the S-waiting-on-stabilization Waiting for a stabilization PR to be merged in the main Rust repository label Jun 3, 2025
@folkertdev folkertdev force-pushed the c-variadic-unsafe-at-any-speed branch from 5d90855 to 3bd778f Compare June 4, 2025 16:53
@folkertdev folkertdev changed the title extern c-variadic functions cannot be safe add a warning about using safe on extern c-variadic functions Jun 4, 2025
@traviscross traviscross removed the S-waiting-on-stabilization Waiting for a stabilization PR to be merged in the main Rust repository label Jun 4, 2025
@traviscross traviscross force-pushed the c-variadic-unsafe-at-any-speed branch from 3bd778f to 5454946 Compare June 4, 2025 21:47
Let's adjust this language so that it talks about the guarantees that
the function must make, and so that it says what may lead to UB rather
than what "is" UB, as the latter implies immediate language UB, when
this is actually library UB.

Let's also add a `SAFETY` comment to the function in the example.
@traviscross traviscross force-pushed the c-variadic-unsafe-at-any-speed branch from 5454946 to c13d2d2 Compare June 4, 2025 21:48
@traviscross
Copy link
Contributor

traviscross commented Jun 4, 2025

Thanks for the PR. Pushed an update to revise a bit, to add a SAFETY comment, to frame this in terms of the guarantees that the function must make, and to use wording that makes it more clear that this would be library-level UB.

@traviscross traviscross added this pull request to the merge queue Jun 4, 2025
Merged via the queue into rust-lang:master with commit c54e1f1 Jun 4, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: The marked PR is awaiting review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants