Skip to content
Merged
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
12 changes: 5 additions & 7 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -698,13 +698,11 @@ impl<'a> AstValidator<'a> {
unreachable!("C variable argument list cannot be used in closures")
};

// C-variadics are not yet implemented in const evaluation.
if let Const::Yes(const_span) = sig.header.constness {
self.dcx().emit_err(errors::ConstAndCVariadic {
spans: vec![const_span, variadic_param.span],
const_span,
variadic_span: variadic_param.span,
});
if let Const::Yes(_) = sig.header.constness
&& !self.features.enabled(sym::const_c_variadic)
{
let msg = format!("c-variadic const function definitions are unstable");
feature_err(&self.sess, sym::const_c_variadic, sig.span, msg).emit();
}

if let Some(coroutine_kind) = sig.header.coroutine_kind {
Expand Down
11 changes: 0 additions & 11 deletions compiler/rustc_ast_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,17 +823,6 @@ pub(crate) struct ConstAndCoroutine {
pub coroutine_kind: &'static str,
}

#[derive(Diagnostic)]
#[diag("functions cannot be both `const` and C-variadic")]
pub(crate) struct ConstAndCVariadic {
#[primary_span]
pub spans: Vec<Span>,
#[label("`const` because of this")]
pub const_span: Span,
#[label("C-variadic because of this")]
pub variadic_span: Span,
}

#[derive(Diagnostic)]
#[diag("functions cannot be both `{$coroutine_kind}` and C-variadic")]
pub(crate) struct CoroutineAndCVariadic {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,10 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
});
}

if self.tcx.fn_sig(callee).skip_binder().c_variadic() {
self.check_op(ops::FnCallCVariadic)
}

// At this point, we are calling a function, `callee`, whose `DefId` is known...

// `begin_panic` and `panic_display` functions accept generic
Expand Down
21 changes: 21 additions & 0 deletions compiler/rustc_const_eval/src/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,27 @@ impl<'tcx> NonConstOp<'tcx> for FnCallIndirect {
}
}

/// A c-variadic function call.
#[derive(Debug)]
pub(crate) struct FnCallCVariadic;
impl<'tcx> NonConstOp<'tcx> for FnCallCVariadic {
fn status_in_item(&self, _ccx: &ConstCx<'_, 'tcx>) -> Status {
Status::Unstable {
gate: sym::const_c_variadic,
gate_already_checked: false,
safe_to_expose_on_stable: false,
is_function_call: true,
}
}

fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> Diag<'tcx> {
ccx.tcx.sess.create_feature_err(
errors::NonConstCVariadicCall { span, kind: ccx.const_kind() },
sym::const_c_variadic,
)
}
}

/// A call to a function that is in a trait, or has trait bounds that make it conditionally-const.
#[derive(Debug)]
pub(crate) struct ConditionallyConstCall<'tcx> {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_errors::msg;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::{self as hir, CRATE_HIR_ID, LangItem};
use rustc_middle::mir::AssertMessage;
use rustc_middle::mir::interpret::{Pointer, ReportedErrorInfo};
use rustc_middle::mir::interpret::ReportedErrorInfo;
use rustc_middle::query::TyCtxtAt;
use rustc_middle::ty::layout::{HasTypingEnv, TyAndLayout, ValidityRequirement};
use rustc_middle::ty::{self, Ty, TyCtxt};
Expand All @@ -22,7 +22,7 @@ use super::error::*;
use crate::errors::{LongRunning, LongRunningWarn};
use crate::interpret::{
self, AllocId, AllocInit, AllocRange, ConstAllocation, CtfeProvenance, FnArg, Frame,
GlobalAlloc, ImmTy, InterpCx, InterpResult, OpTy, PlaceTy, RangeSet, Scalar,
GlobalAlloc, ImmTy, InterpCx, InterpResult, OpTy, PlaceTy, Pointer, RangeSet, Scalar,
compile_time_machine, err_inval, interp_ok, throw_exhaust, throw_inval, throw_ub,
throw_ub_custom, throw_unsup, throw_unsup_format,
};
Expand Down
33 changes: 32 additions & 1 deletion compiler/rustc_const_eval/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,19 @@ pub struct NonConstClosure {
pub non_or_conditionally: &'static str,
}

#[derive(Diagnostic)]
#[diag(r#"calling const c-variadic functions is unstable in {$kind ->
[const] constant
[static] static
[const_fn] constant function
*[other] {""}
}s"#, code = E0015)]
pub struct NonConstCVariadicCall {
#[primary_span]
pub span: Span,
pub kind: ConstContext,
}

#[derive(Subdiagnostic)]
pub enum NonConstClosureNote {
#[note("function defined here, but it is not `const`")]
Expand Down Expand Up @@ -757,11 +770,13 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
WriteToReadOnly(_) => msg!("writing to {$allocation} which is read-only"),
DerefFunctionPointer(_) => msg!("accessing {$allocation} which contains a function"),
DerefVTablePointer(_) => msg!("accessing {$allocation} which contains a vtable"),
DerefVaListPointer(_) => msg!("accessing {$allocation} which contains a variable argument list"),
DerefTypeIdPointer(_) => msg!("accessing {$allocation} which contains a `TypeId`"),
InvalidBool(_) => msg!("interpreting an invalid 8-bit value as a bool: 0x{$value}"),
InvalidChar(_) => msg!("interpreting an invalid 32-bit value as a char: 0x{$value}"),
InvalidTag(_) => msg!("enum value has invalid tag: {$tag}"),
InvalidFunctionPointer(_) => msg!("using {$pointer} as function pointer but it does not point to a function"),
InvalidVaListPointer(_) => msg!("using {$pointer} as variable argument list pointer but it does not point to a variable argument list"),
InvalidVTablePointer(_) => msg!("using {$pointer} as vtable pointer but it does not point to a vtable"),
InvalidVTableTrait { .. } => msg!("using vtable for `{$vtable_dyn_type}` but `{$expected_dyn_type}` was expected"),
InvalidStr(_) => msg!("this string is not valid UTF-8: {$err}"),
Expand All @@ -776,6 +791,9 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
}
AbiMismatchArgument { .. } => msg!("calling a function whose parameter #{$arg_idx} has type {$callee_ty} passing argument of type {$caller_ty}"),
AbiMismatchReturn { .. } => msg!("calling a function with return type {$callee_ty} passing return place of type {$caller_ty}"),
VaArgOutOfBounds => "more C-variadic arguments read than were passed".into(),
CVariadicMismatch { ..} => "calling a function where the caller and callee disagree on whether the function is C-variadic".into(),
CVariadicFixedCountMismatch { .. } => msg!("calling a C-variadic function with {$caller} fixed arguments, but the function expects {$callee}"),
}
}

Expand All @@ -800,6 +818,7 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
| InvalidMeta(InvalidMetaKind::TooBig)
| InvalidUninitBytes(None)
| DeadLocal
| VaArgOutOfBounds
| UninhabitedEnumVariantWritten(_)
| UninhabitedEnumVariantRead(_) => {}

Expand All @@ -820,7 +839,10 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
diag.arg("len", len);
diag.arg("index", index);
}
UnterminatedCString(ptr) | InvalidFunctionPointer(ptr) | InvalidVTablePointer(ptr) => {
UnterminatedCString(ptr)
| InvalidFunctionPointer(ptr)
| InvalidVaListPointer(ptr)
| InvalidVTablePointer(ptr) => {
diag.arg("pointer", ptr);
}
InvalidVTableTrait { expected_dyn_type, vtable_dyn_type } => {
Expand Down Expand Up @@ -874,6 +896,7 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
WriteToReadOnly(alloc)
| DerefFunctionPointer(alloc)
| DerefVTablePointer(alloc)
| DerefVaListPointer(alloc)
| DerefTypeIdPointer(alloc) => {
diag.arg("allocation", alloc);
}
Expand Down Expand Up @@ -910,6 +933,14 @@ impl<'a> ReportErrorExt for UndefinedBehaviorInfo<'a> {
diag.arg("caller_ty", caller_ty);
diag.arg("callee_ty", callee_ty);
}
CVariadicMismatch { caller_is_c_variadic, callee_is_c_variadic } => {
diag.arg("caller_is_c_variadic", caller_is_c_variadic);
diag.arg("callee_is_c_variadic", callee_is_c_variadic);
}
CVariadicFixedCountMismatch { caller, callee } => {
diag.arg("caller", caller);
diag.arg("callee", callee);
}
}
}
}
Expand Down
66 changes: 56 additions & 10 deletions compiler/rustc_const_eval/src/interpret/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use tracing::{info, instrument, trace};
use super::{
CtfeProvenance, FnVal, ImmTy, InterpCx, InterpResult, MPlaceTy, Machine, OpTy, PlaceTy,
Projectable, Provenance, ReturnAction, ReturnContinuation, Scalar, StackPopInfo, interp_ok,
throw_ub, throw_ub_custom, throw_unsup_format,
throw_ub, throw_ub_custom,
};
use crate::enter_trace_span;
use crate::interpret::EnteredTraceSpan;
Expand Down Expand Up @@ -354,13 +354,18 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
) -> InterpResult<'tcx> {
let _trace = enter_trace_span!(M, step::init_stack_frame, %instance, tracing_separate_thread = Empty);

// Compute callee information.
// FIXME: for variadic support, do we have to somehow determine callee's extra_args?
let callee_fn_abi = self.fn_abi_of_instance(instance, ty::List::empty())?;

if callee_fn_abi.c_variadic || caller_fn_abi.c_variadic {
throw_unsup_format!("calling a c-variadic function is not supported");
}
// The first order of business is to figure out the callee signature.
// However, that requires the list of variadic arguments.
// We use the *caller* information to determine where to split the list of arguments,
// and then later check that the callee indeed has the same number of fixed arguments.
let extra_tys = if caller_fn_abi.c_variadic {
let fixed_count = usize::try_from(caller_fn_abi.fixed_count).unwrap();
let extra_tys = args[fixed_count..].iter().map(|arg| arg.layout().ty);
self.tcx.mk_type_list_from_iter(extra_tys)
} else {
ty::List::empty()
};
let callee_fn_abi = self.fn_abi_of_instance(instance, extra_tys)?;

if caller_fn_abi.conv != callee_fn_abi.conv {
throw_ub_custom!(
Expand All @@ -372,6 +377,19 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
)
}

if caller_fn_abi.c_variadic != callee_fn_abi.c_variadic {
throw_ub!(CVariadicMismatch {
caller_is_c_variadic: caller_fn_abi.c_variadic,
callee_is_c_variadic: callee_fn_abi.c_variadic,
});
}
if caller_fn_abi.c_variadic && caller_fn_abi.fixed_count != callee_fn_abi.fixed_count {
throw_ub!(CVariadicFixedCountMismatch {
caller: caller_fn_abi.fixed_count,
callee: callee_fn_abi.fixed_count,
});
}

// Check that all target features required by the callee (i.e., from
// the attribute `#[target_feature(enable = ...)]`) are enabled at
// compile time.
Expand Down Expand Up @@ -444,6 +462,10 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// `pass_argument` would be the loop body. It takes care to
// not advance `caller_iter` for ignored arguments.
let mut callee_args_abis = callee_fn_abi.args.iter().enumerate();
// Determine whether there is a special VaList argument. This is always the
// last argument, and since arguments start at index 1 that's `arg_count`.
let va_list_arg =
callee_fn_abi.c_variadic.then(|| mir::Local::from_usize(body.arg_count));
for local in body.args_iter() {
// Construct the destination place for this argument. At this point all
// locals are still dead, so we cannot construct a `PlaceTy`.
Expand All @@ -452,7 +474,31 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
// type, but the result gets cached so this avoids calling the instantiation
// query *again* the next time this local is accessed.
let ty = self.layout_of_local(self.frame(), local, None)?.ty;
if Some(local) == body.spread_arg {
if Some(local) == va_list_arg {
// This is the last callee-side argument of a variadic function.
// This argument is a VaList holding the remaining caller-side arguments.
self.storage_live(local)?;

let place = self.eval_place(dest)?;
let mplace = self.force_allocation(&place)?;

// Consume the remaining arguments by putting them into the variable argument
// list.
let varargs = self.allocate_varargs(&mut caller_args, &mut callee_args_abis)?;
// When the frame is dropped, these variable arguments are deallocated.
self.frame_mut().va_list = varargs.clone();
let key = self.va_list_ptr(varargs.into());

// Zero the VaList, so it is fully initialized.
self.write_bytes_ptr(
mplace.ptr(),
(0..mplace.layout.size.bytes()).map(|_| 0u8),
)?;

// Store the "key" pointer in the right field.
let key_mplace = self.va_list_key_field(&mplace)?;
self.write_pointer(key, &key_mplace)?;
} else if Some(local) == body.spread_arg {
// Make the local live once, then fill in the value field by field.
self.storage_live(local)?;
// Must be a tuple
Expand Down Expand Up @@ -491,7 +537,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
if instance.def.requires_caller_location(*self.tcx) {
callee_args_abis.next().unwrap();
}
// Now we should have no more caller args or callee arg ABIs
// Now we should have no more caller args or callee arg ABIs.
assert!(
callee_args_abis.next().is_none(),
"mismatch between callee ABI and callee body arguments"
Expand Down
77 changes: 75 additions & 2 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ use super::memory::MemoryKind;
use super::util::ensure_monomorphic_enough;
use super::{
AllocId, CheckInAllocMsg, ImmTy, InterpCx, InterpResult, Machine, OpTy, PlaceTy, Pointer,
PointerArithmetic, Provenance, Scalar, err_ub_custom, err_unsup_format, interp_ok, throw_inval,
throw_ub_custom, throw_ub_format,
PointerArithmetic, Projectable, Provenance, Scalar, err_ub_custom, err_unsup_format, interp_ok,
throw_inval, throw_ub, throw_ub_custom, throw_ub_format, throw_unsup_format,
};
use crate::interpret::Writeable;

Expand Down Expand Up @@ -750,6 +750,57 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
self.float_muladd_intrinsic::<Quad>(args, dest, MulAddType::Nondeterministic)?
}

sym::va_copy => {
let va_list = self.deref_pointer(&args[0])?;
let key_mplace = self.va_list_key_field(&va_list)?;
let key = self.read_pointer(&key_mplace)?;

let varargs = self.get_ptr_va_list(key)?;
let copy_key = self.va_list_ptr(varargs.clone());

let copy_key_mplace = self.va_list_key_field(dest)?;
self.write_pointer(copy_key, &copy_key_mplace)?;
}

sym::va_end => {
let va_list = self.deref_pointer(&args[0])?;
let key_mplace = self.va_list_key_field(&va_list)?;
let key = self.read_pointer(&key_mplace)?;

self.deallocate_va_list(key)?;
}

sym::va_arg => {
let va_list = self.deref_pointer(&args[0])?;
let key_mplace = self.va_list_key_field(&va_list)?;
let key = self.read_pointer(&key_mplace)?;

// Invalidate the old list and get its content. We'll recreate the
// new list (one element shorter) below.
let mut varargs = self.deallocate_va_list(key)?;

let Some(arg_mplace) = varargs.pop_front() else {
throw_ub!(VaArgOutOfBounds);
};

// NOTE: In C some type conversions are allowed (e.g. casting between signed and
// unsigned integers). For now we require c-variadic arguments to be read with the
// exact type they were passed as.
if arg_mplace.layout.ty != dest.layout.ty {
throw_unsup_format!(
"va_arg type mismatch: requested `{}`, but next argument is `{}`",
dest.layout.ty,
arg_mplace.layout.ty
);
}
Comment on lines +789 to +795
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here we do a type comparison, would it be more idiomatic to use layout_compat That would let through usize to u32/u64 conversions and I think pointer casts as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a bit less straightforward to describe than a simple equality though.

Copy link
Member

Choose a reason for hiding this comment

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

I would suggest to open an issue about this, and keep the most strict check for now. I am not entirely certain that layout_compat is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's just go with the simple rule for now then.

// Copy the argument.
self.copy_op(&arg_mplace, dest)?;

// Update the VaList pointer.
let new_key = self.va_list_ptr(varargs);
self.write_pointer(new_key, &key_mplace)?;
}

// Unsupported intrinsic: skip the return_to_block below.
_ => return interp_ok(false),
}
Expand Down Expand Up @@ -1230,4 +1281,26 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
interp_ok(Some(ImmTy::from_scalar(val, cast_to)))
}
}

/// Get the MPlace of the key from the place storing the VaList.
pub(super) fn va_list_key_field<P: Projectable<'tcx, M::Provenance>>(
&self,
va_list: &P,
) -> InterpResult<'tcx, P> {
// The struct wrapped by VaList.
let va_list_inner = self.project_field(va_list, FieldIdx::ZERO)?;

// Find the first pointer field in this struct. The exact index is target-specific.
let ty::Adt(adt, substs) = va_list_inner.layout().ty.kind() else {
bug!("invalid VaListImpl layout");
};

for (i, field) in adt.non_enum_variant().fields.iter().enumerate() {
if field.ty(*self.tcx, substs).is_raw_ptr() {
return self.project_field(&va_list_inner, FieldIdx::from_usize(i));
}
}

bug!("no VaListImpl field is a pointer");
}
}
Loading
Loading