Skip to content

Commit

Permalink
Auto merge of #116930 - RalfJung:raw-ptr-match, r=<try>
Browse files Browse the repository at this point in the history
patterns: reject raw pointers that are not just integers

Matching against `0 as *const i32` is fine, matching against `&42 as *const i32` is not.

Cc `@oli-obk` `@lcnr`
  • Loading branch information
bors committed Oct 19, 2023
2 parents 36b61e5 + 0787e62 commit ebb1189
Show file tree
Hide file tree
Showing 10 changed files with 93 additions and 33 deletions.
37 changes: 27 additions & 10 deletions compiler/rustc_const_eval/src/const_eval/valtrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,27 @@ pub(crate) fn const_to_valtree_inner<'tcx>(
Ok(ty::ValTree::Leaf(val.assert_int()))
}

// Raw pointers are not allowed in type level constants, as we cannot properly test them for
// equality at compile-time (see `ptr_guaranteed_cmp`).
ty::RawPtr(_) => {
// Not all raw pointers are allowed, as we cannot properly test them for
// equality at compile-time (see `ptr_guaranteed_cmp`).
// However we allow those that are just integers in disguise.
// (We could allow wide raw pointers where both sides are integers in the future,
// but for now we reject them.)
let Ok(val) = ecx.read_scalar(place) else {
return Err(ValTreeCreationError::Other);
};
// We are in the CTFE machine, so ptr-to-int casts will fail.
// This can only be `Ok` if `val` already is an integer.
let Ok(val) = val.try_to_int() else {
return Err(ValTreeCreationError::Other);
};
// It's just a ScalarInt!
Ok(ty::ValTree::Leaf(val))
}

// Technically we could allow function pointers (represented as `ty::Instance`), but this is not guaranteed to
// agree with runtime equality tests.
ty::FnPtr(_) | ty::RawPtr(_) => Err(ValTreeCreationError::NonSupportedType),
ty::FnPtr(_) => Err(ValTreeCreationError::NonSupportedType),

ty::Ref(_, _, _) => {
let Ok(derefd_place)= ecx.deref_pointer(place) else {
Expand Down Expand Up @@ -222,12 +238,14 @@ pub fn valtree_to_const_value<'tcx>(
assert!(valtree.unwrap_branch().is_empty());
mir::ConstValue::ZeroSized
}
ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char => match valtree {
ty::ValTree::Leaf(scalar_int) => mir::ConstValue::Scalar(Scalar::Int(scalar_int)),
ty::ValTree::Branch(_) => bug!(
"ValTrees for Bool, Int, Uint, Float or Char should have the form ValTree::Leaf"
),
},
ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char | ty::RawPtr(_) => {
match valtree {
ty::ValTree::Leaf(scalar_int) => mir::ConstValue::Scalar(Scalar::Int(scalar_int)),
ty::ValTree::Branch(_) => bug!(
"ValTrees for Bool, Int, Uint, Float, Char or RawPtr should have the form ValTree::Leaf"
),
}
}
ty::Ref(_, inner_ty, _) => {
let mut ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No);
let imm = valtree_to_ref(&mut ecx, valtree, *inner_ty);
Expand Down Expand Up @@ -281,7 +299,6 @@ pub fn valtree_to_const_value<'tcx>(
| ty::Generator(..)
| ty::GeneratorWitness(..)
| ty::FnPtr(_)
| ty::RawPtr(_)
| ty::Str
| ty::Slice(_)
| ty::Dynamic(..) => bug!("no ValTree should have been created for type {:?}", ty.kind()),
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2217,15 +2217,16 @@ declare_lint! {
///
/// ### Explanation
///
/// Previous versions of Rust allowed function pointers and wide raw pointers in patterns.
/// Previous versions of Rust allowed function pointers and all raw pointers in patterns.
/// While these work in many cases as expected by users, it is possible that due to
/// optimizations pointers are "not equal to themselves" or pointers to different functions
/// compare as equal during runtime. This is because LLVM optimizations can deduplicate
/// functions if their bodies are the same, thus also making pointers to these functions point
/// to the same location. Additionally functions may get duplicated if they are instantiated
/// in different crates and not deduplicated again via LTO.
/// in different crates and not deduplicated again via LTO. Pointer identity for memory
/// created by `const` is similarly unreliable.
pub POINTER_STRUCTURAL_MATCH,
Allow,
Forbid,
"pointers are not structural-match",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ mir_build_overlapping_range_endpoints = multiple patterns overlap on their endpo
mir_build_pattern_not_covered = refutable pattern in {$origin}
.pattern_ty = the matched value is of type `{$pattern_ty}`
mir_build_pointer_pattern = function pointers and unsized pointers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details.
mir_build_pointer_pattern = function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details.
mir_build_privately_uninhabited = pattern `{$witness_1}` is currently uninhabited, but this variant contains private fields which may become inhabited in the future
Expand Down
33 changes: 25 additions & 8 deletions compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ impl<'tcx> ConstToPat<'tcx> {
});
debug!(?check_body_for_struct_match_violation, ?mir_structural_match_violation);

let have_valtree =
matches!(cv, mir::Const::Ty(c) if matches!(c.kind(), ty::ConstKind::Value(_)));
let inlined_const_as_pat = match cv {
mir::Const::Ty(c) => match c.kind() {
ty::ConstKind::Param(_)
Expand Down Expand Up @@ -238,7 +240,9 @@ impl<'tcx> ConstToPat<'tcx> {
}
} else if !self.saw_const_match_lint.get() {
match cv.ty().kind() {
ty::RawPtr(pointee) if pointee.ty.is_sized(self.tcx(), self.param_env) => {}
ty::RawPtr(..) if have_valtree => {
// This is a good raw pointer, it was accepted by valtree construction.
}
ty::FnPtr(..) | ty::RawPtr(..) => {
self.tcx().emit_spanned_lint(
lint::builtin::POINTER_STRUCTURAL_MATCH,
Expand Down Expand Up @@ -389,11 +393,19 @@ impl<'tcx> ConstToPat<'tcx> {
subpatterns: self
.field_pats(cv.unwrap_branch().iter().copied().zip(fields.iter()))?,
},
ty::Adt(def, args) => PatKind::Leaf {
subpatterns: self.field_pats(cv.unwrap_branch().iter().copied().zip(
def.non_enum_variant().fields.iter().map(|field| field.ty(self.tcx(), args)),
))?,
},
ty::Adt(def, args) => {
assert!(!def.is_union()); // Valtree construction would never succeed for unions.
PatKind::Leaf {
subpatterns: self.field_pats(
cv.unwrap_branch().iter().copied().zip(
def.non_enum_variant()
.fields
.iter()
.map(|field| field.ty(self.tcx(), args)),
),
)?,
}
}
ty::Slice(elem_ty) => PatKind::Slice {
prefix: cv
.unwrap_branch()
Expand Down Expand Up @@ -480,10 +492,15 @@ impl<'tcx> ConstToPat<'tcx> {
}
}
},
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) => {
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::RawPtr(..) => {
// The raw pointers we see here have been "vetted" by valtree construction to be
// just integers, so we simply allow them.
PatKind::Constant { value: mir::Const::Ty(ty::Const::new_value(tcx, cv, ty)) }
}
ty::FnPtr(..) | ty::RawPtr(..) => unreachable!(),
ty::FnPtr(..) => {
// Valtree construction would never succeed for these, so this is unreachable.
unreachable!()
}
_ => {
let err = InvalidPattern { span, non_sm_ty: ty };
let e = tcx.sess.emit_err(err);
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/consts/const_in_pattern/issue-44333.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ const BAR: Func = bar;

fn main() {
match test(std::env::consts::ARCH.len()) {
FOO => println!("foo"), //~ WARN pointers in patterns behave unpredictably
FOO => println!("foo"), //~ WARN behave unpredictably
//~^ WARN will become a hard error
BAR => println!("bar"), //~ WARN pointers in patterns behave unpredictably
BAR => println!("bar"), //~ WARN behave unpredictably
//~^ WARN will become a hard error
_ => unreachable!(),
}
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/consts/const_in_pattern/issue-44333.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: function pointers and unsized pointers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details.
warning: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details.
--> $DIR/issue-44333.rs:19:9
|
LL | FOO => println!("foo"),
Expand All @@ -12,7 +12,7 @@ note: the lint level is defined here
LL | #![warn(pointer_structural_match)]
| ^^^^^^^^^^^^^^^^^^^^^^^^

warning: function pointers and unsized pointers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details.
warning: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details.
--> $DIR/issue-44333.rs:21:9
|
LL | BAR => println!("bar"),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
// run-pass

#![warn(pointer_structural_match)]
#![deny(pointer_structural_match)]
#![allow(dead_code)]
const C: *const u8 = &0;

fn foo(x: *const u8) {
match x {
C => {}
C => {} //~ERROR: behave unpredictably
//~| previously accepted
_ => {}
}
}
Expand All @@ -15,7 +14,8 @@ const D: *const [u8; 4] = b"abcd";

fn main() {
match D {
D => {}
D => {} //~ERROR: behave unpredictably
//~| previously accepted
_ => {}
}
}
25 changes: 25 additions & 0 deletions tests/ui/consts/issue-34784-match-on-non-int-raw-ptr.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details.
--> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:7:9
|
LL | C => {}
| ^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #62411 <https://github.com/rust-lang/rust/issues/70861>
note: the lint level is defined here
--> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:1:9
|
LL | #![deny(pointer_structural_match)]
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details.
--> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:17:9
|
LL | D => {}
| ^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #62411 <https://github.com/rust-lang/rust/issues/70861>

error: aborting due to 2 previous errors

Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ fn main() {
let s = B(my_fn);
match s {
B(TEST) => println!("matched"),
//~^ WARN pointers in patterns behave unpredictably
//~^ WARN behave unpredictably
//~| WARN this was previously accepted by the compiler but is being phased out
_ => panic!("didn't match")
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: function pointers and unsized pointers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details.
warning: function pointers and raw pointers not derived from integers in patterns behave unpredictably and should not be relied upon. See https://github.com/rust-lang/rust/issues/70861 for details.
--> $DIR/issue-63479-match-fnptr.rs:35:7
|
LL | B(TEST) => println!("matched"),
Expand Down

0 comments on commit ebb1189

Please sign in to comment.