Skip to content

Commit 77fe9f0

Browse files
oli-obkoli-obk
committed
Validate before reporting interning errors.
validation produces much higher quality errors and already handles most of the cases
1 parent 8b2a4f8 commit 77fe9f0

18 files changed

+253
-513
lines changed

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

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,21 @@ use rustc_middle::traits::Reveal;
1010
use rustc_middle::ty::layout::LayoutOf;
1111
use rustc_middle::ty::print::with_no_trimmed_paths;
1212
use rustc_middle::ty::{self, Ty, TyCtxt};
13+
use rustc_session::lint;
1314
use rustc_span::def_id::LocalDefId;
1415
use rustc_span::Span;
1516
use rustc_target::abi::{self, Abi};
1617

1718
use super::{CanAccessMutGlobal, CompileTimeEvalContext, CompileTimeInterpreter};
1819
use crate::const_eval::CheckAlignment;
19-
use crate::errors;
2020
use crate::errors::ConstEvalError;
21-
use crate::interpret::eval_nullary_intrinsic;
21+
use crate::errors::{self, DanglingPtrInFinal};
2222
use crate::interpret::{
2323
create_static_alloc, intern_const_alloc_recursive, CtfeValidationMode, GlobalId, Immediate,
2424
InternKind, InterpCx, InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking,
2525
StackPopCleanup,
2626
};
27+
use crate::interpret::{eval_nullary_intrinsic, InternResult};
2728
use crate::CTRL_C_RECEIVED;
2829

2930
// Returns a pointer to where the result lives
@@ -89,11 +90,33 @@ fn eval_body_using_ecx<'mir, 'tcx, R: InterpretationResult<'tcx>>(
8990
}
9091

9192
// Intern the result
92-
intern_const_alloc_recursive(ecx, intern_kind, &ret)?;
93+
let intern_result = intern_const_alloc_recursive(ecx, intern_kind, &ret);
9394

9495
// Since evaluation had no errors, validate the resulting constant.
9596
const_validate_mplace(&ecx, &ret, cid)?;
9697

98+
// Only report this after validation, as validaiton produces much better diagnostics.
99+
// FIXME: ensure validation always reports this and stop making interning care about it.
100+
101+
if let Err(InternResult { found_bad_mutable_pointer, found_dangling_pointer }) = intern_result {
102+
if found_dangling_pointer {
103+
return Err(ecx
104+
.tcx
105+
.dcx()
106+
.emit_err(DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind })
107+
.into());
108+
} else if found_bad_mutable_pointer {
109+
// only report mutable pointers if there were no dangling pointers
110+
let err_diag = errors::MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind };
111+
ecx.tcx.emit_node_span_lint(
112+
lint::builtin::CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE,
113+
ecx.best_lint_scope(),
114+
err_diag.span,
115+
err_diag,
116+
)
117+
}
118+
}
119+
97120
Ok(R::make_result(ret, ecx))
98121
}
99122

‎compiler/rustc_const_eval/src/interpret/intern.rs

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,17 @@
1616
use hir::def::DefKind;
1717
use rustc_ast::Mutability;
1818
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
19-
use rustc_errors::ErrorGuaranteed;
2019
use rustc_hir as hir;
2120
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
2221
use rustc_middle::mir::interpret::{ConstAllocation, CtfeProvenance, InterpResult};
2322
use rustc_middle::query::TyCtxtAt;
2423
use rustc_middle::ty::layout::TyAndLayout;
25-
use rustc_session::lint;
2624
use rustc_span::def_id::LocalDefId;
2725
use rustc_span::sym;
2826

2927
use super::{AllocId, Allocation, InterpCx, MPlaceTy, Machine, MemoryKind, PlaceTy};
3028
use crate::const_eval;
31-
use crate::errors::{DanglingPtrInFinal, MutablePtrInFinal, NestedStaticInThreadLocal};
29+
use crate::errors::NestedStaticInThreadLocal;
3230

3331
pub trait CompileTimeMachine<'mir, 'tcx: 'mir, T> = Machine<
3432
'mir,
@@ -134,6 +132,19 @@ pub enum InternKind {
134132
Promoted,
135133
}
136134

135+
#[derive(Default, Debug)]
136+
pub struct InternResult {
137+
pub found_bad_mutable_pointer: bool,
138+
pub found_dangling_pointer: bool,
139+
}
140+
141+
impl InternResult {
142+
fn has_errors(&self) -> bool {
143+
let Self { found_bad_mutable_pointer, found_dangling_pointer } = *self;
144+
found_bad_mutable_pointer || found_dangling_pointer
145+
}
146+
}
147+
137148
/// Intern `ret` and everything it references.
138149
///
139150
/// This *cannot raise an interpreter error*. Doing so is left to validation, which
@@ -149,7 +160,7 @@ pub fn intern_const_alloc_recursive<
149160
ecx: &mut InterpCx<'mir, 'tcx, M>,
150161
intern_kind: InternKind,
151162
ret: &MPlaceTy<'tcx>,
152-
) -> Result<(), ErrorGuaranteed> {
163+
) -> Result<(), InternResult> {
153164
// We are interning recursively, and for mutability we are distinguishing the "root" allocation
154165
// that we are starting in, and all other allocations that we are encountering recursively.
155166
let (base_mutability, inner_mutability, is_static) = match intern_kind {
@@ -201,7 +212,7 @@ pub fn intern_const_alloc_recursive<
201212
// Whether we encountered a bad mutable pointer.
202213
// We want to first report "dangling" and then "mutable", so we need to delay reporting these
203214
// errors.
204-
let mut found_bad_mutable_pointer = false;
215+
let mut result = InternResult::default();
205216

206217
// Keep interning as long as there are things to intern.
207218
// We show errors if there are dangling pointers, or mutable pointers in immutable contexts
@@ -251,7 +262,7 @@ pub fn intern_const_alloc_recursive<
251262
// on the promotion analysis not screwing up to ensure that it is sound to intern
252263
// promoteds as immutable.
253264
trace!("found bad mutable pointer");
254-
found_bad_mutable_pointer = true;
265+
result.found_bad_mutable_pointer = true;
255266
}
256267
if ecx.tcx.try_get_global_alloc(alloc_id).is_some() {
257268
// Already interned.
@@ -269,21 +280,15 @@ pub fn intern_const_alloc_recursive<
269280
// pointers before deciding which allocations can be made immutable; but for now we are
270281
// okay with losing some potential for immutability here. This can anyway only affect
271282
// `static mut`.
272-
todo.extend(intern_shallow(ecx, alloc_id, inner_mutability).map_err(|()| {
273-
ecx.tcx.dcx().emit_err(DanglingPtrInFinal { span: ecx.tcx.span, kind: intern_kind })
274-
})?);
275-
}
276-
if found_bad_mutable_pointer {
277-
let err_diag = MutablePtrInFinal { span: ecx.tcx.span, kind: intern_kind };
278-
ecx.tcx.emit_node_span_lint(
279-
lint::builtin::CONST_EVAL_MUTABLE_PTR_IN_FINAL_VALUE,
280-
ecx.best_lint_scope(),
281-
err_diag.span,
282-
err_diag,
283-
)
283+
match intern_shallow(ecx, alloc_id, inner_mutability) {
284+
Ok(nested) => todo.extend(nested),
285+
Err(()) => {
286+
ecx.tcx.dcx().delayed_bug("found dangling pointer during const interning");
287+
result.found_dangling_pointer = true
288+
}
289+
}
284290
}
285-
286-
Ok(())
291+
if result.has_errors() { Err(result) } else { Ok(()) }
287292
}
288293

289294
/// Intern `ret`. This function assumes that `ret` references no other allocation.

‎compiler/rustc_const_eval/src/interpret/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub use rustc_middle::mir::interpret::*; // have all the `interpret` symbols in
2323
pub use self::eval_context::{format_interp_error, Frame, FrameInfo, InterpCx, StackPopCleanup};
2424
pub use self::intern::{
2525
intern_const_alloc_for_constprop, intern_const_alloc_recursive, HasStaticRootDefId, InternKind,
26+
InternResult,
2627
};
2728
pub use self::machine::{compile_time_machine, AllocMap, Machine, MayLeak, StackPopJump};
2829
pub use self::memory::{AllocKind, AllocRef, AllocRefMut, FnVal, Memory, MemoryKind};

‎tests/ui/consts/const-eval/heap/dealloc_intrinsic_dangling.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,15 @@
22
#![feature(const_heap)]
33
#![feature(const_mut_refs)]
44

5+
// Strip out raw byte dumps to make comparison platform-independent:
6+
//@ normalize-stderr-test "(the raw bytes of the constant) \(size: [0-9]*, align: [0-9]*\)" -> "$1 (size: $$SIZE, align: $$ALIGN)"
7+
//@ normalize-stderr-test "([0-9a-f][0-9a-f] |╾─*A(LLOC)?[0-9]+(\+[a-z0-9]+)?(<imm>)?─*╼ )+ *│.*" -> "HEX_DUMP"
8+
//@ normalize-stderr-test "HEX_DUMP\s*\n\s*HEX_DUMP" -> "HEX_DUMP"
9+
510
use std::intrinsics;
611

712
const _X: &'static u8 = unsafe {
8-
//~^ error: dangling pointer in final value of constant
13+
//~^ error: it is undefined behavior to use this value
914
let ptr = intrinsics::const_allocate(4, 4);
1015
intrinsics::const_deallocate(ptr, 4, 4);
1116
&*ptr

‎tests/ui/consts/const-eval/heap/dealloc_intrinsic_dangling.stderr

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
1-
error: encountered dangling pointer in final value of constant
2-
--> $DIR/dealloc_intrinsic_dangling.rs:7:1
1+
error[E0080]: it is undefined behavior to use this value
2+
--> $DIR/dealloc_intrinsic_dangling.rs:12:1
33
|
44
LL | const _X: &'static u8 = unsafe {
5-
| ^^^^^^^^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a dangling reference (use-after-free)
6+
|
7+
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
8+
= note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
9+
HEX_DUMP
10+
}
611

712
error[E0080]: evaluation of constant value failed
8-
--> $DIR/dealloc_intrinsic_dangling.rs:18:5
13+
--> $DIR/dealloc_intrinsic_dangling.rs:23:5
914
|
1015
LL | *reference
11-
| ^^^^^^^^^^ memory access failed: ALLOC0 has been freed, so this pointer is dangling
16+
| ^^^^^^^^^^ memory access failed: ALLOC1 has been freed, so this pointer is dangling
1217

1318
error: aborting due to 2 previous errors
1419

‎tests/ui/consts/const-mut-refs/mut_ref_in_final_dynamic_check.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ const fn helper_dangling() -> Option<&'static mut i32> { unsafe {
3333
// Undefined behaviour (dangling pointer), who doesn't love tests like this.
3434
Some(&mut *(&mut 42 as *mut i32))
3535
} }
36-
const DANGLING: Option<&mut i32> = helper_dangling(); //~ ERROR encountered dangling pointer
37-
static DANGLING_STATIC: Option<&mut i32> = helper_dangling(); //~ ERROR encountered dangling pointer
36+
const DANGLING: Option<&mut i32> = helper_dangling(); //~ ERROR it is undefined behavior to use this value
37+
static DANGLING_STATIC: Option<&mut i32> = helper_dangling(); //~ ERROR it is undefined behavior to use this value
3838

3939
// These are fine! Just statics pointing to mutable statics, nothing fundamentally wrong with this.
4040
static MUT_STATIC: Option<&mut i32> = helper();

‎tests/ui/consts/const-mut-refs/mut_ref_in_final_dynamic_check.stderr

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,27 @@ LL | static INT2PTR_STATIC: Option<&mut i32> = helper_int2ptr();
3131
HEX_DUMP
3232
}
3333

34-
error: encountered dangling pointer in final value of constant
34+
error[E0080]: it is undefined behavior to use this value
3535
--> $DIR/mut_ref_in_final_dynamic_check.rs:36:1
3636
|
3737
LL | const DANGLING: Option<&mut i32> = helper_dangling();
38-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered a dangling reference (use-after-free)
39+
|
40+
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
41+
= note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
42+
HEX_DUMP
43+
}
3944

40-
error: encountered dangling pointer in final value of static
45+
error[E0080]: it is undefined behavior to use this value
4146
--> $DIR/mut_ref_in_final_dynamic_check.rs:37:1
4247
|
4348
LL | static DANGLING_STATIC: Option<&mut i32> = helper_dangling();
44-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
49+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<enum-variant(Some)>.0: encountered a dangling reference (use-after-free)
50+
|
51+
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
52+
= note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
53+
HEX_DUMP
54+
}
4555

4656
error: aborting due to 5 previous errors
4757

‎tests/ui/consts/dangling-alloc-id-ice.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
// https://github.com/rust-lang/rust/issues/55223
2+
// Strip out raw byte dumps to make comparison platform-independent:
3+
//@ normalize-stderr-test "(the raw bytes of the constant) \(size: [0-9]*, align: [0-9]*\)" -> "$1 (size: $$SIZE, align: $$ALIGN)"
4+
//@ normalize-stderr-test "([0-9a-f][0-9a-f] |╾─*A(LLOC)?[0-9]+(\+[a-z0-9]+)?(<imm>)?─*╼ )+ *│.*" -> "HEX_DUMP"
5+
//@ normalize-stderr-test "HEX_DUMP\s*\n\s*HEX_DUMP" -> "HEX_DUMP"
26

37
union Foo<'a> {
48
y: &'a (),
59
long_live_the_unit: &'static (),
610
}
711

812
const FOO: &() = {
9-
//~^ ERROR encountered dangling pointer in final value of constant
13+
//~^ ERROR it is undefined behavior to use this value
1014
let y = ();
1115
unsafe { Foo { y: &y }.long_live_the_unit }
1216
};

‎tests/ui/consts/dangling-alloc-id-ice.stderr

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
1-
error: encountered dangling pointer in final value of constant
2-
--> $DIR/dangling-alloc-id-ice.rs:8:1
1+
error[E0080]: it is undefined behavior to use this value
2+
--> $DIR/dangling-alloc-id-ice.rs:12:1
33
|
44
LL | const FOO: &() = {
5-
| ^^^^^^^^^^^^^^
5+
| ^^^^^^^^^^^^^^ constructing invalid value: encountered a dangling reference (use-after-free)
6+
|
7+
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
8+
= note: the raw bytes of the constant (size: $SIZE, align: $ALIGN) {
9+
HEX_DUMP
10+
}
611

712
error: aborting due to 1 previous error
813

14+
For more information about this error, try `rustc --explain E0080`.

‎tests/ui/consts/dangling_raw_ptr.rs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,29 @@
1-
const FOO: *const u32 = { //~ ERROR encountered dangling pointer in final value of constant
1+
const FOO: *const u32 = {
2+
//~^ ERROR encountered dangling pointer in final value of constant
23
let x = 42;
34
&x
45
};
56

6-
fn main() {
7-
let x = FOO;
7+
union Union {
8+
ptr: *const u32,
89
}
10+
11+
const BAR: Union = {
12+
//~^ ERROR encountered dangling pointer in final value of constant
13+
let x = 42;
14+
Union { ptr: &x }
15+
};
16+
17+
const BAZ: Union = {
18+
//~^ ERROR encountered dangling pointer in final value of constant
19+
let x = 42_u32;
20+
Union { ptr: &(&x as *const u32) as *const *const u32 as _ }
21+
};
22+
23+
const FOOMP: *const u32 = {
24+
//~^ ERROR encountered dangling pointer in final value of constant
25+
let x = 42_u32;
26+
&(&x as *const u32) as *const *const u32 as _
27+
};
28+
29+
fn main() {}

‎tests/ui/consts/dangling_raw_ptr.stderr

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,23 @@ error: encountered dangling pointer in final value of constant
44
LL | const FOO: *const u32 = {
55
| ^^^^^^^^^^^^^^^^^^^^^
66

7-
error: aborting due to 1 previous error
7+
error: encountered dangling pointer in final value of constant
8+
--> $DIR/dangling_raw_ptr.rs:11:1
9+
|
10+
LL | const BAR: Union = {
11+
| ^^^^^^^^^^^^^^^^
12+
13+
error: encountered dangling pointer in final value of constant
14+
--> $DIR/dangling_raw_ptr.rs:17:1
15+
|
16+
LL | const BAZ: Union = {
17+
| ^^^^^^^^^^^^^^^^
18+
19+
error: encountered dangling pointer in final value of constant
20+
--> $DIR/dangling_raw_ptr.rs:23:1
21+
|
22+
LL | const FOOMP: *const u32 = {
23+
| ^^^^^^^^^^^^^^^^^^^^^^^
24+
25+
error: aborting due to 4 previous errors
826

‎tests/ui/consts/miri_unleashed/mutable_references.rs

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,9 @@
55
#![deny(const_eval_mutable_ptr_in_final_value)]
66
use std::cell::UnsafeCell;
77

8-
// a test demonstrating what things we could allow with a smarter const qualification
9-
8+
// This requires walking nested statics.
109
static FOO: &&mut u32 = &&mut 42;
11-
//~^ ERROR encountered mutable pointer in final value of static
12-
//~| WARNING this was previously accepted by the compiler
13-
//~| ERROR it is undefined behavior to use this value
10+
//~^ ERROR it is undefined behavior to use this value
1411

1512
static BAR: &mut () = &mut ();
1613
//~^ ERROR encountered mutable pointer in final value of static
@@ -27,14 +24,10 @@ struct Meh {
2724
}
2825
unsafe impl Sync for Meh {}
2926
static MEH: Meh = Meh { x: &UnsafeCell::new(42) };
30-
//~^ ERROR encountered mutable pointer in final value of static
31-
//~| WARNING this was previously accepted by the compiler
32-
//~| ERROR it is undefined behavior to use this value
27+
//~^ ERROR it is undefined behavior to use this value
3328

3429
static OH_YES: &mut i32 = &mut 42;
35-
//~^ ERROR encountered mutable pointer in final value of static
36-
//~| WARNING this was previously accepted by the compiler
37-
//~| ERROR it is undefined behavior to use this value
30+
//~^ ERROR it is undefined behavior to use this value
3831

3932
fn main() {
4033
unsafe {

0 commit comments

Comments
 (0)