Strict provenance lints #95599

niluxv · 2022-04-02T20:06:01Z

See #95488.
This PR introduces two unstable (allow by default) lints to which lint on int2ptr and ptr2int casts, as the former is not possible in the strict provenance model and the latter can be written nicer using the .addr() API.
Based on an initial version of the lint by @Gankra in #95199.

rust-highfive · 2022-04-02T20:06:04Z

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

compiler/rustc_lint_defs/src/builtin.rs

compiler/rustc_typeck/src/check/cast.rs

niluxv · 2022-04-03T07:53:12Z

Will address the comments later today.

niluxv · 2022-04-03T14:20:07Z

I can do a follow up PR later to enable these lints in the std library crates and compiler crates. Main question right now is whether to use a separate feature gate or the strict_provenance feature gate for these lints.

Gankra · 2022-04-03T17:04:30Z

credit where it's due: @eddyb actually wrote most of the code here, I mostly just wrote the docs.

compiler/rustc_typeck/src/check/cast.rs

niluxv · 2022-04-03T19:09:24Z

Seems like things are moving in #95588 so I will adjust docs and suggestions hopefully tomorrow.

niluxv · 2022-04-04T09:37:43Z

Done. Also fixed suggestions for ptr2int non-usize casts. Now really ready for review I guess. I can rebase it into a nice history at the end.

niluxv · 2022-04-04T09:39:34Z

@rustbot label A-strict-provenance

michaelwoerister · 2022-04-05T19:21:35Z

I'll try to take a look at this shortly.

michaelwoerister

Thanks for the PR, @niluxv! I left some comments below. But under the assumption that @Gankra has taken a look at this too, this seems basically good to merge, I'd say.

Main question right now is whether to use a separate feature gate or the strict_provenance feature gate for these lints.

I think it's fine to use the same feature gate -- but if there turns out to be a problem with that, we can split it out later.

compiler/rustc_lint_defs/src/builtin.rs

niluxv · 2022-04-07T16:00:21Z

compiler/rustc_typeck/src/check/cast.rs

+                    err.span_suggestion(
+                        self.span,
+                        msg,
+                        format!("({}).addr(){}", snippet, scalar_cast),


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.

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.

Thank you for the hints. Opened #96112.

niluxv · 2022-04-07T16:13:45Z

src/test/ui/lint/lint-strict-provenance-lossy-casts.stderr

+  --> $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`


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.

michaelwoerister · 2022-04-08T11:53:07Z

I think this can be merged in the current state (with the commit history squashed/cleaned up).

The remaining issues seem like minor diagnostics polish, which can be done in a follow up PR.

…casts. ALL OF THEM

… test it * split `fuzzy_provenance_casts` into a ptr2int and a int2ptr lint * feature gate both lints * update documentation to be more realistic short term * add tests for these lints

niluxv · 2022-04-08T15:45:36Z

Rebased onto master and squashed all my commits together (so it is 2 commits now).

michaelwoerister · 2022-04-08T16:32:54Z

Thanks, @niluxv!

@bors r+

bors · 2022-04-08T16:32:55Z

📌 Commit 98a4834 has been approved by michaelwoerister

RalfJung · 2022-04-08T19:35:07Z

compiler/rustc_typeck/src/check/cast.rs

+                    err.help(msg);
+                }
+                err.help(
+                    "if you can't comply with strict provenance and need to expose the pointer\


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.)

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

Yes that'd be good. :)

…chaelwoerister Strict provenance lints See rust-lang#95488. This PR introduces two unstable (allow by default) lints to which lint on int2ptr and ptr2int casts, as the former is not possible in the strict provenance model and the latter can be written nicer using the `.addr()` API. Based on an initial version of the lint by `@Gankra` in rust-lang#95199.

…chaelwoerister Strict provenance lints See rust-lang#95488. This PR introduces two unstable (allow by default) lints to which lint on int2ptr and ptr2int casts, as the former is not possible in the strict provenance model and the latter can be written nicer using the `.addr()` API. Based on an initial version of the lint by ``@Gankra`` in rust-lang#95199.

Rollup of 8 pull requests Successful merges: - rust-lang#90066 (Add new ThinBox type for 1 stack pointer wide heap allocated trait objects) - rust-lang#95374 (assert_uninit_valid: ensure we detect at least arrays of uninhabited types) - rust-lang#95599 (Strict provenance lints) - rust-lang#95751 (Don't report numeric inference ambiguity when we have previous errors) - rust-lang#95764 ([macro_metavar_expr] Add tests to ensure the feature requirement) - rust-lang#95787 (reword panic vs result section to remove recoverable vs unrecoverable framing) - rust-lang#95797 (Remove explicit delimiter token trees from `Delimited`.) - rust-lang#95804 (rustdoc: Fix empty doc comment with backline ICE) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…, r=Dylan-DPC Fix missing space in lossy provenance cast lint See rust-lang#95599 (comment)

Gankra · 2022-04-23T17:26:59Z

thanks so much @niluxv!!!

rust-highfive assigned michaelwoerister Apr 2, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 2, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2022

This comment has been minimized.

Sign in to view

LeSeulArtichaut reviewed Apr 2, 2022

View reviewed changes

niluxv force-pushed the strict-provenance-lint branch from 1b0ed94 to 804c72d Compare April 3, 2022 07:26

This comment has been minimized.

Sign in to view

niluxv force-pushed the strict-provenance-lint branch from 804c72d to f46392e Compare April 3, 2022 07:41

niluxv force-pushed the strict-provenance-lint branch from f46392e to f9764e3 Compare April 3, 2022 12:34

niluxv marked this pull request as ready for review April 3, 2022 14:16

niluxv changed the title ~~WIP: Strict provenance lint~~ Strict provenance lints Apr 3, 2022

Gankra reviewed Apr 3, 2022

View reviewed changes

compiler/rustc_typeck/src/check/cast.rs Show resolved Hide resolved

niluxv force-pushed the strict-provenance-lint branch from fbe942d to 5f81934 Compare April 3, 2022 18:09

rustbot added the A-strict-provenance Area: Strict provenance for raw pointers label Apr 4, 2022

michaelwoerister reviewed Apr 7, 2022

View reviewed changes

compiler/rustc_lint_defs/src/builtin.rs Show resolved Hide resolved

compiler/rustc_lint_defs/src/builtin.rs Outdated Show resolved Hide resolved

compiler/rustc_lint_defs/src/builtin.rs Outdated Show resolved Hide resolved

niluxv force-pushed the strict-provenance-lint branch from 28340e4 to 9fb4fd0 Compare April 7, 2022 15:54

niluxv commented Apr 7, 2022

View reviewed changes

Gankra and others added 2 commits April 8, 2022 17:40

WIP PROOF-OF-CONCEPT: Make the compiler complain about all int<->ptr …

1040cab

…casts. ALL OF THEM

Split fuzzy_provenance_casts into lossy and fuzzy, feature gate and…

98a4834

… test it * split `fuzzy_provenance_casts` into a ptr2int and a int2ptr lint * feature gate both lints * update documentation to be more realistic short term * add tests for these lints

niluxv force-pushed the strict-provenance-lint branch from 9fb4fd0 to 98a4834 Compare April 8, 2022 15:43

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 8, 2022

niluxv mentioned this pull request Apr 8, 2022

[strict provenance] add lints for evil pointer casts #95488

Closed

RalfJung reviewed Apr 8, 2022

View reviewed changes

Dylan-DPC mentioned this pull request Apr 8, 2022

Rollup of 7 pull requests #95827

Closed

Dylan-DPC mentioned this pull request Apr 9, 2022

Rollup of 6 pull requests #95830

Closed

Dylan-DPC mentioned this pull request Apr 9, 2022

Rollup of 8 pull requests #95835

Merged

bors merged commit 525438b into rust-lang:master Apr 9, 2022

rustbot added this to the 1.62.0 milestone Apr 9, 2022

niluxv mentioned this pull request Apr 9, 2022

Fix missing space in lossy provenance cast lint #95852

Merged

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 9, 2022

Rollup merge of rust-lang#95852 - niluxv:strict-provenance-lint-fixup…

8a37db8

…, r=Dylan-DPC Fix missing space in lossy provenance cast lint See rust-lang#95599 (comment)

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 10, 2022

Rollup merge of rust-lang#95852 - niluxv:strict-provenance-lint-fixup…

4f6df46

…, r=Dylan-DPC Fix missing space in lossy provenance cast lint See rust-lang#95599 (comment)

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 10, 2022

Rollup merge of rust-lang#95852 - niluxv:strict-provenance-lint-fixup…

a52eb32

…, r=Dylan-DPC Fix missing space in lossy provenance cast lint See rust-lang#95599 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Strict provenance lints #95599

Strict provenance lints #95599

niluxv commented Apr 2, 2022 •

edited

Loading

rust-highfive commented Apr 2, 2022

This comment has been minimized.

This comment has been minimized.

niluxv commented Apr 3, 2022

niluxv commented Apr 3, 2022

Gankra commented Apr 3, 2022

niluxv commented Apr 3, 2022

niluxv commented Apr 4, 2022

niluxv commented Apr 4, 2022

michaelwoerister commented Apr 5, 2022

michaelwoerister left a comment

niluxv Apr 7, 2022

estebank Apr 14, 2022

niluxv Apr 16, 2022

niluxv Apr 7, 2022

michaelwoerister commented Apr 8, 2022

niluxv commented Apr 8, 2022

michaelwoerister commented Apr 8, 2022

bors commented Apr 8, 2022

RalfJung Apr 8, 2022

niluxv Apr 9, 2022

RalfJung Apr 9, 2022

Gankra commented Apr 23, 2022

	"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 \

Strict provenance lints #95599

Strict provenance lints #95599

Conversation

niluxv commented Apr 2, 2022 • edited Loading

rust-highfive commented Apr 2, 2022

This comment has been minimized.

This comment has been minimized.

niluxv commented Apr 3, 2022

niluxv commented Apr 3, 2022

Gankra commented Apr 3, 2022

niluxv commented Apr 3, 2022

niluxv commented Apr 4, 2022

niluxv commented Apr 4, 2022

michaelwoerister commented Apr 5, 2022

michaelwoerister left a comment

Choose a reason for hiding this comment

niluxv Apr 7, 2022

Choose a reason for hiding this comment

estebank Apr 14, 2022

Choose a reason for hiding this comment

niluxv Apr 16, 2022

Choose a reason for hiding this comment

niluxv Apr 7, 2022

Choose a reason for hiding this comment

michaelwoerister commented Apr 8, 2022

niluxv commented Apr 8, 2022

michaelwoerister commented Apr 8, 2022

bors commented Apr 8, 2022

RalfJung Apr 8, 2022

Choose a reason for hiding this comment

niluxv Apr 9, 2022

Choose a reason for hiding this comment

RalfJung Apr 9, 2022

Choose a reason for hiding this comment

Gankra commented Apr 23, 2022

niluxv commented Apr 2, 2022 •

edited

Loading