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

replace 'UB on raw ptr deref' with UB on place projection/access #1387

Merged
merged 2 commits into from
Oct 7, 2023

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Jul 31, 2023

As discussed by @rust-lang/opsem in rust-lang/opsem-team#9: instead of saying that *ptr is insta-UB for "bad" pointers, only have UB when the place is actually accessed, and when place projections go out-of-bounds.

This makes the following code (which currently has UB) legal:

let ptr = 23 as *const i32;
// `ptr` is not aligned and doesn't point to an actual allocation.
// Still, this is allowed:
let ptr = addr_of!(*ptr);
// Even with a null pointer, this is allowed:
let ptr = addr_of!(*(0 as *const i32));

However, the following code is still UB:

// Accessing (loading from or storing to) a place that is dangling or unaligned
let mem = 0u32;
let odd_ptr = addr_of!(mem).byte_add(1).cast::<u16>();
let _val = *odd_ptr; // pointer is 1-aligned but must be 2-aligned
// Performing a place projection that violates the requirements of in-bounds pointer arithmetic
let ptr = 23 as *const (i32, i32);
let ptr = addr_of!((*ptr).1); // pointer arithmetic not inside the bounds of an allocation

@ehuss ehuss added the T-opsem Team: opsem label Jul 31, 2023
@RalfJung RalfJung added the T-lang Team: Lang label Jul 31, 2023
@RalfJung
Copy link
Member Author

RalfJung commented Jul 31, 2023

I am not sure if this counts as a "small" decision that t-opsem can just make without t-lang involvement, so I'll add both teams. This is an irreversible decision after all and a change to the core "list of UB", that seems quite fundamental.

The justification for why we think this first example should not be UB is mostly that many people are confused about the entire place expression business, so they don't realize addr_of!(*ptr) is "dereferencing" a pointer. There's also not a clear benefit from making it UB -- our current codegen is already completely compatible with this change, the UB we are removing is only recognized by Miri but never exploited by the compiler.

The justification for keeping the later 2 examples UB is that our current codegen actually does exploit both of them, so allowing them comes at a cost. The first example should be rather uncontroversial; the fact that every access has to be in-bounds and aligned has never been up for debate. The second example is probably less clear, but we do want to keep using getelementptr inbounds for place projections, so we have to make this UB. (Maybe we will come back to this later and weaken place projections from "inbounds" to "nowrap". But for now we don't see a good motivation for doing this, and without a good motivation it is unlikely someone is going to put in the work of implementing those semantics in the codegen backend. LLVM currently only supports "inbounds" and "wrapping", and we definitely don't want "wrapping".)

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Jul 31, 2023

Team member @RalfJung has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@JakobDegen
Copy link

Fwiw I don't expect this example to have needed T-lang signoff, but there's no harm.

@rfcbot reviewed

@saethlin
Copy link
Member

saethlin commented Aug 1, 2023

@rfcbot reviewed

@RalfJung
Copy link
Member Author

RalfJung commented Aug 1, 2023

rust-lang/rust#114330 implements this change in Miri. I found some other interesting cases that are allowed now.

A mere "place mention" of a dangling place is no longer UB -- that is in line with addr_of! no longer being UB here:

fn deref_invalid_underscore() {
    unsafe {
        let _ = *ptr::invalid::<i32>(0);
        let _ = *ptr::invalid::<i32>(1);
    }
}

If xptr only partially points to memory, and (*xptr).1 accesses the inbounds part, then this is now allowed -- both the place projection (pointer arithmetic) and the load from that place are fully inbounds, so there's nothing to cause UB here:

fn deref_partially_dangling() {
    let x = (1, 13);
    let xptr = &x as *const _ as *const (i32, i32, i32);
    let val = unsafe { (*xptr).1 };
    assert_eq!(val, 13);
}

@RalfJung
Copy link
Member Author

RalfJung commented Aug 1, 2023

Ah, and here is another fun one that is allowed now:

fn deref_too_big_slice() {
    unsafe {
        let slice: *const [u8] = mem::transmute((1usize, usize::MAX));
        let _val = addr_of!(*slice);
    }
}

addr_of!(*slice) doesn't check the wide pointer metadata for consistency any more.

@lukas-code
Copy link
Member

fn deref_partially_dangling() {
    let x = (1, 13);
    let xptr = &x as *const _ as *const (i32, i32, i32);
    let val = unsafe { (*xptr).1 };
    assert_eq!(val, 13);
}

Have we decided that the order of the fields in the tuple (i32, i32, i32) is guaranteed? In the UCG it specifically says that tuple fields can be reordered, such that .1 could actually be the third field in memory and out-of-bounds.

@RalfJung
Copy link
Member Author

RalfJung commented Aug 1, 2023

No we have not decided that, the proper version of that test would use a repr(C) struct. I was just lazy...

@scottmcm
Copy link
Member

scottmcm commented Aug 6, 2023

This makes good sense to me. Having addr_of!(*p) be a no-op seems like what most people would expect.

@digama0
Copy link

digama0 commented Aug 7, 2023

@rfcbot reviewed

1 similar comment
@CAD97
Copy link

CAD97 commented Aug 7, 2023

@rfcbot reviewed

@tmandry
Copy link
Member

tmandry commented Aug 17, 2023

This makes good sense to me. Having addr_of!(*p) be a no-op seems like what most people would expect.

I agree with this and don't have any blocking concerns over this change specifically. But I am concerned that this will lead users to a mental model where place projections inside addr_of! (the last example in the description) are also a no-op. I found it surprising that the latter example is UB if the first example is not.

At a high level I would like us to strongly prefer simple rules for what is and isn't UB, wherever possible, and justify complexity as either inherently necessary or temporary. "*foo is insta-UB for bad pointers" is quite simple, despite being overly restrictive. "Only if actually accessed" is quite simple. "Only if actually accessed or a place projection goes out of bounds" is less so.

From the description it sounded like this condition might be temporary (and is due to a current limitation in LLVM), but if we could wave a magic wand and fix the LLVM limitation, are we sure we would?

@saethlin
Copy link
Member

saethlin commented Aug 18, 2023

I am concerned that this will lead users to a mental model...

I agree, generally, but I think this is a problem of documentation. Currently we have essentially no documentation for what valid uses of addr_of! are. Our documentation for this currently just punts to "the usual rules" while never defining that as a means to avoid specifying what uses are and aren't valid, because we haven't made decisions about the rules such as the one that this PR is about.

@scottmcm
Copy link
Member

"Only if actually accessed or a place projection goes out of bounds" is less so.

With any luck offset_of! will help here, since we'll be able to talk about how
addr_of!((*p).foo) is p.byte_add(offset_of!(Type, foo)).cast(), and how p.wrapping_byte_add(offset_of!(Type, foo)).cast() is available for cases where you don't want the UB from GEPi.

@RalfJung
Copy link
Member Author

At a high level I would like us to strongly prefer simple rules for what is and isn't UB, wherever possible, and justify complexity as either inherently necessary or temporary. "*foo is insta-UB for bad pointers" is quite simple, despite being overly restrictive.

Those are the rules we currently have. I also think they are simpler. But many, many people have been caught off-guard by the fact that *foo itself can be UB, even in a context like addr_of!(*foo). Yes, our docs are bad, but at this point I think we also have enough evidence that people strongly expect addr_of!(*foo) to never be UB, and it might be better to align the model with people's expectations -- even if that nominally makes the model more complicated, and given in particular that the extra UB does not actually seem that useful.

"Only if actually accessed" is quite simple. "Only if actually accessed or a place projection goes out of bounds" is less so.

You just have to separate alignment and inbounds:

@RalfJung
Copy link
Member Author

RalfJung commented Aug 18, 2023

From the description it sounded like this condition might be temporary (and is due to a current limitation in LLVM), but if we could wave a magic wand and fix the LLVM limitation, are we sure we would?

If this is talking about the requirement that field projections follow the rules of the offset method, then this is intended to be permanent. I can't imagine a good reason for why we'd want field projections and that method to behave differently.

We considered weakening the requirements of offset from inbounds to nowrap. However, we don't have any motivating usecase for this -- as in, we don't know any real code that people might write that would benefit from weakening the requirement. (The one exception is manual offset_of implementations but we consider that solved by the language-native macro.) Without a usecase, it's not worth trying to convince LLVM to add support for this attribute. Also, this weakening causes some trouble for offset as a const fn -- basically, in a const fn we cannot know if the pointer overflows or not since we don't know where in the address space it lies, so we have to still require inbounds there.

@rfcbot
Copy link

rfcbot commented Aug 18, 2023

🔔 This is now entering its final comment period, as per the review above. 🔔

psst @RalfJung, I wasn't able to add the final-comment-period label, please do so.

@ehuss ehuss added this pull request to the merge queue Oct 7, 2023
Merged via the queue into rust-lang:master with commit 142b2ed Oct 7, 2023
1 check passed
@RalfJung RalfJung deleted the raw-ptr-ub branch October 9, 2023 09:29
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 9, 2023
Update books

## rust-lang/reference

2 commits in 5262e1c3b43a2c489df8f6717683a44c7a2260fd..142b2ed77d33f37a9973772bd95e6144ed9dce43
2023-10-07 19:41:21 UTC to 2023-09-26 12:26:35 UTC

- replace 'UB on raw ptr deref' with UB on place projection/access (rust-lang/reference#1387)
- docs: Fix links to ECMA standards in `attributes.md` (rust-lang/reference#1408)

## rust-lang/rust-by-example

11 commits in c954202c1e1720cba5628f99543cc01188c7d6fc..8eb3a01ab74c567b7174784892fb807f2c632d6b
2023-09-26 12:38:17 UTC to 2023-09-26 12:29:10 UTC

- fixed a typo in the lifetime.md (rust-lang/rust-by-example#1737)
- Misleading textual statement in HOF (rust-lang/rust-by-example#1731)
- Equalize title from respective file with title in SUMMARY.md (rust-lang/rust-by-example#1738)
- Added explanation for compiling and executing match_args.rs. (rust-lang/rust-by-example#1739)
- Wrapped long lines and put #[doc] in backquotes. (rust-lang/rust-by-example#1740)
- Update read_lines example to flatten iterator (rust-lang/rust-by-example#1742)
- Update while_let.md: address inconsistent use of fn main between 2 co… (rust-lang/rust-by-example#1744)
- [TRIVIAL] Remove confusing `also` (rust-lang/rust-by-example#1746)
- Fix and extend the explanation of outer vs inner attributes. (rust-lang/rust-by-example#1748)
- Fix uncorresponded back quote (rust-lang/rust-by-example#1749)
- Fix format in constants.md (rust-lang/rust-by-example#1741)

## rust-lang/rustc-dev-guide

3 commits in a13b7c2..b98af7d
2023-10-05 19:48:35 UTC to 2023-09-27 22:57:27 UTC

- update new trait solver docs (rust-lang/rustc-dev-guide#1802)
- update rustc_driver examples (rust-lang/rustc-dev-guide#1803)
- test headers: fix `compile-flags` example (rust-lang/rustc-dev-guide#1800)
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 10, 2023
Update books

## rust-lang/reference

2 commits in 5262e1c3b43a2c489df8f6717683a44c7a2260fd..142b2ed77d33f37a9973772bd95e6144ed9dce43
2023-10-07 19:41:21 UTC to 2023-09-26 12:26:35 UTC

- replace 'UB on raw ptr deref' with UB on place projection/access (rust-lang/reference#1387)
- docs: Fix links to ECMA standards in `attributes.md` (rust-lang/reference#1408)

## rust-lang/rust-by-example

11 commits in c954202c1e1720cba5628f99543cc01188c7d6fc..8eb3a01ab74c567b7174784892fb807f2c632d6b
2023-09-26 12:38:17 UTC to 2023-09-26 12:29:10 UTC

- fixed a typo in the lifetime.md (rust-lang/rust-by-example#1737)
- Misleading textual statement in HOF (rust-lang/rust-by-example#1731)
- Equalize title from respective file with title in SUMMARY.md (rust-lang/rust-by-example#1738)
- Added explanation for compiling and executing match_args.rs. (rust-lang/rust-by-example#1739)
- Wrapped long lines and put #[doc] in backquotes. (rust-lang/rust-by-example#1740)
- Update read_lines example to flatten iterator (rust-lang/rust-by-example#1742)
- Update while_let.md: address inconsistent use of fn main between 2 co… (rust-lang/rust-by-example#1744)
- [TRIVIAL] Remove confusing `also` (rust-lang/rust-by-example#1746)
- Fix and extend the explanation of outer vs inner attributes. (rust-lang/rust-by-example#1748)
- Fix uncorresponded back quote (rust-lang/rust-by-example#1749)
- Fix format in constants.md (rust-lang/rust-by-example#1741)

## rust-lang/rustc-dev-guide

3 commits in a13b7c2..b98af7d
2023-10-05 19:48:35 UTC to 2023-09-27 22:57:27 UTC

- update new trait solver docs (rust-lang/rustc-dev-guide#1802)
- update rustc_driver examples (rust-lang/rustc-dev-guide#1803)
- test headers: fix `compile-flags` example (rust-lang/rustc-dev-guide#1800)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 10, 2023
Update books

## rust-lang/reference

2 commits in 5262e1c3b43a2c489df8f6717683a44c7a2260fd..142b2ed77d33f37a9973772bd95e6144ed9dce43
2023-10-07 19:41:21 UTC to 2023-09-26 12:26:35 UTC

- replace 'UB on raw ptr deref' with UB on place projection/access (rust-lang/reference#1387)
- docs: Fix links to ECMA standards in `attributes.md` (rust-lang/reference#1408)

## rust-lang/rust-by-example

11 commits in c954202c1e1720cba5628f99543cc01188c7d6fc..8eb3a01ab74c567b7174784892fb807f2c632d6b
2023-09-26 12:38:17 UTC to 2023-09-26 12:29:10 UTC

- fixed a typo in the lifetime.md (rust-lang/rust-by-example#1737)
- Misleading textual statement in HOF (rust-lang/rust-by-example#1731)
- Equalize title from respective file with title in SUMMARY.md (rust-lang/rust-by-example#1738)
- Added explanation for compiling and executing match_args.rs. (rust-lang/rust-by-example#1739)
- Wrapped long lines and put #[doc] in backquotes. (rust-lang/rust-by-example#1740)
- Update read_lines example to flatten iterator (rust-lang/rust-by-example#1742)
- Update while_let.md: address inconsistent use of fn main between 2 co… (rust-lang/rust-by-example#1744)
- [TRIVIAL] Remove confusing `also` (rust-lang/rust-by-example#1746)
- Fix and extend the explanation of outer vs inner attributes. (rust-lang/rust-by-example#1748)
- Fix uncorresponded back quote (rust-lang/rust-by-example#1749)
- Fix format in constants.md (rust-lang/rust-by-example#1741)

## rust-lang/rustc-dev-guide

3 commits in a13b7c2..b98af7d
2023-10-05 19:48:35 UTC to 2023-09-27 22:57:27 UTC

- update new trait solver docs (rust-lang/rustc-dev-guide#1802)
- update rustc_driver examples (rust-lang/rustc-dev-guide#1803)
- test headers: fix `compile-flags` example (rust-lang/rustc-dev-guide#1800)
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Oct 10, 2023
Update books

## rust-lang/reference

2 commits in 5262e1c3b43a2c489df8f6717683a44c7a2260fd..142b2ed77d33f37a9973772bd95e6144ed9dce43
2023-10-07 19:41:21 UTC to 2023-09-26 12:26:35 UTC

- replace 'UB on raw ptr deref' with UB on place projection/access (rust-lang/reference#1387)
- docs: Fix links to ECMA standards in `attributes.md` (rust-lang/reference#1408)

## rust-lang/rust-by-example

11 commits in c954202c1e1720cba5628f99543cc01188c7d6fc..8eb3a01ab74c567b7174784892fb807f2c632d6b
2023-09-26 12:38:17 UTC to 2023-09-26 12:29:10 UTC

- fixed a typo in the lifetime.md (rust-lang/rust-by-example#1737)
- Misleading textual statement in HOF (rust-lang/rust-by-example#1731)
- Equalize title from respective file with title in SUMMARY.md (rust-lang/rust-by-example#1738)
- Added explanation for compiling and executing match_args.rs. (rust-lang/rust-by-example#1739)
- Wrapped long lines and put #[doc] in backquotes. (rust-lang/rust-by-example#1740)
- Update read_lines example to flatten iterator (rust-lang/rust-by-example#1742)
- Update while_let.md: address inconsistent use of fn main between 2 co… (rust-lang/rust-by-example#1744)
- [TRIVIAL] Remove confusing `also` (rust-lang/rust-by-example#1746)
- Fix and extend the explanation of outer vs inner attributes. (rust-lang/rust-by-example#1748)
- Fix uncorresponded back quote (rust-lang/rust-by-example#1749)
- Fix format in constants.md (rust-lang/rust-by-example#1741)

## rust-lang/rustc-dev-guide

3 commits in a13b7c2..b98af7d
2023-10-05 19:48:35 UTC to 2023-09-27 22:57:27 UTC

- update new trait solver docs (rust-lang/rustc-dev-guide#1802)
- update rustc_driver examples (rust-lang/rustc-dev-guide#1803)
- test headers: fix `compile-flags` example (rust-lang/rustc-dev-guide#1800)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 10, 2023
Rollup merge of rust-lang#116574 - rustbot:docs-update, r=ehuss

Update books

## rust-lang/reference

2 commits in 5262e1c3b43a2c489df8f6717683a44c7a2260fd..142b2ed77d33f37a9973772bd95e6144ed9dce43
2023-10-07 19:41:21 UTC to 2023-09-26 12:26:35 UTC

- replace 'UB on raw ptr deref' with UB on place projection/access (rust-lang/reference#1387)
- docs: Fix links to ECMA standards in `attributes.md` (rust-lang/reference#1408)

## rust-lang/rust-by-example

11 commits in c954202c1e1720cba5628f99543cc01188c7d6fc..8eb3a01ab74c567b7174784892fb807f2c632d6b
2023-09-26 12:38:17 UTC to 2023-09-26 12:29:10 UTC

- fixed a typo in the lifetime.md (rust-lang/rust-by-example#1737)
- Misleading textual statement in HOF (rust-lang/rust-by-example#1731)
- Equalize title from respective file with title in SUMMARY.md (rust-lang/rust-by-example#1738)
- Added explanation for compiling and executing match_args.rs. (rust-lang/rust-by-example#1739)
- Wrapped long lines and put #[doc] in backquotes. (rust-lang/rust-by-example#1740)
- Update read_lines example to flatten iterator (rust-lang/rust-by-example#1742)
- Update while_let.md: address inconsistent use of fn main between 2 co… (rust-lang/rust-by-example#1744)
- [TRIVIAL] Remove confusing `also` (rust-lang/rust-by-example#1746)
- Fix and extend the explanation of outer vs inner attributes. (rust-lang/rust-by-example#1748)
- Fix uncorresponded back quote (rust-lang/rust-by-example#1749)
- Fix format in constants.md (rust-lang/rust-by-example#1741)

## rust-lang/rustc-dev-guide

3 commits in a13b7c2..b98af7d
2023-10-05 19:48:35 UTC to 2023-09-27 22:57:27 UTC

- update new trait solver docs (rust-lang/rustc-dev-guide#1802)
- update rustc_driver examples (rust-lang/rustc-dev-guide#1803)
- test headers: fix `compile-flags` example (rust-lang/rustc-dev-guide#1800)
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
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
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/rust#114902, so Cc `@joshlf`
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.
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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants