-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Null ptr check doesn't catch null reference to ZSTs #136568
Labels
C-bug
Category: This is a bug.
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Comments
@zjp-CN This cannot be checked by library code. |
Hm, the pass does insert check for borrows... odd. |
We don't insert null checks for places of ZSTs regardless of how those places are being used. I opened #136601 to ensure that we enforce this for borrows even for ZSTs. |
Urgau
added a commit
to Urgau/rust
that referenced
this issue
Feb 8, 2025
…=saethlin Detect (non-raw) borrows of null ZST pointers in CheckNull Fixes rust-lang#136568. Ensures that we check that borrows of derefs are non-null in the `CheckNull` pass **even if** it's a ZST pointee. I'm actually surprised that this is UB in Miri, but if it's certainly UB, then this PR modifies the null check to be stricter. I couldn't find anywhere in https://doc.rust-lang.org/reference/behavior-considered-undefined.html that discusses this case specifically, but I didn't read it too closely, or perhaps it's just missing a bullet point. On the contrary, if this is actually erroneous UB in Miri, then I'm happy to close this (and perhaps fix the null check in Miri to exclude ZSTs?) On the double contrary, if this is still an "open question", I'm also happy to close this and wait for a decision to be made. r? `@saethlin` cc `@RalfJung` (perhaps you feel strongly about this change)
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this issue
Feb 9, 2025
Rollup merge of rust-lang#136601 - compiler-errors:borrow-null-zst, r=saethlin Detect (non-raw) borrows of null ZST pointers in CheckNull Fixes rust-lang#136568. Ensures that we check that borrows of derefs are non-null in the `CheckNull` pass **even if** it's a ZST pointee. I'm actually surprised that this is UB in Miri, but if it's certainly UB, then this PR modifies the null check to be stricter. I couldn't find anywhere in https://doc.rust-lang.org/reference/behavior-considered-undefined.html that discusses this case specifically, but I didn't read it too closely, or perhaps it's just missing a bullet point. On the contrary, if this is actually erroneous UB in Miri, then I'm happy to close this (and perhaps fix the null check in Miri to exclude ZSTs?) On the double contrary, if this is still an "open question", I'm also happy to close this and wait for a decision to be made. r? ``@saethlin`` cc ``@RalfJung`` (perhaps you feel strongly about this change)
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.
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
As #134424 recently landed, for the following code, I'd expect the check will catch null reference as Miri does, but actually not:
As per rust-lang/unsafe-code-guidelines#472 , access to a ptr to ZST is not UB, so
*ptr
is ok.But it could be better to catch null reference
&*ptr
:Not sure if I put up a real issue. I just thought the check would act on null reference, since in some cases the term pointer may refer to a reference as well.
I guess we need another check for null reference, because null ptr is not harmful, dereferencing to it is harmful, while it doesn't need dereferencing for null reference to be UB.
The text was updated successfully, but these errors were encountered: