Skip to content

Commit c27b7c2

Browse files
jhprattjhpratt
authored
Rollup merge of #141361 - folkertdev:varargs-cfg, r=workingjubilee
use `cfg_select!` to select the right `VaListImpl` definition tracking issue: #44930 Just a bit of cleanup really. We could use `PhantomInvariantLifetime<'f>` (#135806) to make it more precise what that `PhantomData<&'f mut &'f c_void>` marker is doing. I'm not sure how ready that feature is though, `@jhpratt` are these types good to use internally? --- Some research into the lifetimes of `VaList` and `VaListImpl`: It's easy to see why the lifetime of these types should not be extended, a `VaList` or `VaListImpl` escaping its function is a bad idea. I don't currently see why coercing the lifetime to a shorter lifetime is problematic though, but probably I just don't understand variance well enough to see it. The history does not provide much explanation: - immunant@0814087 original implementation - immunant@b9ea653 adds `VaListImpl<'f>`, but it is only covariant in `'f` - #62639 makes `VaListImpl<'f>` invariant over `'f` (because `VaList<'a, 'f>` is already invariant over `'f`, but I think that is just an implementation detail?) Beyond that I don't see how the lifetime situation can be simplified significantly, e.g. this function really needs `'copy` to be unconstrained. ```rust /// Copies the `va_list` at the current location. pub unsafe fn with_copy<F, R>(&self, f: F) -> R where F: for<'copy> FnOnce(VaList<'copy, 'f>) -> R, { let mut ap = self.clone(); let ret = f(ap.as_va_list()); // SAFETY: the caller must uphold the safety contract for `va_end`. unsafe { va_end(&mut ap); } ret } ``` `@rustbot` label +F-c_variadic r? `@workingjubilee`
2 parents 3338ff7 + 89a8abc commit c27b7c2

File tree

1 file changed

+136
-175
lines changed

1 file changed

+136
-175
lines changed

‎library/core/src/ffi/va_list.rs‎

Lines changed: 136 additions & 175 deletions
Original file line numberDiff line numberDiff line change
@@ -5,148 +5,120 @@
55
use crate::ffi::c_void;
66
#[allow(unused_imports)]
77
use crate::fmt;
8-
use crate::marker::PhantomData;
8+
use crate::marker::{PhantomData, PhantomInvariantLifetime};
99
use crate::ops::{Deref, DerefMut};
1010

11-
/// Basic implementation of a `va_list`.
1211
// The name is WIP, using `VaListImpl` for now.
13-
#[cfg(any(
12+
//
13+
// Most targets explicitly specify the layout of `va_list`, this layout is matched here.
14+
crate::cfg_select! {
1415
all(
15-
not(target_arch = "aarch64"),
16-
not(target_arch = "powerpc"),
17-
not(target_arch = "s390x"),
18-
not(target_arch = "xtensa"),
19-
not(target_arch = "x86_64")
20-
),
21-
all(target_arch = "aarch64", target_vendor = "apple"),
22-
target_family = "wasm",
23-
target_os = "uefi",
24-
windows,
25-
))]
26-
#[repr(transparent)]
27-
#[lang = "va_list"]
28-
pub struct VaListImpl<'f> {
29-
ptr: *mut c_void,
30-
31-
// Invariant over `'f`, so each `VaListImpl<'f>` object is tied to
32-
// the region of the function it's defined in
33-
_marker: PhantomData<&'f mut &'f c_void>,
34-
}
35-
36-
#[cfg(any(
37-
all(
38-
not(target_arch = "aarch64"),
39-
not(target_arch = "powerpc"),
40-
not(target_arch = "s390x"),
41-
not(target_arch = "xtensa"),
42-
not(target_arch = "x86_64")
43-
),
44-
all(target_arch = "aarch64", target_vendor = "apple"),
45-
target_family = "wasm",
46-
target_os = "uefi",
47-
windows,
48-
))]
49-
impl<'f> fmt::Debug for VaListImpl<'f> {
50-
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
51-
write!(f, "va_list* {:p}", self.ptr)
16+
target_arch = "aarch64",
17+
not(target_vendor = "apple"),
18+
not(target_os = "uefi"),
19+
not(windows),
20+
) => {
21+
/// AArch64 ABI implementation of a `va_list`. See the
22+
/// [AArch64 Procedure Call Standard] for more details.
23+
///
24+
/// [AArch64 Procedure Call Standard]:
25+
/// http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
26+
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
27+
#[derive(Debug)]
28+
#[lang = "va_list"]
29+
pub struct VaListImpl<'f> {
30+
stack: *mut c_void,
31+
gr_top: *mut c_void,
32+
vr_top: *mut c_void,
33+
gr_offs: i32,
34+
vr_offs: i32,
35+
_marker: PhantomInvariantLifetime<'f>,
36+
}
37+
}
38+
all(target_arch = "powerpc", not(target_os = "uefi"), not(windows)) => {
39+
/// PowerPC ABI implementation of a `va_list`.
40+
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
41+
#[derive(Debug)]
42+
#[lang = "va_list"]
43+
pub struct VaListImpl<'f> {
44+
gpr: u8,
45+
fpr: u8,
46+
reserved: u16,
47+
overflow_arg_area: *mut c_void,
48+
reg_save_area: *mut c_void,
49+
_marker: PhantomInvariantLifetime<'f>,
50+
}
51+
}
52+
target_arch = "s390x" => {
53+
/// s390x ABI implementation of a `va_list`.
54+
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
55+
#[derive(Debug)]
56+
#[lang = "va_list"]
57+
pub struct VaListImpl<'f> {
58+
gpr: i64,
59+
fpr: i64,
60+
overflow_arg_area: *mut c_void,
61+
reg_save_area: *mut c_void,
62+
_marker: PhantomInvariantLifetime<'f>,
63+
}
64+
}
65+
all(target_arch = "x86_64", not(target_os = "uefi"), not(windows)) => {
66+
/// x86_64 ABI implementation of a `va_list`.
67+
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
68+
#[derive(Debug)]
69+
#[lang = "va_list"]
70+
pub struct VaListImpl<'f> {
71+
gp_offset: i32,
72+
fp_offset: i32,
73+
overflow_arg_area: *mut c_void,
74+
reg_save_area: *mut c_void,
75+
_marker: PhantomInvariantLifetime<'f>,
76+
}
77+
}
78+
target_arch = "xtensa" => {
79+
/// Xtensa ABI implementation of a `va_list`.
80+
#[repr(C)]
81+
#[derive(Debug)]
82+
#[lang = "va_list"]
83+
pub struct VaListImpl<'f> {
84+
stk: *mut i32,
85+
reg: *mut i32,
86+
ndx: i32,
87+
_marker: PhantomInvariantLifetime<'f>,
88+
}
5289
}
53-
}
54-
55-
/// AArch64 ABI implementation of a `va_list`. See the
56-
/// [AArch64 Procedure Call Standard] for more details.
57-
///
58-
/// [AArch64 Procedure Call Standard]:
59-
/// http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
60-
#[cfg(all(
61-
target_arch = "aarch64",
62-
not(target_vendor = "apple"),
63-
not(target_os = "uefi"),
64-
not(windows),
65-
))]
66-
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
67-
#[derive(Debug)]
68-
#[lang = "va_list"]
69-
pub struct VaListImpl<'f> {
70-
stack: *mut c_void,
71-
gr_top: *mut c_void,
72-
vr_top: *mut c_void,
73-
gr_offs: i32,
74-
vr_offs: i32,
75-
_marker: PhantomData<&'f mut &'f c_void>,
76-
}
77-
78-
/// PowerPC ABI implementation of a `va_list`.
79-
#[cfg(all(target_arch = "powerpc", not(target_os = "uefi"), not(windows)))]
80-
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
81-
#[derive(Debug)]
82-
#[lang = "va_list"]
83-
pub struct VaListImpl<'f> {
84-
gpr: u8,
85-
fpr: u8,
86-
reserved: u16,
87-
overflow_arg_area: *mut c_void,
88-
reg_save_area: *mut c_void,
89-
_marker: PhantomData<&'f mut &'f c_void>,
90-
}
91-
92-
/// s390x ABI implementation of a `va_list`.
93-
#[cfg(target_arch = "s390x")]
94-
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
95-
#[derive(Debug)]
96-
#[lang = "va_list"]
97-
pub struct VaListImpl<'f> {
98-
gpr: i64,
99-
fpr: i64,
100-
overflow_arg_area: *mut c_void,
101-
reg_save_area: *mut c_void,
102-
_marker: PhantomData<&'f mut &'f c_void>,
103-
}
10490

105-
/// x86_64 ABI implementation of a `va_list`.
106-
#[cfg(all(target_arch = "x86_64", not(target_os = "uefi"), not(windows)))]
107-
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
108-
#[derive(Debug)]
109-
#[lang = "va_list"]
110-
pub struct VaListImpl<'f> {
111-
gp_offset: i32,
112-
fp_offset: i32,
113-
overflow_arg_area: *mut c_void,
114-
reg_save_area: *mut c_void,
115-
_marker: PhantomData<&'f mut &'f c_void>,
116-
}
91+
// The fallback implementation, used for:
92+
//
93+
// - apple aarch64 (see https://github.com/rust-lang/rust/pull/56599)
94+
// - windows
95+
// - uefi
96+
// - any other target for which we don't specify the `VaListImpl` above
97+
//
98+
// In this implementation the `va_list` type is just an alias for an opaque pointer.
99+
// That pointer is probably just the next variadic argument on the caller's stack.
100+
_ => {
101+
/// Basic implementation of a `va_list`.
102+
#[repr(transparent)]
103+
#[lang = "va_list"]
104+
pub struct VaListImpl<'f> {
105+
ptr: *mut c_void,
106+
107+
// Invariant over `'f`, so each `VaListImpl<'f>` object is tied to
108+
// the region of the function it's defined in
109+
_marker: PhantomInvariantLifetime<'f>,
110+
}
117111

118-
/// Xtensa ABI implementation of a `va_list`.
119-
#[cfg(target_arch = "xtensa")]
120-
#[repr(C)]
121-
#[derive(Debug)]
122-
#[lang = "va_list"]
123-
pub struct VaListImpl<'f> {
124-
stk: *mut i32,
125-
reg: *mut i32,
126-
ndx: i32,
127-
_marker: PhantomData<&'f mut &'f c_void>,
112+
impl<'f> fmt::Debug for VaListImpl<'f> {
113+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
114+
write!(f, "va_list* {:p}", self.ptr)
115+
}
116+
}
117+
}
128118
}
129119

130-
/// A wrapper for a `va_list`
131-
#[repr(transparent)]
132-
#[derive(Debug)]
133-
pub struct VaList<'a, 'f: 'a> {
134-
#[cfg(any(
135-
all(
136-
not(target_arch = "aarch64"),
137-
not(target_arch = "powerpc"),
138-
not(target_arch = "s390x"),
139-
not(target_arch = "x86_64")
140-
),
141-
target_arch = "xtensa",
142-
all(target_arch = "aarch64", target_vendor = "apple"),
143-
target_family = "wasm",
144-
target_os = "uefi",
145-
windows,
146-
))]
147-
inner: VaListImpl<'f>,
148-
149-
#[cfg(all(
120+
crate::cfg_select! {
121+
all(
150122
any(
151123
target_arch = "aarch64",
152124
target_arch = "powerpc",
@@ -158,52 +130,41 @@ pub struct VaList<'a, 'f: 'a> {
158130
not(target_family = "wasm"),
159131
not(target_os = "uefi"),
160132
not(windows),
161-
))]
162-
inner: &'a mut VaListImpl<'f>,
133+
) => {
134+
/// A wrapper for a `va_list`
135+
#[repr(transparent)]
136+
#[derive(Debug)]
137+
pub struct VaList<'a, 'f: 'a> {
138+
inner: &'a mut VaListImpl<'f>,
139+
_marker: PhantomData<&'a mut VaListImpl<'f>>,
140+
}
163141

164-
_marker: PhantomData<&'a mut VaListImpl<'f>>,
165-
}
166142

167-
#[cfg(any(
168-
all(
169-
not(target_arch = "aarch64"),
170-
not(target_arch = "powerpc"),
171-
not(target_arch = "s390x"),
172-
not(target_arch = "x86_64")
173-
),
174-
target_arch = "xtensa",
175-
all(target_arch = "aarch64", target_vendor = "apple"),
176-
target_family = "wasm",
177-
target_os = "uefi",
178-
windows,
179-
))]
180-
impl<'f> VaListImpl<'f> {
181-
/// Converts a `VaListImpl` into a `VaList` that is binary-compatible with C's `va_list`.
182-
#[inline]
183-
pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
184-
VaList { inner: VaListImpl { ..*self }, _marker: PhantomData }
143+
impl<'f> VaListImpl<'f> {
144+
/// Converts a [`VaListImpl`] into a [`VaList`] that is binary-compatible with C's `va_list`.
145+
#[inline]
146+
pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
147+
VaList { inner: self, _marker: PhantomData }
148+
}
149+
}
185150
}
186-
}
187151

188-
#[cfg(all(
189-
any(
190-
target_arch = "aarch64",
191-
target_arch = "powerpc",
192-
target_arch = "s390x",
193-
target_arch = "xtensa",
194-
target_arch = "x86_64"
195-
),
196-
not(target_arch = "xtensa"),
197-
any(not(target_arch = "aarch64"), not(target_vendor = "apple")),
198-
not(target_family = "wasm"),
199-
not(target_os = "uefi"),
200-
not(windows),
201-
))]
202-
impl<'f> VaListImpl<'f> {
203-
/// Converts a `VaListImpl` into a `VaList` that is binary-compatible with C's `va_list`.
204-
#[inline]
205-
pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
206-
VaList { inner: self, _marker: PhantomData }
152+
_ => {
153+
/// A wrapper for a `va_list`
154+
#[repr(transparent)]
155+
#[derive(Debug)]
156+
pub struct VaList<'a, 'f: 'a> {
157+
inner: VaListImpl<'f>,
158+
_marker: PhantomData<&'a mut VaListImpl<'f>>,
159+
}
160+
161+
impl<'f> VaListImpl<'f> {
162+
/// Converts a [`VaListImpl`] into a [`VaList`] that is binary-compatible with C's `va_list`.
163+
#[inline]
164+
pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
165+
VaList { inner: VaListImpl { ..*self }, _marker: PhantomData }
166+
}
167+
}
207168
}
208169
}
209170

0 commit comments

Comments
 (0)