Skip to content

Raw pointer deref of uninhabited type allowed in const fn #77694

Closed

Description

@DutchGhost

The code below compiles as of 1.46, but I'm not sure it should, since it dereferences a raw pointer in a const block, which is only allowed in nightly with the #![feature(const_raw_ptr_deref)] flag:

const unsafe fn unreachable() -> ! {
    use core::convert::Infallible;
    
    const INFALLIBLE: *const Infallible = [].as_ptr();
    match *INFALLIBLE {}
}

fn main() {
}

Activity

  1. ohsayan commented on Oct 8, 2020

    @ohsayan
    Contributor

    @rustbot modify labels to +A-raw-pointers +A-const-fn

  2. added
    A-stabilityArea: `#[stable]`, `#[unstable]` etc.
    C-bugCategory: This is a bug.
    I-prioritizeIssue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}
    on Oct 8, 2020
  3. jonas-schievink commented on Oct 8, 2020

    @jonas-schievink
    Contributor

    cc @rust-lang/wg-const-eval

  4. RalfJung commented on Oct 8, 2020

    @RalfJung
    Member

    Ah, that looks like the const version of the old "uninhabited match" problem...

    The MIR for unreachable is

    fn unreachable() -> ! {
        let mut _0: !;                       // return place in scope 0 at src/main.rs:1:34: 1:35
    
        bb0: {
            unreachable;                     // scope 0 at src/main.rs:5:11: 5:22
        }
    }
    

    So somehow the deref already vanished, and hence the const analysis cannot catch it.

    But doesn't unsafety checking also happen on the MIR? Why does that work?

  5. DutchGhost commented on Oct 8, 2020

    @DutchGhost
    Author

    It seems that whenever the dereference is not bound to a variable, it compiles, but whenever bound to a variable, it fails to compile:

    const unsafe fn const_deref() {
        use core::convert::Infallible;
        const INFALLIBLE: *const Infallible = [].as_ptr();
        
        match *INFALLIBLE {
            n => {}, // <--- rightfully claims I need #![feature(const_raw_ptr_deref)]
            //_ => {}, // <--- compiles fine
        };
    }
  6. added
    I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
    on Oct 8, 2020
  7. oli-obk commented on Oct 8, 2020

    @oli-obk
    Contributor

    I don't believe this is an unsoundness. Unsafe code is required to cause anything here.

    The MIR for unreachable is

    That's the final MIR, since without unsafe fn this will fail to compile, it must be there for unsafety checking, thus it must also be there for const checking. Not sure why we fail to catch this.

  8. RalfJung commented on Oct 8, 2020

    @RalfJung
    Member

    I don't believe this is an unsoundness. Unsafe code is required to cause anything here.

    Agreed. This is a case of "oops a nightly-only feature is available on stable", which is bad, but it is not an unsoundness.

  9. removed
    I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness
    on Oct 8, 2020
  10. camelid commented on Oct 8, 2020

    @camelid
    Member

    Assigning P-high and removing I-prioritize as discussed in the prioritization working group.

  11. added
    P-highHigh priority
    and removed
    I-prioritizeIssue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}
    on Oct 8, 2020
  12. RalfJung commented on Dec 7, 2020

    @RalfJung
    Member

    The initial MIR looks like this:

    fn unreachable() -> ! {
        let mut _0: !;                       // return place in scope 0 at test2.rs:1:34: 1:35
        let mut _1: !;                       // in scope 0 at test2.rs:1:36: 6:2
        let mut _2: !;                       // in scope 0 at test2.rs:5:5: 5:25
        let mut _3: *const std::convert::Infallible; // in scope 0 at test2.rs:5:12: 5:22
    
        bb0: {
            StorageLive(_1);                 // scope 0 at test2.rs:1:36: 6:2
            StorageLive(_2);                 // scope 0 at test2.rs:5:5: 5:25
            StorageLive(_3);                 // scope 0 at test2.rs:5:12: 5:22
            _3 = const INFALLIBLE;           // scope 0 at test2.rs:5:12: 5:22
                                             // ty::Const
                                             // + ty: *const std::convert::Infallible
                                             // + val: Unevaluated(WithOptConstParam { did: DefId(0:5 ~ test2[317d]::unreachable::INFALLIBLE), const_param_did: None }, [], None)
                                             // mir::Constant
                                             // + span: test2.rs:5:12: 5:22
                                             // + literal: Const { ty: *const std::convert::Infallible, val: Unevaluated(WithOptConstParam { did: DefId(0:5 ~ test2[317d]::unreachable::INFALLIBLE), const_param_did: None }, [], None) }
            FakeRead(ForMatchedPlace, (*_3)); // scope 0 at test2.rs:5:11: 5:22
            unreachable;                     // scope 0 at test2.rs:5:11: 5:22
        }
    
        bb1: {
            unreachable;                     // scope 0 at test2.rs:5:5: 5:25
        }
    
        bb2: {
            StorageDead(_2);                 // scope 0 at test2.rs:5:24: 5:25
            unreachable;                     // scope 0 at test2.rs:1:36: 6:2
        }
    
        bb3: {
            StorageDead(_3);                 // scope 0 at test2.rs:6:1: 6:2
            StorageDead(_1);                 // scope 0 at test2.rs:6:1: 6:2
            return;                          // scope 0 at test2.rs:6:2: 6:2
        }
    }

    So probably the issue is that const-checking ignores FakeRead?

  13. oli-obk commented on Dec 7, 2020

    @oli-obk
    Contributor

    oh... yea.. we ignored it on purpose... "because it doesn't actually do anything" 🙃

    I guess we should visit all places within statements that we otherwise ignore during const-checking?

  14. RalfJung commented on Dec 7, 2020

    @RalfJung
    Member

    Well, at least we need to visit FakeRead. ;)

  15. RalfJung commented on Dec 19, 2020

    @RalfJung
    Member

    oh... yea.. we ignored it on purpose... "because it doesn't actually do anything" upside_down_face

    Do we really? The Deref check is done in visit_projection_elem, which should be called even for the projection in the place in the fake read.

  16. RalfJung commented on Dec 19, 2020

    @RalfJung
    Member

    Oh wait, we don't call super_statement unconditionally! That seems like... a problem.^^

  17. RalfJung commented on Dec 19, 2020

    @RalfJung
    Member

    Proposed a fix: #80199

  18. added 2 commits that reference this issue on Dec 21, 2020
  19. added
    A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)
    on Dec 1, 2024
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-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)A-raw-pointersArea: raw pointers, MaybeUninit, NonNullA-stabilityArea: `#[stable]`, `#[unstable]` etc.C-bugCategory: This is a bug.P-highHigh priority

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions