offset_from: do not require pointers to be inbounds #112712

RalfJung · 2023-06-16T17:00:26Z

In offset_from, we currently require both pointers to be in-bounds of the same allocation.

There's not really any good motivation for this requirement: all codegen backends implement this intrinsic by casting to integers and doing the subtraction there, and const-eval (the original motivation for having this function in the first place) is fine as long as both pointers are derived from the same allocation, but doesn't care if they are in-bounds.

This restriction also causes problems, as noted in #92512. We allow CTFE to move pointers out-of-bounds with wrapping_offset, but it is currently impossible to determine how far they have been moved, as offset_from is the only "pointer subtraction" operation available in const and it has this restriction on things being in-bounds.

Hence I propose we remove this restriction. @rust-lang/opsem what do you think? (For FCP we'll probably want to add T-lang since this is stable.)

The one interesting question this open is how to interpret the requirement about not wrapping the edge of the address space in CTFE, where we don't know where that edge is. Currently we basically pretend that the edge is at the beginning of the allocation, so a pointer to 4 bytes before that beginning is considered to "wrap" when used with a pointer to 4 bytes after that beginning.

The Miri implementation also needs some work for wildcard pointers (as returned by int2ptr casts). But I figured I'd see if we even want this change before implementing all that.

rustbot · 2023-06-16T17:00:33Z

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-06-16T17:00:37Z

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

rust-log-analyzer · 2023-06-16T17:14:45Z

The job x86_64-gnu-llvm-14 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

98 error[E0080]: could not evaluate static initializer
-   --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
+   --> $SRC_DIR/core/src/slice/raw.rs:LL:COL
100    |
-    = note: out-of-bounds `offset_from`: null pointer is a dangling pointer (it has no provenance)
102    |
- note: inside `ptr::const_ptr::<impl *const u32>::sub_ptr`
-   --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
-   --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
+ note: inside `std::slice::from_raw_parts::<'_, u32>`
+   --> $SRC_DIR/core/src/slice/raw.rs:LL:COL
105 note: inside `from_ptr_range::<'_, u32>`
106   --> $SRC_DIR/core/src/slice/raw.rs:LL:COL
107 note: inside `R0`

The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/const-ptr/forbidden_slices/forbidden_slices.stderr
To update references, rerun the tests and pass the `--bless` flag
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args const-ptr/forbidden_slices.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/const-ptr/forbidden_slices.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Cstrip=debuginfo" "--remap-path-prefix=/checkout/tests/ui=fake-test-src-base" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/const-ptr/forbidden_slices" "-A" "unused" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/const-ptr/forbidden_slices/auxiliary"
stdout: none
error[E0080]: could not evaluate static initializer
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
   |
   = note: dereferencing pointer failed: null pointer is a dangling pointer (it has no provenance)
   = note: dereferencing pointer failed: null pointer is a dangling pointer (it has no provenance)
   |
note: inside `std::slice::from_raw_parts::<'_, u32>`
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
note: inside `S0`
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:19:34
   |
LL | pub static S0: &[u32] = unsafe { from_raw_parts(ptr::null(), 0) };

error[E0080]: could not evaluate static initializer
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
   |
   |
   = note: dereferencing pointer failed: null pointer is a dangling pointer (it has no provenance)
   |
note: inside `std::slice::from_raw_parts::<'_, ()>`
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
note: inside `S1`
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:20:33
   |
LL | pub static S1: &[()] = unsafe { from_raw_parts(ptr::null(), 0) };

error[E0080]: could not evaluate static initializer
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
   |
   |
   = note: dereferencing pointer failed: alloc18 has size 4, so pointer to 8 bytes starting at offset 0 is out-of-bounds
   |
note: inside `std::slice::from_raw_parts::<'_, u32>`
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
note: inside `S2`
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:23:34
   |
LL | pub static S2: &[u32] = unsafe { from_raw_parts(&D0, 2) };

error[E0080]: it is undefined behavior to use this value
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:26:1
   |
   |
LL | pub static S4: &[u8] = unsafe { from_raw_parts((&D1) as *const _ as _, 1) }; //~ ERROR: it is undefined behavior to use this value
   | ^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered uninitialized bytes
   |
   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
   = note: the raw bytes of the constant (size: 16, align: 8) {
           }

error[E0080]: it is undefined behavior to use this value
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:28:1
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:28:1
   |
LL | pub static S5: &[u8] = unsafe { from_raw_parts((&D3) as *const _ as _, size_of::<&u32>()) }; //~ ERROR: it is undefined behavior to use t...
   | ^^^^^^^^^^^^^^^^^^^^ unable to turn pointer into raw bytes
   |
   = note: the raw bytes of the constant (size: 16, align: 8) {
           }
   = help: this code performed an operation that depends on the underlying bytes representing a pointer
   = help: this code performed an operation that depends on the underlying bytes representing a pointer
   = help: the absolute address of a pointer is not known at compile-time, so such operations are not supported
error[E0080]: it is undefined behavior to use this value
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:30:1
   |
   |
LL | pub static S6: &[bool] = unsafe { from_raw_parts((&D0) as *const _ as _, 4) }; //~ ERROR: it is undefined behavior to use this value
   | ^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered 0x11, but expected a boolean
   |
   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
   = note: the raw bytes of the constant (size: 16, align: 8) {
           }

error[E0080]: it is undefined behavior to use this value
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:33:1
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:33:1
   |
LL | pub static S7: &[u16] = unsafe {
   | ^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[1]: encountered uninitialized bytes
   |
   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
   = note: the raw bytes of the constant (size: 16, align: 8) {
               ╾─────alloc50+0x2─────╼ 04 00 00 00 00 00 00 00 │ ╾──────╼........

error[E0080]: could not evaluate static initializer
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
   |
   |
   = note: dereferencing pointer failed: alloc58 has size 8, so pointer to 8 bytes starting at offset 1 is out-of-bounds
   |
note: inside `std::slice::from_raw_parts::<'_, u64>`
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
note: inside `S8`
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:44:5
LL |     from_raw_parts(ptr, 1)
   |     ^^^^^^^^^^^^^^^^^^^^^^

error[E0080]: could not evaluate static initializer
error[E0080]: could not evaluate static initializer
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
   |
   = note: dereferencing pointer failed: null pointer is a dangling pointer (it has no provenance)
   |
note: inside `std::slice::from_raw_parts::<'_, u32>`
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
note: inside `from_ptr_range::<'_, u32>`
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:227:14
note: inside `R0`
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:47:34
   |
LL | pub static R0: &[u32] = unsafe { from_ptr_range(ptr::null()..ptr::null()) };

error[E0080]: could not evaluate static initializer
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:801:9
   |
   |
   = note: the evaluated program panicked at 'assertion failed: 0 < pointee_size && pointee_size <= isize::MAX as usize', /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:801:9
note: inside `ptr::const_ptr::<impl *const ()>::sub_ptr`
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:801:9
note: inside `from_ptr_range::<'_, ()>`
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:227:42
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:227:42
note: inside `R1`
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:48:33
   |
LL | pub static R1: &[()] = unsafe { from_ptr_range(ptr::null()..ptr::null()) };
   = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0080]: could not evaluate static initializer
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:925:18
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:925:18
   |
   = note: out-of-bounds pointer arithmetic: alloc103 has size 4, so pointer to 8 bytes starting at offset 0 is out-of-bounds
note: inside `ptr::const_ptr::<impl *const u32>::add`
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:925:18
note: inside `R2`
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:51:25
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:51:25
   |
LL |     from_ptr_range(ptr..ptr.add(2))

error[E0080]: it is undefined behavior to use this value
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:53:1
   |
   |
LL | pub static R4: &[u8] = unsafe {
   | ^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered uninitialized bytes
   |
   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
   = note: the raw bytes of the constant (size: 16, align: 8) {
           }

error[E0080]: it is undefined behavior to use this value
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:58:1
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:58:1
   |
LL | pub static R5: &[u8] = unsafe {
   | ^^^^^^^^^^^^^^^^^^^^ unable to turn pointer into raw bytes
   |
   = note: the raw bytes of the constant (size: 16, align: 8) {
           }
   = help: this code performed an operation that depends on the underlying bytes representing a pointer
   = help: this code performed an operation that depends on the underlying bytes representing a pointer
   = help: the absolute address of a pointer is not known at compile-time, so such operations are not supported
error[E0080]: it is undefined behavior to use this value
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:63:1
   |
   |
LL | pub static R6: &[bool] = unsafe {
   | ^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .<deref>[0]: encountered 0x11, but expected a boolean
   |
   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
   = note: the raw bytes of the constant (size: 16, align: 8) {
           }

error[E0080]: could not evaluate static initializer
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
   |
   = note: accessing memory with alignment 1, but alignment 2 is required
   |
note: inside `std::slice::from_raw_parts::<'_, u16>`
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:100:9
note: inside `from_ptr_range::<'_, u16>`
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:227:14
note: inside `R7`
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:70:5
   |
LL |     from_ptr_range(ptr..ptr.add(4)) //~ inside `R7`

error[E0080]: could not evaluate static initializer
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:925:18
   |
   |
   = note: out-of-bounds pointer arithmetic: alloc141 has size 8, so pointer to 8 bytes starting at offset 1 is out-of-bounds
note: inside `ptr::const_ptr::<impl *const u64>::add`
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:925:18
note: inside `R8`
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:74:25
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:74:25
   |
LL |     from_ptr_range(ptr..ptr.add(1)) //~ inside `R8`

error[E0080]: could not evaluate static initializer
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:803:18
   |
   |
   = note: `ptr_offset_from_unsigned` called on pointers into different allocations
   |
note: inside `ptr::const_ptr::<impl *const u32>::sub_ptr`
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:803:18
note: inside `from_ptr_range::<'_, u32>`
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:227:42
note: inside `R9`
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:79:34
   |
LL | pub static R9: &[u32] = unsafe { from_ptr_range(&D0..(&D0 as *const u32).add(1)) };

error[E0080]: could not evaluate static initializer
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:803:18
   |
   |
   = note: `ptr_offset_from_unsigned` called on pointers into different allocations
   |
note: inside `ptr::const_ptr::<impl *const u32>::sub_ptr`
  --> /rustc/FAKE_PREFIX/library/core/src/ptr/const_ptr.rs:803:18
note: inside `from_ptr_range::<'_, u32>`
  --> /rustc/FAKE_PREFIX/library/core/src/slice/raw.rs:227:42
note: inside `R10`
  --> fake-test-src-base/const-ptr/forbidden_slices.rs:80:35
   |
LL | pub static R10: &[u32] = unsafe { from_ptr_range(&D0..&D0) };

error: aborting due to 18 previous errors

For more information about this error, try `rustc --explain E0080`.

the8472 · 2023-06-16T17:43:24Z

How does this affect sub_ptr? It has its own intrinsic but its documentation points to the offset_from requirements.

RalfJung · 2023-06-16T20:10:24Z

It is affected the same way, so the docs did not need to change.

JakobDegen · 2023-06-17T06:57:33Z

@RalfJung it's not quite clear to me how I'm expected to be able to use this feature correctly in a const context. If I offset a pointer to outside its allocation, is it ever possible for me to know that such an operation won't overflow the address space? That does happen to be the case right now because all const allocations are at the beginning of the address space, but I don't think that's stable.

Unrelated: Does having the "same allocations" restriction as a safety requirement make sense? It seems like we could 1. drop this requirement outside of const contexts, and 2. promise to throw an error and halt compilation if this requirement is violated in a const context. That seems like a much more user friendly version of things than UB

RalfJung · 2023-06-17T15:17:17Z

Yeah I realized what I really want is a wrapping_offset_from that still requires "based on same alloc" but makes no overflow restrictions. Regarding the second point, as of now we that would be an option though we don't have precedent for such an operation. However, the "based on same alloc" is actually useful for optimizations; it means this operation does not "escape" the provenance (in an alias analysis sense, not an ptr2int semantics sense).

JakobDegen · 2023-06-17T16:10:55Z

I realize I didn't say it before, but I should clarify: Even with my two above points (which I do think should be considered) this PR doesn't make things worse and I'm in support regardless.

Regarding the second point, as of now we that would be an option though we don't have precedent for such an operation. However, the "based on same alloc" is actually useful for optimizations; it means this operation does not "escape" the provenance (in an alias analysis sense, not an ptr2int semantics sense).

I see the point in theory, but I can't actually think of an example where it's useful. Might be worth taking to a separate Zulip thread though?

yotamofek · 2023-06-18T15:18:27Z

library/core/src/ptr/const_ptr.rs

@@ -644,6 +642,12 @@ impl<T: ?Sized> *const T {
    /// (Note that [`offset`] and [`add`] also have a similar limitation and hence cannot be used on
    /// such large allocations either.)
    ///
+    /// The requirement for pointers to be derived from the same allocated object is primarily
+    /// equired for `const`-compatibility: at compile-time, pointers into *different* allocated


Suggested change

/// equired for `const`-compatibility: at compile-time, pointers into *different* allocated

/// required for `const`-compatibility: at compile-time, pointers into *different* allocated

😄

RalfJung · 2023-06-18T18:08:27Z

I realize I didn't say it before, but I should clarify: Even with my two above points (which I do think should be considered) this PR doesn't make things worse and I'm in support regardless.

I tend to disagree. If we add wrapping_offset_from as described above, we have a reasonable symmetry:

if ptr2 was obtained from ptr1 via offset, then offset_from is legal, and conversely if offset_from is legal then obtaining ptr2 from ptr via offset would be possible
if ptr2 was obtained from ptr1 via wrapping_offset, then wrapping_offset_from is legal, and conversely if wrapping_offset_from is legal then obtaining ptr2 from ptr via wrapping_offset would be possible

To maintain that symmetry we should drop the inbounds requirement from both offset and offset_from, not just one of them.

RalfJung · 2023-06-20T11:29:57Z

This PR is superseded by #112837.

RalfJung added 3 commits June 16, 2023 19:00

offset_from: do not require pointers to be inbounds

4e51e91

cleanup

1a4d11a

update tests

db7e809

rustbot assigned b-naber Jun 16, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 16, 2023

yotamofek reviewed Jun 18, 2023

View reviewed changes

RalfJung closed this Jun 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

offset_from: do not require pointers to be inbounds #112712

offset_from: do not require pointers to be inbounds #112712

RalfJung commented Jun 16, 2023

rustbot commented Jun 16, 2023

rustbot commented Jun 16, 2023

rust-log-analyzer commented Jun 16, 2023

the8472 commented Jun 16, 2023

RalfJung commented Jun 16, 2023 via email

JakobDegen commented Jun 17, 2023

RalfJung commented Jun 17, 2023 via email

JakobDegen commented Jun 17, 2023

yotamofek Jun 18, 2023

RalfJung commented Jun 18, 2023

RalfJung commented Jun 20, 2023

	/// equired for `const`-compatibility: at compile-time, pointers into different allocated
	/// required for `const`-compatibility: at compile-time, pointers into different allocated

offset_from: do not require pointers to be inbounds #112712

offset_from: do not require pointers to be inbounds #112712

Conversation

RalfJung commented Jun 16, 2023

rustbot commented Jun 16, 2023

rustbot commented Jun 16, 2023

rust-log-analyzer commented Jun 16, 2023

the8472 commented Jun 16, 2023

RalfJung commented Jun 16, 2023 via email

JakobDegen commented Jun 17, 2023

RalfJung commented Jun 17, 2023 via email

JakobDegen commented Jun 17, 2023

yotamofek Jun 18, 2023

Choose a reason for hiding this comment

RalfJung commented Jun 18, 2023

RalfJung commented Jun 20, 2023