Skip to content

Enable a weaker form of -Zrandomize-layout when using debug-assertions #139719

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+112 −35
Draft

Enable a weaker form of -Zrandomize-layout when using debug-assertions #139719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions compiler/rustc_abi/src/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1167,6 +1167,23 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
}
// Otherwise we just leave things alone and actually optimize the type's fields
} else {
// Equal to can_randomize_type_layout, only doesn't need the -Zrandomize-layout
// flag
if !repr.never_randomize_type_layout() && cfg!(feature = "randomize") {
#[cfg(feature = "randomize")]
{
use rand::SeedableRng;
use rand::seq::SliceRandom;
// `ReprOptions.field_shuffle_seed` is a deterministic seed we can use to randomize field
// ordering.
let mut rng = rand_xoshiro::Xoshiro128StarStar::seed_from_u64(
field_seed.wrapping_add(repr.field_shuffle_seed).as_u64(),
);

// Shuffle the ordering of the fields.
optimizing.shuffle(&mut rng);
Comment on lines +1183 to +1184
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍, I like this general approach. We saw in crater that enough places have const asserts on sizes that trying to run with full arbitrary randomization just didn't work, but just shuffling first then still running the layout optimization should get us a meaningful part of the checking. (Though with niche awareness and such now too, there's rather alot of types that it just can't touch, so I don't know if it's even a majority of types where the shuffling will really end up permuting things.)

}
}
// To allow unsizing `&Foo<Type>` -> `&Foo<dyn Trait>`, the layout of the struct must
// not depend on the layout of the tail.
let max_field_align =
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_abi/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ bitflags! {
// Other flags can still inhibit reordering and thus randomization.
// The seed stored in `ReprOptions.field_shuffle_seed`.
const RANDOMIZE_LAYOUT = 1 << 4;
// If true, the type itself has opted out of layout randomization, even under the
// "default-enabled" mode. Internal only, just used for tests that assume the layout of
// types. Does *not* inhibit reordering, just randomization.
const NEVER_RANDOMIZE_LAYOUT = 1 << 5;
// Any of these flags being set prevent field reordering optimisation.
const FIELD_ORDER_UNOPTIMIZABLE = ReprFlags::IS_C.bits()
| ReprFlags::IS_SIMD.bits()
Expand Down Expand Up @@ -207,6 +211,12 @@ impl ReprOptions {
!self.inhibit_struct_field_reordering() && self.flags.contains(ReprFlags::RANDOMIZE_LAYOUT)
}

pub fn never_randomize_type_layout(&self) -> bool {
self.inhibit_struct_field_reordering()
|| self.flags.contains(ReprFlags::NEVER_RANDOMIZE_LAYOUT)
|| self.packed()
}

/// Returns `true` if this `#[repr()]` should inhibit union ABI optimisations.
pub fn inhibits_union_abi_opt(&self) -> bool {
self.c()
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_attr_data_structures/src/attributes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ pub enum AttributeKind {
MacroTransparency(Transparency),
Repr(ThinVec<(ReprAttr, Span)>),
RustcMacroEdition2021,
RustcNeverRandomizeLayout,
Stability {
stability: Stability,
/// Span of the `#[stable(...)]` or `#[unstable(...)]` attribute
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,12 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
(note that the compiler does not even check whether the type indeed is being non-null-optimized; \
it is your responsibility to ensure that the attribute is only used on types that are optimized)",
),
rustc_attr!(
rustc_never_randomize_layout, Normal, template!(Word), ErrorFollowing,
EncodeCrossCrate::Yes,
"the `#[rustc_never_randomize_layout]` attribute is just used to inhibit \
field randomization in rustc tests",
),

// ==========================================================================
// Internal attributes, Misc:
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1484,6 +1484,15 @@ impl<'tcx> TyCtxt<'tcx> {
field_shuffle_seed ^= user_seed;
}

if self.has_attr(did, sym::rustc_never_randomize_layout) {
flags.insert(ReprFlags::NEVER_RANDOMIZE_LAYOUT);
}

// Never randomize layout if we're not running under debug assertions.
if !self.sess.opts.debug_assertions {
flags.insert(ReprFlags::NEVER_RANDOMIZE_LAYOUT);
}
Comment on lines +1491 to +1494
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure: should we have a -Z flag for always/never/if-debug-assertions so that this can be exercised independently? Might be nice to have the way to just turn it off broadly without needing to update the type, like for debugging if something breaks to see if it's because of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm...

Probably, yeah. I don't think it could use the existing -Zrandomize-layout flag since this is a different form of layout randomization. But yeah, a -Zrandomize-layout-weak=true/false/if-debug-assertions (or just making unspecified mean if-debug-assertions) flag would probably be the right move.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or just making unspecified mean if-debug-assertions

Yeah, that's what I was thinking -- use the opt_bool parser and leave it defaulting to exactly what you have here, but have the way for people to force it to one or the other if they really need to.

(Could even then always pass that when compiling library, if it came down to it too, though that's just mentioned as musing. I haven't thought about whether it's actually a good idea or not, and it might be terrible.)


if let Some(reprs) = attr::find_attr!(self.get_all_attrs(did), AttributeKind::Repr(r) => r)
{
for (r, _) in reprs {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1825,6 +1825,7 @@ symbols! {
rustc_main,
rustc_mir,
rustc_must_implement_one_of,
rustc_never_randomize_layout,
rustc_never_returns_null_ptr,
rustc_never_type_options,
rustc_no_mir_inline,
Expand Down
1 change: 1 addition & 0 deletions tests/assembly/issue-83585-small-pod-struct-equality.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//@ assembly-output: emit-asm
//@ compile-flags: -Copt-level=3
//@ only-x86_64
//@ ignore-std-debug-assertions

#![crate_type = "lib"]

Expand Down
1 change: 1 addition & 0 deletions tests/codegen/issues/issue-101082.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//@ compile-flags: -Copt-level=3
//@ revisions: host x86-64-v3
//@ min-llvm-version: 20
//@ ignore-std-debug-assertions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...that would also let tests like this just pass a -Z flag instead of not running.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do codegen tests have the ability to rebuild std? I assume it's some type in std that's being randomized and causing the worse codegen. When I tried disabling debug assertions, it didn't seem to change the codegen at all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this one might have been a bad one to put that comment on.

That said, I'm surprised you're not getting a conflict on this file, 'cause I just updated it yesterday https://github.com/rust-lang/rust/pull/139430/files#diff-a6f7809c5adbbdcc5080d91b5615b2d7ec2e69a5df97470dd3d7d13a9a90cab0R2

(It's been flakey lately in general, so we ended up just disabling the v3 part of it, so if that was the problem you were hitting, it might be already fixed on rebase.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do codegen tests have the ability to rebuild std?

They do not.


// This particular CPU regressed in #131563
//@[x86-64-v3] only-x86_64
Expand Down
5 changes: 4 additions & 1 deletion tests/mir-opt/const_prop/offset_of.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,22 @@
//@ test-mir-pass: GVN
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY

#![feature(offset_of_enum)]
#![feature(offset_of_enum, rustc_attrs)]

use std::marker::PhantomData;
use std::mem::offset_of;

#[rustc_never_randomize_layout]
struct Alpha {
x: u8,
y: u16,
z: Beta,
}

#[rustc_never_randomize_layout]
struct Beta(u8, u8);

#[rustc_never_randomize_layout]
struct Gamma<T> {
x: u8,
y: u16,
Expand Down
4 changes: 4 additions & 0 deletions tests/mir-opt/dataflow-const-prop/offset_of.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
//@ test-mir-pass: DataflowConstProp
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
#![feature(rustc_attrs)]

use std::marker::PhantomData;
use std::mem::offset_of;

#[rustc_never_randomize_layout]
struct Alpha {
x: u8,
y: u16,
z: Beta,
}

#[rustc_never_randomize_layout]
struct Beta(u8, u8);

#[rustc_never_randomize_layout]
struct Gamma<T> {
x: u8,
y: u16,
Expand Down
1 change: 1 addition & 0 deletions tests/ui/abi/abi-sysv64-arg-passing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ mod tests {
pub struct Quad { a: u64, b: u64, c: u64, d: u64 }

#[derive(Copy, Clone)]
#[repr(C)]
pub struct QuadFloats { a: f32, b: f32, c: f32, d: f32 }

#[repr(C)]
Expand Down
1 change: 1 addition & 0 deletions tests/ui/abi/extern/extern-return-FiveU16s.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//@ run-pass
#![allow(improper_ctypes)]

#[repr(C)]
pub struct FiveU16s {
one: u16,
two: u16,
Expand Down
1 change: 1 addition & 0 deletions tests/ui/abi/issue-28676.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![allow(improper_ctypes)]

#[derive(Copy, Clone)]
#[repr(C)]
pub struct Quad {
a: u64,
b: u64,
Expand Down
1 change: 1 addition & 0 deletions tests/ui/abi/issues/issue-62350-sysv-neg-reg-counts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#![allow(improper_ctypes)]

#[derive(Copy, Clone)]
#[repr(C)]
pub struct QuadFloats {
a: f32,
b: f32,
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/consts/const-eval/ub-incorrect-vtable.64bit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ LL | const INVALID_VTABLE_UB: W<&dyn Trait> =
}

error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-incorrect-vtable.rs:91:1
--> $DIR/ub-incorrect-vtable.rs:94:1
|
LL | const G: Wide = unsafe { Transmute { t: FOO }.u };
| ^^^^^^^^^^^^^ constructing invalid value at .1: encountered a dangling reference (going beyond the bounds of its allocation)
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/consts/const-eval/ub-incorrect-vtable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ const INVALID_VTABLE_UB: W<&dyn Trait> =
// Trying to access the data in a vtable does not work, either.

#[derive(Copy, Clone)]
#[repr(C)]
struct Wide<'a>(&'a Foo, &'static VTable);

#[repr(C)]
struct VTable {
drop: Option<for<'a> fn(&'a mut Foo)>,
size: usize,
Expand All @@ -62,6 +64,7 @@ trait Bar {
fn bar(&self) -> u32;
}

#[repr(C)]
struct Foo {
foo: u32,
bar: bool,
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/layout/debug.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
#![crate_type = "lib"]

#[rustc_layout(debug)]
#[rustc_never_randomize_layout]
#[derive(Copy, Clone)]
enum E { Foo, Bar(!, i32, i32) } //~ ERROR: layout_of

#[rustc_layout(debug)]
#[rustc_never_randomize_layout]
struct S { f1: i32, f2: (), f3: i32 } //~ ERROR: layout_of

#[rustc_layout(debug)]
Expand Down
40 changes: 20 additions & 20 deletions tests/ui/layout/debug.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: unions cannot have zero fields
--> $DIR/debug.rs:84:1
--> $DIR/debug.rs:86:1
|
LL | union EmptyUnion {}
| ^^^^^^^^^^^^^^^^^^^
Expand Down Expand Up @@ -101,7 +101,7 @@ error: layout_of(E) = Layout {
unadjusted_abi_align: Align(4 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:8:1
--> $DIR/debug.rs:9:1
|
LL | enum E { Foo, Bar(!, i32, i32) }
| ^^^^^^
Expand Down Expand Up @@ -149,7 +149,7 @@ error: layout_of(S) = Layout {
unadjusted_abi_align: Align(4 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:11:1
--> $DIR/debug.rs:13:1
|
LL | struct S { f1: i32, f2: (), f3: i32 }
| ^^^^^^^^
Expand All @@ -175,7 +175,7 @@ error: layout_of(U) = Layout {
unadjusted_abi_align: Align(4 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:14:1
--> $DIR/debug.rs:16:1
|
LL | union U { f1: (i32, i32), f3: i32 }
| ^^^^^^^
Expand Down Expand Up @@ -316,7 +316,7 @@ error: layout_of(Result<i32, i32>) = Layout {
unadjusted_abi_align: Align(4 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:17:1
--> $DIR/debug.rs:19:1
|
LL | type Test = Result<i32, i32>;
| ^^^^^^^^^
Expand Down Expand Up @@ -346,7 +346,7 @@ error: layout_of(i32) = Layout {
unadjusted_abi_align: Align(4 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:20:1
--> $DIR/debug.rs:22:1
|
LL | type T = impl std::fmt::Debug;
| ^^^^^^
Expand All @@ -372,7 +372,7 @@ error: layout_of(V) = Layout {
unadjusted_abi_align: Align(2 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:27:1
--> $DIR/debug.rs:29:1
|
LL | pub union V {
| ^^^^^^^^^^^
Expand All @@ -398,7 +398,7 @@ error: layout_of(W) = Layout {
unadjusted_abi_align: Align(2 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:33:1
--> $DIR/debug.rs:35:1
|
LL | pub union W {
| ^^^^^^^^^^^
Expand All @@ -424,7 +424,7 @@ error: layout_of(Y) = Layout {
unadjusted_abi_align: Align(2 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:39:1
--> $DIR/debug.rs:41:1
|
LL | pub union Y {
| ^^^^^^^^^^^
Expand All @@ -450,7 +450,7 @@ error: layout_of(P1) = Layout {
unadjusted_abi_align: Align(1 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:46:1
--> $DIR/debug.rs:48:1
|
LL | union P1 { x: u32 }
| ^^^^^^^^
Expand All @@ -476,7 +476,7 @@ error: layout_of(P2) = Layout {
unadjusted_abi_align: Align(1 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:50:1
--> $DIR/debug.rs:52:1
|
LL | union P2 { x: (u32, u32) }
| ^^^^^^^^
Expand All @@ -502,7 +502,7 @@ error: layout_of(P3) = Layout {
unadjusted_abi_align: Align(1 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:58:1
--> $DIR/debug.rs:60:1
|
LL | union P3 { x: F32x4 }
| ^^^^^^^^
Expand All @@ -528,7 +528,7 @@ error: layout_of(P4) = Layout {
unadjusted_abi_align: Align(1 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:62:1
--> $DIR/debug.rs:64:1
|
LL | union P4 { x: E }
| ^^^^^^^^
Expand Down Expand Up @@ -559,7 +559,7 @@ error: layout_of(P5) = Layout {
unadjusted_abi_align: Align(1 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:66:1
--> $DIR/debug.rs:68:1
|
LL | union P5 { zst: [u16; 0], byte: u8 }
| ^^^^^^^^
Expand Down Expand Up @@ -590,19 +590,19 @@ error: layout_of(MaybeUninit<u8>) = Layout {
unadjusted_abi_align: Align(1 bytes),
randomization_seed: $SEED,
}
--> $DIR/debug.rs:69:1
--> $DIR/debug.rs:71:1
|
LL | type X = std::mem::MaybeUninit<u8>;
| ^^^^^^

error: `#[rustc_layout]` can only be applied to `struct`/`enum`/`union` declarations and type aliases
--> $DIR/debug.rs:72:1
--> $DIR/debug.rs:74:1
|
LL | const C: () = ();
| ^^^^^^^^^^^

error[E0277]: the size for values of type `str` cannot be known at compilation time
--> $DIR/debug.rs:80:19
--> $DIR/debug.rs:82:19
|
LL | type Impossible = (str, str);
| ^^^^^^^^^^ doesn't have a size known at compile-time
Expand All @@ -611,19 +611,19 @@ LL | type Impossible = (str, str);
= note: only the last element of a tuple may have a dynamically sized type

error: the type has an unknown layout
--> $DIR/debug.rs:84:1
--> $DIR/debug.rs:86:1
|
LL | union EmptyUnion {}
| ^^^^^^^^^^^^^^^^

error: the type `T` does not have a fixed layout
--> $DIR/debug.rs:90:1
--> $DIR/debug.rs:92:1
|
LL | type TooGeneric<T> = T;
| ^^^^^^^^^^^^^^^^^^

error: `#[rustc_layout]` can only be applied to `struct`/`enum`/`union` declarations and type aliases
--> $DIR/debug.rs:76:5
--> $DIR/debug.rs:78:5
|
LL | const C: () = ();
| ^^^^^^^^^^^
Expand Down
Loading
Loading