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

enable fuzzy_provenance_casts lint in liballoc and libstd #104647

Merged
merged 3 commits into from
Nov 23, 2022

Conversation

RalfJung
Copy link
Member

r? @thomcc

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 20, 2022
@rustbot
Copy link
Collaborator

rustbot commented Nov 20, 2022

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@@ -184,7 +185,7 @@ unsafe fn read_encoded_pointer(
};

if encoding & DW_EH_PE_indirect != 0 {
result = *(result as *const usize);
result = *ptr::from_exposed_addr::<usize>(result);
Copy link
Member Author

Choose a reason for hiding this comment

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

I hope this can only happen if we previously had the DW_EH_PE_pcrel case above and exposed 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.

Who knows, but given that as *const usize is essentially semantically equivalent to from_exposed_addr::<usize>(...), this shouldn't add additional UB or anything if that's not how it is.

Copy link
Member Author

Choose a reason for hiding this comment

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

This could add UB only if the code relied on the round_up stuff above also exposing the pointer. But that did look like a slam-dunk case for with_addr so I went with that. (Actually it would be an awesome case for map_addr except that it would need ? inside the closure so of course that doesn't work...)

Copy link
Member

Choose a reason for hiding this comment

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

This could add UB only if the code relied on the round_up stuff above also exposing the pointer

That would be incredibly goofy and surprising, so I'm going to assume it's not the case.

@@ -47,6 +47,7 @@
#![feature(strict_provenance)]
#![feature(once_cell)]
#![feature(drain_keep_rest)]
#![deny(fuzzy_provenance_casts)]
Copy link
Member

Choose a reason for hiding this comment

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

A little surprised this is the only lint we turn on in the liballoc tests, but I'll look into that later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah me too... I briefly tried denying unsafe_op_in_unsafe_fn and got too many red lines to pursue this further right now. ;)

@thomcc
Copy link
Member

thomcc commented Nov 20, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 20, 2022

📌 Commit 7f5addd has been approved by thomcc

It is now in the queue for this repository.

@bors 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 Nov 20, 2022
@thomcc
Copy link
Member

thomcc commented Nov 20, 2022

Ah, wait, this might cause platform-specific failures. So maybe rollup=always is too optimistic.

@bors rollup=maybe

@RalfJung
Copy link
Member Author

Ah, wait, this might cause platform-specific failures. So maybe rollup=always is too optimistic.

Yeah. I did check builds on Linux, Android, macOS, Windows targets (which did find some more cases) but there are tons of targets left untested -- this should probably be
@bors rollup=iffy

@bors
Copy link
Contributor

bors commented Nov 20, 2022

⌛ Testing commit 7f5addd with merge 5d1ad57001036df80dc2d48130ba9a461b4a81a6...

@bors
Copy link
Contributor

bors commented Nov 20, 2022

💔 Test failed - checks-actions

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

This comment has been minimized.

@RalfJung
Copy link
Member Author

@bors r=thomcc

@bors
Copy link
Contributor

bors commented Nov 21, 2022

📌 Commit 1a69666 has been approved by thomcc

It is now in the queue for this repository.

@bors 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 Nov 21, 2022
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
  IMAGE: x86_64-gnu-tools
##[endgroup]
From https://github.com/rust-lang/rust
 * branch              master     -> FETCH_HEAD
Searching for toolstate changes between 736c675d2ab65bcde6554e1b73340c2dbc27c85a and 999d40d8101aaba38fb0a99d64656bd6471b8e5a
Tool subtrees were updated
##[group]Run src/ci/scripts/verify-channel.sh
src/ci/scripts/verify-channel.sh
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
---
.......... (80/88)
........   (88/88)


/checkout/src/test/rustdoc-gui/code-sidebar-toggle.goml code-sidebar-toggle... FAILED
[ERROR] (line 4) Error: Execution context was destroyed, most likely because of a navigation.: for command `wait-for: "#sidebar-toggle"`
Build completed unsuccessfully in 0:02:17

@RalfJung
Copy link
Member Author

Flaky rustdoc test?

/checkout/src/test/rustdoc-gui/code-sidebar-toggle.goml code-sidebar-toggle... FAILED

Cc @rust-lang/rustdoc

@notriddle
Copy link
Contributor

notriddle commented Nov 21, 2022

Yeah, flaky rustdoc test. Nothing you changed here could've affected it. #93784

Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 22, 2022
…r=thomcc

enable fuzzy_provenance_casts lint in liballoc and libstd

r? `@thomcc`
Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 22, 2022
…r=thomcc

enable fuzzy_provenance_casts lint in liballoc and libstd

r? ``@thomcc``
Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 23, 2022
…r=thomcc

enable fuzzy_provenance_casts lint in liballoc and libstd

r? ```@thomcc```
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 23, 2022
…earth

Rollup of 6 pull requests

Successful merges:

 - rust-lang#103488 (Allow opaque types in trait impl headers and rely on coherence to reject unsound cases)
 - rust-lang#104359 (Refactor must_use lint into two parts)
 - rust-lang#104612 (Lower return type outside async block creation)
 - rust-lang#104621 (Fix --extern library finding errors)
 - rust-lang#104647 (enable fuzzy_provenance_casts lint in liballoc and libstd)
 - rust-lang#104750 (Bump `fd-lock` in `bootstrap` again)

Failed merges:

 - rust-lang#104732 (Refactor `ty::ClosureKind` related stuff)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 316bda8 into rust-lang:master Nov 23, 2022
@rustbot rustbot added this to the 1.67.0 milestone Nov 23, 2022
Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 23, 2022
…safe_fn, r=Mark-Simulacrum

Add `#![deny(unsafe_op_in_unsafe_fn)]` in liballoc tests

In rust-lang#104647 (comment) it was mentioned that liballoc tests should probably have this enabled (we have it pretty much everywhere else in the stdlib), so I added it.

This will probably conflict with rust-lang#104647 so I'll rebase after that lands.
Manishearth added a commit to Manishearth/rust that referenced this pull request Nov 23, 2022
…safe_fn, r=Mark-Simulacrum

Add `#![deny(unsafe_op_in_unsafe_fn)]` in liballoc tests

In rust-lang#104647 (comment) it was mentioned that liballoc tests should probably have this enabled (we have it pretty much everywhere else in the stdlib), so I added it.

This will probably conflict with rust-lang#104647 so I'll rebase after that lands.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 25, 2022
…safe_fn, r=Mark-Simulacrum

Add `#![deny(unsafe_op_in_unsafe_fn)]` in liballoc tests

In rust-lang#104647 (comment) it was mentioned that liballoc tests should probably have this enabled (we have it pretty much everywhere else in the stdlib), so I added it.

This will probably conflict with rust-lang#104647 so I'll rebase after that lands.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 25, 2022
…safe_fn, r=Mark-Simulacrum

Add `#![deny(unsafe_op_in_unsafe_fn)]` in liballoc tests

In rust-lang#104647 (comment) it was mentioned that liballoc tests should probably have this enabled (we have it pretty much everywhere else in the stdlib), so I added it.

This will probably conflict with rust-lang#104647 so I'll rebase after that lands.
@RalfJung RalfJung deleted the alloc-strict-provenance branch December 2, 2022 11:11
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Feb 10, 2023
…r=Mark-Simulacrum

Add `#![deny(unsafe_op_in_unsafe_fn)]` in liballoc tests

In rust-lang/rust#104647 (comment) it was mentioned that liballoc tests should probably have this enabled (we have it pretty much everywhere else in the stdlib), so I added it.

This will probably conflict with rust-lang/rust#104647 so I'll rebase after that lands.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants