Skip to content

Commit d6f8271

Browse files
committed
Revert rust-lang#94158, "Apply noundef metadata to loads of types that do not permit raw init"
In rust-lang#94158, we started emitting `noundef`, which means that functions returning uninitialized references emit IR with is both `ret void` and also `noundef`: https://godbolt.org/z/hbjsKKfc3 More generally, this change makes `mem::uninitialized` dangerous in a way that it wasn't before. This `noundef` change was shipped in the past 2 stable releases, 1.61.0 and 1.62.0. This concerns me because in rust-lang#66151 we have thus far decided against issuing a panic for creating uninitialized references within arrays, on account of the breakage that shows up in crater. If this pattern is so widely-used that we're not willing to put in a runtime check for it, then it doesn't seem prudent to invite LLVM to exploit this UB. The pattern of creating uninit references in arrays shows up real code because the 0.11, 0.12, and 0.13 release series for `hyper` all use `mem::uninitialized` to construct arrays of `httparse::Header`, which contains a `&str` and `&[u8]`. There is no patch available within these release series, so a `cargo update` is not a viable way to escape the UB here. Also note that the 0.11 release series of `hyper` predates `MaybeUninit`, so any source-level patch for that will incur runtime overhead. Which would be unfortunate, but preferable to UB. I discussed this with @thomcc on the community Discord, and we think that it would be prudent to revert this introduction of `noundef` until we have a mitigation in place for the UB that this may unleash. We haven't been able to cook up any examples of surprising optimizations due to this pattern, but the whole point of `noundef` is to enable optimizations, and we know that there is code which uses it in a way which is definitely instant UB and which we have declined to inform users of. If possible, we would like to see `freeze` applied to the return value of `mem::uninitialized` as a mitigation for this problem. That may be able to keep old code functioning without introducing a performance hit. @rustbot labels add +T-compiler +I-compiler-nominated
1 parent 7b46aa5 commit d6f8271

File tree

2 files changed

+0
-109
lines changed

2 files changed

+0
-109
lines changed

compiler/rustc_codegen_llvm/src/builder.rs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -480,10 +480,6 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
480480
layout: TyAndLayout<'tcx>,
481481
offset: Size,
482482
) {
483-
if !scalar.is_always_valid(bx) {
484-
bx.noundef_metadata(load);
485-
}
486-
487483
match scalar.primitive() {
488484
abi::Int(..) => {
489485
if !scalar.is_always_valid(bx) {
@@ -1249,16 +1245,6 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
12491245
}
12501246
}
12511247

1252-
fn noundef_metadata(&mut self, load: &'ll Value) {
1253-
unsafe {
1254-
llvm::LLVMSetMetadata(
1255-
load,
1256-
llvm::MD_noundef as c_uint,
1257-
llvm::LLVMMDNodeInContext(self.cx.llcx, ptr::null(), 0),
1258-
);
1259-
}
1260-
}
1261-
12621248
pub fn minnum(&mut self, lhs: &'ll Value, rhs: &'ll Value) -> &'ll Value {
12631249
unsafe { llvm::LLVMRustBuildMinNum(self.llbuilder, lhs, rhs) }
12641250
}

src/test/codegen/loads.rs

Lines changed: 0 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -2,51 +2,17 @@
22

33
#![crate_type = "lib"]
44

5-
use std::mem::MaybeUninit;
6-
use std::num::NonZeroU16;
7-
85
pub struct Bytes {
96
a: u8,
107
b: u8,
118
c: u8,
129
d: u8,
1310
}
1411

15-
#[derive(Copy, Clone)]
16-
pub enum MyBool {
17-
True,
18-
False,
19-
}
20-
21-
#[repr(align(16))]
22-
pub struct Align16(u128);
23-
2412
// CHECK: @ptr_alignment_helper({{.*}}align [[PTR_ALIGNMENT:[0-9]+]]
2513
#[no_mangle]
2614
pub fn ptr_alignment_helper(x: &&()) {}
2715

28-
// CHECK-LABEL: @load_ref
29-
#[no_mangle]
30-
pub fn load_ref<'a>(x: &&'a i32) -> &'a i32 {
31-
// CHECK: load {{i32\*|ptr}}, {{i32\*\*|ptr}} %x, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_4_META:[0-9]+]], !noundef !{{[0-9]+}}
32-
*x
33-
}
34-
35-
// CHECK-LABEL: @load_ref_higher_alignment
36-
#[no_mangle]
37-
pub fn load_ref_higher_alignment<'a>(x: &&'a Align16) -> &'a Align16 {
38-
// CHECK: load {{%Align16\*|i128\*|ptr}}, {{%Align16\*\*|i128\*\*|ptr}} %x, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_16_META:[0-9]+]], !noundef !{{[0-9]+}}
39-
*x
40-
}
41-
42-
// CHECK-LABEL: @load_scalar_pair
43-
#[no_mangle]
44-
pub fn load_scalar_pair<'a>(x: &(&'a i32, &'a Align16)) -> (&'a i32, &'a Align16) {
45-
// CHECK: load {{i32\*|ptr}}, {{i32\*\*|ptr}} %{{.+}}, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_4_META]], !noundef !{{[0-9]+}}
46-
// CHECK: load {{i64\*|ptr}}, {{i64\*\*|ptr}} %{{.+}}, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_16_META]], !noundef !{{[0-9]+}}
47-
*x
48-
}
49-
5016
// CHECK-LABEL: @load_raw_pointer
5117
#[no_mangle]
5218
pub fn load_raw_pointer<'a>(x: &*const i32) -> *const i32 {
@@ -55,62 +21,6 @@ pub fn load_raw_pointer<'a>(x: &*const i32) -> *const i32 {
5521
*x
5622
}
5723

58-
// CHECK-LABEL: @load_box
59-
#[no_mangle]
60-
pub fn load_box<'a>(x: Box<Box<i32>>) -> Box<i32> {
61-
// CHECK: load {{i32\*|ptr}}, {{i32\*\*|ptr}} %{{.*}}, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_4_META]], !noundef !{{[0-9]+}}
62-
*x
63-
}
64-
65-
// CHECK-LABEL: @load_bool
66-
#[no_mangle]
67-
pub fn load_bool(x: &bool) -> bool {
68-
// CHECK: load i8, {{i8\*|ptr}} %x, align 1, !range ![[BOOL_RANGE:[0-9]+]], !noundef !{{[0-9]+}}
69-
*x
70-
}
71-
72-
// CHECK-LABEL: @load_maybeuninit_bool
73-
#[no_mangle]
74-
pub fn load_maybeuninit_bool(x: &MaybeUninit<bool>) -> MaybeUninit<bool> {
75-
// CHECK: load i8, {{i8\*|ptr}} %x, align 1{{$}}
76-
*x
77-
}
78-
79-
// CHECK-LABEL: @load_enum_bool
80-
#[no_mangle]
81-
pub fn load_enum_bool(x: &MyBool) -> MyBool {
82-
// CHECK: load i8, {{i8\*|ptr}} %x, align 1, !range ![[BOOL_RANGE]], !noundef !{{[0-9]+}}
83-
*x
84-
}
85-
86-
// CHECK-LABEL: @load_maybeuninit_enum_bool
87-
#[no_mangle]
88-
pub fn load_maybeuninit_enum_bool(x: &MaybeUninit<MyBool>) -> MaybeUninit<MyBool> {
89-
// CHECK: load i8, {{i8\*|ptr}} %x, align 1{{$}}
90-
*x
91-
}
92-
93-
// CHECK-LABEL: @load_int
94-
#[no_mangle]
95-
pub fn load_int(x: &u16) -> u16 {
96-
// CHECK: load i16, {{i16\*|ptr}} %x, align 2{{$}}
97-
*x
98-
}
99-
100-
// CHECK-LABEL: @load_nonzero_int
101-
#[no_mangle]
102-
pub fn load_nonzero_int(x: &NonZeroU16) -> NonZeroU16 {
103-
// CHECK: load i16, {{i16\*|ptr}} %x, align 2, !range ![[NONZEROU16_RANGE:[0-9]+]], !noundef !{{[0-9]+}}
104-
*x
105-
}
106-
107-
// CHECK-LABEL: @load_option_nonzero_int
108-
#[no_mangle]
109-
pub fn load_option_nonzero_int(x: &Option<NonZeroU16>) -> Option<NonZeroU16> {
110-
// CHECK: load i16, {{i16\*|ptr}} %x, align 2{{$}}
111-
*x
112-
}
113-
11424
// CHECK-LABEL: @borrow
11525
#[no_mangle]
11626
pub fn borrow(x: &i32) -> &i32 {
@@ -145,8 +55,3 @@ pub fn small_struct_alignment(x: Bytes) -> Bytes {
14555
// CHECK: ret i32 [[VAR]]
14656
x
14757
}
148-
149-
// CHECK-DAG: ![[BOOL_RANGE]] = !{i8 0, i8 2}
150-
// CHECK-DAG: ![[NONZEROU16_RANGE]] = !{i16 1, i16 0}
151-
// CHECK-DAG: ![[ALIGN_4_META]] = !{i64 4}
152-
// CHECK-DAG: ![[ALIGN_16_META]] = !{i64 16}

0 commit comments

Comments
 (0)