Skip to content

Commit 605ea9d

Browse files
committed
Auto merge of #59625 - immunant:copy_variadics_typealias, r=eddyb
Refactor C FFI variadics to more closely match their C counterparts, and add Clone implementation We had to make some changes to expose `va_copy` and `va_end` directly to users (mainly for C2Rust, but not exclusively): - redefine the Rust variadic structures to more closely correspond to C: `VaList` now matches `va_list`, and `VaListImpl` matches `__va_list_tag` - add `Clone` for `VaListImpl` - add explicit `as_va_list()` conversion function from `VaListImpl` to `VaList` - add deref coercion from `VaList` to `VaListImpl` - add support for the `asmjs` target All these changes were needed for use cases like: ```Rust let mut ap2 = va_copy(ap); vprintf(fmt, ap2); va_end(&mut ap2); ```
2 parents 04a3dd8 + b9ea653 commit 605ea9d

File tree

26 files changed

+419
-223
lines changed

26 files changed

+419
-223
lines changed

src/libcore/ffi.rs

+187-44
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
//! Utilities related to FFI bindings.
66
77
use crate::fmt;
8+
use crate::marker::PhantomData;
9+
use crate::ops::{Deref, DerefMut};
810

911
/// Equivalent to C's `void` type when used as a [pointer].
1012
///
@@ -45,25 +47,33 @@ impl fmt::Debug for c_void {
4547
}
4648

4749
/// Basic implementation of a `va_list`.
50+
// The name is WIP, using `VaListImpl` for now.
4851
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
49-
not(target_arch = "x86_64")),
52+
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
5053
all(target_arch = "aarch64", target_os = "ios"),
5154
windows))]
55+
#[repr(transparent)]
5256
#[unstable(feature = "c_variadic",
5357
reason = "the `c_variadic` feature has not been properly tested on \
5458
all supported platforms",
5559
issue = "44930")]
56-
extern {
57-
type VaListImpl;
60+
#[lang = "va_list"]
61+
pub struct VaListImpl<'f> {
62+
ptr: *mut c_void,
63+
_marker: PhantomData<&'f c_void>,
5864
}
5965

6066
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
61-
not(target_arch = "x86_64")),
67+
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
6268
all(target_arch = "aarch64", target_os = "ios"),
6369
windows))]
64-
impl fmt::Debug for VaListImpl {
70+
#[unstable(feature = "c_variadic",
71+
reason = "the `c_variadic` feature has not been properly tested on \
72+
all supported platforms",
73+
issue = "44930")]
74+
impl<'f> fmt::Debug for VaListImpl<'f> {
6575
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
66-
write!(f, "va_list* {:p}", self)
76+
write!(f, "va_list* {:p}", self.ptr)
6777
}
6878
}
6979

@@ -79,12 +89,14 @@ impl fmt::Debug for VaListImpl {
7989
reason = "the `c_variadic` feature has not been properly tested on \
8090
all supported platforms",
8191
issue = "44930")]
82-
struct VaListImpl {
92+
#[lang = "va_list"]
93+
pub struct VaListImpl<'f> {
8394
stack: *mut c_void,
8495
gr_top: *mut c_void,
8596
vr_top: *mut c_void,
8697
gr_offs: i32,
8798
vr_offs: i32,
99+
_marker: PhantomData<&'f c_void>,
88100
}
89101

90102
/// PowerPC ABI implementation of a `va_list`.
@@ -95,12 +107,14 @@ struct VaListImpl {
95107
reason = "the `c_variadic` feature has not been properly tested on \
96108
all supported platforms",
97109
issue = "44930")]
98-
struct VaListImpl {
110+
#[lang = "va_list"]
111+
pub struct VaListImpl<'f> {
99112
gpr: u8,
100113
fpr: u8,
101114
reserved: u16,
102115
overflow_arg_area: *mut c_void,
103116
reg_save_area: *mut c_void,
117+
_marker: PhantomData<&'f c_void>,
104118
}
105119

106120
/// x86_64 ABI implementation of a `va_list`.
@@ -111,22 +125,131 @@ struct VaListImpl {
111125
reason = "the `c_variadic` feature has not been properly tested on \
112126
all supported platforms",
113127
issue = "44930")]
114-
struct VaListImpl {
128+
#[lang = "va_list"]
129+
pub struct VaListImpl<'f> {
115130
gp_offset: i32,
116131
fp_offset: i32,
117132
overflow_arg_area: *mut c_void,
118133
reg_save_area: *mut c_void,
134+
_marker: PhantomData<&'f c_void>,
119135
}
120136

121-
/// A wrapper for a `va_list`
137+
/// asm.js ABI implementation of a `va_list`.
138+
// asm.js uses the PNaCl ABI, which specifies that a `va_list` is
139+
// an array of 4 32-bit integers, according to the old PNaCl docs at
140+
// https://web.archive.org/web/20130518054430/https://www.chromium.org/nativeclient/pnacl/bitcode-abi#TOC-Derived-Types
141+
// and clang does the same in `CreatePNaClABIBuiltinVaListDecl` from `lib/AST/ASTContext.cpp`
142+
#[cfg(all(target_arch = "asmjs", not(windows)))]
143+
#[repr(C)]
144+
#[unstable(feature = "c_variadic",
145+
reason = "the `c_variadic` feature has not been properly tested on \
146+
all supported platforms",
147+
issue = "44930")]
122148
#[lang = "va_list"]
123-
#[derive(Debug)]
149+
pub struct VaListImpl<'f> {
150+
inner: [crate::mem::MaybeUninit<i32>; 4],
151+
_marker: PhantomData<&'f c_void>,
152+
}
153+
154+
#[cfg(all(target_arch = "asmjs", not(windows)))]
124155
#[unstable(feature = "c_variadic",
125156
reason = "the `c_variadic` feature has not been properly tested on \
126157
all supported platforms",
127158
issue = "44930")]
159+
impl<'f> fmt::Debug for VaListImpl<'f> {
160+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
161+
unsafe {
162+
write!(f, "va_list* [{:#x}, {:#x}, {:#x}, {:#x}]",
163+
self.inner[0].read(), self.inner[1].read(),
164+
self.inner[2].read(), self.inner[3].read())
165+
}
166+
}
167+
}
168+
169+
/// A wrapper for a `va_list`
128170
#[repr(transparent)]
129-
pub struct VaList<'a>(&'a mut VaListImpl);
171+
#[derive(Debug)]
172+
#[unstable(feature = "c_variadic",
173+
reason = "the `c_variadic` feature has not been properly tested on \
174+
all supported platforms",
175+
issue = "44930")]
176+
pub struct VaList<'a, 'f: 'a> {
177+
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
178+
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
179+
all(target_arch = "aarch64", target_os = "ios"),
180+
windows))]
181+
inner: VaListImpl<'f>,
182+
183+
#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc",
184+
target_arch = "x86_64", target_arch = "asmjs"),
185+
any(not(target_arch = "aarch64"), not(target_os = "ios")),
186+
not(windows)))]
187+
inner: &'a mut VaListImpl<'f>,
188+
189+
_marker: PhantomData<&'a mut VaListImpl<'f>>,
190+
}
191+
192+
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
193+
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
194+
all(target_arch = "aarch64", target_os = "ios"),
195+
windows))]
196+
#[unstable(feature = "c_variadic",
197+
reason = "the `c_variadic` feature has not been properly tested on \
198+
all supported platforms",
199+
issue = "44930")]
200+
impl<'f> VaListImpl<'f> {
201+
/// Convert a `VaListImpl` into a `VaList` that is binary-compatible with C's `va_list`.
202+
#[inline]
203+
pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
204+
VaList {
205+
inner: VaListImpl { ..*self },
206+
_marker: PhantomData,
207+
}
208+
}
209+
}
210+
211+
#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc",
212+
target_arch = "x86_64", target_arch = "asmjs"),
213+
any(not(target_arch = "aarch64"), not(target_os = "ios")),
214+
not(windows)))]
215+
#[unstable(feature = "c_variadic",
216+
reason = "the `c_variadic` feature has not been properly tested on \
217+
all supported platforms",
218+
issue = "44930")]
219+
impl<'f> VaListImpl<'f> {
220+
/// Convert a `VaListImpl` into a `VaList` that is binary-compatible with C's `va_list`.
221+
#[inline]
222+
pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
223+
VaList {
224+
inner: self,
225+
_marker: PhantomData,
226+
}
227+
}
228+
}
229+
230+
#[unstable(feature = "c_variadic",
231+
reason = "the `c_variadic` feature has not been properly tested on \
232+
all supported platforms",
233+
issue = "44930")]
234+
impl<'a, 'f: 'a> Deref for VaList<'a, 'f> {
235+
type Target = VaListImpl<'f>;
236+
237+
#[inline]
238+
fn deref(&self) -> &VaListImpl<'f> {
239+
&self.inner
240+
}
241+
}
242+
243+
#[unstable(feature = "c_variadic",
244+
reason = "the `c_variadic` feature has not been properly tested on \
245+
all supported platforms",
246+
issue = "44930")]
247+
impl<'a, 'f: 'a> DerefMut for VaList<'a, 'f> {
248+
#[inline]
249+
fn deref_mut(&mut self) -> &mut VaListImpl<'f> {
250+
&mut self.inner
251+
}
252+
}
130253

131254
// The VaArgSafe trait needs to be used in public interfaces, however, the trait
132255
// itself must not be allowed to be used outside this module. Allowing users to
@@ -175,56 +298,76 @@ impl<T> sealed_trait::VaArgSafe for *mut T {}
175298
issue = "44930")]
176299
impl<T> sealed_trait::VaArgSafe for *const T {}
177300

178-
impl<'a> VaList<'a> {
301+
#[unstable(feature = "c_variadic",
302+
reason = "the `c_variadic` feature has not been properly tested on \
303+
all supported platforms",
304+
issue = "44930")]
305+
#[cfg(not(bootstrap))]
306+
impl<'f> VaListImpl<'f> {
179307
/// Advance to the next arg.
180-
#[unstable(feature = "c_variadic",
181-
reason = "the `c_variadic` feature has not been properly tested on \
182-
all supported platforms",
183-
issue = "44930")]
308+
#[inline]
184309
pub unsafe fn arg<T: sealed_trait::VaArgSafe>(&mut self) -> T {
185310
va_arg(self)
186311
}
187312

188313
/// Copies the `va_list` at the current location.
189-
#[unstable(feature = "c_variadic",
190-
reason = "the `c_variadic` feature has not been properly tested on \
191-
all supported platforms",
192-
issue = "44930")]
193314
pub unsafe fn with_copy<F, R>(&self, f: F) -> R
194-
where F: for<'copy> FnOnce(VaList<'copy>) -> R {
195-
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
196-
not(target_arch = "x86_64")),
197-
all(target_arch = "aarch64", target_os = "ios"),
198-
windows))]
199-
let mut ap = va_copy(self);
200-
#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc", target_arch = "x86_64"),
201-
not(windows), not(all(target_arch = "aarch64", target_os = "ios"))))]
202-
let mut ap_inner = va_copy(self);
203-
#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc", target_arch = "x86_64"),
204-
not(windows), not(all(target_arch = "aarch64", target_os = "ios"))))]
205-
let mut ap = VaList(&mut ap_inner);
206-
let ret = f(VaList(ap.0));
315+
where F: for<'copy> FnOnce(VaList<'copy, 'f>) -> R {
316+
let mut ap = self.clone();
317+
let ret = f(ap.as_va_list());
207318
va_end(&mut ap);
208319
ret
209320
}
210321
}
211322

323+
#[unstable(feature = "c_variadic",
324+
reason = "the `c_variadic` feature has not been properly tested on \
325+
all supported platforms",
326+
issue = "44930")]
327+
#[cfg(not(bootstrap))]
328+
impl<'f> Clone for VaListImpl<'f> {
329+
#[inline]
330+
fn clone(&self) -> Self {
331+
let mut dest = crate::mem::MaybeUninit::uninit();
332+
unsafe {
333+
va_copy(dest.as_mut_ptr(), self);
334+
dest.assume_init()
335+
}
336+
}
337+
}
338+
339+
#[unstable(feature = "c_variadic",
340+
reason = "the `c_variadic` feature has not been properly tested on \
341+
all supported platforms",
342+
issue = "44930")]
343+
#[cfg(not(bootstrap))]
344+
impl<'f> Drop for VaListImpl<'f> {
345+
fn drop(&mut self) {
346+
// FIXME: this should call `va_end`, but there's no clean way to
347+
// guarantee that `drop` always gets inlined into its caller,
348+
// so the `va_end` would get directly called from the same function as
349+
// the corresponding `va_copy`. `man va_end` states that C requires this,
350+
// and LLVM basically follows the C semantics, so we need to make sure
351+
// that `va_end` is always called from the same function as `va_copy`.
352+
// For more details, see https://github.com/rust-lang/rust/pull/59625
353+
// and https://llvm.org/docs/LangRef.html#llvm-va-end-intrinsic.
354+
//
355+
// This works for now, since `va_end` is a no-op on all current LLVM targets.
356+
}
357+
}
358+
212359
extern "rust-intrinsic" {
213360
/// Destroy the arglist `ap` after initialization with `va_start` or
214361
/// `va_copy`.
215-
fn va_end(ap: &mut VaList<'_>);
362+
#[cfg(not(bootstrap))]
363+
fn va_end(ap: &mut VaListImpl<'_>);
216364

217365
/// Copies the current location of arglist `src` to the arglist `dst`.
218-
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
219-
not(target_arch = "x86_64")),
220-
all(target_arch = "aarch64", target_os = "ios"),
221-
windows))]
222-
fn va_copy<'a>(src: &VaList<'a>) -> VaList<'a>;
223-
#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc", target_arch = "x86_64"),
224-
not(windows), not(all(target_arch = "aarch64", target_os = "ios"))))]
225-
fn va_copy(src: &VaList<'_>) -> VaListImpl;
366+
#[cfg(not(bootstrap))]
367+
fn va_copy<'f>(dest: *mut VaListImpl<'f>, src: &VaListImpl<'f>);
226368

227369
/// Loads an argument of type `T` from the `va_list` `ap` and increment the
228370
/// argument `ap` points to.
229-
fn va_arg<T: sealed_trait::VaArgSafe>(ap: &mut VaList<'_>) -> T;
371+
#[cfg(not(bootstrap))]
372+
fn va_arg<T: sealed_trait::VaArgSafe>(ap: &mut VaListImpl<'_>) -> T;
230373
}

src/librustc/hir/lowering.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1674,7 +1674,7 @@ impl<'a> LoweringContext<'a> {
16741674
}
16751675
TyKind::Mac(_) => bug!("`TyMac` should have been expanded by now."),
16761676
TyKind::CVarArgs => {
1677-
// Create the implicit lifetime of the "spoofed" `VaList`.
1677+
// Create the implicit lifetime of the "spoofed" `VaListImpl`.
16781678
let span = self.sess.source_map().next_point(t.span.shrink_to_lo());
16791679
let lt = self.new_implicit_lifetime(span);
16801680
hir::TyKind::CVarArgs(lt)

src/librustc/hir/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1930,7 +1930,7 @@ pub enum TyKind {
19301930
Infer,
19311931
/// Placeholder for a type that has failed to be defined.
19321932
Err,
1933-
/// Placeholder for C-variadic arguments. We "spoof" the `VaList` created
1933+
/// Placeholder for C-variadic arguments. We "spoof" the `VaListImpl` created
19341934
/// from the variadic arguments. This type is only valid up to typeck.
19351935
CVarArgs(Lifetime),
19361936
}

src/librustc/ty/layout.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2711,7 +2711,7 @@ where
27112711
}
27122712

27132713
// If this is a C-variadic function, this is not the return value,
2714-
// and there is one or more fixed arguments; ensure that the `VaList`
2714+
// and there is one or more fixed arguments; ensure that the `VaListImpl`
27152715
// is ignored as an argument.
27162716
if sig.c_variadic {
27172717
match (last_arg_idx, arg_idx) {
@@ -2722,7 +2722,7 @@ where
27222722
};
27232723
match ty.sty {
27242724
ty::Adt(def, _) if def.did == va_list_did => {
2725-
// This is the "spoofed" `VaList`. Set the arguments mode
2725+
// This is the "spoofed" `VaListImpl`. Set the arguments mode
27262726
// so that it will be ignored.
27272727
arg.mode = PassMode::Ignore(IgnoreMode::CVarArgs);
27282728
}

0 commit comments

Comments
 (0)