Skip to content

Commit fdaaaf9

Browse files
borsbors
committed
Auto merge of #116930 - RalfJung:raw-ptr-match, r=davidtwco
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. This extends the existing check against function pointers and wide pointers: we now uniformly reject all these pointer types during valtree construction, and then later lint because of that. See [here](#116930 (comment)) for some more explanation and context. Also fixes #116929. Cc `@oli-obk` `@lcnr`
2 parents 90fdc1f + 3058865 commit fdaaaf9

File tree

18 files changed

+375
-98
lines changed

18 files changed

+375
-98
lines changed

‎compiler/rustc_const_eval/src/const_eval/valtrees.rs

+27-10
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,27 @@ pub(crate) fn const_to_valtree_inner<'tcx>(
9797
Ok(ty::ValTree::Leaf(val.assert_int()))
9898
}
9999

100-
// Raw pointers are not allowed in type level constants, as we cannot properly test them for
101-
// equality at compile-time (see `ptr_guaranteed_cmp`).
100+
ty::RawPtr(_) => {
101+
// Not all raw pointers are allowed, as we cannot properly test them for
102+
// equality at compile-time (see `ptr_guaranteed_cmp`).
103+
// However we allow those that are just integers in disguise.
104+
// (We could allow wide raw pointers where both sides are integers in the future,
105+
// but for now we reject them.)
106+
let Ok(val) = ecx.read_scalar(place) else {
107+
return Err(ValTreeCreationError::Other);
108+
};
109+
// We are in the CTFE machine, so ptr-to-int casts will fail.
110+
// This can only be `Ok` if `val` already is an integer.
111+
let Ok(val) = val.try_to_int() else {
112+
return Err(ValTreeCreationError::Other);
113+
};
114+
// It's just a ScalarInt!
115+
Ok(ty::ValTree::Leaf(val))
116+
}
117+
102118
// Technically we could allow function pointers (represented as `ty::Instance`), but this is not guaranteed to
103119
// agree with runtime equality tests.
104-
ty::FnPtr(_) | ty::RawPtr(_) => Err(ValTreeCreationError::NonSupportedType),
120+
ty::FnPtr(_) => Err(ValTreeCreationError::NonSupportedType),
105121

106122
ty::Ref(_, _, _) => {
107123
let Ok(derefd_place)= ecx.deref_pointer(place) else {
@@ -222,12 +238,14 @@ pub fn valtree_to_const_value<'tcx>(
222238
assert!(valtree.unwrap_branch().is_empty());
223239
mir::ConstValue::ZeroSized
224240
}
225-
ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char => match valtree {
226-
ty::ValTree::Leaf(scalar_int) => mir::ConstValue::Scalar(Scalar::Int(scalar_int)),
227-
ty::ValTree::Branch(_) => bug!(
228-
"ValTrees for Bool, Int, Uint, Float or Char should have the form ValTree::Leaf"
229-
),
230-
},
241+
ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char | ty::RawPtr(_) => {
242+
match valtree {
243+
ty::ValTree::Leaf(scalar_int) => mir::ConstValue::Scalar(Scalar::Int(scalar_int)),
244+
ty::ValTree::Branch(_) => bug!(
245+
"ValTrees for Bool, Int, Uint, Float, Char or RawPtr should have the form ValTree::Leaf"
246+
),
247+
}
248+
}
231249
ty::Ref(_, inner_ty, _) => {
232250
let mut ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No);
233251
let imm = valtree_to_ref(&mut ecx, valtree, *inner_ty);
@@ -281,7 +299,6 @@ pub fn valtree_to_const_value<'tcx>(
281299
| ty::Coroutine(..)
282300
| ty::CoroutineWitness(..)
283301
| ty::FnPtr(_)
284-
| ty::RawPtr(_)
285302
| ty::Str
286303
| ty::Slice(_)
287304
| ty::Dynamic(..) => bug!("no ValTree should have been created for type {:?}", ty.kind()),

‎compiler/rustc_lint_defs/src/builtin.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -2217,15 +2217,16 @@ declare_lint! {
22172217
///
22182218
/// ### Explanation
22192219
///
2220-
/// Previous versions of Rust allowed function pointers and wide raw pointers in patterns.
2220+
/// Previous versions of Rust allowed function pointers and all raw pointers in patterns.
22212221
/// While these work in many cases as expected by users, it is possible that due to
22222222
/// optimizations pointers are "not equal to themselves" or pointers to different functions
22232223
/// compare as equal during runtime. This is because LLVM optimizations can deduplicate
22242224
/// functions if their bodies are the same, thus also making pointers to these functions point
22252225
/// to the same location. Additionally functions may get duplicated if they are instantiated
2226-
/// in different crates and not deduplicated again via LTO.
2226+
/// in different crates and not deduplicated again via LTO. Pointer identity for memory
2227+
/// created by `const` is similarly unreliable.
22272228
pub POINTER_STRUCTURAL_MATCH,
2228-
Allow,
2229+
Warn,
22292230
"pointers are not structural-match",
22302231
@future_incompatible = FutureIncompatibleInfo {
22312232
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,

‎compiler/rustc_mir_build/messages.ftl

+1-1
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ mir_build_overlapping_range_endpoints = multiple patterns overlap on their endpo
247247
mir_build_pattern_not_covered = refutable pattern in {$origin}
248248
.pattern_ty = the matched value is of type `{$pattern_ty}`
249249
250-
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.
250+
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.
251251
252252
mir_build_privately_uninhabited = pattern `{$witness_1}` is currently uninhabited, but this variant contains private fields which may become inhabited in the future
253253

‎compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs

+31-30
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ impl<'tcx> ConstToPat<'tcx> {
123123
});
124124
debug!(?check_body_for_struct_match_violation, ?mir_structural_match_violation);
125125

126+
let have_valtree =
127+
matches!(cv, mir::Const::Ty(c) if matches!(c.kind(), ty::ConstKind::Value(_)));
126128
let inlined_const_as_pat = match cv {
127129
mir::Const::Ty(c) => match c.kind() {
128130
ty::ConstKind::Param(_)
@@ -209,16 +211,6 @@ impl<'tcx> ConstToPat<'tcx> {
209211
} else if !self.saw_const_match_lint.get() {
210212
if let Some(mir_structural_match_violation) = mir_structural_match_violation {
211213
match non_sm_ty.kind() {
212-
ty::RawPtr(pointee)
213-
if pointee.ty.is_sized(self.tcx(), self.param_env) => {}
214-
ty::FnPtr(..) | ty::RawPtr(..) => {
215-
self.tcx().emit_spanned_lint(
216-
lint::builtin::POINTER_STRUCTURAL_MATCH,
217-
self.id,
218-
self.span,
219-
PointerPattern,
220-
);
221-
}
222214
ty::Adt(..) if mir_structural_match_violation => {
223215
self.tcx().emit_spanned_lint(
224216
lint::builtin::INDIRECT_STRUCTURAL_MATCH,
@@ -236,19 +228,15 @@ impl<'tcx> ConstToPat<'tcx> {
236228
}
237229
}
238230
}
239-
} else if !self.saw_const_match_lint.get() {
240-
match cv.ty().kind() {
241-
ty::RawPtr(pointee) if pointee.ty.is_sized(self.tcx(), self.param_env) => {}
242-
ty::FnPtr(..) | ty::RawPtr(..) => {
243-
self.tcx().emit_spanned_lint(
244-
lint::builtin::POINTER_STRUCTURAL_MATCH,
245-
self.id,
246-
self.span,
247-
PointerPattern,
248-
);
249-
}
250-
_ => {}
251-
}
231+
} else if !have_valtree && !self.saw_const_match_lint.get() {
232+
// The only way valtree construction can fail without the structural match
233+
// checker finding a violation is if there is a pointer somewhere.
234+
self.tcx().emit_spanned_lint(
235+
lint::builtin::POINTER_STRUCTURAL_MATCH,
236+
self.id,
237+
self.span,
238+
PointerPattern,
239+
);
252240
}
253241

254242
// Always check for `PartialEq`, even if we emitted other lints. (But not if there were
@@ -389,11 +377,19 @@ impl<'tcx> ConstToPat<'tcx> {
389377
subpatterns: self
390378
.field_pats(cv.unwrap_branch().iter().copied().zip(fields.iter()))?,
391379
},
392-
ty::Adt(def, args) => PatKind::Leaf {
393-
subpatterns: self.field_pats(cv.unwrap_branch().iter().copied().zip(
394-
def.non_enum_variant().fields.iter().map(|field| field.ty(self.tcx(), args)),
395-
))?,
396-
},
380+
ty::Adt(def, args) => {
381+
assert!(!def.is_union()); // Valtree construction would never succeed for unions.
382+
PatKind::Leaf {
383+
subpatterns: self.field_pats(
384+
cv.unwrap_branch().iter().copied().zip(
385+
def.non_enum_variant()
386+
.fields
387+
.iter()
388+
.map(|field| field.ty(self.tcx(), args)),
389+
),
390+
)?,
391+
}
392+
}
397393
ty::Slice(elem_ty) => PatKind::Slice {
398394
prefix: cv
399395
.unwrap_branch()
@@ -480,10 +476,15 @@ impl<'tcx> ConstToPat<'tcx> {
480476
}
481477
}
482478
},
483-
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) => {
479+
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::RawPtr(..) => {
480+
// The raw pointers we see here have been "vetted" by valtree construction to be
481+
// just integers, so we simply allow them.
484482
PatKind::Constant { value: mir::Const::Ty(ty::Const::new_value(tcx, cv, ty)) }
485483
}
486-
ty::FnPtr(..) | ty::RawPtr(..) => unreachable!(),
484+
ty::FnPtr(..) => {
485+
// Valtree construction would never succeed for these, so this is unreachable.
486+
unreachable!()
487+
}
487488
_ => {
488489
let err = InvalidPattern { span, non_sm_ty: ty };
489490
let e = tcx.sess.emit_err(err);

‎library/core/src/marker.rs

+1
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,7 @@ marker_impls! {
253253
///
254254
/// const CFN: Wrap<fn(&())> = Wrap(higher_order);
255255
///
256+
/// #[allow(pointer_structural_match)]
256257
/// fn main() {
257258
/// match CFN {
258259
/// CFN => {}

‎tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ pub fn edge_case_str(event: String) {
2626
pub fn edge_case_raw_ptr(event: *const i32) {
2727
let _ = || {
2828
match event {
29-
NUMBER_POINTER => (),
29+
NUMBER_POINTER => (), //~WARN behave unpredictably
30+
//~| previously accepted
3031
_ => (),
3132
};
3233
};

‎tests/ui/closures/2229_closure_analysis/match/match-edge-cases_1.stderr

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
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.
2+
--> $DIR/match-edge-cases_1.rs:29:13
3+
|
4+
LL | NUMBER_POINTER => (),
5+
| ^^^^^^^^^^^^^^
6+
|
7+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
8+
= note: for more information, see issue #62411 <https://github.com/rust-lang/rust/issues/70861>
9+
= note: `#[warn(pointer_structural_match)]` on by default
10+
11+
warning: 1 warning emitted
12+

‎tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.rs

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#![deny(pointer_structural_match)]
2+
#![allow(dead_code)]
3+
4+
const C: *const u8 = &0;
5+
// Make sure we also find pointers nested in other types.
6+
const C_INNER: (*const u8, u8) = (C, 0);
7+
8+
fn foo(x: *const u8) {
9+
match x {
10+
C => {} //~ERROR: behave unpredictably
11+
//~| previously accepted
12+
_ => {}
13+
}
14+
}
15+
16+
fn foo2(x: *const u8) {
17+
match (x, 1) {
18+
C_INNER => {} //~ERROR: behave unpredictably
19+
//~| previously accepted
20+
_ => {}
21+
}
22+
}
23+
24+
const D: *const [u8; 4] = b"abcd";
25+
26+
fn main() {
27+
match D {
28+
D => {} //~ERROR: behave unpredictably
29+
//~| previously accepted
30+
_ => {}
31+
}
32+
}

‎tests/ui/consts/const_in_pattern/issue-34784-match-on-non-int-raw-ptr.stderr

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
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.
2+
--> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:10:9
3+
|
4+
LL | C => {}
5+
| ^
6+
|
7+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
8+
= note: for more information, see issue #62411 <https://github.com/rust-lang/rust/issues/70861>
9+
note: the lint level is defined here
10+
--> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:1:9
11+
|
12+
LL | #![deny(pointer_structural_match)]
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^
14+
15+
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.
16+
--> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:18:9
17+
|
18+
LL | C_INNER => {}
19+
| ^^^^^^^
20+
|
21+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
22+
= note: for more information, see issue #62411 <https://github.com/rust-lang/rust/issues/70861>
23+
24+
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.
25+
--> $DIR/issue-34784-match-on-non-int-raw-ptr.rs:28:9
26+
|
27+
LL | D => {}
28+
| ^
29+
|
30+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
31+
= note: for more information, see issue #62411 <https://github.com/rust-lang/rust/issues/70861>
32+
33+
error: aborting due to 3 previous errors
34+

‎tests/ui/consts/const_in_pattern/issue-44333.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ const BAR: Func = bar;
1616

1717
fn main() {
1818
match test(std::env::consts::ARCH.len()) {
19-
FOO => println!("foo"), //~ WARN pointers in patterns behave unpredictably
19+
FOO => println!("foo"), //~ WARN behave unpredictably
2020
//~^ WARN will become a hard error
21-
BAR => println!("bar"), //~ WARN pointers in patterns behave unpredictably
21+
BAR => println!("bar"), //~ WARN behave unpredictably
2222
//~^ WARN will become a hard error
2323
_ => unreachable!(),
2424
}

‎tests/ui/consts/const_in_pattern/issue-44333.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
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.
1+
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.
22
--> $DIR/issue-44333.rs:19:9
33
|
44
LL | FOO => println!("foo"),
@@ -12,7 +12,7 @@ note: the lint level is defined here
1212
LL | #![warn(pointer_structural_match)]
1313
| ^^^^^^^^^^^^^^^^^^^^^^^^
1414

15-
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.
15+
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.
1616
--> $DIR/issue-44333.rs:21:9
1717
|
1818
LL | BAR => println!("bar"),

‎tests/ui/consts/issue-34784.rs

-21
This file was deleted.

‎tests/ui/pattern/usefulness/consts-opaque.rs

+16-8
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,10 @@ fn main() {
9595
const QUUX: Quux = quux;
9696

9797
match QUUX {
98-
QUUX => {}
99-
QUUX => {}
98+
QUUX => {} //~WARN behave unpredictably
99+
//~| previously accepted
100+
QUUX => {} //~WARN behave unpredictably
101+
//~| previously accepted
100102
_ => {}
101103
}
102104

@@ -105,14 +107,17 @@ fn main() {
105107
const WRAPQUUX: Wrap<Quux> = Wrap(quux);
106108

107109
match WRAPQUUX {
108-
WRAPQUUX => {}
109-
WRAPQUUX => {}
110+
WRAPQUUX => {} //~WARN behave unpredictably
111+
//~| previously accepted
112+
WRAPQUUX => {} //~WARN behave unpredictably
113+
//~| previously accepted
110114
Wrap(_) => {}
111115
}
112116

113117
match WRAPQUUX {
114118
Wrap(_) => {}
115-
WRAPQUUX => {}
119+
WRAPQUUX => {} //~WARN behave unpredictably
120+
//~| previously accepted
116121
}
117122

118123
match WRAPQUUX {
@@ -121,7 +126,8 @@ fn main() {
121126

122127
match WRAPQUUX {
123128
//~^ ERROR: non-exhaustive patterns: `Wrap(_)` not covered
124-
WRAPQUUX => {}
129+
WRAPQUUX => {} //~WARN behave unpredictably
130+
//~| previously accepted
125131
}
126132

127133
#[derive(PartialEq, Eq)]
@@ -132,9 +138,11 @@ fn main() {
132138
const WHOKNOWSQUUX: WhoKnows<Quux> = WhoKnows::Yay(quux);
133139

134140
match WHOKNOWSQUUX {
135-
WHOKNOWSQUUX => {}
141+
WHOKNOWSQUUX => {} //~WARN behave unpredictably
142+
//~| previously accepted
136143
WhoKnows::Yay(_) => {}
137-
WHOKNOWSQUUX => {}
144+
WHOKNOWSQUUX => {} //~WARN behave unpredictably
145+
//~| previously accepted
138146
WhoKnows::Nope => {}
139147
}
140148
}

0 commit comments

Comments
 (0)