Skip to content
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

Constant gets evaluated even when it contains invalid transmute #79047

Closed
RalfJung opened this issue Nov 14, 2020 · 3 comments · Fixed by #96046
Closed

Constant gets evaluated even when it contains invalid transmute #79047

RalfJung opened this issue Nov 14, 2020 · 3 comments · Fixed by #96046
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

Consider the following code:

const DST: &[u8] = unsafe { std::mem::transmute(1usize) };

fn main() {
    match &b""[..] {
        DST => {}
    }
}

This code hits a peculiar code path in the interpreter:

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 likey 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(
self.cur_span(),
"size-changing transmute, should have been caught by transmute checking",
);
throw_inval!(TransmuteSizeDiff(src.layout.ty, dest.layout.ty));
}

This code path should be unreachable: just like the interpreter can assume that code code it runs on is well-typed, it should be able to assume that the code it runs on has its transmutes checked. But something seems to be different about transmute when compared with "normal" type-checking.

TransmuteSizeDiff is a hack; we should instead arrange things in a way that failing the transmute check inhibits const-evaluation the same way that true + 4 inhibitis const-evaluation.

@jonas-schievink jonas-schievink added A-const-eval Area: Constant evaluation (MIR interpretation) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 14, 2020
@RalfJung
Copy link
Member Author

RalfJung commented Jan 22, 2021

The same issue also arises with SizeOf and its requirement that the type be sized.
Cc #81185

@osa1
Copy link
Contributor

osa1 commented Jan 24, 2021

The same issue also arises with SizeOf and its requirement that the type be sized.

Is it really the same issue? My understanding is the SizeOf issue is a type error (as the argument's Sized bound is not satisfied), but the transmute error is caught not by the type checker, but rather it has its own check in

fn check_transmute(&self, span: Span, from: Ty<'tcx>, to: Ty<'tcx>) {
let sk_from = SizeSkeleton::compute(from, self.tcx, self.param_env);
let sk_to = SizeSkeleton::compute(to, self.tcx, self.param_env);
// Check for same size using the skeletons.
if let (Ok(sk_from), Ok(sk_to)) = (sk_from, sk_to) {
if sk_from.same_size(sk_to) {
return;
}
// Special-case transmuting from `typeof(function)` and
// `Option<typeof(function)>` to present a clearer error.
let from = unpack_option_like(self.tcx, from);
if let (&ty::FnDef(..), SizeSkeleton::Known(size_to)) = (from.kind(), sk_to) {
if size_to == Pointer.size(&self.tcx) {
struct_span_err!(self.tcx.sess, span, E0591, "can't transmute zero-sized type")
.note(&format!("source type: {}", from))
.note(&format!("target type: {}", to))
.help("cast with `as` to a pointer instead")
.emit();
return;
}
}
}
// Try to display a sensible error with as much information as possible.
let skeleton_string = |ty: Ty<'tcx>, sk| match sk {
Ok(SizeSkeleton::Known(size)) => format!("{} bits", size.bits()),
Ok(SizeSkeleton::Pointer { tail, .. }) => format!("pointer to `{}`", tail),
Err(LayoutError::Unknown(bad)) => {
if bad == ty {
"this type does not have a fixed size".to_owned()
} else {
format!("size can vary because of {}", bad)
}
}
Err(err) => err.to_string(),
};
let mut err = struct_span_err!(
self.tcx.sess,
span,
E0512,
"cannot transmute between types of different sizes, \
or dependently-sized types"
);
if from == to {
err.note(&format!("`{}` does not have a fixed size", from));
} else {
err.note(&format!("source type: `{}` ({})", from, skeleton_string(from, sk_from)))
.note(&format!("target type: `{}` ({})", to, skeleton_string(to, sk_to)));
}
err.emit()
}

So I think it's possible that we have two different bugs here.

@RalfJung
Copy link
Member Author

Well, maybe not exactly the same issue, but something very similar at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation (MIR interpretation) C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants