incorrect UB when when a ! place is constructed (but not loaded!)

@Nadrieril

This code shouldn't have UB but Miri reports UB and rustc generates a SIGILL:

#![feature(never_type)]
fn main() {
    unsafe {
        let x = 3u8;
        let x: *const ! = &x as *const u8 as *const _;
        let _ = *x;
    }
}

Thanks to @Nadrieril for finding this.

Zulip discussion

Labeling as "unsound" since under the rules that @rust-lang/opsem decided for *, and how we've been treating _ in general, this shouldn't SIGILL.

Note that this triggers for ! but not enum Void{} so it's likely a type-based special case

@cjgillot

This is already wrong in the initially generated MIR:

        _6 = &_2;
        _5 = &raw const (*_6);
        _4 = move _5 as *const ! (PtrToPtr);
        AscribeUserType(_4, o, UserTypeProjection { base: UserType(1), projs: [] });
        _3 = _4;
        StorageDead(_5);
        FakeRead(ForLet(None), _3);
        AscribeUserType(_3, o, UserTypeProjection { base: UserType(2), projs: [] });
        StorageDead(_6);
        StorageDead(_4);
        StorageLive(_7);
        StorageLive(_8);
        _8 = (*_3); // an actual load from the place!

If we use bool instead we get just a PlaceMention, as we should

        _5 = &_1;
        _4 = &raw const (*_5);
        _3 = move _4 as *const bool (PtrToPtr);
        AscribeUserType(_3, o, UserTypeProjection { base: UserType(1), projs: [] });
        _2 = _3;
        StorageDead(_4);
        FakeRead(ForLet(None), _2);
        AscribeUserType(_2, o, UserTypeProjection { base: UserType(2), projs: [] });
        StorageDead(_5);
        StorageDead(_3);
        PlaceMention((*_2)); // no load

@cjgillot any idea why we don't just get a PlaceMention for !?

-Zunpretty=thir-tree shows a NeverToAny expression. Maybe this has something to do with it?

Oh, then this looks like fallback behavior: if I annotate let _: ! = *x; I only get a "entering unreachable code" error

Yeah that's also wrong, but different.^^ MIR for that case:

        _6 = &_2;
        _5 = &raw const (*_6);
        _4 = move _5 as *const ! (PtrToPtr);
        AscribeUserType(_4, o, UserTypeProjection { base: UserType(1), projs: [] });
        _3 = _4;
        StorageDead(_5);
        FakeRead(ForLet(None), _3);
        AscribeUserType(_3, o, UserTypeProjection { base: UserType(2), projs: [] });
        StorageDead(_6);
        StorageDead(_4);
        PlaceMention((*_3));
        AscribeUserType((*_3), +, UserTypeProjection { base: UserType(4), projs: [] });
        StorageDead(_3);
        StorageDead(_2);
        unreachable;

So we get a PlaceMention but then we also get a unreachable...

-Zunpretty=thir-tree shows a NeverToAny expression. Maybe this has something to do with it?

Yeah that's probably the right trail. MIR building might not even be at fault, the THIR is already wrong since it thinks there's a never value being constructed here. These THIR nodes seem to be constructed from Adjust::NeverToAny; no idea if there's any way to dump those.

Who are the right people to ping for THIR issues...?

No this bug is specific to !. Equivalent code works fine for empty enums

This is now blocking #129248: making addr_of!(*ptr) a safe operation.

@Nadrieril

Well, I can put up a hack to unblock that specifically. As @Nadrieril pointed out, this is actually two different "bugs" in HIR typeck:

#![feature(never_type)]
fn main() {
    unsafe {
        let x = 3u8;
        let x: *const ! = &x as *const u8 as *const _;
        let _ /* (1.) */ = *x;
        /* (2.) */
    }
}

In the above code:

At (1.), we actually do never type fallback to (). Coercing the ! to () does constitute a read, and therefore is UB.
At (2.), HIR typeck considers the fact that *x evalauted to ! to mean that all following code diverges, and therefore counts the block's type as ! as well.

Only (2.) actually matters for #129248, and if we were to land that PR right now, it would be unsound:

#![feature(never_type)]

fn make_up_a_value<T>() -> T {
    let x: *const ! = 0 as _;
    &raw const *x;
    // Since `*x` is `!`, HIR typeck thinks it diverges and allows the block to coerce to any value.
}

I will probably just put up a hack for (2.), since it's basically the only expression¹ where this matters. This doesn't fix the let _: ! = *x; case, but that should remain unsafe at least, and therefore just regular UB and not unsound.

Other combinations of HIR, such as match scrutinees that have no reads should also ideally be fixed... but that's really hard to fix I fear. ↩

I would say it is unsound in the sense that the reference doesn't say that this is UB. Admittedly the reference isn't clear on the entire point about let _ only evaluating the place expression, but that's what we've been consistently communicating for a while now and it has informed other decisions as well (around match, for instance).

But fixing one of two bugs is still progress, so that'd still be a great first step. :)

Auto merge of rust-lang#129392 - compiler-errors:raw-ref-op-doesnt-di…

Rollup merge of rust-lang#129392 - compiler-errors:raw-ref-op-doesnt-…

Auto merge of rust-lang#129392 - compiler-errors:raw-ref-op-doesnt-di…

Unrolled build for rust-lang#129392

Navigation Menu

Repository navigation

incorrect UB when when a `!` place is constructed (but not loaded!) #117288

41 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

incorrect UB when when a ! place is constructed (but not loaded!) #117288

Description

Activity

RalfJung commented on Oct 27, 2023

Nadrieril commented on Oct 27, 2023

RalfJung commented on Oct 27, 2023

bjorn3 commented on Oct 27, 2023

Nadrieril commented on Oct 27, 2023

RalfJung commented on Oct 27, 2023

RalfJung commented on Oct 28, 2023

41 remaining items

LunarLambda commented on Jun 27, 2024

RalfJung commented on Aug 21, 2024

compiler-errors commented on Aug 21, 2024

Footnotes

RalfJung commented on Aug 21, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

incorrect UB when when a `!` place is constructed (but not loaded!) #117288