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

Promotion rules are surprising and insufficiently documented #105270

Open
AnthonyMikh opened this issue Dec 4, 2022 · 6 comments
Open

Promotion rules are surprising and insufficiently documented #105270

AnthonyMikh opened this issue Dec 4, 2022 · 6 comments
Labels
C-bug Category: This is a bug. needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged.

Comments

@AnthonyMikh
Copy link
Contributor

The code in question (playground link):

const S_CONST: &'static [u8] = unsafe {
    std::slice::from_raw_parts(
        std::convert::identity([0]).as_ptr(),
        1,
    )
};

fn main() {
    let s_rt: &'static [u8] = unsafe {
        std::slice::from_raw_parts(
            std::convert::identity([0]).as_ptr(),
            1,
        )
    };
    println!("{s_rt:?}");
    println!("{S_CONST:?}");
}

When I run this code under miri, it complains about dangling reference on attempt to print s_rt:

error: Undefined Behavior: constructing invalid value: encountered a dangling reference (use-after-free)
    --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:2382:1
     |
2382 | fmt_refs! { Debug, Display, Octal, Binary, LowerHex, UpperHex, LowerExp, UpperExp }
     | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered a dangling reference (use-after-free)
     |
     = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
     = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
     = note: BACKTRACE:
     = note: inside `<&[u8] as std::fmt::Debug>::fmt` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:2372:71
     = note: inside `std::fmt::write` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:1208:17
     = note: inside `<std::io::StdoutLock<'_> as std::io::Write>::write_fmt` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/mod.rs:1682:15
     = note: inside `<&std::io::Stdout as std::io::Write>::write_fmt` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:716:9
     = note: inside `<std::io::Stdout as std::io::Write>::write_fmt` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:690:9
     = note: inside `std::io::stdio::print_to::<std::io::Stdout>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:1008:21
     = note: inside `std::io::_print` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/io/stdio.rs:1075:5
note: inside `main` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/macros.rs:136:9
    --> src/main.rs:15:5
     |
15   |     println!("{s_rt:?}");
     |     ^^^^^^^^^^^^^^^^^^^^
     = note: this error originates in the macro `fmt_refs` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)

However, if I comment out this line and keep the line which prints S_CONST, miri runs this program just fine.

Meta

I used the latest stable at the moment (1.65.0), but since miri requires nightly toolchain anyway I believe it used latest nightly (2022-12-03 2341517).

@AnthonyMikh AnthonyMikh added the C-bug Category: This is a bug. label Dec 4, 2022
@steffahn
Copy link
Member

steffahn commented Feb 2, 2023

As unfortunately somewhat underdocumented, but pointed out here

[in] the bodies of const/static initializers […] we will promote calls to arbitrary const fn

whereas for normal function bodies

we only promote code that can never fail to evaluate


This means that the result of the call to std::convert::identity([0]) which is implicitly borrowed by as_ptr() will be promoted to a constant, so that a &'static [u8; 1] is crated, which is then coerced into &[u8], and passed to as_ptr. This value refers to static memory and thus will never be dropped.

On the other hand, in the main funciton, the same kind of code will not result in promotion, because std::convert::identity is (conservatively) assumed to be a function that can fail; so that – without constant promotion – the result of std::convert::identity([0]) is computed at run-time and stored in a temporary that gets dropped at the end of the statement.

So as a result, there is no bug here :-)

@AnthonyMikh
Copy link
Contributor Author

Okay, maybe it is not a bug per se, but it just looks very weird when compile-time unsafe code has greater capabilities than the same runtime code. Besides the rules for const promotion are not properly documented and are subject to change over time, so the situation when miri accepts this code seems fragile to me.

@RalfJung
Copy link
Member

Yeah here's no Miri bug here, we just (unfortunately) desugar code differently in const vs regular code. I'd love to not promote this in const but that would be too much of a breaking change.

So if there is a bug here it is in documenting our promotion rules. I don't know where those docs would go.

@RalfJung RalfJung changed the title miri considers the very same code UB in runtime but ok at compile time Promotion rules are surprising and insufficiently documented Apr 26, 2023
@RalfJung
Copy link
Member

Cc @rust-lang/wg-const-eval

@AnthonyMikh
Copy link
Contributor Author

Yeah here's no Miri bug here, we just (unfortunately) desugar code differently in const vs regular code.

Could you elaborate on the difference?

I'd love to not promote this in const but that would be too much of a breaking change.

Is it just your guess? If so, it would be better to check it via crater run, I presume.

So if there is a bug here it is in documenting our promotion rules. I don't know where those docs would go.

Both Reference and Nomicon seem fitting.

@RalfJung
Copy link
Member

RalfJung commented Apr 29, 2023

Could you elaborate on the difference?

Everything behaves as intended. And the part of the compiler that doesn't behave the way you expect it to isn't even in Miri, it is in rustc. Even without Miri, your code (the runtime version of this) has UB at runtime, it accesses dangling memory.

Is it just your guess? If so, it would be better to check it via crater run, I presume.

We did crater runs for this a few years ago. Also see #80619.

@ChrisDenton ChrisDenton added the needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged. label Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. needs-triage-legacy Old issue that were never triaged. Remove this label once the issue has been sufficiently triaged.
Projects
None yet
Development

No branches or pull requests

4 participants