`-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-errors · 2024-11-24T04:51:47Z

compiler/rustc_abi/src/layout.rs

@@ -119,6 +119,8 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
            .chain(Niche::from_scalar(dl, Size::ZERO, a))
            .max_by_key(|niche| niche.available(dl));

+        let combined_seed = a.size(&self.cx).bytes().wrapping_add(b.size(&self.cx).bytes());


Why do we need to do any of this seed computation if -Zrandomize-layout is not set?

Because if -Zrandomize-layout is not set, then keeping this set to 0 means we should also be able to avoid all that //@ normalize-stderr-test: below.

Because randomization is toggled on a per-crate basis, so we want types from non-randomized crates still contribute a seed to randomized crates.

Additionally one CI job builds with randomization enabled, which includes std, so it wouldn't actually help the tests.

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%)

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

the8472 · 2024-11-28T22:01:04Z

I have also changed some comments at the request of Ralf to explain places where randomization explicitly does not randomize is not a guarantee but instead a conservative approach until the UCG questions are settled.

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