Skip to content

Commit

Permalink
Auto merge of #59625 - immunant:copy_variadics_typealias, r=eddyb
Browse files Browse the repository at this point in the history
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);
```
  • Loading branch information
bors committed Jun 18, 2019
2 parents 04a3dd8 + b9ea653 commit 605ea9d
Show file tree
Hide file tree
Showing 26 changed files with 419 additions and 223 deletions.
231 changes: 187 additions & 44 deletions src/libcore/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
//! Utilities related to FFI bindings.

use crate::fmt;
use crate::marker::PhantomData;
use crate::ops::{Deref, DerefMut};

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

/// Basic implementation of a `va_list`.
// The name is WIP, using `VaListImpl` for now.
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
not(target_arch = "x86_64")),
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
all(target_arch = "aarch64", target_os = "ios"),
windows))]
#[repr(transparent)]
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
extern {
type VaListImpl;
#[lang = "va_list"]
pub struct VaListImpl<'f> {
ptr: *mut c_void,
_marker: PhantomData<&'f c_void>,
}

#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
not(target_arch = "x86_64")),
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
all(target_arch = "aarch64", target_os = "ios"),
windows))]
impl fmt::Debug for VaListImpl {
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
impl<'f> fmt::Debug for VaListImpl<'f> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "va_list* {:p}", self)
write!(f, "va_list* {:p}", self.ptr)
}
}

Expand All @@ -79,12 +89,14 @@ impl fmt::Debug for VaListImpl {
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
struct VaListImpl {
#[lang = "va_list"]
pub struct VaListImpl<'f> {
stack: *mut c_void,
gr_top: *mut c_void,
vr_top: *mut c_void,
gr_offs: i32,
vr_offs: i32,
_marker: PhantomData<&'f c_void>,
}

/// PowerPC ABI implementation of a `va_list`.
Expand All @@ -95,12 +107,14 @@ struct VaListImpl {
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
struct VaListImpl {
#[lang = "va_list"]
pub struct VaListImpl<'f> {
gpr: u8,
fpr: u8,
reserved: u16,
overflow_arg_area: *mut c_void,
reg_save_area: *mut c_void,
_marker: PhantomData<&'f c_void>,
}

/// x86_64 ABI implementation of a `va_list`.
Expand All @@ -111,22 +125,131 @@ struct VaListImpl {
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
struct VaListImpl {
#[lang = "va_list"]
pub struct VaListImpl<'f> {
gp_offset: i32,
fp_offset: i32,
overflow_arg_area: *mut c_void,
reg_save_area: *mut c_void,
_marker: PhantomData<&'f c_void>,
}

/// A wrapper for a `va_list`
/// asm.js ABI implementation of a `va_list`.
// asm.js uses the PNaCl ABI, which specifies that a `va_list` is
// an array of 4 32-bit integers, according to the old PNaCl docs at
// https://web.archive.org/web/20130518054430/https://www.chromium.org/nativeclient/pnacl/bitcode-abi#TOC-Derived-Types
// and clang does the same in `CreatePNaClABIBuiltinVaListDecl` from `lib/AST/ASTContext.cpp`
#[cfg(all(target_arch = "asmjs", not(windows)))]
#[repr(C)]
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
#[lang = "va_list"]
#[derive(Debug)]
pub struct VaListImpl<'f> {
inner: [crate::mem::MaybeUninit<i32>; 4],
_marker: PhantomData<&'f c_void>,
}

#[cfg(all(target_arch = "asmjs", not(windows)))]
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
impl<'f> fmt::Debug for VaListImpl<'f> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
unsafe {
write!(f, "va_list* [{:#x}, {:#x}, {:#x}, {:#x}]",
self.inner[0].read(), self.inner[1].read(),
self.inner[2].read(), self.inner[3].read())
}
}
}

/// A wrapper for a `va_list`
#[repr(transparent)]
pub struct VaList<'a>(&'a mut VaListImpl);
#[derive(Debug)]
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
pub struct VaList<'a, 'f: 'a> {
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
all(target_arch = "aarch64", target_os = "ios"),
windows))]
inner: VaListImpl<'f>,

#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc",
target_arch = "x86_64", target_arch = "asmjs"),
any(not(target_arch = "aarch64"), not(target_os = "ios")),
not(windows)))]
inner: &'a mut VaListImpl<'f>,

_marker: PhantomData<&'a mut VaListImpl<'f>>,
}

#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
not(target_arch = "x86_64"), not(target_arch = "asmjs")),
all(target_arch = "aarch64", target_os = "ios"),
windows))]
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
impl<'f> VaListImpl<'f> {
/// Convert a `VaListImpl` into a `VaList` that is binary-compatible with C's `va_list`.
#[inline]
pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
VaList {
inner: VaListImpl { ..*self },
_marker: PhantomData,
}
}
}

#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc",
target_arch = "x86_64", target_arch = "asmjs"),
any(not(target_arch = "aarch64"), not(target_os = "ios")),
not(windows)))]
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
impl<'f> VaListImpl<'f> {
/// Convert a `VaListImpl` into a `VaList` that is binary-compatible with C's `va_list`.
#[inline]
pub fn as_va_list<'a>(&'a mut self) -> VaList<'a, 'f> {
VaList {
inner: self,
_marker: PhantomData,
}
}
}

#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
impl<'a, 'f: 'a> Deref for VaList<'a, 'f> {
type Target = VaListImpl<'f>;

#[inline]
fn deref(&self) -> &VaListImpl<'f> {
&self.inner
}
}

#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
impl<'a, 'f: 'a> DerefMut for VaList<'a, 'f> {
#[inline]
fn deref_mut(&mut self) -> &mut VaListImpl<'f> {
&mut self.inner
}
}

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

impl<'a> VaList<'a> {
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
#[cfg(not(bootstrap))]
impl<'f> VaListImpl<'f> {
/// Advance to the next arg.
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
#[inline]
pub unsafe fn arg<T: sealed_trait::VaArgSafe>(&mut self) -> T {
va_arg(self)
}

/// Copies the `va_list` at the current location.
#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
pub unsafe fn with_copy<F, R>(&self, f: F) -> R
where F: for<'copy> FnOnce(VaList<'copy>) -> R {
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
not(target_arch = "x86_64")),
all(target_arch = "aarch64", target_os = "ios"),
windows))]
let mut ap = va_copy(self);
#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc", target_arch = "x86_64"),
not(windows), not(all(target_arch = "aarch64", target_os = "ios"))))]
let mut ap_inner = va_copy(self);
#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc", target_arch = "x86_64"),
not(windows), not(all(target_arch = "aarch64", target_os = "ios"))))]
let mut ap = VaList(&mut ap_inner);
let ret = f(VaList(ap.0));
where F: for<'copy> FnOnce(VaList<'copy, 'f>) -> R {
let mut ap = self.clone();
let ret = f(ap.as_va_list());
va_end(&mut ap);
ret
}
}

#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
#[cfg(not(bootstrap))]
impl<'f> Clone for VaListImpl<'f> {
#[inline]
fn clone(&self) -> Self {
let mut dest = crate::mem::MaybeUninit::uninit();
unsafe {
va_copy(dest.as_mut_ptr(), self);
dest.assume_init()
}
}
}

#[unstable(feature = "c_variadic",
reason = "the `c_variadic` feature has not been properly tested on \
all supported platforms",
issue = "44930")]
#[cfg(not(bootstrap))]
impl<'f> Drop for VaListImpl<'f> {
fn drop(&mut self) {
// FIXME: this should call `va_end`, but there's no clean way to
// guarantee that `drop` always gets inlined into its caller,
// so the `va_end` would get directly called from the same function as
// the corresponding `va_copy`. `man va_end` states that C requires this,
// and LLVM basically follows the C semantics, so we need to make sure
// that `va_end` is always called from the same function as `va_copy`.
// For more details, see https://github.com/rust-lang/rust/pull/59625
// and https://llvm.org/docs/LangRef.html#llvm-va-end-intrinsic.
//
// This works for now, since `va_end` is a no-op on all current LLVM targets.
}
}

extern "rust-intrinsic" {
/// Destroy the arglist `ap` after initialization with `va_start` or
/// `va_copy`.
fn va_end(ap: &mut VaList<'_>);
#[cfg(not(bootstrap))]
fn va_end(ap: &mut VaListImpl<'_>);

/// Copies the current location of arglist `src` to the arglist `dst`.
#[cfg(any(all(not(target_arch = "aarch64"), not(target_arch = "powerpc"),
not(target_arch = "x86_64")),
all(target_arch = "aarch64", target_os = "ios"),
windows))]
fn va_copy<'a>(src: &VaList<'a>) -> VaList<'a>;
#[cfg(all(any(target_arch = "aarch64", target_arch = "powerpc", target_arch = "x86_64"),
not(windows), not(all(target_arch = "aarch64", target_os = "ios"))))]
fn va_copy(src: &VaList<'_>) -> VaListImpl;
#[cfg(not(bootstrap))]
fn va_copy<'f>(dest: *mut VaListImpl<'f>, src: &VaListImpl<'f>);

/// Loads an argument of type `T` from the `va_list` `ap` and increment the
/// argument `ap` points to.
fn va_arg<T: sealed_trait::VaArgSafe>(ap: &mut VaList<'_>) -> T;
#[cfg(not(bootstrap))]
fn va_arg<T: sealed_trait::VaArgSafe>(ap: &mut VaListImpl<'_>) -> T;
}
2 changes: 1 addition & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1674,7 +1674,7 @@ impl<'a> LoweringContext<'a> {
}
TyKind::Mac(_) => bug!("`TyMac` should have been expanded by now."),
TyKind::CVarArgs => {
// Create the implicit lifetime of the "spoofed" `VaList`.
// Create the implicit lifetime of the "spoofed" `VaListImpl`.
let span = self.sess.source_map().next_point(t.span.shrink_to_lo());
let lt = self.new_implicit_lifetime(span);
hir::TyKind::CVarArgs(lt)
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1930,7 +1930,7 @@ pub enum TyKind {
Infer,
/// Placeholder for a type that has failed to be defined.
Err,
/// Placeholder for C-variadic arguments. We "spoof" the `VaList` created
/// Placeholder for C-variadic arguments. We "spoof" the `VaListImpl` created
/// from the variadic arguments. This type is only valid up to typeck.
CVarArgs(Lifetime),
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2711,7 +2711,7 @@ where
}

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

0 comments on commit 605ea9d

Please sign in to comment.