`-Zrandomize-layout` harder. `Foo<T> != Foo` #133088

the8472 · 2024-11-16T00:58:18Z

Tracking issue: #106764

Previously randomize-layout only used a deterministic shuffle based on the seed stored in an Adt's ReprOptions, meaning that Foo<T> and Foo were shuffled by the same seed. This change adds a similar seed to each calculated LayoutData so that a struct can be randomized both based on the layout of its fields and its per-type seed.
Primitives start with simple seed derived from some of their properties. Though some types can no longer be distinguished at that point, e.g. usize and u64 will still be treated the same.

rustbot · 2024-11-16T00:58:28Z

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-11-16T01:49:18Z

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

petrochenkov · 2024-11-16T19:10:58Z

r? compiler

compiler-errors · 2024-11-24T04:49:36Z

@bors try @rust-timer queue

`-Zrandomize-layout` harder. `Foo<T> != Foo` Tracking issue: rust-lang#106764 Previously randomize-layout only used a deterministic shuffle based on the seed stored in an Adt's ReprOptions, meaning that `Foo<T>` and `Foo` were shuffled by the same seed. This change adds a similar seed to each calculated LayoutData so that a struct can be randomized both based on the layout of its fields and its per-type seed. Primitives start with simple seed derived from some of their properties. Though some types can no longer be distinguished at that point, e.g. usize and u64 will still be treated the same.

bors · 2024-11-24T04:50:48Z

⌛ Trying commit c276116 with merge 0e4cbcd...

compiler/rustc_abi/src/layout.rs

compiler-errors · 2024-11-24T04:52:43Z

compiler/rustc_abi/src/layout.rs

@@ -1043,10 +1067,12 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
                {
                    use rand::SeedableRng;
                    use rand::seq::SliceRandom;
+                    //let field_entropy = fields_excluding_tail.iter().map(|f| f.).sum();


remove commented out code

bors · 2024-11-24T06:38:33Z

☀️ Try build successful - checks-actions
Build commit: 0e4cbcd (0e4cbcded5bee0aabf1c9b10281afc7b466a961d)

rust-timer · 2024-11-24T07:55:42Z

Finished benchmarking commit (0e4cbcd): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (secondary 3.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.5%	[3.5%, 3.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 797.356s -> 796.07s (-0.16%)
Artifact size: 336.34 MiB -> 336.30 MiB (-0.01%)

bors · 2024-12-27T16:23:58Z

☔ The latest upstream changes (presumably #134822) made this pull request unmergeable. Please resolve the merge conflicts.

tests/ui/layout/thumb-enum.stderr

tests/ui/layout/issue-96185-overaligned-enum.stderr

compiler/rustc_abi/src/lib.rs

the8472 · 2025-01-08T23:55:43Z

I have updated the randomization UI test and added some cases that people on zulip were concerned about.

tests/ui/layout/randomize.rs

workingjubilee · 2025-01-09T22:55:30Z

tests/ui/layout/randomize.rs

Hm. Do we already have a test for MaybeUninit<T> and T having matching layouts, even under the influence of -Zrandomize-layout? I'm surprised it wasn't diffed here if so (or maybe I'm missing something in the GH view...)

MaybeUninit is repr(transparent) not repr(Rust). Randomization only applies to the latter.

If randomization injected front padding then Option would be an interesting case because it makes guarantees without having a special repr. But currently randomization only affects field order (and thus only structs with more than 1 field), it does not yet add padding beyond what's required by the alignment of the reordered fields.

I've added the Option case for future randomization improvements, though I'd expect things to blow up earlier if someone broke this.

thanks! yeah I was more wondering about "is there something we need to be making sure we aren't screwing up because it's already semantically guaranteed...?" but you're right, the transparent repr should take care of that.

compiler/rustc_abi/src/layout.rs

workingjubilee · 2025-01-09T23:01:38Z

compiler/rustc_abi/src/lib.rs

@@ -1748,6 +1759,7 @@ impl<FieldIdx: Idx, VariantIdx: Idx> LayoutData<FieldIdx, VariantIdx> {
            align,
            max_repr_align: None,
            unadjusted_abi_align: align.abi,
+            randomization_seed: size.bytes().wrapping_add(seed_extra << 32),


Can you lift this up into a single let randomization_seed and comment that the combination of choices is to always distinguish the seeds of the minimal pairs

f32, f64 (byte size)

*mut (), usize (base type)

i32, u32 (signedness)

or just "all the primitive types, layout-wise, should get their own seed, so later things like accumulating seeds based on scalars works"

all the primitive types, layout-wise

This is an implementation artifact. All primitive types should get a distinct seed, but some of the information is already erased at this point so this impl only is a conservative approximation of the layout freedoms we have.

Thanks for poking here. This lead me to include the valid-range information for primitives, so more primitives can be distinguished now.

I was wondering if there was something we could do to distinguish bool and u8! excellent.

previously field ordering was using the same seed for all instances of Foo, now we pass seed values through the layout tree so that not only the struct itself affects layout but also its fields

workingjubilee · 2025-01-10T02:17:11Z

thank you! the impl is a bit spread out but the overarching strategy being documented should hopefully prevent that from desyncing.

@bors r+ rollup

bors · 2025-01-10T02:17:14Z

📌 Commit d89b6d5 has been approved by workingjubilee

It is now in the queue for this repository.

bors · 2025-01-10T02:17:15Z

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

…kingjubilee `-Zrandomize-layout` harder. `Foo<T> != Foo` Tracking issue: rust-lang#106764 Previously randomize-layout only used a deterministic shuffle based on the seed stored in an Adt's ReprOptions, meaning that `Foo<T>` and `Foo` were shuffled by the same seed. This change adds a similar seed to each calculated LayoutData so that a struct can be randomized both based on the layout of its fields and its per-type seed. Primitives start with simple seed derived from some of their properties. Though some types can no longer be distinguished at that point, e.g. usize and u64 will still be treated the same.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#133088 (`-Zrandomize-layout` harder. `Foo<T> != Foo`) - rust-lang#134619 (Improve prose around `as_slice` example of IterMut) - rust-lang#134855 (Add `default_field_values` entry to unstable book) - rust-lang#134908 (Fix `ptr::from_ref` documentation example comment) - rust-lang#135275 (Add Pin::as_deref_mut to 1.84 relnotes) - rust-lang#135294 (Make `bare-fn-no-impl-fn-ptr-99875` test less dependent on path width) - rust-lang#135304 (Add tests cases from review of rust-lang#132289) - rust-lang#135308 (Make sure to walk into nested const blocks in `RegionResolutionVisitor`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#133088 - the8472:randomize-me-harder, r=workingjubilee `-Zrandomize-layout` harder. `Foo<T> != Foo` Tracking issue: rust-lang#106764 Previously randomize-layout only used a deterministic shuffle based on the seed stored in an Adt's ReprOptions, meaning that `Foo<T>` and `Foo` were shuffled by the same seed. This change adds a similar seed to each calculated LayoutData so that a struct can be randomized both based on the layout of its fields and its per-type seed. Primitives start with simple seed derived from some of their properties. Though some types can no longer be distinguished at that point, e.g. usize and u64 will still be treated the same.

rustbot assigned petrochenkov Nov 16, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 16, 2024

the8472 mentioned this pull request Nov 16, 2024

Crater run with -Zrandomize-layout #132292

Closed