-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Drop the dereferenceable attribute when a reference is passed in an enum #131834
Comments
@rustbot label -needs-triage |
It's less about the niche and more about the enum. The logic that computes the attribute has to just stop at the enum. An argument of type |
While looking into this I noticed that #![feature(rustc_attrs)]
#![allow(internal_features)]
#![allow(dead_code)]
#[rustc_layout_scalar_valid_range_start(1)]
#[rustc_layout_scalar_valid_range_end(0xfffffffffffffff0)]
pub struct RestrictedAddress(&'static i8);
enum E {
A(RestrictedAddress),
B,
C,
}
fn main() {
std::hint::black_box(E::B);
} causes a debug-only ICE ( |
#131698 should fix that |
I have a fix for this up at #132745 |
Turns out there is already another PR related to this issue, which strangely was not mentioned here: #131739. However, it doesn't fix this issue -- see the comment I posted there. |
This is a very good catch btw, a really subtle bug to spot! |
Rollup merge of rust-lang#132745 - RalfJung:pointee-info-inside-enum, r=DianQK pointee_info_at: fix logic for recursing into enums Fixes rust-lang#131834 The logic in `pointee_info_at` was likely written at a time when the null pointer optimization was the *only* enum layout optimization -- and as `Variant::Multiple` kept getting expanded, nobody noticed that the logic is now unsound. The job of this function is to figure out whether there is a dereferenceable-or-null and aligned pointer at a given offset inside a type. So when we recurse into a multi-variant enum, we better make sure that all the other enum variants must be null! This is the part that was forgotten, and this PR adds it. The reason this didn't explode in many ways so far is that our references only have 1 niche value (null), so it's not possible on stable to have a multi-variant enum with a dereferenceable pointer and other enum variants that are not null. But with `rustc_layout_scalar_valid_range` attributes one can force such a layout, and if `@the8472's` work on alignment niches ever lands, that will make this possible on stable.
… r=DianQK pointee_info_at: fix logic for recursing into enums Fixes rust-lang#131834 The logic in `pointee_info_at` was likely written at a time when the null pointer optimization was the *only* enum layout optimization -- and as `Variant::Multiple` kept getting expanded, nobody noticed that the logic is now unsound. The job of this function is to figure out whether there is a dereferenceable-or-null and aligned pointer at a given offset inside a type. So when we recurse into a multi-variant enum, we better make sure that all the other enum variants must be null! This is the part that was forgotten, and this PR adds it. The reason this didn't explode in many ways so far is that our references only have 1 niche value (null), so it's not possible on stable to have a multi-variant enum with a dereferenceable pointer and other enum variants that are not null. But with `rustc_layout_scalar_valid_range` attributes one can force such a layout, and if `@the8472's` work on alignment niches ever lands, that will make this possible on stable.
Considering the following code, it should print 0 with a release build, but I get a segmentation violation.
This is because we are adding the
dereferenceable
attribute, which allows LLVM to hoist the load instruction. In this case, we need to drop thedereferenceable
attribute.@rustbot label +requires-nightly +I-miscompile +A-codegen
The text was updated successfully, but these errors were encountered: