Skip to content

Commit f52a0d9

Browse files
committed
pointee_info_at: fix logic for recursing into enums
1 parent 3d1dba8 commit f52a0d9

File tree

4 files changed

+53
-16
lines changed

4 files changed

+53
-16
lines changed

compiler/rustc_abi/src/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -1745,6 +1745,10 @@ pub enum PointerKind {
17451745

17461746
/// Note that this information is advisory only, and backends are free to ignore it.
17471747
/// It can only be used to encode potential optimizations, but no critical information.
1748+
///
1749+
/// Note that all this information is implicitly combined with "or null", i.e.,
1750+
/// no matter the invariant encoded here, a null pointer is always considered to
1751+
/// satisfy it. This is so that we can give a `PointeeInfo` to `Option<&T>`.
17481752
#[derive(Copy, Clone, Debug)]
17491753
pub struct PointeeInfo {
17501754
pub size: Size,

compiler/rustc_middle/src/ty/layout.rs

+27-11
Original file line numberDiff line numberDiff line change
@@ -1011,25 +1011,41 @@ where
10111011
}
10121012

10131013
_ => {
1014-
let mut data_variant = match this.variants {
1014+
let mut data_variant = match &this.variants {
10151015
// Within the discriminant field, only the niche itself is
10161016
// always initialized, so we only check for a pointer at its
10171017
// offset.
10181018
//
1019-
// If the niche is a pointer, it's either valid (according
1020-
// to its type), or null (which the niche field's scalar
1021-
// validity range encodes). This allows using
1022-
// `dereferenceable_or_null` for e.g., `Option<&T>`, and
1023-
// this will continue to work as long as we don't start
1024-
// using more niches than just null (e.g., the first page of
1025-
// the address space, or unaligned pointers).
1019+
// Our goal here is to check whether this represents a
1020+
// "dereferenceable or null" pointer, so we need to ensure
1021+
// that there is only one other variant, and it must be null.
1022+
// Below, we will then check whether the pointer is indeed
1023+
// dereferenceable.
10261024
Variants::Multiple {
1027-
tag_encoding: TagEncoding::Niche { untagged_variant, .. },
1025+
tag_encoding:
1026+
TagEncoding::Niche { untagged_variant, niche_variants, niche_start },
10281027
tag_field,
1028+
variants,
10291029
..
1030-
} if this.fields.offset(tag_field) == offset => {
1031-
Some(this.for_variant(cx, untagged_variant))
1030+
} if variants.len() == 2 && this.fields.offset(*tag_field) == offset => {
1031+
let tagged_variant = if untagged_variant.as_u32() == 0 {
1032+
VariantIdx::from_u32(1)
1033+
} else {
1034+
VariantIdx::from_u32(0)
1035+
};
1036+
assert_eq!(tagged_variant, *niche_variants.start());
1037+
if *niche_start == 0 {
1038+
// The other variant is encoded as "null", so we can recurse searching for
1039+
// a pointer here. This relies on the fact that the codegen backend
1040+
// only adds "dereferenceable" if there's also a "nonnull" proof,
1041+
// and that null is aligned for all alignments so it's okay to forward
1042+
// the pointer's alignment.
1043+
Some(this.for_variant(cx, *untagged_variant))
1044+
} else {
1045+
None
1046+
}
10321047
}
1048+
Variants::Multiple { .. } => None,
10331049
_ => Some(this),
10341050
};
10351051

compiler/rustc_target/src/callconv/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ pub struct ArgAttributes {
143143
pub regular: ArgAttribute,
144144
pub arg_ext: ArgExtension,
145145
/// The minimum size of the pointee, guaranteed to be valid for the duration of the whole call
146-
/// (corresponding to LLVM's dereferenceable and dereferenceable_or_null attributes).
146+
/// (corresponding to LLVM's dereferenceable_or_null attributes, i.e., it is okay for this to be
147+
/// set on a null pointer, but all non-null pointers must be dereferenceable).
147148
pub pointee_size: Size,
148149
pub pointee_align: Option<Align>,
149150
}

tests/codegen/function-arguments.rs

+20-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//@ compile-flags: -O -C no-prepopulate-passes
22
#![crate_type = "lib"]
3+
#![feature(rustc_attrs)]
34
#![feature(dyn_star)]
45
#![feature(allocator_api)]
56

@@ -143,13 +144,28 @@ pub fn indirect_struct(_: S) {}
143144
#[no_mangle]
144145
pub fn borrowed_struct(_: &S) {}
145146

146-
// CHECK: @option_borrow(ptr noalias noundef readonly align 4 dereferenceable_or_null(4) %x)
147+
// CHECK: @option_borrow(ptr noalias noundef readonly align 4 dereferenceable_or_null(4) %_x)
147148
#[no_mangle]
148-
pub fn option_borrow(x: Option<&i32>) {}
149+
pub fn option_borrow(_x: Option<&i32>) {}
149150

150-
// CHECK: @option_borrow_mut(ptr noalias noundef align 4 dereferenceable_or_null(4) %x)
151+
// CHECK: @option_borrow_mut(ptr noalias noundef align 4 dereferenceable_or_null(4) %_x)
151152
#[no_mangle]
152-
pub fn option_borrow_mut(x: Option<&mut i32>) {}
153+
pub fn option_borrow_mut(_x: Option<&mut i32>) {}
154+
155+
// Function that must NOT have `dereferenceable` or `align`.
156+
#[rustc_layout_scalar_valid_range_start(16)]
157+
pub struct RestrictedAddress(&'static i16);
158+
enum E {
159+
A(RestrictedAddress),
160+
B,
161+
C,
162+
}
163+
// If the `nonnull` ever goes missing, you might have to tweak the
164+
// scalar_valid_range on `RestrictedAddress` to get it back. You
165+
// might even have to add a `rustc_layout_scalar_valid_range_end`.
166+
// CHECK: @nonnull_and_nondereferenceable(ptr noundef nonnull %_x)
167+
#[no_mangle]
168+
pub fn nonnull_and_nondereferenceable(_x: E) {}
153169

154170
// CHECK: @raw_struct(ptr noundef %_1)
155171
#[no_mangle]

0 commit comments

Comments
 (0)