enable -Zrandomize-layout in debug CI builds #101339

the8472 · 2022-09-02T19:06:32Z

This builds rustc/libs/tools with -Zrandomize-layout on *-debug CI runners.

Only a handful of tests and asserts break with that enabled, which is promising. One test was fixable, the rest is dealt with by disabling them through new cargo features or compiletest directives.

The config.toml flag rust.randomize-layout defaults to false, so it has to be explicitly enabled for now.

rust-highfive · 2022-09-02T19:06:36Z

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

the8472 · 2022-09-02T19:07:55Z

r? @jyn514

jyn514

This seems reasonable, but I'm worried it makes tests more flaky. What's the plan if we merge this and suddenly a quarter of all PRs start failing randomly? I guess "revert until the tests are fixed" is good enough.

I don't see any changes to src/ci - are there already some builders that enable rust.debug? Which builders are those? Because this disables tests I want to be careful about where we enable it, we should have at least one builder on each platform with randomized layouts disabled.

config.toml.example

jyn514 · 2022-09-02T19:45:30Z

src/bootstrap/test.rs

+        if builder.config.rust_randomize_layout {
+            cmd.arg("--rust-randomized-layout");
+        }


I've been meaning to refactor this for a while, your change only affects compiletest :( there's no one place to put it so it also affects src/test/{codegen,run-make,debuginfo}.

compiletest seems good enough for now, but it would be nice to add this everywhere else if you have time.

That flag is there for the needs-deterministic-layouts directive. As long as those test types don't get broken by randomization we won't need it.

As long as those test types don't get broken by randomization we won't need it.

well, that's what I'm worried about 😅 but given this is all internal-only, I'm fine with landing this for only a subset of the testsuite.

library/alloc/src/collections/btree/node/tests.rs

jyn514 · 2022-09-02T19:55:56Z

I don't see any changes to src/ci - are there already some builders that enable rust.debug? Which builders are those? Because this disables tests I want to be careful about where we enable it, we should have at least one builder on each platform with randomized layouts disabled.

alternatively we can try and get the two failing UI tests to work with randomized layouts, that would probably be my preference. but won't block on it.

the8472 · 2022-09-02T20:24:07Z

I don't see any changes to src/ci - are there already some builders that enable rust.debug?

Ah, good question. x86_64-gnu-debug seems to be the only one enabling rust.debug. Other CI jobs only toggle some of the more specific flags in a pattern that's not entirely clear to me.

That builder only runs run-make-fulldeps so that doesn't get us much test-coverage, but it does make a stage-2 build, so it builds a compiler with a randomized compiler which would hopefully give us some coverage... but not all that much.
Maybe enable it for the llvm-13 builder? That even runs on PRs.

This seems reasonable, but I'm worried it makes tests more flaky. What's the plan if we merge this and suddenly a quarter of all PRs start failing randomly?

If it ever becomes too big of a problem we could set -Zlayout-seed to use a fixed randomization seed. Until that happens it seems better to stochastically cover more bases.

jyn514 · 2022-09-02T20:28:05Z

Maybe enable it for the llvm-13 builder? That even runs on PRs.

That seems fine :) random-layout shouldn't add any additional compile time, and running it in PRs will get us more coverage.

It does mean this will only run on linux platforms, but that seems ~fine for now, better than no coverage.

the8472 · 2022-09-02T20:35:37Z

alternatively we can try and get the two failing UI tests to work with randomized layouts, that would probably be my preference. but won't block on it.

One prints hir statistics from the compiler and checks them via the test-output. Those statistics involve struct sizes. So layout randomization of the compiler breaks this test quite directly. It's not the structs in the test that get randomized, it's the compiler itself.

The other one involves diagnostics that print memory contents of a std type which can get swapped under randomization.

jyn514 · 2022-09-02T20:46:49Z

One prints hir statistics from the compiler and checks them via the test-output. Those statistics involve struct sizes. So layout randomization of the compiler breaks this test quite directly. It's not the structs in the test that get randomized, it's the compiler itself.

You can normalize the sizes with a regex: https://rustc-dev-guide.rust-lang.org/tests/ui.html#normalization. I don't think we're directly testing the sizes, just that we output a number.

The other one involves diagnostics that print memory contents of a std type which can get swapped under randomization.

Hmm, I agree that one's harder. Not sure how to fix it without digging into it myself.

src/bootstrap/configure.py

library/std/Cargo.toml

the8472 · 2022-09-02T20:54:51Z

One prints hir statistics from the compiler and checks them via the test-output. Those statistics involve struct sizes. So layout randomization of the compiler breaks this test quite directly. It's not the structs in the test that get randomized, it's the compiler itself.

You can normalize the sizes with a regex: https://rustc-dev-guide.rust-lang.org/tests/ui.html#normalization. I don't think we're directly testing the sizes, just that we output a number.

I guess they might catch struct size-regressions if they're not all of those structs are covered by asserts in the compiler. Ping @nnethercote to clarify.

nnethercote · 2022-09-04T22:40:43Z

If you are talking about src/test/ui/stats/hir-stats.rs, it exists to test the output of -Zhir-stats. Using regex normalization to hide the exact numbers would weaken the test somewhat, though I guess it would be acceptable. But it's possible that randomization might also change the order of the entries, which would be harder to normalize for.

…layout Additionally teach compiletest to ignore tests that rely on deterministic layout. Tests themselves aren't built with randomization but they can still observe slight changes in std or rustc

…l struct sizes

the8472 · 2024-08-31T21:59:50Z

@bors r=Mark-Simulacrum

I have left it as disabled-by-default and updated the config.toml.example file to reflect that. Only the x86_64-gnu-llvm-17 runner or explicit opt-in is required for now. This can still be changed later to inherit from rust.debug.

bors · 2024-08-31T21:59:53Z

📌 Commit c218c75 has been approved by Mark-Simulacrum

It is now in the queue for this repository.

…-Simulacrum enable -Zrandomize-layout in debug CI builds This builds rustc/libs/tools with `-Zrandomize-layout` on *-debug CI runners. Only a handful of tests and asserts break with that enabled, which is promising. One test was fixable, the rest is dealt with by disabling them through new cargo features or compiletest directives. The config.toml flag `rust.randomize-layout` defaults to false, so it has to be explicitly enabled for now.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#101339 (enable -Zrandomize-layout in debug CI builds ) - rust-lang#127021 (Add target support for RTEMS Arm) - rust-lang#127692 (Suggest `impl Trait` for References to Bare Trait in Function Header) - rust-lang#128701 (Don't Suggest Labeling `const` and `unsafe` Blocks ) - rust-lang#128871 (bypass linker configuration and cross target check for specific commands) - rust-lang#129624 (Adjust `memchr` pinning and run `cargo update`) - rust-lang#129706 (Rename dump of coroutine by-move-body to be more consistent, fix ICE in dump_mir) - rust-lang#129789 (rustdoc: use strategic boxing to shrink `clean::Item`) r? `@ghost` `@rustbot` modify labels: rollup

…-Simulacrum enable -Zrandomize-layout in debug CI builds This builds rustc/libs/tools with `-Zrandomize-layout` on *-debug CI runners. Only a handful of tests and asserts break with that enabled, which is promising. One test was fixable, the rest is dealt with by disabling them through new cargo features or compiletest directives. The config.toml flag `rust.randomize-layout` defaults to false, so it has to be explicitly enabled for now.

Rollup of 8 pull requests Successful merges: - rust-lang#101339 (enable -Zrandomize-layout in debug CI builds ) - rust-lang#127021 (Add target support for RTEMS Arm) - rust-lang#128871 (bypass linker configuration and cross target check for specific commands) - rust-lang#129471 ([rustdoc] Sort impl associated items by kinds and then by appearance) - rust-lang#129529 (Add test to build crates used by r-a on stable) - rust-lang#129624 (Adjust `memchr` pinning and run `cargo update`) - rust-lang#129796 (Unify scraped examples with other code examples) - rust-lang#129939 (explain why Rvalue::Len still exists) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#101339 (enable -Zrandomize-layout in debug CI builds ) - rust-lang#120736 (rustdoc: add header map to the table of contents) - rust-lang#127021 (Add target support for RTEMS Arm) - rust-lang#128928 (CI: rfl: add more tools and steps) - rust-lang#129584 (warn the user if the upstream master branch is old) - rust-lang#129664 (Arbitrary self types v2: pointers feature gate.) - rust-lang#129752 (Make supertrait and implied predicates queries defaulted) - rust-lang#129918 (Update docs of `missing_abi` lint) - rust-lang#129919 (Stabilize `waker_getters`) - rust-lang#129925 (remove deprecated option `rust.split-debuginfo`) Failed merges: - rust-lang#129789 (rustdoc: use strategic boxing to shrink `clean::Item`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#101339 - the8472:ci-randomize-debug, r=Mark-Simulacrum enable -Zrandomize-layout in debug CI builds This builds rustc/libs/tools with `-Zrandomize-layout` on *-debug CI runners. Only a handful of tests and asserts break with that enabled, which is promising. One test was fixable, the rest is dealt with by disabling them through new cargo features or compiletest directives. The config.toml flag `rust.randomize-layout` defaults to false, so it has to be explicitly enabled for now.

rust-highfive assigned Mark-Simulacrum Sep 2, 2022

rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Sep 2, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 2, 2022

rust-highfive assigned jyn514 and unassigned Mark-Simulacrum Sep 2, 2022

the8472 force-pushed the ci-randomize-debug branch from 1acae66 to 66b5d6c Compare September 2, 2022 19:31

jyn514 reviewed Sep 2, 2022

View reviewed changes

the8472 force-pushed the ci-randomize-debug branch from 66b5d6c to e4dcd3c Compare September 2, 2022 20:45

rustbot added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Sep 2, 2022

jyn514 reviewed Sep 2, 2022

View reviewed changes

src/bootstrap/configure.py Outdated Show resolved Hide resolved

bjorn3 reviewed Sep 2, 2022

View reviewed changes

library/std/Cargo.toml Outdated Show resolved Hide resolved

the8472 force-pushed the ci-randomize-debug branch from e4dcd3c to 4fee75e Compare September 2, 2022 22:04

This comment has been minimized.

Sign in to view

the8472 force-pushed the ci-randomize-debug branch from 4fee75e to 0ee6fa7 Compare September 2, 2022 22:11

This comment has been minimized.

Sign in to view

the8472 force-pushed the ci-randomize-debug branch from 0ee6fa7 to 0eaac8f Compare September 3, 2022 09:23

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 31, 2024

the8472 added 7 commits August 31, 2024 23:56

Add rust.randomize-layout config to build artifacts with -Zrandomize-…

121e9f4

…layout Additionally teach compiletest to ignore tests that rely on deterministic layout. Tests themselves aren't built with randomization but they can still observe slight changes in std or rustc

ignore/fix layout-sensitive tests

f3bc08a

disable size asserts in the compiler when randomizing layouts

5bf8eeb

when -Zrandomize-layout is enabled disable alloc test testing interna…

e3169f7

…l struct sizes

enable layout randomization in x86_64-gnu-llvm-17 CI job

1e377c5

inhibit layout randomization for Box

df20808

exclude tools with deps that have size asserts

c218c75

the8472 force-pushed the ci-randomize-debug branch from 1ab5394 to c218c75 Compare August 31, 2024 21:56

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 31, 2024

matthiaskrgr mentioned this pull request Sep 2, 2024

Rollup of 8 pull requests #129900

Closed

tgross35 mentioned this pull request Sep 4, 2024

Rollup of 8 pull requests #129949

Closed

matthiaskrgr mentioned this pull request Sep 5, 2024

Rollup of 10 pull requests #129978

Merged

bors merged commit 8a60d0a into rust-lang:master Sep 5, 2024
6 checks passed

rustbot added this to the 1.83.0 milestone Sep 5, 2024

the8472 mentioned this pull request Mar 5, 2023

Tracking issue for -Z randomize-layout #106764

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable -Zrandomize-layout in debug CI builds #101339

enable -Zrandomize-layout in debug CI builds #101339

the8472 commented Sep 2, 2022 •

edited

Loading

rust-highfive commented Sep 2, 2022

the8472 commented Sep 2, 2022

jyn514 left a comment •

edited

Loading

jyn514 Sep 2, 2022

the8472 Sep 2, 2022

jyn514 Sep 2, 2022

jyn514 commented Sep 2, 2022

the8472 commented Sep 2, 2022

jyn514 commented Sep 2, 2022

the8472 commented Sep 2, 2022

jyn514 commented Sep 2, 2022

the8472 commented Sep 2, 2022

This comment has been minimized.

This comment has been minimized.

nnethercote commented Sep 4, 2022

the8472 commented Aug 31, 2024

bors commented Aug 31, 2024

enable -Zrandomize-layout in debug CI builds #101339

enable -Zrandomize-layout in debug CI builds #101339

Conversation

the8472 commented Sep 2, 2022 • edited Loading

rust-highfive commented Sep 2, 2022

the8472 commented Sep 2, 2022

jyn514 left a comment • edited Loading

Choose a reason for hiding this comment

jyn514 Sep 2, 2022

Choose a reason for hiding this comment

the8472 Sep 2, 2022

Choose a reason for hiding this comment

jyn514 Sep 2, 2022

Choose a reason for hiding this comment

jyn514 commented Sep 2, 2022

the8472 commented Sep 2, 2022

jyn514 commented Sep 2, 2022

the8472 commented Sep 2, 2022

jyn514 commented Sep 2, 2022

the8472 commented Sep 2, 2022

This comment has been minimized.

This comment has been minimized.

nnethercote commented Sep 4, 2022

the8472 commented Aug 31, 2024

bors commented Aug 31, 2024

the8472 commented Sep 2, 2022 •

edited

Loading

jyn514 left a comment •

edited

Loading