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.

Sign up for GitHub

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

Jump to bottom

Strict provenance lint diagnostics improvements #96112

Merged
merged 2 commits into from
Apr 17, 2022
Merged

Strict provenance lint diagnostics improvements #96112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
02d12bc
Improve `lossy_provenance_casts` lint diagnostics
niluxv Apr 16, 2022
1d63d6d
Improve `fuzzy_provenance_casts` lint diagnostics
niluxv Apr 16, 2022
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
49 changes: 29 additions & 20 deletions compiler/rustc_typeck/src/check/cast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,20 +993,33 @@ impl<'a, 'tcx> CastCheck<'tcx> {
));

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),
};

let expr_prec = self.expr.precedence().order();
let needs_parens = expr_prec < rustc_ast::util::parser::PREC_POSTFIX;

let scalar_cast = match t_c {
ty::cast::IntTy::U(ty::UintTy::Usize) => String::new(),
_ => format!(" as {}", self.cast_ty),
};

let cast_span = self.expr_span.shrink_to_hi().to(self.cast_span);

if needs_parens {
let suggestions = vec![
(self.expr_span.shrink_to_lo(), String::from("(")),
(cast_span, format!(").addr(){scalar_cast}")),
];

err.multipart_suggestion(msg, suggestions, Applicability::MaybeIncorrect);
} else {
err.span_suggestion(
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why this doesn't use multipart_suggestion like in the branch above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it is not multipart? It is just a single part (single span, single suggestion String)...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does crate some inconsistency in the diagnostics depending on whether parenthesis are necessary, so maybe it would actually be better to use a multipart suggestion here too? But it just feels weird to do a multipart suggestion with only one part...

Copy link
Contributor

Choose a reason for hiding this comment

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

@niluxv If you want the suggestion to always have its own subwindow, you can use span_suggestion_verbose.

self.span,
cast_span,
msg,
format!("({snippet}).addr(){scalar_cast}"),
Applicability::MaybeIncorrect
format!(".addr(){scalar_cast}"),
Applicability::MaybeIncorrect,
);
} else {
err.help(msg);
}

err.help(
"if you can't comply with strict provenance and need to expose the pointer \
provenance you can use `.expose_addr()` instead"
Expand All @@ -1028,16 +1041,12 @@ impl<'a, 'tcx> CastCheck<'tcx> {
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);
}
let suggestions = vec![
(self.expr_span.shrink_to_lo(), String::from("(...).with_addr(")),
(self.expr_span.shrink_to_hi().to(self.cast_span), String::from(")")),
];

err.multipart_suggestion(msg, suggestions, Applicability::MaybeIncorrect);
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"
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/lint/lint-strict-provenance-fuzzy-casts.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ LL | #![deny(fuzzy_provenance_casts)]
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

7 changes: 7 additions & 0 deletions src/test/ui/lint/lint-strict-provenance-lossy-casts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,11 @@ fn main() {

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`

// don't add unnecessary parens in the suggestion
let ptr = &x as *const u8;
let ptr_addr = ptr as usize;
//~^ ERROR under strict provenance it is considered bad style to cast pointer `*const u8` to integer `usize`
let ptr_addr_32bit = ptr as u32;
//~^ ERROR under strict provenance it is considered bad style to cast pointer `*const u8` to integer `u32`
}
34 changes: 31 additions & 3 deletions src/test/ui/lint/lint-strict-provenance-lossy-casts.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,50 @@ error: under strict provenance it is considered bad style to cast pointer `*cons
--> $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 pointer provenance you can use `.expose_addr()` instead
help: use `.addr()` to obtain the address of a pointer
|
LL | let addr: usize = (&x as *const u8).addr();
| + ~~~~~~~~

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`
| ^^^^^^^^^^^^^^^^^^^^^^
|
= help: if you can't comply with strict provenance and need to expose the pointer provenance you can use `.expose_addr()` instead
help: use `.addr()` to obtain the address of a pointer
|
LL | let addr_32bit = (&x as *const u8).addr() as u32;
| + ~~~~~~~~~~~~~~~

error: under strict provenance it is considered bad style to cast pointer `*const u8` to integer `usize`
--> $DIR/lint-strict-provenance-lossy-casts.rs:14:20
|
LL | let ptr_addr = ptr as usize;
| ^^^---------
| |
| help: use `.addr()` to obtain the address of a pointer: `.addr()`
|
= help: if you can't comply with strict provenance and need to expose the pointer provenance 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:16:26
|
LL | let ptr_addr_32bit = ptr as u32;
| ^^^-------
| |
| help: use `.addr()` to obtain the address of a pointer: `.addr() as u32`
|
= help: if you can't comply with strict provenance and need to expose the pointer provenance you can use `.expose_addr()` instead

error: aborting due to 2 previous errors
error: aborting due to 4 previous errors