Skip to content

Commit 5b6ed25

Browse files
borsbors
committed
Auto merge of #102513 - RalfJung:no-more-unaligned-reference, r=cjgillot,scottmcm
make unaligned_reference a hard error The `unaligned_references` lint has been warn-by-default since Rust 1.53 (#82525) and deny-by-default with mention in cargo future-incompat reports since Rust 1.62 (#95372). Current nightly will become Rust 1.66, so (unless major surprises show up with crater) I think it is time we make this a hard error, and close this old soundness gap in the language. EDIT: Turns out this will only land for Rust 1.67, so there is another 6 weeks of time here for crates to adjust. Fixes #82523.
2 parents dc1d9d5 + dfc4a7b commit 5b6ed25

27 files changed

+170
-686
lines changed

‎compiler/rustc_error_codes/src/error_codes.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
// /!\ IMPORTANT /!\
66
//
77
// Error messages' format must follow the RFC 1567 available here:
8-
// https://github.com/rust-lang/rfcs/pull/1567
8+
// https://rust-lang.github.io/rfcs/1567-long-error-codes-explanation-normalization.html
99

1010
register_diagnostics! {
1111
E0001: include_str!("./error_codes/E0001.md"),
@@ -510,6 +510,7 @@ E0789: include_str!("./error_codes/E0789.md"),
510510
E0790: include_str!("./error_codes/E0790.md"),
511511
E0791: include_str!("./error_codes/E0791.md"),
512512
E0792: include_str!("./error_codes/E0792.md"),
513+
E0793: include_str!("./error_codes/E0793.md"),
513514
;
514515
// E0006, // merged with E0005
515516
// E0008, // cannot bind by-move into a pattern guard

‎compiler/rustc_error_codes/src/error_codes/E0793.md

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
An unaligned references to a field of a [packed] struct got created.
2+
3+
Erroneous code example:
4+
5+
```compile_fail,E0793
6+
#[repr(packed)]
7+
pub struct Foo {
8+
field1: u64,
9+
field2: u8,
10+
}
11+
12+
unsafe {
13+
let foo = Foo { field1: 0, field2: 0 };
14+
// Accessing the field directly is fine.
15+
let val = foo.field1;
16+
// A reference to a packed field causes a error.
17+
let val = &foo.field1; // ERROR
18+
// An implicit `&` is added in format strings, causing the same error.
19+
println!("{}", foo.field1); // ERROR
20+
}
21+
```
22+
23+
Creating a reference to an insufficiently aligned packed field is
24+
[undefined behavior] and therefore disallowed. Using an `unsafe` block does not
25+
change anything about this. Instead, the code should do a copy of the data in
26+
the packed field or use raw pointers and unaligned accesses.
27+
28+
```
29+
#[repr(packed)]
30+
pub struct Foo {
31+
field1: u64,
32+
field2: u8,
33+
}
34+
35+
unsafe {
36+
let foo = Foo { field1: 0, field2: 0 };
37+
38+
// Instead of a reference, we can create a raw pointer...
39+
let ptr = std::ptr::addr_of!(foo.field1);
40+
// ... and then (crucially!) access it in an explicitly unaligned way.
41+
let val = unsafe { ptr.read_unaligned() };
42+
// This would *NOT* be correct:
43+
// let val = unsafe { *ptr }; // Undefined Behavior due to unaligned load!
44+
45+
// For formatting, we can create a copy to avoid the direct reference.
46+
let copy = foo.field1;
47+
println!("{}", copy);
48+
// Creating a copy can be written in a single line with curly braces.
49+
// (This is equivalent to the two lines above.)
50+
println!("{}", { foo.field1 });
51+
}
52+
```
53+
54+
### Additional information
55+
56+
Note that this error is specifically about *references* to packed fields.
57+
Direct by-value access of those fields is fine, since then the compiler has
58+
enough information to generate the correct kind of access.
59+
60+
See [issue #82523] for more information.
61+
62+
[packed]: https://doc.rust-lang.org/reference/type-layout.html#the-alignment-modifiers
63+
[undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
64+
[issue #82523]: https://github.com/rust-lang/rust/issues/82523

‎compiler/rustc_lint/src/lib.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,6 @@ fn register_builtins(store: &mut LintStore) {
324324
store.register_renamed("exceeding_bitshifts", "arithmetic_overflow");
325325
store.register_renamed("redundant_semicolon", "redundant_semicolons");
326326
store.register_renamed("overlapping_patterns", "overlapping_range_endpoints");
327-
store.register_renamed("safe_packed_borrows", "unaligned_references");
328327
store.register_renamed("disjoint_capture_migration", "rust_2021_incompatible_closure_captures");
329328
store.register_renamed("or_patterns_back_compat", "rust_2021_incompatible_or_patterns");
330329
store.register_renamed("non_fmt_panic", "non_fmt_panics");
@@ -487,6 +486,16 @@ fn register_builtins(store: &mut LintStore) {
487486
"converted into hard error, see issue #71800 \
488487
<https://github.com/rust-lang/rust/issues/71800> for more information",
489488
);
489+
store.register_removed(
490+
"safe_packed_borrows",
491+
"converted into hard error, see issue #82523 \
492+
<https://github.com/rust-lang/rust/issues/82523> for more information",
493+
);
494+
store.register_removed(
495+
"unaligned_references",
496+
"converted into hard error, see issue #82523 \
497+
<https://github.com/rust-lang/rust/issues/82523> for more information",
498+
);
490499
}
491500

492501
fn register_internals(store: &mut LintStore) {

‎compiler/rustc_lint_defs/src/builtin.rs

-46
Original file line numberDiff line numberDiff line change
@@ -1187,51 +1187,6 @@ declare_lint! {
11871187
"lints that have been renamed or removed"
11881188
}
11891189

1190-
declare_lint! {
1191-
/// The `unaligned_references` lint detects unaligned references to fields
1192-
/// of [packed] structs.
1193-
///
1194-
/// [packed]: https://doc.rust-lang.org/reference/type-layout.html#the-alignment-modifiers
1195-
///
1196-
/// ### Example
1197-
///
1198-
/// ```rust,compile_fail
1199-
/// #[repr(packed)]
1200-
/// pub struct Foo {
1201-
/// field1: u64,
1202-
/// field2: u8,
1203-
/// }
1204-
///
1205-
/// fn main() {
1206-
/// unsafe {
1207-
/// let foo = Foo { field1: 0, field2: 0 };
1208-
/// let _ = &foo.field1;
1209-
/// println!("{}", foo.field1); // An implicit `&` is added here, triggering the lint.
1210-
/// }
1211-
/// }
1212-
/// ```
1213-
///
1214-
/// {{produces}}
1215-
///
1216-
/// ### Explanation
1217-
///
1218-
/// Creating a reference to an insufficiently aligned packed field is [undefined behavior] and
1219-
/// should be disallowed. Using an `unsafe` block does not change anything about this. Instead,
1220-
/// the code should do a copy of the data in the packed field or use raw pointers and unaligned
1221-
/// accesses. See [issue #82523] for more information.
1222-
///
1223-
/// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
1224-
/// [issue #82523]: https://github.com/rust-lang/rust/issues/82523
1225-
pub UNALIGNED_REFERENCES,
1226-
Deny,
1227-
"detects unaligned references to fields of packed structs",
1228-
@future_incompatible = FutureIncompatibleInfo {
1229-
reference: "issue #82523 <https://github.com/rust-lang/rust/issues/82523>",
1230-
reason: FutureIncompatibilityReason::FutureReleaseErrorReportNow,
1231-
};
1232-
report_in_external_macro
1233-
}
1234-
12351190
declare_lint! {
12361191
/// The `const_item_mutation` lint detects attempts to mutate a `const`
12371192
/// item.
@@ -3308,7 +3263,6 @@ declare_lint_pass! {
33083263
PUB_USE_OF_PRIVATE_EXTERN_CRATE,
33093264
INVALID_TYPE_PARAM_DEFAULT,
33103265
RENAMED_AND_REMOVED_LINTS,
3311-
UNALIGNED_REFERENCES,
33123266
CONST_ITEM_MUTATION,
33133267
PATTERNS_IN_FNS_WITHOUT_BODY,
33143268
MISSING_FRAGMENT_SPECIFIER,

‎compiler/rustc_mir_transform/src/check_packed_ref.rs

+17-26
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1+
use rustc_errors::struct_span_err;
12
use rustc_middle::mir::visit::{PlaceContext, Visitor};
23
use rustc_middle::mir::*;
34
use rustc_middle::ty::{self, TyCtxt};
4-
use rustc_session::lint::builtin::UNALIGNED_REFERENCES;
55

66
use crate::util;
77
use crate::MirLint;
@@ -49,31 +49,22 @@ impl<'tcx> Visitor<'tcx> for PackedRefChecker<'_, 'tcx> {
4949
// shouldn't do.
5050
unreachable!();
5151
} else {
52-
let source_info = self.source_info;
53-
let lint_root = self.body.source_scopes[source_info.scope]
54-
.local_data
55-
.as_ref()
56-
.assert_crate_local()
57-
.lint_root;
58-
self.tcx.struct_span_lint_hir(
59-
UNALIGNED_REFERENCES,
60-
lint_root,
61-
source_info.span,
62-
"reference to packed field is unaligned",
63-
|lint| {
64-
lint
65-
.note(
66-
"fields of packed structs are not properly aligned, and creating \
67-
a misaligned reference is undefined behavior (even if that \
68-
reference is never dereferenced)",
69-
)
70-
.help(
71-
"copy the field contents to a local variable, or replace the \
72-
reference with a raw pointer and use `read_unaligned`/`write_unaligned` \
73-
(loads and stores via `*p` must be properly aligned even when using raw pointers)"
74-
)
75-
},
76-
);
52+
struct_span_err!(
53+
self.tcx.sess,
54+
self.source_info.span,
55+
E0793,
56+
"reference to packed field is unaligned"
57+
)
58+
.note(
59+
"fields of packed structs are not properly aligned, and creating \
60+
a misaligned reference is undefined behavior (even if that \
61+
reference is never dereferenced)",
62+
).help(
63+
"copy the field contents to a local variable, or replace the \
64+
reference with a raw pointer and use `read_unaligned`/`write_unaligned` \
65+
(loads and stores via `*p` must be properly aligned even when using raw pointers)"
66+
)
67+
.emit();
7768
}
7869
}
7970
}

‎src/tools/miri/tests/fail/unaligned_pointers/reference_to_packed.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,26 @@
11
// This should fail even without validation/SB
22
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows
33

4-
#![allow(dead_code, unused_variables, unaligned_references)]
4+
#![allow(dead_code, unused_variables)]
5+
6+
use std::{ptr, mem};
57

68
#[repr(packed)]
79
struct Foo {
810
x: i32,
911
y: i32,
1012
}
1113

14+
unsafe fn raw_to_ref<'a, T>(x: *const T) -> &'a T {
15+
mem::transmute(x)
16+
}
17+
1218
fn main() {
1319
// Try many times as this might work by chance.
1420
for _ in 0..20 {
1521
let foo = Foo { x: 42, y: 99 };
16-
let p = &foo.x;
17-
let i = *p; //~ERROR: alignment 4 is required
22+
// There seem to be implicit reborrows, which make the error already appear here
23+
let p: &i32 = unsafe { raw_to_ref(ptr::addr_of!(foo.x)) }; //~ERROR: alignment 4 is required
24+
let i = *p;
1825
}
1926
}

‎src/tools/miri/tests/fail/unaligned_pointers/reference_to_packed.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
error: Undefined Behavior: accessing memory with alignment ALIGN, but alignment ALIGN is required
22
--> $DIR/reference_to_packed.rs:LL:CC
33
|
4-
LL | let i = *p;
5-
| ^^ accessing memory with alignment ALIGN, but alignment ALIGN is required
4+
LL | let p: &i32 = unsafe { raw_to_ref(ptr::addr_of!(foo.x)) };
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ accessing memory with alignment ALIGN, but alignment ALIGN is required
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

‎tests/ui/binding/issue-53114-safety-checks.rs

-4
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,11 @@ fn let_wild_gets_unsafe_field() {
2121
let u2 = U { a: I(1) };
2222
let p = P { a: &2, b: &3 };
2323
let _ = &p.b; //~ ERROR reference to packed field
24-
//~^ WARN will become a hard error
2524
let _ = u1.a; // #53114: should eventually signal error as well
2625
let _ = &u2.a; //~ ERROR [E0133]
2726

2827
// variation on above with `_` in substructure
2928
let (_,) = (&p.b,); //~ ERROR reference to packed field
30-
//~^ WARN will become a hard error
3129
let (_,) = (u1.a,); //~ ERROR [E0133]
3230
let (_,) = (&u2.a,); //~ ERROR [E0133]
3331
}
@@ -37,13 +35,11 @@ fn match_unsafe_field_to_wild() {
3735
let u2 = U { a: I(1) };
3836
let p = P { a: &2, b: &3 };
3937
match &p.b { _ => { } } //~ ERROR reference to packed field
40-
//~^ WARN will become a hard error
4138
match u1.a { _ => { } } //~ ERROR [E0133]
4239
match &u2.a { _ => { } } //~ ERROR [E0133]
4340

4441
// variation on above with `_` in substructure
4542
match (&p.b,) { (_,) => { } } //~ ERROR reference to packed field
46-
//~^ WARN will become a hard error
4743
match (u1.a,) { (_,) => { } } //~ ERROR [E0133]
4844
match (&u2.a,) { (_,) => { } } //~ ERROR [E0133]
4945
}

0 commit comments

Comments
 (0)