Skip to content

use cfg_select! to select the right VaListImpl definition #141361

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

use cfg_select! to select the right VaListImpl definition #141361

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
311 changes: 136 additions & 175 deletions library/core/src/ffi/va_list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,148 +5,120 @@
use crate::ffi::c_void;
#[allow(unused_imports)]
use crate::fmt;
use crate::marker::PhantomData;
use crate::marker::{PhantomData, PhantomInvariantLifetime};
use crate::ops::{Deref, DerefMut};

/// Basic implementation of a `va_list`.
// The name is WIP, using `VaListImpl` for now.
#[cfg(any(
//
// Most targets explicitly specify the layout of `va_list`, this layout is matched here.
crate::cfg_select! {
all(
not(target_arch = "aarch64"),
not(target_arch = "powerpc"),
not(target_arch = "s390x"),
not(target_arch = "xtensa"),
not(target_arch = "x86_64")
),
all(target_arch = "aarch64", target_vendor = "apple"),
target_family = "wasm",
target_os = "uefi",
windows,
))]
#[repr(transparent)]
#[lang = "va_list"]
pub struct VaListImpl<'f> {
ptr: *mut c_void,

// Invariant over `'f`, so each `VaListImpl<'f>` object is tied to
// the region of the function it's defined in
_marker: PhantomData<&'f mut &'f c_void>,
}

#[cfg(any(
all(
not(target_arch = "aarch64"),
not(target_arch = "powerpc"),
not(target_arch = "s390x"),
not(target_arch = "xtensa"),
not(target_arch = "x86_64")
),
all(target_arch = "aarch64", target_vendor = "apple"),
target_family = "wasm",
target_os = "uefi",
windows,
))]
impl<'f> fmt::Debug for VaListImpl<'f> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "va_list* {:p}", self.ptr)
target_arch = "aarch64",
not(target_vendor = "apple"),
not(target_os = "uefi"),
not(windows),
) => {
/// AArch64 ABI implementation of a `va_list`. See the
/// [AArch64 Procedure Call Standard] for more details.
///
/// [AArch64 Procedure Call Standard]:
/// http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
#[derive(Debug)]
#[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: PhantomInvariantLifetime<'f>,
}
}
all(target_arch = "powerpc", not(target_os = "uefi"), not(windows)) => {
/// PowerPC ABI implementation of a `va_list`.
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
#[derive(Debug)]
#[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: PhantomInvariantLifetime<'f>,
}
}
target_arch = "s390x" => {
/// s390x ABI implementation of a `va_list`.
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
#[derive(Debug)]
#[lang = "va_list"]
pub struct VaListImpl<'f> {
gpr: i64,
fpr: i64,
overflow_arg_area: *mut c_void,
reg_save_area: *mut c_void,
_marker: PhantomInvariantLifetime<'f>,
}
}
all(target_arch = "x86_64", not(target_os = "uefi"), not(windows)) => {
/// x86_64 ABI implementation of a `va_list`.
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
#[derive(Debug)]
#[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: PhantomInvariantLifetime<'f>,
}
}
target_arch = "xtensa" => {
/// Xtensa ABI implementation of a `va_list`.
#[repr(C)]
#[derive(Debug)]
#[lang = "va_list"]
pub struct VaListImpl<'f> {
stk: *mut i32,
reg: *mut i32,
ndx: i32,
_marker: PhantomInvariantLifetime<'f>,
}
}
}

/// AArch64 ABI implementation of a `va_list`. See the
/// [AArch64 Procedure Call Standard] for more details.
///
/// [AArch64 Procedure Call Standard]:
/// http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf
#[cfg(all(
target_arch = "aarch64",
not(target_vendor = "apple"),
not(target_os = "uefi"),
not(windows),
))]
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
#[derive(Debug)]
#[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 mut &'f c_void>,
}

/// PowerPC ABI implementation of a `va_list`.
#[cfg(all(target_arch = "powerpc", not(target_os = "uefi"), not(windows)))]
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
#[derive(Debug)]
#[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 mut &'f c_void>,
}

/// s390x ABI implementation of a `va_list`.
#[cfg(target_arch = "s390x")]
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
#[derive(Debug)]
#[lang = "va_list"]
pub struct VaListImpl<'f> {
gpr: i64,
fpr: i64,
overflow_arg_area: *mut c_void,
reg_save_area: *mut c_void,
_marker: PhantomData<&'f mut &'f c_void>,
}

/// x86_64 ABI implementation of a `va_list`.
#[cfg(all(target_arch = "x86_64", not(target_os = "uefi"), not(windows)))]
#[cfg_attr(not(doc), repr(C))] // work around https://github.com/rust-lang/rust/issues/66401
#[derive(Debug)]
#[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 mut &'f c_void>,
}
// The fallback implementation, used for:
//
// - apple aarch64 (see https://github.com/rust-lang/rust/pull/56599)
// - windows
// - uefi
// - any other target for which we don't specify the `VaListImpl` above
//
// In this implementation the `va_list` type is just an alias for an opaque pointer.
// That pointer is probably just the next variadic argument on the caller's stack.
_ => {
/// Basic implementation of a `va_list`.
#[repr(transparent)]
#[lang = "va_list"]
pub struct VaListImpl<'f> {
ptr: *mut c_void,

// Invariant over `'f`, so each `VaListImpl<'f>` object is tied to
// the region of the function it's defined in
_marker: PhantomInvariantLifetime<'f>,
}

/// Xtensa ABI implementation of a `va_list`.
#[cfg(target_arch = "xtensa")]
#[repr(C)]
#[derive(Debug)]
#[lang = "va_list"]
pub struct VaListImpl<'f> {
stk: *mut i32,
reg: *mut i32,
ndx: i32,
_marker: PhantomData<&'f mut &'f c_void>,
impl<'f> fmt::Debug for VaListImpl<'f> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "va_list* {:p}", self.ptr)
}
}
}
}

/// A wrapper for a `va_list`
#[repr(transparent)]
#[derive(Debug)]
pub struct VaList<'a, 'f: 'a> {
#[cfg(any(
all(
not(target_arch = "aarch64"),
not(target_arch = "powerpc"),
not(target_arch = "s390x"),
not(target_arch = "x86_64")
),
target_arch = "xtensa",
all(target_arch = "aarch64", target_vendor = "apple"),
target_family = "wasm",
target_os = "uefi",
windows,
))]
inner: VaListImpl<'f>,

#[cfg(all(
crate::cfg_select! {
all(
any(
target_arch = "aarch64",
target_arch = "powerpc",
Expand All @@ -158,52 +130,41 @@ pub struct VaList<'a, 'f: 'a> {
not(target_family = "wasm"),
not(target_os = "uefi"),
not(windows),
))]
inner: &'a mut VaListImpl<'f>,
) => {
/// A wrapper for a `va_list`
#[repr(transparent)]
#[derive(Debug)]
pub struct VaList<'a, 'f: 'a> {
inner: &'a mut VaListImpl<'f>,
_marker: PhantomData<&'a mut VaListImpl<'f>>,
}

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

#[cfg(any(
all(
not(target_arch = "aarch64"),
not(target_arch = "powerpc"),
not(target_arch = "s390x"),
not(target_arch = "x86_64")
),
target_arch = "xtensa",
all(target_arch = "aarch64", target_vendor = "apple"),
target_family = "wasm",
target_os = "uefi",
windows,
))]
impl<'f> VaListImpl<'f> {
/// Converts 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 }
impl<'f> VaListImpl<'f> {
/// Converts 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 }
}
}
}
}

#[cfg(all(
any(
target_arch = "aarch64",
target_arch = "powerpc",
target_arch = "s390x",
target_arch = "xtensa",
target_arch = "x86_64"
),
not(target_arch = "xtensa"),
any(not(target_arch = "aarch64"), not(target_vendor = "apple")),
not(target_family = "wasm"),
not(target_os = "uefi"),
not(windows),
))]
impl<'f> VaListImpl<'f> {
/// Converts 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 }
_ => {
/// A wrapper for a `va_list`
#[repr(transparent)]
#[derive(Debug)]
pub struct VaList<'a, 'f: 'a> {
inner: VaListImpl<'f>,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by this implementation that is used when we don't know the layout of va_list (the _ => { ... } case in the cfg_match! above). It is inconsistent with the others.

Could we not have &'a mut VaListImpl<'f> here like above, and make the VaListImpl a ZST containing just the invariance marker?

Is that fallback implementation even meaningful?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok no, after reading a bit more there are just two approaches

  1. va_list is just an opaque pointer (probably just the next variadic argument on the stack, though it's opaque so we can't know)
  2. va_list is some sort of struct. It looks like this is used when registers are also at play for passing variadic arguments

These two approaches are captured by the two different ways of representing VaList. I added a comment with some of that information, in particular #56599 had some useful info.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does raise the question: what is VaList for? it has no methods of its own, so I don't see its purpose.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VaList is supposed to be ABI compatible with C's va_list

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was confused by the C definition, until I rediscovered that nasty thing where sizeof in C is not the same as size_of in rust: for arrays, sizeof gives the length of the array, though it is really used/passed as a pointer type.

Anyway, #44930 (comment) has an updated design for this api, which keeps VaListImpl private.

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

impl<'f> VaListImpl<'f> {
/// Converts 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 }
}
}
}
}

Expand Down
Loading