Skip to content

Self-referential generators vs dereferenceable #381

Open
@RalfJung

Description

@RalfJung
Member

Miri recently started checking the dereferenceable attribute of &mut !Unpin references by doing "fake reads" when such functions start executing. Interestingly, that makes this safe piece of code fail.

This makes sense, I think, for the same reason that we cannot do "fake reads" of &!Freeze references: when doing such reads, we might invalidate other aliasing references. In the case of this future, what happens is that

  • a reference to the field where Delay::new(1) is stored gets created
  • next time poll gets called, we do a "fake read" of the entire future, which invalidates the previously created reference (a noalias reference doesn't like other pointers being used for reads, that's kind of the point)
  • then we access the reference, causing UB

What I do not fully understand yet is why something like this is not sufficient to cause the issue.

How could we solve this? I am honestly not entirely sure, but see two avenues worth pursuing:

  • Figure out whether these "fake reads" are really needed to model dereferenceable. I think they are, because otherwise I don't think we get the right interaction between noalias and dereferenceable, but I asked the LLVM folks about this.
  • Remove dereferenceable from &mut !Unpin references. This is obviously correct but codegen/optimization people will probably be unhappy...

The latter point might be what we have to do, and somewhat mirrors the fact that we remove dereferenceable from &!Freeze references, but not really -- for shared references we thought dereferenceable_on_entry would still be a sound attribute to add, but with this problem it looks like that might not be the case (neither for &mut !Unpin nor for &!Freeze), at least if the "fake read" model of dereferenceable prevails.

The problematic code condenses to something like this:

struct NotUnpinType {
  delay: usize,
  delay_ref: &mut usize, // points to `delay`
}

fn poll(self: &mut NotUnpinType) {
  let fake_read = *self;
  self.delay_ref -= 1; // self.delay_ref is a noalias reference
  // so we should be able to reorder the last two lines, but that changes the value stored in fake_read.
}

Activity

added a commit that references this issue on Dec 3, 2022

Auto merge of #2713 - RalfJung:not-unpin-fake-read, r=RalfJung

RalfJung

RalfJung commented on Dec 3, 2022

@RalfJung
MemberAuthor

I'll make Miri not complain about this, to avoid blocking other people on this (rust-lang/miri#2713) -- so playground might not reproduce these errors if you are trying this in the future.

RalfJung

RalfJung commented on Dec 3, 2022

@RalfJung
MemberAuthor

What I do not fully understand yet is why something like this is not sufficient to cause the issue.

Interestingly, this is enough to trigger the problem. Somehow wrapping the reference in Pin actually adds more UB? Very strange.

EDIT: Any wrapper struct will do, it doesn't have to be Pin.

RalfJung

RalfJung commented on Dec 4, 2022

@RalfJung
MemberAuthor

I think the fact that a wrapper struct is needed is due to the fact that add_retags "gives up" on direct assignments to places like (((*_8) as variant#3).1: &mut i32), where it is not sure that the place still points to the same thing after the assignment is done.

added a commit that references this issue on Dec 5, 2022

Auto merge of rust-lang#2713 - RalfJung:not-unpin-fake-read, r=RalfJung

added a commit that references this issue on Dec 5, 2022

Rollup merge of rust-lang#105301 - RalfJung:miri, r=oli-obk

2a030bb
added a commit that references this issue on Dec 5, 2022

Rollup merge of rust-lang#105301 - RalfJung:miri, r=oli-obk

612e89a

21 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-dereferenceableTopic: when exactly does a reference need to point to regular dereferenceable memory?

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @RalfJung@JakobDegen

        Issue actions

          Self-referential generators vs `dereferenceable` · Issue #381 · rust-lang/unsafe-code-guidelines