-
Notifications
You must be signed in to change notification settings - Fork 587
add c-variadic function definitions #2177
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
base: master
Are you sure you want to change the base?
Changes from all commits
fe7af94
4d20c3b
3f29b09
0807e68
56c3a47
9880b05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,7 +83,7 @@ r[items.fn.params.self-restriction] | |
| Functions with a self parameter may only appear as an [associated function] in a [trait] or [implementation]. | ||
|
|
||
| r[items.fn.params.varargs] | ||
| A parameter with the `...` token indicates a [variadic function], and may only be used as the last parameter of an [external block] function. The variadic parameter may have an optional identifier, such as `args: ...`. | ||
| A parameter with the `...` token indicates a [c-variadic function], and may only be used as the last parameter. In an [`extern` block], the c-variadic parameter may have an optional identifier, such as `args: ...`, and in a [c-variadic function definition], the identifier is mandatory. | ||
|
|
||
| r[items.fn.body] | ||
| ## Function body | ||
|
|
@@ -332,6 +332,100 @@ Note that this behavior is a consequence of the desugaring to a function that re | |
|
|
||
| Unsafe is used on an async function in precisely the same way that it is used on other functions: it indicates that the function imposes some additional obligations on its caller to ensure soundness. As in any other unsafe function, these conditions may extend beyond the initial call itself -- in the snippet above, for example, the `unsafe_example` function took a pointer `x` as argument, and then (when awaited) dereferenced that pointer. This implies that `x` would have to be valid until the future is finished executing, and it is the caller's responsibility to ensure that. | ||
|
|
||
| r[items.fn.c-variadic] | ||
| ## C-variadic functions | ||
|
|
||
| r[items.fn.c-variadic.intro] | ||
| A *c-variadic* function accepts a variable argument list `pat: ...` as its final parameter. | ||
|
|
||
| ```rust | ||
| unsafe extern "C" fn example(ap: ...) -> f64 { | ||
| unsafe { ap.next_arg::<f64>() } | ||
| } | ||
| ``` | ||
|
|
||
| This parameter stands in for an arbitrary number of arguments that may be passed by the caller. | ||
|
|
||
| > [!WARNING] | ||
| > Passing an unexpected number of arguments or arguments of unexpected type to a c-variadic function may lead to [undefined behavior][undefined]. | ||
|
|
||
|
folkertdev marked this conversation as resolved.
|
||
| [items.fn.c-variadic.stable-targets] | ||
| Support for c-variadic function definitions is stable on the following architectures: | ||
|
|
||
| - x86 and x86-64 | ||
| - ARM | ||
| - AArch64 and Arm64EC | ||
| - RISC-V (except when using the ilp32e ABI) | ||
| - LoongArch | ||
| - s390x | ||
| - PowerPC and PowerPC64 | ||
| - AmdGpu and Nvptx64 | ||
| - wasm32 and wasm64 | ||
| - csky | ||
| - xtensa | ||
| - hexagon | ||
| - sparc64 | ||
| - mips | ||
|
|
||
| r[items.fn.c-variadic.c-variadic-parameter-type] | ||
| The type of `pat` in the function body is [`VaList`]. | ||
|
|
||
| r[items.fn.c-variadic.desugar-brief] | ||
| A c-variadic function definition is roughly equivalent to a function operating on a [`VaList`]. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is "roughly" about it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In how arguments are actually passed, these signatures are (very) different fn foo(ap: ...) { /* ... */ }
fn bar(ap: VaList) { /* ... */ }Those two functions could have exactly the same body though. |
||
|
|
||
| ```rust | ||
| // Source | ||
| unsafe extern "C" fn example(mut ap: ...) -> i32 { | ||
| unsafe { ap.next_arg::<i32>() } | ||
| } | ||
| ``` | ||
|
|
||
| is roughly equivalent to: | ||
|
|
||
| ```rust | ||
| # use std::ffi::VaList; | ||
| // Desugared | ||
| unsafe extern "C" fn example() -> i32 { | ||
| let mut ap: VaList<'_> = /* compiler initializes the VaList */; | ||
|
|
||
| unsafe { ap.next_arg::<i32>() } | ||
|
|
||
| va_end(ap) | ||
| } | ||
|
Comment on lines
+388
to
+394
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
let mut ap: VaList<'_> = /* ... */; // Initializes the VaList.the |
||
| ``` | ||
|
|
||
| r[items.fn.c-variadic.lifetime] | ||
| The lifetime of a `VaList` is that of the function that created it. Hence, the `VaList` value can never outlive the function that created it. | ||
|
Comment on lines
+397
to
+398
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bringing the discussion from rust-lang/rust#155697 (comment) here because I think it's getting too in the weeds for the stabilization PR. @RalfJung said:
What makes that case UB, but not the The question I want the reference to answer is: What semantic difference does it make to call the destructor of a VaList (and, by extension, what are its implications on the correctness of programs)?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. apparently there are targets (not currently supported by Rust) where We mostly decided that we aren't supporting the requirement for the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way we model varargs in Miri is that
This model is intended to operationally capture some of the UB that C ascribes to variadics. But there are some things C makes UB that we say are allowed. Specifically C says it is UB to not call va_end, and it is UB to call va_end in a different function than where the VaList got created. We decided to not have all the UB (and we'll require backends to honor that of course) because we'd have to significantly redesign the API to make it sound under such a model. For LLVM this requirement is trivially upheld because va_end at the moment is a NOP on all targets we care about -- but the rules are designed in a way that we can later support targets where it is not a NOP. We just can't support targets where it's UB to actually leak a VaList. We doubt there'll ever be such a target, where would the UB even come from? So, this is what I'd say the underlying spec for vararg UB is in Rust. The goal was to capture as much as possible of the UB in C. The cases that are UB in C but allowed in Rust satisfy all of the following conditions:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I wouldn't quite say that, but it definitely falls beyond what we expect even the craziest platforms to need: |
||
|
|
||
| r[items.fn.c-variadic.ffi-compatibility] | ||
| The rust [`VaList`] is ABI-compatible with the C `va_list` type. | ||
|
|
||
| r[items.fn.c-variadic.abi] | ||
| Only `extern "C"` and `extern "C-unwind"` function defintions can accept a variable argument list. | ||
|
|
||
| r[items.fn.c-variadic.safety] | ||
| Only `unsafe` functions can accept a variable argument list. | ||
|
|
||
| r[items.fn.c-variadic.async] | ||
| A c-variadic functions cannot be `async` | ||
|
|
||
| r[items.fn.c-variadic.const] | ||
| A c-variadic functions cannot be `const` | ||
|
|
||
| r[items.fn.c-variadic.platform-support] | ||
| Some ABIs do not support c-variadic function definitions. The compiler errors in this case. | ||
|
|
||
| ```text | ||
| error: the `bpfel` target does not support c-variadic functions | ||
| --> $DIR/not-supported.rs:23:31 | ||
| | | ||
| LL | unsafe extern "C" fn variadic(_: ...) {} | ||
| | ^^^^^^ | ||
| ``` | ||
|
|
||
| r[items.fn.c-variadic.dyn-compat] | ||
| When a trait method is c-variadic, the trait is no longer [dyn-compatible]. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we move this to the dyn-compatibility section? I assume it would just be added to the list in "Dispatchable functions must…". I realize there's a bit of a challenge whether to document or link things in multiple places when they cover multiple concepts (dyn-compatibility and c-variadic in this case). In this particular case, I think it's fine to consolidate the rules in the dyn-compatibility list. For example, when describing Self or async or receivers, they don't mention their impact on dyn-compatibility. |
||
|
|
||
| r[items.fn.attributes] | ||
| ## Attributes on functions | ||
|
|
||
|
|
@@ -424,6 +518,8 @@ fn foo_oof(#[some_inert_attribute] arg: u8) { | |
| [associated function]: associated-items.md#associated-functions-and-methods | ||
| [implementation]: implementations.md | ||
| [value namespace]: ../names/namespaces.md | ||
| [variadic function]: external-blocks.md#variadic-functions | ||
| [c-variadic function]: external-blocks.md#variadic-functions | ||
| [`extern` block]: external-blocks.md | ||
| [zero-sized]: glossary.zst | ||
| [`VaList`]: std::ffi::VaList | ||
| [dyn-compatible]: traits.md#dyn-compatibility | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for c-variadic definitions only
pat: ...is accepted semantically.plain
...is currently parsed, thevarargs_without_patternlint is meant to eventually disallow it. This syntax can only be used as an input to macros, when a bare...makes it past macro expansion, that will emit an error.I didn't mention this here, but maybe we should: we follow C23 in that
pat: ...may be the only argument. Earlier versions of C required at least one standard argument before the....There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is anything holding back changing
varargs_without_patterntoreport_in_deps: true?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not as far as I know. I can make that change an nominate for T-lang.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on rust-lang/rust#143619 (comment) there was only one actual impacted crate https://crates.io/crates/binrw. The issue was fixed in jam1garner/binrw#342, however there has not been a release since.
So, if we made this warn in dependencies there would be nothing that users of that crate could do. I'll ping for a release with the fix.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the releases happened: jam1garner/binrw#342 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, PR is up at rust-lang/rust#154599