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

Confusing diagnostic from const eval when offsetting a pointer out of bounds #93881

Open
saethlin opened this issue Feb 10, 2022 · 8 comments
Open
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@saethlin
Copy link
Member

saethlin commented Feb 10, 2022

Given the following code: playground link

#![feature(const_ptr_offset)]
const fn demo() -> *const u8 {
    let x = 0u8;
    let ptr = &x as *const u8;
    unsafe { ptr.offset(3) }
}

const P: *const u8 = demo();

fn main() {}

The current output is:

error[E0080]: evaluation of constant value failed
   --> /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/const_ptr.rs:295:18
    |
295 |         unsafe { intrinsics::offset(self, count) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
    |                  pointer arithmetic failed: alloc2 has size 1, so pointer to 3 bytes starting at offset 0 is out-of-bounds
    |                  inside `ptr::const_ptr::<impl *const u8>::offset` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/const_ptr.rs:295:18

Ideally the output should look like:

error[E0080]: evaluation of constant value failed
   --> /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/const_ptr.rs:295:18
    |
295 |         unsafe { intrinsics::offset(self, count) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
    |                  pointer arithmetic failed: alloc2 has size 1, so pointer to 1 byte starting at offset 3 is out-of-bounds
    |                  inside `ptr::const_ptr::<impl *const u8>::offset` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/const_ptr.rs:295:18

I originally found this through Miri, in huonw/primal#35, though the diagnostic is generated by rustc so I'm opening an issue here. I spent a while in the original example trying to figure out how a *const u8 became a pointer to 3 bytes. Reading over the code that implements this diagnostic, it almost looks like some generic pointer out-of-bounds code was repurposed to provide a diagnostic for invalid offsets. I'd implement an improvement myself but I really can't figure out how to get the size of the pointee type.

@saethlin saethlin added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 10, 2022
@cuviper cuviper added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Feb 10, 2022
@Noratrieb
Copy link
Member

Noratrieb commented Jul 25, 2022

Locally, I get an improved (but still not the desired, the offsets are weird) message like this now on nightly (nightly-2022-07-23):

error[E0080]: evaluation of constant value failed
   --> /home/nilsh/projects/rust/library/core/src/ptr/const_ptr.rs:457:18
    |
457 |         unsafe { intrinsics::offset(self, count) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |                  |
    |                  out-of-bounds pointer arithmetic: alloc3 has size 1, so pointer to 3 bytes starting at offset 0 is out-of-bounds
    |                  inside `ptr::const_ptr::<impl *const u8>::offset` at /home/nilsh/projects/rust/library/core/src/ptr/const_ptr.rs:457:18
    |
   ::: lol.rs:4:14
    |
4   |     unsafe { ptr.offset(3) }
    |              ------------- inside `demo` at lol.rs:4:14
...
7   | const P: *const u8 = demo();
    |                      ------ inside `P` at lol.rs:7:22

But for some reason, there is no error message at all on the playground and it just says

error[E0080]: evaluation of constant value failed
    |
   ::: src/lib.rs:4:14
    |
4   |     unsafe { ptr.offset(3) }
    |              ------------- inside `demo` at src/lib.rs:4:14
...
7   | pub const P: *const u8 = demo();
    |                          ------ inside `P` at src/lib.rs:7:26

on nightly on the playground.

@estebank
Copy link
Contributor

Output after #136503:

error[E0080]: evaluation of constant value failed
   --> roo.rs:8:22
    |
8   | const P: *const u8 = demo();
    |                      ^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to 3 bytes of memory, but got alloc3 which is only 1 byte from the end of the allocation
    |
note: inside `demo`
   --> roo.rs:5:26
    |
5   |                 unsafe { ptr.offset(3) }
    |                          ^^^^^^^^^^^^^
note: inside `std::ptr::const_ptr::<impl *const u8>::offset`
   --> /home/gh-estebank/rust/library/core/src/ptr/const_ptr.rs:455:18
    |
455 |         unsafe { intrinsics::offset(self, count) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the failure occurred here

I believe that that output will be enough to close this ticket. Would you agree?

@saethlin
Copy link
Member Author

I don't even remember filing this issue, and if I could I would emphasize the fact that the panic message itself from const eval is confusing.

But the backtrace is quite helpful.

@RalfJung
Copy link
Member

RalfJung commented Feb 28, 2025

I spent a while in the original example trying to figure out how a *const u8 became a pointer to 3 bytes. Reading over the code that implements this diagnostic, it almost looks like some generic pointer out-of-bounds code was repurposed to provide a diagnostic for invalid offsets.

The pointee type of a raw pointer is irrelevant, and shouldn't affect the error message.

offset documents that the memory range between the old and new pointer must be in-bounds. In this case, that range has size 3. So there is a requirement that those 3 bytes be in-bounds.

Happy for suggestions for how to improve it, but the wording you suggested would not be correct. Specifically, the byte after the new pointer ("1 byte starting at offset 3") does not even have be be in-bounds.

@saethlin
Copy link
Member Author

saethlin commented Feb 28, 2025

The problem is that the diagnostic refers to the trick that the interpreter is using to check this condition, not recognizable attributes of the program. Whether or not the pointee type is relevant to the opsem, it is highly relevant to the user because the behavior of offset depends on the pointee type, and mixing up offsets in terms of T vs bytes is a well-known bug pattern.

out-of-bounds pointer arithmetic: The resulting pointer would point to 1 byte at offset 3 which is not in-bounds for alloc3 which has size 1.

out-of-bounds pointer arithmetic: Offsetting from a pointer to alloc3[0x0..0x1] by 3 bytes would produce a pointer to alloc3[0x3..0x4] which is out of bounds because alloc3 has size 1.

@RalfJung
Copy link
Member

RalfJung commented Feb 28, 2025

would produce a pointer to alloc3[0x3..0x4] which is out of bounds

Again, it doesn't matter whether the produced pointer is out-of-bounds. The produced pointer can actually be in-bounds when this error occurs, e.g. if you start with a pointer at offset 10 and add -5 and the allocation has size 8.

But I agree the wording is confusing. I just don't want it to become incorrect. :)
(I wouldn't call it a "trick"; that memory range being in-bounds is the definition.)

So what about something like:
out-of-bounds pointer arithmetic: $alloc has size $N, so offsetting a pointer to offset $O by $B bytes moves the pointer over a memory range that is not entirely in-bounds of the allocation

@saethlin
Copy link
Member Author

The produced pointer can actually be in-bounds when this error occurs, e.g. if you start with a pointer at offset 10 and add -5 and the allocation has size 8.

We don't have to use the same error message template for every scenario 😉

@RalfJung
Copy link
Member

RalfJung commented Feb 28, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants