Skip to content

Move various checks to typeck so them failing causes the typeck result to get tainted #96046

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 9 commits into from
May 27, 2022
12 changes: 4 additions & 8 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -906,16 +906,12 @@ where
}
// We still require the sizes to match.
if src.layout.size != dest.layout.size {
// FIXME: This should be an assert instead of an error, but if we transmute within an
// array length computation, `typeck` may not have yet been run and errored out. In fact
// most likely we *are* running `typeck` right now. Investigate whether we can bail out
// on `typeck_results().has_errors` at all const eval entry points.
debug!("Size mismatch when transmuting!\nsrc: {:#?}\ndest: {:#?}", src, dest);
self.tcx.sess.delay_span_bug(
span_bug!(
self.cur_span(),
"size-changing transmute, should have been caught by transmute checking",
"size-changing transmute, should have been caught by transmute checking: {:#?}\ndest: {:#?}",
src,
dest
);
throw_inval!(TransmuteSizeDiff(src.layout.ty, dest.layout.ty));
}
// Unsized copies rely on interpreting `src.meta` with `dest.layout`, we want
// to avoid that here.
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,6 @@ fn analysis(tcx: TyCtxt<'_>, (): ()) -> Result<()> {
//
// maybe move the check to a MIR pass?
tcx.ensure().check_mod_liveness(module);
tcx.ensure().check_mod_intrinsics(module);
});
});
}
Expand Down
7 changes: 0 additions & 7 deletions compiler/rustc_middle/src/mir/interpret/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,6 @@ pub enum InvalidProgramInfo<'tcx> {
/// (which unfortunately typeck does not reject).
/// Not using `FnAbiError` as that contains a nested `LayoutError`.
FnAbiAdjustForForeignAbi(call::AdjustForForeignAbiError),
/// An invalid transmute happened.
TransmuteSizeDiff(Ty<'tcx>, Ty<'tcx>),
Copy link
Member

Choose a reason for hiding this comment

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

That's great. :D So does this also fix #79047 ?

What about SizeOfUnsizedType, which seems similar to me?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

similar, but when I looked into it it was a different beast that I'll tackle seperately

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's great. :D So does this also fix #79047 ?

yes, the corresponding test has been updated

Copy link
Member

@RalfJung RalfJung Apr 14, 2022

Choose a reason for hiding this comment

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

We might want to keep the issue open for SizeOfUnsizedType though. Do we have a nice example of that (or an existing separate issue)?

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 think I saw an example when I removed that variant ^^ but I don't remember where

Copy link
Member

Choose a reason for hiding this comment

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

What about SizeOfUnsizedType, which seems similar to me?

I have opened #97477 for this.

/// SizeOf of unsized type was requested.
SizeOfUnsizedType(Ty<'tcx>),
}
Expand All @@ -166,11 +164,6 @@ impl fmt::Display for InvalidProgramInfo<'_> {
}
Layout(ref err) => write!(f, "{}", err),
FnAbiAdjustForForeignAbi(ref err) => write!(f, "{}", err),
TransmuteSizeDiff(from_ty, to_ty) => write!(
f,
"transmuting `{}` to `{}` is not possible, because these types do not have the same size",
from_ty, to_ty
),
SizeOfUnsizedType(ty) => write!(f, "size_of called on unsized type `{}`", ty),
}
}
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -804,10 +804,6 @@ rustc_queries! {
desc { |tcx| "checking privacy in {}", describe_as_module(key, tcx) }
}

query check_mod_intrinsics(key: LocalDefId) -> () {
desc { |tcx| "checking intrinsics in {}", describe_as_module(key, tcx) }
}

query check_mod_liveness(key: LocalDefId) -> () {
desc { |tcx| "checking liveness of variables in {}", describe_as_module(key, tcx) }
}
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_passes/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ mod diagnostic_items;
pub mod entry;
pub mod hir_id_validator;
pub mod hir_stats;
mod intrinsicck;
mod lang_items;
pub mod layout_test;
mod lib_features;
Expand All @@ -54,7 +53,6 @@ pub fn provide(providers: &mut Providers) {
loops::provide(providers);
naked_functions::provide(providers);
liveness::provide(providers);
intrinsicck::provide(providers);
reachable::provide(providers);
stability::provide(providers);
upvars::provide(providers);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
error: &SelectionError<'tcx>,
fallback_has_occurred: bool,
) {
self.set_tainted_by_errors();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this change anything?

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. It avoids ICEs and bogus errors in CTFE.

let tcx = self.tcx;
let mut span = obligation.cause.span;

Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_typeck/src/check/check.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use crate::check::wfcheck::for_item;

use super::coercion::CoerceMany;
use super::compare_method::check_type_bounds;
use super::compare_method::{compare_const_impl, compare_impl_method, compare_ty_impl};
Expand Down Expand Up @@ -871,6 +873,14 @@ pub fn check_item_type<'tcx>(tcx: TyCtxt<'tcx>, id: hir::ItemId) {
}
}
}
DefKind::GlobalAsm => {
let it = tcx.hir().item(id);
let hir::ItemKind::GlobalAsm(asm) = it.kind else { span_bug!(it.span, "DefKind::GlobalAsm but got {:#?}", it) };
for_item(tcx, it).with_fcx(|fcx| {
fcx.check_asm(asm, it.hir_id());
Default::default()
})
}
_ => {}
}
}
Expand Down
18 changes: 16 additions & 2 deletions compiler/rustc_typeck/src/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ use rustc_span::lev_distance::find_best_match_for_name;
use rustc_span::source_map::Span;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{BytePos, Pos};
use rustc_target::spec::abi::Abi::RustIntrinsic;
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::{self, ObligationCauseCode};

Expand Down Expand Up @@ -294,7 +295,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.check_lang_item_path(lang_item, expr, hir_id)
}
ExprKind::Path(ref qpath) => self.check_expr_path(qpath, expr, &[]),
ExprKind::InlineAsm(asm) => self.check_expr_asm(asm),
ExprKind::InlineAsm(asm) => {
// We defer some asm checks as we may not have resolved the input and output types yet (they may still be infer vars).
self.deferred_asm_checks.borrow_mut().push((asm, expr.hir_id));
self.check_expr_asm(asm)
}
ExprKind::Break(destination, ref expr_opt) => {
self.check_expr_break(destination, expr_opt.as_deref(), expr)
}
Expand Down Expand Up @@ -530,8 +535,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
_ => self.instantiate_value_path(segs, opt_ty, res, expr.span, expr.hir_id).0,
};

if let ty::FnDef(..) = ty.kind() {
if let ty::FnDef(did, ..) = *ty.kind() {
let fn_sig = ty.fn_sig(tcx);
if tcx.fn_sig(did).abi() == RustIntrinsic && tcx.item_name(did) == sym::transmute {
let from = fn_sig.inputs().skip_binder()[0];
let to = fn_sig.output().skip_binder();
// We defer the transmute to the end of typeck, once all inference vars have
// been resolved or we errored. This is important as we can only check transmute
// on concrete types, but the output type may not be known yet (it would only
// be known if explicitly specified via turbofish).
self.deferred_transmute_checks.borrow_mut().push((from, to, expr.span));
}
if !tcx.features().unsized_fn_params {
// We want to remove some Sized bounds from std functions,
// but don't want to expose the removal to stable Rust.
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

pub(in super::super) fn check_transmutes(&self) {
let mut deferred_transmute_checks = self.deferred_transmute_checks.borrow_mut();
debug!("FnCtxt::check_transmutes: {} deferred checks", deferred_transmute_checks.len());
for (from, to, span) in deferred_transmute_checks.drain(..) {
self.check_transmute(span, from, to);
}
}

pub(in super::super) fn check_asms(&self) {
let mut deferred_asm_checks = self.deferred_asm_checks.borrow_mut();
debug!("FnCtxt::check_asm: {} deferred checks", deferred_asm_checks.len());
for (asm, hir_id) in deferred_asm_checks.drain(..) {
let enclosing_id = self.tcx.hir().enclosing_body_owner(hir_id);
self.check_asm(asm, enclosing_id);
}
}

pub(in super::super) fn check_method_argument_types(
&self,
sp: Span,
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_typeck/src/check/inherited.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ pub struct Inherited<'a, 'tcx> {

pub(super) deferred_cast_checks: RefCell<Vec<super::cast::CastCheck<'tcx>>>,

pub(super) deferred_transmute_checks: RefCell<Vec<(Ty<'tcx>, Ty<'tcx>, Span)>>,

pub(super) deferred_asm_checks: RefCell<Vec<(&'tcx hir::InlineAsm<'tcx>, hir::HirId)>>,

pub(super) deferred_generator_interiors:
RefCell<Vec<(hir::BodyId, Ty<'tcx>, hir::GeneratorKind)>>,

Expand Down Expand Up @@ -113,6 +117,8 @@ impl<'a, 'tcx> Inherited<'a, 'tcx> {
deferred_sized_obligations: RefCell::new(Vec::new()),
deferred_call_resolutions: RefCell::new(Default::default()),
deferred_cast_checks: RefCell::new(Vec::new()),
deferred_transmute_checks: RefCell::new(Vec::new()),
deferred_asm_checks: RefCell::new(Vec::new()),
deferred_generator_interiors: RefCell::new(Vec::new()),
diverging_type_vars: RefCell::new(Default::default()),
body_id,
Expand Down
Loading