Skip to content

Rollup of 6 pull requests #103436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/rustc_error_codes/src/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// /!\ IMPORTANT /!\
//
// Error messages' format must follow the RFC 1567 available here:
// https://github.com/rust-lang/rfcs/pull/1567
// https://rust-lang.github.io/rfcs/1567-long-error-codes-explanation-normalization.html

register_diagnostics! {
E0001: include_str!("./error_codes/E0001.md"),
Expand Down Expand Up @@ -494,6 +494,7 @@ E0786: include_str!("./error_codes/E0786.md"),
E0787: include_str!("./error_codes/E0787.md"),
E0788: include_str!("./error_codes/E0788.md"),
E0790: include_str!("./error_codes/E0790.md"),
E0791: include_str!("./error_codes/E0791.md"),
;
// E0006, // merged with E0005
// E0008, // cannot bind by-move into a pattern guard
Expand Down
64 changes: 64 additions & 0 deletions compiler/rustc_error_codes/src/error_codes/E0791.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
An unaligned reference to a field of a [packed] struct got created.

Erroneous code example:

```compile_fail,E0791
#[repr(packed)]
pub struct Foo {
field1: u64,
field2: u8,
}

unsafe {
let foo = Foo { field1: 0, field2: 0 };
// Accessing the field directly is fine.
let val = foo.field1;
// A reference to a packed field causes a error.
let val = &foo.field1; // ERROR
// An implicit `&` is added in format strings, causing the same error.
println!("{}", foo.field1); // ERROR
}
```

Creating a reference to an insufficiently aligned packed field is
[undefined behavior] and therefore disallowed. Using an `unsafe` block does not
change anything about this. Instead, the code should do a copy of the data in
the packed field or use raw pointers and unaligned accesses.

```
#[repr(packed)]
pub struct Foo {
field1: u64,
field2: u8,
}

unsafe {
let foo = Foo { field1: 0, field2: 0 };

// Instead of a reference, we can create a raw pointer...
let ptr = std::ptr::addr_of!(foo.field1);
// ... and then (crucially!) access it in an explicitly unaligned way.
let val = unsafe { ptr.read_unaligned() };
// This would *NOT* be correct:
// let val = unsafe { *ptr }; // Undefined Behavior due to unaligned load!

// For formatting, we can create a copy to avoid the direct reference.
let copy = foo.field1;
println!("{}", copy);
// Creating a copy can be written in a single line with curly braces.
// (This is equivalent to the two lines above.)
println!("{}", { foo.field1 });
}
```

### Additional information

Note that this error is specifically about *references* to packed fields.
Direct by-value access of those fields is fine, since then the compiler has
enough information to generate the correct kind of access.

See [issue #82523] for more information.

[packed]: https://doc.rust-lang.org/reference/type-layout.html#the-alignment-modifiers
[undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
[issue #82523]: https://github.com/rust-lang/rust/issues/82523
5 changes: 4 additions & 1 deletion compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2526,7 +2526,10 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
// return `Bound::Excluded`. (And we have tests checking that we
// handle the attribute correctly.)
// We don't add a span since users cannot declare such types anyway.
(Bound::Included(lo), _) if lo > 0 => {
(Bound::Included(lo), Bound::Included(hi)) if 0 < lo && lo < hi => {
return Some((format!("`{}` must be non-null", ty), None));
}
(Bound::Included(lo), Bound::Unbounded) if 0 < lo => {
return Some((format!("`{}` must be non-null", ty), None));
}
(Bound::Included(_), _) | (_, Bound::Included(_))
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,6 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
store.register_renamed("exceeding_bitshifts", "arithmetic_overflow");
store.register_renamed("redundant_semicolon", "redundant_semicolons");
store.register_renamed("overlapping_patterns", "overlapping_range_endpoints");
store.register_renamed("safe_packed_borrows", "unaligned_references");
store.register_renamed("disjoint_capture_migration", "rust_2021_incompatible_closure_captures");
store.register_renamed("or_patterns_back_compat", "rust_2021_incompatible_or_patterns");
store.register_renamed("non_fmt_panic", "non_fmt_panics");
Expand Down Expand Up @@ -530,6 +529,16 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
"converted into hard error, see issue #71800 \
<https://github.com/rust-lang/rust/issues/71800> for more information",
);
store.register_removed(
"safe_packed_borrows",
"converted into hard error, see issue #82523 \
<https://github.com/rust-lang/rust/issues/82523> for more information",
);
store.register_removed(
"unaligned_references",
"converted into hard error, see issue #82523 \
<https://github.com/rust-lang/rust/issues/82523> for more information",
);
}

fn register_internals(store: &mut LintStore) {
Expand Down
46 changes: 0 additions & 46 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1146,51 +1146,6 @@ declare_lint! {
"lints that have been renamed or removed"
}

declare_lint! {
/// The `unaligned_references` lint detects unaligned references to fields
/// of [packed] structs.
///
/// [packed]: https://doc.rust-lang.org/reference/type-layout.html#the-alignment-modifiers
///
/// ### Example
///
/// ```compile_fail
/// #[repr(packed)]
/// pub struct Foo {
/// field1: u64,
/// field2: u8,
/// }
///
/// fn main() {
/// unsafe {
/// let foo = Foo { field1: 0, field2: 0 };
/// let _ = &foo.field1;
/// println!("{}", foo.field1); // An implicit `&` is added here, triggering the lint.
/// }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Creating a reference to an insufficiently aligned packed field is [undefined behavior] and
/// should be disallowed. Using an `unsafe` block does not change anything about this. Instead,
/// the code should do a copy of the data in the packed field or use raw pointers and unaligned
/// accesses. See [issue #82523] for more information.
///
/// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
/// [issue #82523]: https://github.com/rust-lang/rust/issues/82523
pub UNALIGNED_REFERENCES,
Deny,
"detects unaligned references to fields of packed structs",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #82523 <https://github.com/rust-lang/rust/issues/82523>",
reason: FutureIncompatibilityReason::FutureReleaseErrorReportNow,
};
report_in_external_macro
}

declare_lint! {
/// The `const_item_mutation` lint detects attempts to mutate a `const`
/// item.
Expand Down Expand Up @@ -3266,7 +3221,6 @@ declare_lint_pass! {
PUB_USE_OF_PRIVATE_EXTERN_CRATE,
INVALID_TYPE_PARAM_DEFAULT,
RENAMED_AND_REMOVED_LINTS,
UNALIGNED_REFERENCES,
CONST_ITEM_MUTATION,
PATTERNS_IN_FNS_WITHOUT_BODY,
MISSING_FRAGMENT_SPECIFIER,
Expand Down
57 changes: 18 additions & 39 deletions compiler/rustc_mir_transform/src/check_packed_ref.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use rustc_errors::struct_span_err;
use rustc_hir::def_id::LocalDefId;
use rustc_middle::mir::visit::{PlaceContext, Visitor};
use rustc_middle::mir::*;
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{self, TyCtxt};
use rustc_session::lint::builtin::UNALIGNED_REFERENCES;

use crate::util;
use crate::MirLint;
Expand Down Expand Up @@ -31,11 +31,6 @@ struct PackedRefChecker<'a, 'tcx> {
}

fn unsafe_derive_on_repr_packed(tcx: TyCtxt<'_>, def_id: LocalDefId) {
let lint_hir_id = tcx.hir().local_def_id_to_hir_id(def_id);

// FIXME: when we make this a hard error, this should have its
// own error code.

let extra = if tcx.generics_of(def_id).own_requires_monomorphization() {
"with type or const parameters"
} else {
Expand All @@ -46,14 +41,7 @@ fn unsafe_derive_on_repr_packed(tcx: TyCtxt<'_>, def_id: LocalDefId) {
tcx.item_name(tcx.trait_id_of_impl(def_id.to_def_id()).expect("derived trait name")),
extra
);

tcx.struct_span_lint_hir(
UNALIGNED_REFERENCES,
lint_hir_id,
tcx.def_span(def_id),
message,
|lint| lint,
);
struct_span_err!(tcx.sess, tcx.def_span(def_id), E0791, "{message}").emit();
}

impl<'tcx> Visitor<'tcx> for PackedRefChecker<'_, 'tcx> {
Expand Down Expand Up @@ -82,31 +70,22 @@ impl<'tcx> Visitor<'tcx> for PackedRefChecker<'_, 'tcx> {
// the impl containing that method should also be.
self.tcx.ensure().unsafe_derive_on_repr_packed(impl_def_id.expect_local());
} else {
let source_info = self.source_info;
let lint_root = self.body.source_scopes[source_info.scope]
.local_data
.as_ref()
.assert_crate_local()
.lint_root;
self.tcx.struct_span_lint_hir(
UNALIGNED_REFERENCES,
lint_root,
source_info.span,
"reference to packed field is unaligned",
|lint| {
lint
.note(
"fields of packed structs are not properly aligned, and creating \
a misaligned reference is undefined behavior (even if that \
reference is never dereferenced)",
)
.help(
"copy the field contents to a local variable, or replace the \
reference with a raw pointer and use `read_unaligned`/`write_unaligned` \
(loads and stores via `*p` must be properly aligned even when using raw pointers)"
)
},
);
struct_span_err!(
self.tcx.sess,
self.source_info.span,
E0791,
"reference to packed field is unaligned"
)
.note(
"fields of packed structs are not properly aligned, and creating \
a misaligned reference is undefined behavior (even if that \
reference is never dereferenced)",
).help(
"copy the field contents to a local variable, or replace the \
reference with a raw pointer and use `read_unaligned`/`write_unaligned` \
(loads and stores via `*p` must be properly aligned even when using raw pointers)"
)
.emit();
}
}
}
Expand Down
16 changes: 14 additions & 2 deletions compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_attr::StabilityLevel;
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::intern::Interned;
use rustc_data_structures::sync::Lrc;
use rustc_errors::struct_span_err;
use rustc_errors::{struct_span_err, Applicability};
use rustc_expand::base::{Annotatable, DeriveResolutions, Indeterminate, ResolverExpand};
use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
use rustc_expand::compile_declarative_macro;
Expand Down Expand Up @@ -694,7 +694,19 @@ impl<'a> Resolver<'a> {
check_consistency(self, &path, path_span, kind, initial_res, res)
}
path_res @ PathResult::NonModule(..) | path_res @ PathResult::Failed { .. } => {
let mut suggestion = None;
let (span, label) = if let PathResult::Failed { span, label, .. } = path_res {
// try to suggest if it's not a macro, maybe a function
if let PathResult::NonModule(partial_res) = self.maybe_resolve_path(&path, Some(ValueNS), &parent_scope)
&& partial_res.unresolved_segments() == 0 {
let sm = self.session.source_map();
let exclamation_span = sm.next_point(span);
suggestion = Some((
vec![(exclamation_span, "".to_string())],
format!("{} is not a macro, but a {}, try to remove `!`", Segment::names_to_string(&path), partial_res.base_res().descr()),
Applicability::MaybeIncorrect
));
}
(span, label)
} else {
(
Expand All @@ -708,7 +720,7 @@ impl<'a> Resolver<'a> {
};
self.report_error(
span,
ResolutionError::FailedToResolve { label, suggestion: None },
ResolutionError::FailedToResolve { label, suggestion },
);
}
PathResult::Module(..) | PathResult::Indeterminate => unreachable!(),
Expand Down
20 changes: 12 additions & 8 deletions library/std/src/sys/solid/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,15 +175,19 @@ impl Iterator for ReadDir {
type Item = io::Result<DirEntry>;

fn next(&mut self) -> Option<io::Result<DirEntry>> {
unsafe {
let mut out_dirent = MaybeUninit::uninit();
error::SolidError::err_if_negative(abi::SOLID_FS_ReadDir(
let entry = unsafe {
let mut out_entry = MaybeUninit::uninit();
match error::SolidError::err_if_negative(abi::SOLID_FS_ReadDir(
self.inner.dirp,
out_dirent.as_mut_ptr(),
))
.ok()?;
Some(Ok(DirEntry { entry: out_dirent.assume_init(), inner: Arc::clone(&self.inner) }))
}
out_entry.as_mut_ptr(),
)) {
Ok(_) => out_entry.assume_init(),
Err(e) if e.as_raw() == abi::SOLID_ERR_NOTFOUND => return None,
Err(e) => return Some(Err(e.as_io_error())),
}
};

(entry.d_name[0] != 0).then(|| Ok(DirEntry { entry, inner: Arc::clone(&self.inner) }))
}
}

Expand Down
41 changes: 23 additions & 18 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -774,31 +774,36 @@ fn clean_ty_generics<'tcx>(
let mut where_predicates =
where_predicates.into_iter().flat_map(|p| clean_predicate(*p, cx)).collect::<Vec<_>>();

// Type parameters have a Sized bound by default unless removed with
// ?Sized. Scan through the predicates and mark any type parameter with
// a Sized bound, removing the bounds as we find them.
// In the surface language, all type parameters except `Self` have an
// implicit `Sized` bound unless removed with `?Sized`.
// However, in the list of where-predicates below, `Sized` appears like a
// normal bound: It's either present (the type is sized) or
// absent (the type is unsized) but never *maybe* (i.e. `?Sized`).
//
// Note that associated types also have a sized bound by default, but we
// This is unsuitable for rendering.
// Thus, as a first step remove all `Sized` bounds that should be implicit.
//
// Note that associated types also have an implicit `Sized` bound but we
// don't actually know the set of associated types right here so that's
// handled in cleaning associated types
// handled when cleaning associated types.
let mut sized_params = FxHashSet::default();
where_predicates.retain(|pred| match *pred {
WherePredicate::BoundPredicate { ty: Generic(ref g), ref bounds, .. } => {
if bounds.iter().any(|b| b.is_sized_bound(cx)) {
sized_params.insert(*g);
false
} else {
true
}
where_predicates.retain(|pred| {
if let WherePredicate::BoundPredicate { ty: Generic(g), bounds, .. } = pred
&& *g != kw::SelfUpper
&& bounds.iter().any(|b| b.is_sized_bound(cx))
{
sized_params.insert(*g);
false
} else {
true
}
_ => true,
});

// Run through the type parameters again and insert a ?Sized
// unbound for any we didn't find to be Sized.
// As a final step, go through the type parameters again and insert a
// `?Sized` bound for each one we didn't find to be `Sized`.
for tp in &stripped_params {
if matches!(tp.kind, types::GenericParamDefKind::Type { .. })
&& !sized_params.contains(&tp.name)
if let types::GenericParamDefKind::Type { .. } = tp.kind
&& !sized_params.contains(&tp.name)
{
where_predicates.push(WherePredicate::BoundPredicate {
ty: Type::Generic(tp.name),
Expand Down
1 change: 0 additions & 1 deletion src/librustdoc/html/static/css/rustdoc.css
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ h4.code-header {
}
.code-header {
font-weight: 600;
border-bottom-style: none;
margin: 0;
padding: 0;
}
Expand Down
Loading