Skip to content

Commit

Permalink
rustc: rely on c_variadic == true instead of CVarArgs in HIR/Ty fn si…
Browse files Browse the repository at this point in the history
…gnatures.
  • Loading branch information
eddyb committed Sep 28, 2019
1 parent 7683d1c commit a88d181
Show file tree
Hide file tree
Showing 30 changed files with 295 additions and 386 deletions.
1 change: 0 additions & 1 deletion src/librustc/hir/intravisit.rs
Expand Up @@ -633,7 +633,6 @@ pub fn walk_ty<'v, V: Visitor<'v>>(visitor: &mut V, typ: &'v Ty) {
TyKind::Typeof(ref expression) => { TyKind::Typeof(ref expression) => {
visitor.visit_anon_const(expression) visitor.visit_anon_const(expression)
} }
TyKind::CVarArgs |
TyKind::Infer | TyKind::Err => {} TyKind::Infer | TyKind::Err => {}
} }
} }
Expand Down
26 changes: 21 additions & 5 deletions src/librustc/hir/lowering.rs
Expand Up @@ -1335,8 +1335,8 @@ impl<'a> LoweringContext<'a> {
} }
} }
} }
TyKind::Mac(_) => bug!("`TyMac` should have been expanded by now"), TyKind::Mac(_) => bug!("`TyKind::Mac` should have been expanded by now"),
TyKind::CVarArgs => hir::TyKind::CVarArgs, TyKind::CVarArgs => bug!("`TyKind::CVarArgs` should have been handled elsewhere"),
}; };


hir::Ty { hir::Ty {
Expand Down Expand Up @@ -2088,7 +2088,14 @@ impl<'a> LoweringContext<'a> {
} }


fn lower_fn_params_to_names(&mut self, decl: &FnDecl) -> hir::HirVec<Ident> { fn lower_fn_params_to_names(&mut self, decl: &FnDecl) -> hir::HirVec<Ident> {
decl.inputs // Skip the `...` (`CVarArgs`) trailing arguments from the AST,
// as they are not explicit in HIR/Ty function signatures.
// (instead, the `c_variadic` flag is set to `true`)
let mut inputs = &decl.inputs[..];
if decl.c_variadic() {
inputs = &inputs[..inputs.len() - 1];
}
inputs
.iter() .iter()
.map(|param| match param.pat.kind { .map(|param| match param.pat.kind {
PatKind::Ident(_, ident, _) => ident, PatKind::Ident(_, ident, _) => ident,
Expand Down Expand Up @@ -2125,10 +2132,19 @@ impl<'a> LoweringContext<'a> {
self.anonymous_lifetime_mode self.anonymous_lifetime_mode
}; };


let c_variadic = decl.c_variadic();

// Remember how many lifetimes were already around so that we can // Remember how many lifetimes were already around so that we can
// only look at the lifetime parameters introduced by the arguments. // only look at the lifetime parameters introduced by the arguments.
let inputs = self.with_anonymous_lifetime_mode(lt_mode, |this| { let inputs = self.with_anonymous_lifetime_mode(lt_mode, |this| {
decl.inputs // Skip the `...` (`CVarArgs`) trailing arguments from the AST,
// as they are not explicit in HIR/Ty function signatures.
// (instead, the `c_variadic` flag is set to `true`)
let mut inputs = &decl.inputs[..];
if c_variadic {
inputs = &inputs[..inputs.len() - 1];
}
inputs
.iter() .iter()
.map(|param| { .map(|param| {
if let Some((_, ibty)) = &mut in_band_ty_params { if let Some((_, ibty)) = &mut in_band_ty_params {
Expand Down Expand Up @@ -2163,7 +2179,7 @@ impl<'a> LoweringContext<'a> {
P(hir::FnDecl { P(hir::FnDecl {
inputs, inputs,
output, output,
c_variadic: decl.c_variadic(), c_variadic,
implicit_self: decl.inputs.get(0).map_or( implicit_self: decl.inputs.get(0).map_or(
hir::ImplicitSelfKind::None, hir::ImplicitSelfKind::None,
|arg| { |arg| {
Expand Down
3 changes: 0 additions & 3 deletions src/librustc/hir/mod.rs
Expand Up @@ -2016,9 +2016,6 @@ pub enum TyKind {
Infer, Infer,
/// Placeholder for a type that has failed to be defined. /// Placeholder for a type that has failed to be defined.
Err, Err,
/// Placeholder for C-variadic arguments. We "spoof" the `VaListImpl` created
/// from the variadic arguments. This type is only valid up to typeck.
CVarArgs,
} }


#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, HashStable)] #[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, HashStable)]
Expand Down
3 changes: 0 additions & 3 deletions src/librustc/hir/print.rs
Expand Up @@ -361,9 +361,6 @@ impl<'a> State<'a> {
self.s.word("/*ERROR*/"); self.s.word("/*ERROR*/");
self.pclose(); self.pclose();
} }
hir::TyKind::CVarArgs => {
self.s.word("...");
}
} }
self.end() self.end()
} }
Expand Down
35 changes: 2 additions & 33 deletions src/librustc/ty/layout.rs
Expand Up @@ -25,7 +25,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher,
pub use rustc_target::abi::*; pub use rustc_target::abi::*;
use rustc_target::spec::{HasTargetSpec, abi::Abi as SpecAbi}; use rustc_target::spec::{HasTargetSpec, abi::Abi as SpecAbi};
use rustc_target::abi::call::{ use rustc_target::abi::call::{
ArgAttribute, ArgAttributes, ArgType, Conv, FnType, IgnoreMode, PassMode, Reg, RegKind ArgAttribute, ArgAttributes, ArgType, Conv, FnType, PassMode, Reg, RegKind
}; };


pub trait IntegerExt { pub trait IntegerExt {
Expand Down Expand Up @@ -2722,14 +2722,6 @@ where
} }
}; };


// Store the index of the last argument. This is useful for working with
// C-compatible variadic arguments.
let last_arg_idx = if sig.inputs().is_empty() {
None
} else {
Some(sig.inputs().len() - 1)
};

let arg_of = |ty: Ty<'tcx>, arg_idx: Option<usize>| { let arg_of = |ty: Ty<'tcx>, arg_idx: Option<usize>| {
let is_return = arg_idx.is_none(); let is_return = arg_idx.is_none();
let mut arg = mk_arg_type(ty, arg_idx); let mut arg = mk_arg_type(ty, arg_idx);
Expand All @@ -2739,30 +2731,7 @@ where
// The same is true for s390x-unknown-linux-gnu // The same is true for s390x-unknown-linux-gnu
// and sparc64-unknown-linux-gnu. // and sparc64-unknown-linux-gnu.
if is_return || rust_abi || (!win_x64_gnu && !linux_s390x && !linux_sparc64) { if is_return || rust_abi || (!win_x64_gnu && !linux_s390x && !linux_sparc64) {
arg.mode = PassMode::Ignore(IgnoreMode::Zst); arg.mode = PassMode::Ignore;
}
}

// If this is a C-variadic function, this is not the return value,
// 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) {
(Some(last_idx), Some(cur_idx)) if last_idx == cur_idx => {
let va_list_did = match cx.tcx().lang_items().va_list() {
Some(did) => did,
None => bug!("`va_list` lang item required for C-variadic functions"),
};
match ty.kind {
ty::Adt(def, _) if def.did == va_list_did => {
// This is the "spoofed" `VaListImpl`. Set the arguments mode
// so that it will be ignored.
arg.mode = PassMode::Ignore(IgnoreMode::CVarArgs);
}
_ => (),
}
}
_ => {}
} }
} }


Expand Down
12 changes: 5 additions & 7 deletions src/librustc_codegen_llvm/abi.rs
Expand Up @@ -264,7 +264,7 @@ impl ArgTypeExt<'ll, 'tcx> for ArgType<'tcx, Ty<'tcx>> {
val val
}; };
match self.mode { match self.mode {
PassMode::Ignore(_) => {} PassMode::Ignore => {}
PassMode::Pair(..) => { PassMode::Pair(..) => {
OperandValue::Pair(next(), next()).store(bx, dst); OperandValue::Pair(next(), next()).store(bx, dst);
} }
Expand Down Expand Up @@ -319,9 +319,7 @@ impl<'tcx> FnTypeLlvmExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
); );


let llreturn_ty = match self.ret.mode { let llreturn_ty = match self.ret.mode {
PassMode::Ignore(IgnoreMode::Zst) => cx.type_void(), PassMode::Ignore => cx.type_void(),
PassMode::Ignore(IgnoreMode::CVarArgs) =>
bug!("`va_list` should never be a return type"),
PassMode::Direct(_) | PassMode::Pair(..) => { PassMode::Direct(_) | PassMode::Pair(..) => {
self.ret.layout.immediate_llvm_type(cx) self.ret.layout.immediate_llvm_type(cx)
} }
Expand All @@ -339,7 +337,7 @@ impl<'tcx> FnTypeLlvmExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
} }


let llarg_ty = match arg.mode { let llarg_ty = match arg.mode {
PassMode::Ignore(_) => continue, PassMode::Ignore => continue,
PassMode::Direct(_) => arg.layout.immediate_llvm_type(cx), PassMode::Direct(_) => arg.layout.immediate_llvm_type(cx),
PassMode::Pair(..) => { PassMode::Pair(..) => {
llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 0, true)); llargument_tys.push(arg.layout.scalar_pair_element_llvm_type(cx, 0, true));
Expand Down Expand Up @@ -408,7 +406,7 @@ impl<'tcx> FnTypeLlvmExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
apply(&ArgAttributes::new(), None); apply(&ArgAttributes::new(), None);
} }
match arg.mode { match arg.mode {
PassMode::Ignore(_) => {} PassMode::Ignore => {}
PassMode::Direct(ref attrs) | PassMode::Direct(ref attrs) |
PassMode::Indirect(ref attrs, None) => apply(attrs, Some(arg.layout.llvm_type(cx))), PassMode::Indirect(ref attrs, None) => apply(attrs, Some(arg.layout.llvm_type(cx))),
PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => { PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => {
Expand Down Expand Up @@ -455,7 +453,7 @@ impl<'tcx> FnTypeLlvmExt<'tcx> for FnType<'tcx, Ty<'tcx>> {
apply(&ArgAttributes::new(), None); apply(&ArgAttributes::new(), None);
} }
match arg.mode { match arg.mode {
PassMode::Ignore(_) => {} PassMode::Ignore => {}
PassMode::Direct(ref attrs) | PassMode::Direct(ref attrs) |
PassMode::Indirect(ref attrs, None) => apply(attrs, Some(arg.layout.llvm_type(bx))), PassMode::Indirect(ref attrs, None) => apply(attrs, Some(arg.layout.llvm_type(bx))),
PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => { PassMode::Indirect(ref attrs, Some(ref extra_attrs)) => {
Expand Down
32 changes: 3 additions & 29 deletions src/librustc_codegen_ssa/mir/block.rs
Expand Up @@ -3,7 +3,7 @@ use rustc::ty::{self, Ty, TypeFoldable, Instance};
use rustc::ty::layout::{self, LayoutOf, HasTyCtxt, FnTypeExt}; use rustc::ty::layout::{self, LayoutOf, HasTyCtxt, FnTypeExt};
use rustc::mir::{self, Place, PlaceBase, Static, StaticKind}; use rustc::mir::{self, Place, PlaceBase, Static, StaticKind};
use rustc::mir::interpret::PanicInfo; use rustc::mir::interpret::PanicInfo;
use rustc_target::abi::call::{ArgType, FnType, PassMode, IgnoreMode}; use rustc_target::abi::call::{ArgType, FnType, PassMode};
use rustc_target::spec::abi::Abi; use rustc_target::spec::abi::Abi;
use crate::base; use crate::base;
use crate::MemFlags; use crate::MemFlags;
Expand Down Expand Up @@ -242,15 +242,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
return; return;
} }
let llval = match self.fn_ty.ret.mode { let llval = match self.fn_ty.ret.mode {
PassMode::Ignore(IgnoreMode::Zst) | PassMode::Indirect(..) => { PassMode::Ignore | PassMode::Indirect(..) => {
bx.ret_void(); bx.ret_void();
return; return;
} }


PassMode::Ignore(IgnoreMode::CVarArgs) => {
bug!("C-variadic arguments should never be the return type");
}

PassMode::Direct(_) | PassMode::Pair(..) => { PassMode::Direct(_) | PassMode::Pair(..) => {
let op = let op =
self.codegen_consume(&mut bx, &mir::Place::return_place().as_ref()); self.codegen_consume(&mut bx, &mir::Place::return_place().as_ref());
Expand Down Expand Up @@ -502,10 +498,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
return; return;
} }


// The "spoofed" `VaListImpl` added to a C-variadic functions signature let extra_args = &args[sig.inputs().len()..];
// should not be included in the `extra_args` calculation.
let extra_args_start_idx = sig.inputs().len() - if sig.c_variadic { 1 } else { 0 };
let extra_args = &args[extra_args_start_idx..];
let extra_args = extra_args.iter().map(|op_arg| { let extra_args = extra_args.iter().map(|op_arg| {
let op_ty = op_arg.ty(self.mir, bx.tcx()); let op_ty = op_arg.ty(self.mir, bx.tcx());
self.monomorphize(&op_ty) self.monomorphize(&op_ty)
Expand Down Expand Up @@ -691,26 +684,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
(&args[..], None) (&args[..], None)
}; };


// Useful determining if the current argument is the "spoofed" `VaListImpl`
let last_arg_idx = if sig.inputs().is_empty() {
None
} else {
Some(sig.inputs().len() - 1)
};
'make_args: for (i, arg) in first_args.iter().enumerate() { 'make_args: for (i, arg) in first_args.iter().enumerate() {
// If this is a C-variadic function the function signature contains
// an "spoofed" `VaListImpl`. This argument is ignored, but we need to
// populate it with a dummy operand so that the users real arguments
// are not overwritten.
let i = if sig.c_variadic && last_arg_idx.map(|x| i >= x).unwrap_or(false) {
if i + 1 < fn_ty.args.len() {
i + 1
} else {
break 'make_args
}
} else {
i
};
let mut op = self.codegen_operand(&mut bx, arg); let mut op = self.codegen_operand(&mut bx, arg);


if let (0, Some(ty::InstanceDef::Virtual(_, idx))) = (i, def) { if let (0, Some(ty::InstanceDef::Virtual(_, idx))) = (i, def) {
Expand Down
58 changes: 30 additions & 28 deletions src/librustc_codegen_ssa/mir/mod.rs
Expand Up @@ -2,7 +2,7 @@ use rustc::ty::{self, Ty, TypeFoldable, UpvarSubsts, Instance};
use rustc::ty::layout::{TyLayout, HasTyCtxt, FnTypeExt}; use rustc::ty::layout::{TyLayout, HasTyCtxt, FnTypeExt};
use rustc::mir::{self, Body}; use rustc::mir::{self, Body};
use rustc::session::config::DebugInfo; use rustc::session::config::DebugInfo;
use rustc_target::abi::call::{FnType, PassMode, IgnoreMode}; use rustc_target::abi::call::{FnType, PassMode};
use rustc_target::abi::{Variants, VariantIdx}; use rustc_target::abi::{Variants, VariantIdx};
use crate::base; use crate::base;
use crate::debuginfo::{self, VariableAccess, VariableKind, FunctionDebugContext}; use crate::debuginfo::{self, VariableAccess, VariableKind, FunctionDebugContext};
Expand Down Expand Up @@ -441,15 +441,6 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
None None
}; };


// Store the index of the last argument. This is used to
// call va_start on the va_list instead of attempting
// to store_fn_arg.
let last_arg_idx = if fx.fn_ty.args.is_empty() {
None
} else {
Some(fx.fn_ty.args.len() - 1)
};

mir.args_iter().enumerate().map(|(arg_index, local)| { mir.args_iter().enumerate().map(|(arg_index, local)| {
let arg_decl = &mir.local_decls[local]; let arg_decl = &mir.local_decls[local];


Expand Down Expand Up @@ -503,6 +494,33 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
return LocalRef::Place(place); return LocalRef::Place(place);
} }


if fx.fn_ty.c_variadic && arg_index == fx.fn_ty.args.len() {
let arg_ty = fx.monomorphize(&arg_decl.ty);

let va_list = PlaceRef::alloca(bx, bx.layout_of(arg_ty));
bx.set_var_name(va_list.llval, name);
bx.va_start(va_list.llval);
// FIXME(eddyb) remove `va_list_ref`.
*va_list_ref = Some(va_list);

arg_scope.map(|scope| {
let variable_access = VariableAccess::DirectVariable {
alloca: va_list.llval
};
bx.declare_local(
&fx.debug_context,
arg_decl.name.unwrap_or(kw::Invalid),
va_list.layout.ty,
scope,
variable_access,
VariableKind::ArgumentVariable(arg_index + 1),
DUMMY_SP
);
});

return LocalRef::Place(va_list);
}

let arg = &fx.fn_ty.args[idx]; let arg = &fx.fn_ty.args[idx];
idx += 1; idx += 1;
if arg.pad.is_some() { if arg.pad.is_some() {
Expand All @@ -515,10 +533,9 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
// of putting everything in allocas just so we can use llvm.dbg.declare. // of putting everything in allocas just so we can use llvm.dbg.declare.
let local = |op| LocalRef::Operand(Some(op)); let local = |op| LocalRef::Operand(Some(op));
match arg.mode { match arg.mode {
PassMode::Ignore(IgnoreMode::Zst) => { PassMode::Ignore => {
return local(OperandRef::new_zst(bx, arg.layout)); return local(OperandRef::new_zst(bx, arg.layout));
} }
PassMode::Ignore(IgnoreMode::CVarArgs) => {}
PassMode::Direct(_) => { PassMode::Direct(_) => {
let llarg = bx.get_param(llarg_idx); let llarg = bx.get_param(llarg_idx);
bx.set_var_name(llarg, &name); bx.set_var_name(llarg, &name);
Expand Down Expand Up @@ -568,22 +585,7 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
} else { } else {
let tmp = PlaceRef::alloca(bx, arg.layout); let tmp = PlaceRef::alloca(bx, arg.layout);
bx.set_var_name(tmp.llval, name); bx.set_var_name(tmp.llval, name);
if fx.fn_ty.c_variadic && last_arg_idx.map(|idx| arg_index == idx).unwrap_or(false) { bx.store_fn_arg(arg, &mut llarg_idx, tmp);
let va_list_did = match tcx.lang_items().va_list() {
Some(did) => did,
None => bug!("`va_list` lang item required for C-variadic functions"),
};
match arg_decl.ty.kind {
ty::Adt(def, _) if def.did == va_list_did => {
// Call `va_start` on the spoofed `VaListImpl`.
bx.va_start(tmp.llval);
*va_list_ref = Some(tmp);
},
_ => bug!("last argument of variadic function is not a `va_list`")
}
} else {
bx.store_fn_arg(arg, &mut llarg_idx, tmp);
}
tmp tmp
}; };
let upvar_debuginfo = &mir.__upvar_debuginfo_codegen_only_do_not_use; let upvar_debuginfo = &mir.__upvar_debuginfo_codegen_only_do_not_use;
Expand Down
9 changes: 1 addition & 8 deletions src/librustc_lint/types.rs
Expand Up @@ -944,15 +944,8 @@ impl<'a, 'tcx> ImproperCTypesVisitor<'a, 'tcx> {
let def_id = self.cx.tcx.hir().local_def_id(id); let def_id = self.cx.tcx.hir().local_def_id(id);
let sig = self.cx.tcx.fn_sig(def_id); let sig = self.cx.tcx.fn_sig(def_id);
let sig = self.cx.tcx.erase_late_bound_regions(&sig); let sig = self.cx.tcx.erase_late_bound_regions(&sig);
let inputs = if sig.c_variadic {
// Don't include the spoofed `VaListImpl` in the functions list
// of inputs.
&sig.inputs()[..sig.inputs().len() - 1]
} else {
&sig.inputs()[..]
};


for (input_ty, input_hir) in inputs.iter().zip(&decl.inputs) { for (input_ty, input_hir) in sig.inputs().iter().zip(&decl.inputs) {
self.check_type_for_ffi_and_report_errors(input_hir.span, input_ty); self.check_type_for_ffi_and_report_errors(input_hir.span, input_ty);
} }


Expand Down

0 comments on commit a88d181

Please sign in to comment.