replace 'UB on raw ptr deref' with UB on place projection/access by RalfJung · Pull Request #1387 · rust-lang/reference

RalfJung

bors added a commit to rust-lang-ci/rust that referenced this pull request

Oct 9, 2023

bors added a commit to rust-lang-ci/rust that referenced this pull request

Oct 10, 2023

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Oct 10, 2023

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Oct 10, 2023

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Oct 10, 2023

bors added a commit to rust-lang-ci/rust that referenced this pull request

Oct 16, 2023
don't UB on dangling ptr deref, instead check inbounds on projections

This implements rust-lang/reference#1387 in Miri. See that PR for what the change is about.

Detecting dangling references in `let x = &...;` is now done by validity checking only, so some tests need to have validity checking enabled. There is no longer inherently a "nodangle" check in evaluating the expression `&*ptr` (aside from the aliasing model).

r? `@oli-obk`

Based on:
- rust-lang/reference#1387
- rust-lang#115524

This was referenced

Oct 16, 2023

github-actions bot pushed a commit to rust-lang/miri that referenced this pull request

Oct 17, 2023
don't UB on dangling ptr deref, instead check inbounds on projections

This implements rust-lang/reference#1387 in Miri. See that PR for what the change is about.

Detecting dangling references in `let x = &...;` is now done by validity checking only, so some tests need to have validity checking enabled. There is no longer inherently a "nodangle" check in evaluating the expression `&*ptr` (aside from the aliasing model).

r? `@oli-obk`

Based on:
- rust-lang/reference#1387
- rust-lang/rust#115524

bors added a commit to rust-lang-ci/rust that referenced this pull request

Nov 4, 2023
Update the alignment checks to match rust-lang/reference#1387

Previously, we had a special case to not check `Rvalue::AddressOf` in this pass because we weren't quite sure if pointers needed to be aligned in the Place passed to it: rust-lang#112026

Since rust-lang/reference#1387 merged, this PR updates this pass to match. The behavior of the check is nearly unchanged, except we also avoid inserting a check for creating references. Most of the changes in this PR are cleanup and new tests.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Nov 4, 2023
Update the alignment checks to match rust-lang/reference#1387

Previously, we had a special case to not check `Rvalue::AddressOf` in this pass because we weren't quite sure if pointers needed to be aligned in the Place passed to it: rust-lang#112026

Since rust-lang/reference#1387 merged, this PR updates this pass to match. The behavior of the check is nearly unchanged, except we also avoid inserting a check for creating references. Most of the changes in this PR are cleanup and new tests.

bors added a commit to rust-lang-ci/rust that referenced this pull request

Nov 10, 2023
update and clarify addr_of docs

This updates the docs to match rust-lang/reference#1387. Cc `@rust-lang/opsem`

`@chorman0773` not sure if you had anything else you wanted to say here, I'd be happy to get your feedback. :)

Fixes rust-lang#114902, so Cc `@joshlf`

github-actions bot pushed a commit to rust-lang/miri that referenced this pull request

Nov 15, 2023
Update the alignment checks to match rust-lang/reference#1387

Previously, we had a special case to not check `Rvalue::AddressOf` in this pass because we weren't quite sure if pointers needed to be aligned in the Place passed to it: rust-lang/rust#112026

Since rust-lang/reference#1387 merged, this PR updates this pass to match. The behavior of the check is nearly unchanged, except we also avoid inserting a check for creating references. Most of the changes in this PR are cleanup and new tests.

github-actions bot pushed a commit to rust-lang/miri that referenced this pull request

Nov 15, 2023

lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request

Apr 7, 2024
Update the alignment checks to match rust-lang/reference#1387

Previously, we had a special case to not check `Rvalue::AddressOf` in this pass because we weren't quite sure if pointers needed to be aligned in the Place passed to it: rust-lang/rust#112026

Since rust-lang/reference#1387 merged, this PR updates this pass to match. The behavior of the check is nearly unchanged, except we also avoid inserting a check for creating references. Most of the changes in this PR are cleanup and new tests.

RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request

Apr 27, 2024
Update the alignment checks to match rust-lang/reference#1387

Previously, we had a special case to not check `Rvalue::AddressOf` in this pass because we weren't quite sure if pointers needed to be aligned in the Place passed to it: rust-lang/rust#112026

Since rust-lang/reference#1387 merged, this PR updates this pass to match. The behavior of the check is nearly unchanged, except we also avoid inserting a check for creating references. Most of the changes in this PR are cleanup and new tests.

@CAD97 CAD97 mentioned this pull request

Aug 20, 2024

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request

Oct 24, 2024
…nethercote

Taking a raw ref (`&raw (const|mut)`) of a deref of pointer (`*ptr`) is always safe

T-opsem decided in rust-lang/reference#1387 that `*ptr` is only unsafe if the place is accessed. This means that taking a raw ref of a deref expr is always safe, since it doesn't constitute a read.

This also relaxes the `DEREF_NULLPTR` lint to stop warning in the case of raw ref of a deref'd nullptr, and updates its docs to reflect that change in the UB specification.

This does not change the behavior of `addr_of!((*ptr).field)`, since field projections still require the projection is in-bounds.

I'm on the fence whether this requires an FCP, since it's something that is guaranteed by the reference you could ostensibly call this a bugfix since we were counting truly safe operations as unsafe. Perhaps someone on opsem has a strong opinion? cc `@rust-lang/opsem`

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Oct 24, 2024
Rollup merge of rust-lang#129248 - compiler-errors:raw-ref-deref, r=nnethercote

Taking a raw ref (`&raw (const|mut)`) of a deref of pointer (`*ptr`) is always safe

T-opsem decided in rust-lang/reference#1387 that `*ptr` is only unsafe if the place is accessed. This means that taking a raw ref of a deref expr is always safe, since it doesn't constitute a read.

This also relaxes the `DEREF_NULLPTR` lint to stop warning in the case of raw ref of a deref'd nullptr, and updates its docs to reflect that change in the UB specification.

This does not change the behavior of `addr_of!((*ptr).field)`, since field projections still require the projection is in-bounds.

I'm on the fence whether this requires an FCP, since it's something that is guaranteed by the reference you could ostensibly call this a bugfix since we were counting truly safe operations as unsafe. Perhaps someone on opsem has a strong opinion? cc `@rust-lang/opsem`

jhpratt added a commit to jhpratt/rust that referenced this pull request

Jan 15, 2025
Update `ReadDir::next` in `std::sys::pal::unix::fs` to use `&raw const (*p).field` instead of `p.byte_offset().cast()`

Since rust-lang/reference#1387 and rust-lang#117572, `&raw mut (*p).field`/`addr_of!((*p).field)` is defined to have the same inbounds preconditions as `ptr::offset`/`ptr::byte_offset`. I.e. `&raw const (*p).field` does not require that `p: *const T` point to a full `size_of::<T>()` bytes of memory, only that `p.byte_add(offset_of!(T, field))` is defined.

The old comment "[...] we don't even get to use `&raw const (*entry_ptr).d_name` because that operation requires the full extent of *entry_ptr to be in bounds of the same allocation, which is not necessarily the case here [...]" is now outdated, and the code can be simplified to use `&raw const (*entry_ptr).field`.

-------

There should be no behavior differences from this PR.

The `: *const dirent64` on line 716 and the `const _: usize = mem::offset_of!(dirent64, $field);` and comment on lines 749-751 are just sanity checks and should not affect semantics.

Since the `offset_ptr!` macro is only called three times, and all with the same local variable entry_ptr, I just used the local variable directly in the macro instead of taking it as an input, and renamed the macro to `entry_field_ptr!`.

The whole macro could also be removed and replaced with just using `&raw const (*entry_ptr).field`  in the three places, but the comments on the macro seemed worthwhile to keep.

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Jan 15, 2025
Rollup merge of rust-lang#134678 - zachs18:offset-ptr-update, r=tgross35

Update `ReadDir::next` in `std::sys::pal::unix::fs` to use `&raw const (*p).field` instead of `p.byte_offset().cast()`

Since rust-lang/reference#1387 and rust-lang#117572, `&raw mut (*p).field`/`addr_of!((*p).field)` is defined to have the same inbounds preconditions as `ptr::offset`/`ptr::byte_offset`. I.e. `&raw const (*p).field` does not require that `p: *const T` point to a full `size_of::<T>()` bytes of memory, only that `p.byte_add(offset_of!(T, field))` is defined.

The old comment "[...] we don't even get to use `&raw const (*entry_ptr).d_name` because that operation requires the full extent of *entry_ptr to be in bounds of the same allocation, which is not necessarily the case here [...]" is now outdated, and the code can be simplified to use `&raw const (*entry_ptr).field`.

-------

There should be no behavior differences from this PR.

The `: *const dirent64` on line 716 and the `const _: usize = mem::offset_of!(dirent64, $field);` and comment on lines 749-751 are just sanity checks and should not affect semantics.

Since the `offset_ptr!` macro is only called three times, and all with the same local variable entry_ptr, I just used the local variable directly in the macro instead of taking it as an input, and renamed the macro to `entry_field_ptr!`.

The whole macro could also be removed and replaced with just using `&raw const (*entry_ptr).field`  in the three places, but the comments on the macro seemed worthwhile to keep.

github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request

Mar 11, 2025
Update `ReadDir::next` in `std::sys::pal::unix::fs` to use `&raw const (*p).field` instead of `p.byte_offset().cast()`

Since rust-lang/reference#1387 and rust-lang#117572, `&raw mut (*p).field`/`addr_of!((*p).field)` is defined to have the same inbounds preconditions as `ptr::offset`/`ptr::byte_offset`. I.e. `&raw const (*p).field` does not require that `p: *const T` point to a full `size_of::<T>()` bytes of memory, only that `p.byte_add(offset_of!(T, field))` is defined.

The old comment "[...] we don't even get to use `&raw const (*entry_ptr).d_name` because that operation requires the full extent of *entry_ptr to be in bounds of the same allocation, which is not necessarily the case here [...]" is now outdated, and the code can be simplified to use `&raw const (*entry_ptr).field`.

-------

There should be no behavior differences from this PR.

The `: *const dirent64` on line 716 and the `const _: usize = mem::offset_of!(dirent64, $field);` and comment on lines 749-751 are just sanity checks and should not affect semantics.

Since the `offset_ptr!` macro is only called three times, and all with the same local variable entry_ptr, I just used the local variable directly in the macro instead of taking it as an input, and renamed the macro to `entry_field_ptr!`.

The whole macro could also be removed and replaced with just using `&raw const (*entry_ptr).field`  in the three places, but the comments on the macro seemed worthwhile to keep.