Skip to content
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

Strict provenance lints #95599

Merged
merged 2 commits into from
Apr 9, 2022
Merged
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
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,8 @@ declare_features! (
(active, static_nobundle, "1.16.0", Some(37403), None),
/// Allows attributes on expressions and non-item statements.
(active, stmt_expr_attributes, "1.6.0", Some(15701), None),
/// Allows lints part of the strict provenance effort.
(active, strict_provenance, "1.61.0", Some(95228), None),
/// Allows the use of `#[target_feature]` on safe functions.
(active, target_feature_11, "1.45.0", Some(69098), None),
/// Allows using `#[thread_local]` on `static` items.
Expand Down
92 changes: 92 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2648,6 +2648,96 @@ declare_lint! {
};
}

declare_lint! {
/// The `fuzzy_provenance_casts` lint detects an `as` cast between an integer
/// and a pointer.
///
/// ### Example
///
/// ```rust
/// #![feature(strict_provenance)]
/// #![warn(fuzzy_provenance_casts)]
///
/// fn main() {
/// let _dangling = 16_usize as *const u8;
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// This lint is part of the strict provenance effort, see [issue #95228].
/// Casting an integer to a pointer is considered bad style, as a pointer
/// contains, besides the *address* also a *provenance*, indicating what
/// memory the pointer is allowed to read/write. Casting an integer, which
/// doesn't have provenance, to a pointer requires the compiler to assign
/// (guess) provenance. The compiler assigns "all exposed valid" (see the
/// docs of [`ptr::from_exposed_addr`] for more information about this
/// "exposing"). This penalizes the optimiser and is not well suited for
/// dynamic analysis/dynamic program verification (e.g. Miri or CHERI
/// platforms).
///
/// It is much better to use [`ptr::with_addr`] instead to specify the
/// provenance you want. If using this function is not possible because the
/// code relies on exposed provenance then there is as an escape hatch
/// [`ptr::from_exposed_addr`].
niluxv marked this conversation as resolved.
Show resolved Hide resolved
///
/// [issue #95228]: https://github.com/rust-lang/rust/issues/95228
/// [`ptr::with_addr`]: https://doc.rust-lang.org/core/ptr/fn.with_addr
/// [`ptr::from_exposed_addr`]: https://doc.rust-lang.org/core/ptr/fn.from_exposed_addr
pub FUZZY_PROVENANCE_CASTS,
Allow,
"a fuzzy integer to pointer cast is used",
@feature_gate = sym::strict_provenance;
}

declare_lint! {
/// The `lossy_provenance_casts` lint detects an `as` cast between a pointer
/// and an integer.
///
/// ### Example
///
/// ```rust
/// #![feature(strict_provenance)]
/// #![warn(lossy_provenance_casts)]
///
/// fn main() {
/// let x: u8 = 37;
/// let _addr: usize = &x as *const u8 as usize;
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// This lint is part of the strict provenance effort, see [issue #95228].
/// Casting a pointer to an integer is a lossy operation, because beyond
/// just an *address* a pointer may be associated with a particular
/// *provenance*. This information is used by the optimiser and for dynamic
/// analysis/dynamic program verification (e.g. Miri or CHERI platforms).
///
/// Since this cast is lossy, it is considered good style to use the
/// [`ptr::addr`] method instead, which has a similar effect, but doesn't
/// "expose" the pointer provenance. This improves optimisation potential.
/// See the docs of [`ptr::addr`] and [`ptr::expose_addr`] for more information
/// about exposing pointer provenance.
///
/// If your code can't comply with strict provenance and needs to expose
/// the provenance, then there is [`ptr::expose_addr`] as an escape hatch,
/// which preserves the behaviour of `as usize` casts while being explicit
/// about the semantics.
///
/// [issue #95228]: https://github.com/rust-lang/rust/issues/95228
/// [`ptr::addr`]: https://doc.rust-lang.org/core/ptr/fn.addr
/// [`ptr::expose_addr`]: https://doc.rust-lang.org/core/ptr/fn.expose_addr
pub LOSSY_PROVENANCE_CASTS,
Allow,
"a lossy pointer to integer cast is used",
@feature_gate = sym::strict_provenance;
}

declare_lint! {
/// The `const_evaluatable_unchecked` lint detects a generic constant used
/// in a type.
Expand Down Expand Up @@ -3101,6 +3191,8 @@ declare_lint_pass! {
UNSAFE_OP_IN_UNSAFE_FN,
INCOMPLETE_INCLUDE,
CENUM_IMPL_DROP_CAST,
FUZZY_PROVENANCE_CASTS,
LOSSY_PROVENANCE_CASTS,
CONST_EVALUATABLE_UNCHECKED,
INEFFECTIVE_UNSTABLE_TRAIT_IMPL,
MUST_NOT_SUSPEND,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1348,6 +1348,7 @@ symbols! {
str_trim,
str_trim_end,
str_trim_start,
strict_provenance,
stringify,
stringify_macro,
struct_field_attributes,
Expand Down
87 changes: 83 additions & 4 deletions compiler/rustc_typeck/src/check/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -807,11 +807,22 @@ impl<'a, 'tcx> CastCheck<'tcx> {

// ptr -> *
(Ptr(m_e), Ptr(m_c)) => self.check_ptr_ptr_cast(fcx, m_e, m_c), // ptr-ptr-cast
(Ptr(m_expr), Int(_)) => self.check_ptr_addr_cast(fcx, m_expr), // ptr-addr-cast
(FnPtr, Int(_)) => Ok(CastKind::FnPtrAddrCast),

// * -> ptr
(Int(_), Ptr(mt)) => self.check_addr_ptr_cast(fcx, mt), // addr-ptr-cast
// ptr-addr-cast
(Ptr(m_expr), Int(t_c)) => {
self.lossy_provenance_ptr2int_lint(fcx, t_c);
self.check_ptr_addr_cast(fcx, m_expr)
}
(FnPtr, Int(_)) => {
// FIXME(#95489): there should eventually be a lint for these casts
Ok(CastKind::FnPtrAddrCast)
niluxv marked this conversation as resolved.
Show resolved Hide resolved
}
// addr-ptr-cast
(Int(_), Ptr(mt)) => {
self.fuzzy_provenance_int2ptr_lint(fcx);
self.check_addr_ptr_cast(fcx, mt)
}
// fn-ptr-cast
(FnPtr, Ptr(mt)) => self.check_fptr_ptr_cast(fcx, mt),

// prim -> prim
Expand Down Expand Up @@ -973,6 +984,74 @@ impl<'a, 'tcx> CastCheck<'tcx> {
}
}
}

fn lossy_provenance_ptr2int_lint(&self, fcx: &FnCtxt<'a, 'tcx>, t_c: ty::cast::IntTy) {
fcx.tcx.struct_span_lint_hir(
lint::builtin::LOSSY_PROVENANCE_CASTS,
self.expr.hir_id,
self.span,
|err| {
let mut err = err.build(&format!(
"under strict provenance it is considered bad style to cast pointer `{}` to integer `{}`",
self.expr_ty, self.cast_ty
));

let msg = "use `.addr()` to obtain the address of a pointer";
if let Ok(snippet) = fcx.tcx.sess.source_map().span_to_snippet(self.expr.span) {
let scalar_cast = match t_c {
ty::cast::IntTy::U(ty::UintTy::Usize) => String::new(),
_ => format!(" as {}", self.cast_ty),
};
err.span_suggestion(
self.span,
msg,
format!("({}).addr(){}", snippet, scalar_cast),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggestion can contain unnecessary parenthesis but I have no idea how to reliably check whether snippet requires enclosing parenthesis, and also don't know whether fixing that would be worth it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we have access to the hir::Node::Expr we can check its op_precedence. We do it in some diagnostics already. It's also usually better to use a multipart_suggestion instead of relying on getting the code's snippet when possible. We used not to do that because the output is more verbose, but it is so much easier to read that I think it is worth it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the hints. Opened #96112.

Applicability::MaybeIncorrect
);
} else {
err.help(msg);
}
err.help(
"if you can't comply with strict provenance and need to expose the pointer\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"if you can't comply with strict provenance and need to expose the pointer\
"if you can't comply with strict provenance and need to expose the pointer \

Currently it is printed without a space, I doubt that is intentional. (Test output also needs to be adjusted then.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm too late to modify this PR. Should I open a new one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that'd be good. :)

provenance you can use `.expose_addr()` instead"
);

err.emit();
},
);
}

fn fuzzy_provenance_int2ptr_lint(&self, fcx: &FnCtxt<'a, 'tcx>) {
fcx.tcx.struct_span_lint_hir(
lint::builtin::FUZZY_PROVENANCE_CASTS,
self.expr.hir_id,
self.span,
|err| {

let mut err = err.build(&format!(
"strict provenance disallows casting integer `{}` to pointer `{}`",
self.expr_ty, self.cast_ty
));
let msg = "use `.with_addr()` to adjust a valid pointer in the same allocation, to this address";
if let Ok(snippet) = fcx.tcx.sess.source_map().span_to_snippet(self.expr.span) {
err.span_suggestion(
self.span,
msg,
format!("(...).with_addr({})", snippet),
Applicability::HasPlaceholders,
);
} else {
err.help(msg);
}
err.help(
"if you can't comply with strict provenance and don't have a pointer with \
the correct provenance you can use `std::ptr::from_exposed_addr()` instead"
);

err.emit();
},
);
}
}

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Expand Down
22 changes: 22 additions & 0 deletions src/doc/unstable-book/src/language-features/strict-provenance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# `strict_provenance`

The tracking issue for this feature is: [#95228]

[#95228]: https://github.com/rust-lang/rust/issues/95228
-----

The `strict_provenance` feature allows to enable the `fuzzy_provenance_casts` and `lossy_provenance_casts` lints.
These lint on casts between integers and pointers, that are recommended against or invalid in the strict provenance model.
The same feature gate is also used for the experimental strict provenance API in `std` (actually `core`).

## Example

```rust
#![feature(strict_provenance)]
#![warn(fuzzy_provenance_casts)]

fn main() {
let _dangling = 16_usize as *const u8;
//~^ WARNING: strict provenance disallows casting integer `usize` to pointer `*const u8`
}
```
19 changes: 19 additions & 0 deletions src/test/ui/feature-gates/feature-gate-strict_provenance.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// check-pass

#![deny(fuzzy_provenance_casts)]
//~^ WARNING unknown lint: `fuzzy_provenance_casts`
//~| WARNING unknown lint: `fuzzy_provenance_casts`
//~| WARNING unknown lint: `fuzzy_provenance_casts`
#![deny(lossy_provenance_casts)]
//~^ WARNING unknown lint: `lossy_provenance_casts`
//~| WARNING unknown lint: `lossy_provenance_casts`
//~| WARNING unknown lint: `lossy_provenance_casts`

fn main() {
// no warnings emitted since the lints are not activated

let _dangling = 16_usize as *const u8;

let x: u8 = 37;
let _addr: usize = &x as *const u8 as usize;
}
63 changes: 63 additions & 0 deletions src/test/ui/feature-gates/feature-gate-strict_provenance.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
warning: unknown lint: `fuzzy_provenance_casts`
--> $DIR/feature-gate-strict_provenance.rs:3:1
|
LL | #![deny(fuzzy_provenance_casts)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(unknown_lints)]` on by default
= note: the `fuzzy_provenance_casts` lint is unstable
= note: see issue #95228 <https://github.com/rust-lang/rust/issues/95228> for more information
= help: add `#![feature(strict_provenance)]` to the crate attributes to enable

warning: unknown lint: `lossy_provenance_casts`
--> $DIR/feature-gate-strict_provenance.rs:7:1
|
LL | #![deny(lossy_provenance_casts)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the `lossy_provenance_casts` lint is unstable
= note: see issue #95228 <https://github.com/rust-lang/rust/issues/95228> for more information
= help: add `#![feature(strict_provenance)]` to the crate attributes to enable

warning: unknown lint: `fuzzy_provenance_casts`
--> $DIR/feature-gate-strict_provenance.rs:3:1
|
LL | #![deny(fuzzy_provenance_casts)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the `fuzzy_provenance_casts` lint is unstable
= note: see issue #95228 <https://github.com/rust-lang/rust/issues/95228> for more information
= help: add `#![feature(strict_provenance)]` to the crate attributes to enable

warning: unknown lint: `lossy_provenance_casts`
--> $DIR/feature-gate-strict_provenance.rs:7:1
|
LL | #![deny(lossy_provenance_casts)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the `lossy_provenance_casts` lint is unstable
= note: see issue #95228 <https://github.com/rust-lang/rust/issues/95228> for more information
= help: add `#![feature(strict_provenance)]` to the crate attributes to enable

warning: unknown lint: `fuzzy_provenance_casts`
--> $DIR/feature-gate-strict_provenance.rs:3:1
|
LL | #![deny(fuzzy_provenance_casts)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the `fuzzy_provenance_casts` lint is unstable
= note: see issue #95228 <https://github.com/rust-lang/rust/issues/95228> for more information
= help: add `#![feature(strict_provenance)]` to the crate attributes to enable

warning: unknown lint: `lossy_provenance_casts`
--> $DIR/feature-gate-strict_provenance.rs:7:1
|
LL | #![deny(lossy_provenance_casts)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the `lossy_provenance_casts` lint is unstable
= note: see issue #95228 <https://github.com/rust-lang/rust/issues/95228> for more information
= help: add `#![feature(strict_provenance)]` to the crate attributes to enable

warning: 6 warnings emitted

7 changes: 7 additions & 0 deletions src/test/ui/lint/lint-strict-provenance-fuzzy-casts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#![feature(strict_provenance)]
#![deny(fuzzy_provenance_casts)]

fn main() {
let dangling = 16_usize as *const u8;
//~^ ERROR strict provenance disallows casting integer `usize` to pointer `*const u8`
}
19 changes: 19 additions & 0 deletions src/test/ui/lint/lint-strict-provenance-fuzzy-casts.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error: strict provenance disallows casting integer `usize` to pointer `*const u8`
--> $DIR/lint-strict-provenance-fuzzy-casts.rs:5:20
|
LL | let dangling = 16_usize as *const u8;
| ^^^^^^^^^^^^^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/lint-strict-provenance-fuzzy-casts.rs:2:9
|
LL | #![deny(fuzzy_provenance_casts)]
| ^^^^^^^^^^^^^^^^^^^^^^
= help: if you can't comply with strict provenance and don't have a pointer with the correct provenance you can use `std::ptr::from_exposed_addr()` instead
help: use `.with_addr()` to adjust a valid pointer in the same allocation, to this address
|
LL | let dangling = (...).with_addr(16_usize);
| ~~~~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to previous error

11 changes: 11 additions & 0 deletions src/test/ui/lint/lint-strict-provenance-lossy-casts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#![feature(strict_provenance)]
#![deny(lossy_provenance_casts)]

fn main() {
let x: u8 = 37;
let addr: usize = &x as *const u8 as usize;
//~^ ERROR under strict provenance it is considered bad style to cast pointer `*const u8` to integer `usize`

let addr_32bit = &x as *const u8 as u32;
//~^ ERROR under strict provenance it is considered bad style to cast pointer `*const u8` to integer `u32`
}
23 changes: 23 additions & 0 deletions src/test/ui/lint/lint-strict-provenance-lossy-casts.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error: under strict provenance it is considered bad style to cast pointer `*const u8` to integer `usize`
--> $DIR/lint-strict-provenance-lossy-casts.rs:6:23
|
LL | let addr: usize = &x as *const u8 as usize;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: use `.addr()` to obtain the address of a pointer: `(&x as *const u8).addr()`
|
note: the lint level is defined here
--> $DIR/lint-strict-provenance-lossy-casts.rs:2:9
|
LL | #![deny(lossy_provenance_casts)]
| ^^^^^^^^^^^^^^^^^^^^^^
= help: if you can't comply with strict provenance and need to expose the pointerprovenance you can use `.expose_addr()` instead

error: under strict provenance it is considered bad style to cast pointer `*const u8` to integer `u32`
--> $DIR/lint-strict-provenance-lossy-casts.rs:9:22
|
LL | let addr_32bit = &x as *const u8 as u32;
| ^^^^^^^^^^^^^^^^^^^^^^ help: use `.addr()` to obtain the address of a pointer: `(&x as *const u8).addr() as u32`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As seen in this example the suggestion lines tend to get really long. Feels suboptimal, but I don't know how to change the way the suggestion is shown.

|
= help: if you can't comply with strict provenance and need to expose the pointerprovenance you can use `.expose_addr()` instead

error: aborting due to 2 previous errors