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

-Zdrop-tracking doesn't work if the value is borrowed #101135

Open
ChayimFriedman2 opened this issue Aug 28, 2022 · 4 comments
Open

-Zdrop-tracking doesn't work if the value is borrowed #101135

ChayimFriedman2 opened this issue Aug 28, 2022 · 4 comments
Labels
A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug.

Comments

@ChayimFriedman2
Copy link
Contributor

I tried this code with -Zdrop-tracking:

fn assert_send<T: Send>(_v: T) {}

struct NonSend(*const ());
fn use_non_send(_: &NonSend) {}

fn main() {
    assert_send(async {
        let non_send = NonSend(&());
        use_non_send(&non_send);
        drop(non_send);
        async {}.await;
    });
}

I expected to see this happen: this should compiler, as -Zdrop-tracking should see that we drop the non-Send type before the yield point.

Instead, this happened: this does not compile with "future cannot be sent between threads safely" pointing to the NonSend type.

If I comment the use_non_send(&non_send); line it works fine.

I don't know if this is supposed to work but this looks like a bug.

Meta

rustc --version --verbose:

rustc 1.65.0-nightly (c07a8b4e0 2022-08-26)
binary: rustc
commit-hash: c07a8b4e09f356c7468b69c50cac7fc5b5000b8a
commit-date: 2022-08-26
host: x86_64-pc-windows-msvc
release: 1.65.0-nightly
LLVM version: 15.0.0
@ChayimFriedman2 ChayimFriedman2 added the C-bug Category: This is a bug. label Aug 28, 2022
@jyn514
Copy link
Member

jyn514 commented Aug 29, 2022

cc #97331, @eholk

I don't think this needs to block stabilization.

@jyn514 jyn514 added the A-async-await Area: Async & Await label Aug 29, 2022
@eholk
Copy link
Contributor

eholk commented Sep 9, 2022

Agreed, drop tracking is still a significant improvement over the status quo and should be compatible with any fixes we do here. That said, I'd really like to fix this issue as well.

This will need some work in the MIR side of things, since that is where the more conservative treatment of borrowing comes from. On the typeck side, I think drop tracking is naturally more precise, but we've had to make it more pessimistic to agree with what MIR does.

Once the MIR is more precise, all we need to do from the typeck is basically remove anything to do with borrowed_temporaries.

@eholk
Copy link
Contributor

eholk commented Sep 12, 2022

We discussed this in the wg-async triage meeting today. Fixing this bug is something I'd like to see, but it sounds like it might be hard from a semantics standpoint. @tmandry points out that we don't know anything about about what use_non_send does, and that it could save a pointer that's used in unsafe code in ways that are considered sound today. So fixing this is going to require careful consideration and possibly even adjustment of unsafe code rules.

@rustbot label +AsyncAwait-Triaged

@rustbot rustbot added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Sep 12, 2022
@jyn514
Copy link
Member

jyn514 commented Sep 12, 2022

@tmandry I don't understand how this would invalidate any code that was sound before; the value is dropped before the await, so any unsafe code using it afterwards is already UB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

4 participants