Skip to content

improve core::ffi::VaList #141835

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 16 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let va_list_arg_idx = self.fn_abi.args.len();
match self.locals[mir::Local::from_usize(1 + va_list_arg_idx)] {
LocalRef::Place(va_list) => {
bx.va_end(va_list.val.llval);
if super::is_va_list_struct_on_stack(bx) {
// Call `va_end` on the `&mut VaListTag` that is stored in `va_list`.
let inner_field = va_list.project_field(bx, 0);

let tag_ptr = bx.load(
bx.backend_type(inner_field.layout),
inner_field.val.llval,
inner_field.layout.align.abi,
);

bx.va_end(tag_ptr);
} else {
// Call `va_end` on the `VaListTag` that is stored in `va_list`.
let tag_ptr = va_list.project_field(bx, 0);
bx.va_end(tag_ptr.val.llval);
}
}
_ => bug!("C-variadic function must have a `VaList` place"),
}
Expand Down
67 changes: 64 additions & 3 deletions compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::iter;

use rustc_ast::Mutability;
use rustc_hir as hir;
use rustc_index::IndexVec;
use rustc_index::bit_set::DenseBitSet;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
Expand Down Expand Up @@ -370,6 +372,41 @@ fn optimize_use_clone<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
mir
}

/// Whether on this target, the `VaList` is a reference to a struct on the stack.
///
/// If this function returns `true`, the `core` crate defines a `VaListTag` struct
/// matching the varargs ABI for this target.
///
/// In other cases, the `VaList` is an opaque pointer. Generally this is just a pointer to the
/// caller's stack where the variadic arguments are stored sequentially.
fn is_va_list_struct_on_stack<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(bx: &mut Bx) -> bool {
let did = bx.tcx().require_lang_item(hir::LangItem::VaList, None);
let ty = bx.tcx().type_of(did).instantiate_identity();

// Check how `VaList` is implemented for the current target. It can be either:
//
// - a reference to a value on the stack
// - a struct wrapping a pointer
let Some(adt_def) = ty.ty_adt_def() else { bug!("invalid `VaList`") };
let variant = adt_def.non_enum_variant();

if variant.fields.len() != 1 {
bug!("`VaList` must have exactly 1 field")
}
let field = variant.fields.iter().next().unwrap();
let field_ty = bx.tcx().type_of(field.did).instantiate_identity();

if field_ty.is_adt() {
return false;
}

if let Some(Mutability::Mut) = field_ty.ref_mutability() {
return true;
}

bug!("invalid `VaList` field type")
}

/// Produces, for each argument, a `Value` pointing at the
/// argument's value. As arguments are places, these are always
/// indirect.
Expand Down Expand Up @@ -436,10 +473,34 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
}

if fx.fn_abi.c_variadic && arg_index == fx.fn_abi.args.len() {
let va_list = PlaceRef::alloca(bx, bx.layout_of(arg_ty));
bx.va_start(va_list.val.llval);
use rustc_hir::LangItem;

let va_list_tag_ty = {
let did = bx.tcx().require_lang_item(LangItem::VaListTag, None);
let ty = bx.tcx().type_of(did).instantiate_identity();
bx.tcx().normalize_erasing_regions(fx.cx.typing_env(), ty)
};
let va_list_tag_layout = bx.layout_of(va_list_tag_ty);

// Construct the `VaListTag` on the stack.
let va_list_tag = PlaceRef::alloca(bx, va_list_tag_layout);

// Initialize the alloca.
bx.va_start(va_list_tag.val.llval);

let va_list_tag = if is_va_list_struct_on_stack(bx) {
va_list_tag.val.llval
} else {
bx.load(
bx.backend_type(va_list_tag_layout),
va_list_tag.val.llval,
va_list_tag.layout.align.abi,
)
};

return LocalRef::Place(va_list);
let tmp = PlaceRef::alloca(bx, bx.layout_of(arg_ty));
bx.store_to_place(va_list_tag, tmp.val);
return LocalRef::Place(tmp);
}

let arg = &fx.fn_abi.args[idx];
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ language_item_table! {
UnsafePinned, sym::unsafe_pinned, unsafe_pinned_type, Target::Struct, GenericRequirement::None;

VaList, sym::va_list, va_list, Target::Struct, GenericRequirement::None;
VaListTag, sym::va_list_tag, va_list_tag, Target::Struct, GenericRequirement::None;

Deref, sym::deref, deref_trait, Target::Trait, GenericRequirement::Exact(0);
DerefMut, sym::deref_mut, deref_mut_trait, Target::Trait, GenericRequirement::Exact(0);
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ pub(crate) fn check_intrinsic_type(
ty::BoundVariableKind::Region(ty::BoundRegionKind::Anon),
ty::BoundVariableKind::Region(ty::BoundRegionKind::ClosureEnv),
]);
let mk_va_list_ty = |mutbl| {
let did = tcx.require_lang_item(LangItem::VaList, Some(span));
let mk_va_list_tag_ty = |mutbl| {
let did = tcx.require_lang_item(LangItem::VaListTag, Some(span));
let region = ty::Region::new_bound(
tcx,
ty::INNERMOST,
Expand Down Expand Up @@ -567,16 +567,16 @@ pub(crate) fn check_intrinsic_type(
}

sym::va_start | sym::va_end => {
(0, 0, vec![mk_va_list_ty(hir::Mutability::Mut).0], tcx.types.unit)
(0, 0, vec![mk_va_list_tag_ty(hir::Mutability::Mut).0], tcx.types.unit)
}

sym::va_copy => {
let (va_list_ref_ty, va_list_ty) = mk_va_list_ty(hir::Mutability::Not);
let (va_list_ref_ty, va_list_ty) = mk_va_list_tag_ty(hir::Mutability::Not);
let va_list_ptr_ty = Ty::new_mut_ptr(tcx, va_list_ty);
(0, 0, vec![va_list_ptr_ty, va_list_ref_ty], tcx.types.unit)
}

sym::va_arg => (1, 0, vec![mk_va_list_ty(hir::Mutability::Mut).0], param(0)),
sym::va_arg => (1, 0, vec![mk_va_list_tag_ty(hir::Mutability::Mut).0], param(0)),

sym::nontemporal_store => {
(1, 0, vec![Ty::new_mut_ptr(tcx, param(0)), param(0)], tcx.types.unit)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2275,6 +2275,7 @@ symbols! {
va_copy,
va_end,
va_list,
va_list_tag,
va_start,
val,
validity,
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/ffi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub mod c_str;
issue = "44930",
reason = "the `c_variadic` feature has not been properly tested on all supported platforms"
)]
pub use self::va_list::{VaArgSafe, VaList, VaListImpl};
pub use self::va_list::{VaArgSafe, VaList, VaListTag, va_copy};

#[unstable(
feature = "c_variadic",
Expand Down
Loading
Loading