Link `std` statically in `rustc_driver` #122362

Zoxc · 2024-03-12T04:11:51Z

This makes rustc_driver statically link to std. This is done by not passing -C prefer-dynamic when building rustc_driver. However building rustc-main won't work currently as it tries to dynamically link to both rustc_driver and std resulting in a crate graph with std duplicated. To fix that new command line option -Z prefer_deps_of_dynamic is added which prevents linking to a dylib if there's a static variant of it already statically linked into another dylib dependency.

The main motivation for this change is to enable #[global_allocator] to be used in rustc_driver allowing overriding the allocator used in rustc on all platforms.

Instead of adding -Z prefer_deps_of_dynamic, this PR is changed to crate opt-in to the linking change via the rustc_private feature instead, as that would be typically needed to link to rustc_driver anyway.

try-job: aarch64-apple
try-job: x86_64-msvc
try-job: i686-mingw
try-job: dist-x86_64-msvc
try-job: aarch64-gnu

rustbot · 2024-03-12T04:11:58Z

r? @oli-obk

rustbot has assigned @oli-obk.
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

oli-obk · 2024-03-12T06:30:18Z

enable #[global_allocator] to be used in rustc_driver allowing overriding the allocator used in rustc on all platforms.

why can't this be done in rustc-main?

Will this PR require all custom drivers (including our tools) to use -Z prefer_deps_of_dynamic?

Noratrieb · 2024-03-12T06:31:01Z

does this mean that external tools using rustc driver now also need to use this flag? would be nice if the duplicate std error message mentioned that.
it would be even nicer if this flag could be made the default.. but I'm not sure whether that would be a breaking change

Zoxc · 2024-03-12T07:03:12Z

why can't this be done in rustc-main?

The global allocator must be statically linked with std.

Will this PR require all custom drivers (including our tools) to use -Z prefer_deps_of_dynamic?

I made the rustc_private feature imply -Z prefer_deps_of_dynamic, which should avoid that.

Zoxc · 2024-03-12T08:19:33Z

This could probably use a perf run as there could be some ThinLTO interactions.

Kobzol · 2024-03-12T08:20:24Z

@bors try @rust-timer queue

Link `std` statically in `rustc_driver` This makes `rustc_driver` statically link to `std`. This is done by not passing `-C prefer-dynamic` when building `rustc_driver`. However building `rustc-main` won't work currently as it tries to dynamically link to both `rustc_driver` and `std` resulting in a crate graph with `std` duplicated. To fix that new command line option `-Z prefer_deps_of_dynamic` is added which prevents linking to a dylib if there's a static variant of it already statically linked into another dylib dependency. The main motivation for this change is to enable `#[global_allocator]` to be used in `rustc_driver` allowing overriding the allocator used in rustc on all platforms.

bors · 2024-03-12T08:21:35Z

⌛ Trying commit d5cdb9e with merge 380a85b...

bors · 2024-03-12T09:48:52Z

☀️ Try build successful - checks-actions
Build commit: 380a85b (380a85bad098d06e75548877d9061302a9dd5312)

Use `mimalloc` as `global_allocator` for `rustc_driver` This changes the Rust global allocator for `rustc_driver` to `mimalloc`, leaving LLVM unaffected. Based on rust-lang#122362.

rust-timer · 2024-03-12T13:24:13Z

Finished benchmarking commit (380a85b): comparison URL.

Overall result: ✅ improvements - 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 is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-1.7%, -0.4%]	178
Improvements ✅ (secondary)	-0.9%	[-1.7%, -0.3%]	164
All ❌✅ (primary)	-0.9%	[-1.7%, -0.4%]	178

Max RSS (memory usage)

Results

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)	-	-	0
Improvements ✅ (primary)	-2.7%	[-2.7%, -2.7%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.7%	[-2.7%, -2.7%]	1

Cycles

Results

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)	17.9%	[17.8%, 18.0%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.1%	[-1.2%, -1.0%]	2
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 672.448s -> 671.639s (-0.12%)
Artifact size: 310.05 MiB -> 298.66 MiB (-3.67%)

src/bootstrap/src/core/builder.rs

src/bootstrap/src/bin/rustc.rs

bjorn3 · 2024-03-12T14:37:30Z

compiler/rustc_metadata/src/dependency_format.rs

+                }
+            }
+        }
+    }


Instead of all this interspersed logic would it be possible to at the end iterate over all dylibs and mark all crates they have linked into them as IncludedFromDylib?

Not sure if that's correct. The current logic starts by picking the dylibs then adding other crates as needed. That seems less likely to end up with missing crates than removing them after.

My suggestion is not to remove crates, but to change their linkage to IncludeFromDylib after it has determined which crates to include.

That's what I mean by remove as IncludeFromDylib won't be linked to the crate. It also won't work with the current duplicate crate detection as that happens as the crates are added.

bjorn3 · 2024-03-12T14:38:17Z

compiler/rustc_session/src/options.rs

@@ -1810,6 +1810,8 @@ options! {
        "use a more precise version of drop elaboration for matches on enums (default: yes). \
        This results in better codegen, but has caused miscompilations on some tier 2 platforms. \
        See #77382 and #74551."),
+    prefer_deps_of_dynamic: bool = (false, parse_bool, [TRACKED],
+        "prefer linking to static dependencies of dynamic libraries over available dynamic libraries (default: no)"),


If it works robustly, I feel like this should be unconditionally enabled.

I agree, but I'm not confident in there being no regressions. We don't really have many dylib and multi crate type tests.

bjorn3 · 2024-03-12T14:39:27Z

src/bootstrap/src/core/build_steps/compile.rs

+            let can_be_rustc_dep = filename.starts_with("rustc_driver-")
+                || filename.starts_with("librustc_driver-")
+                || build_compiler.stage == 0;
+            if can_be_rustc_dep


What is this extra logic needed for?

That's to prevent the std dynamic library from also being included with the compiler, as it's no longer needed.

That sounds like it'd be worth a comment in the code. (That's probably true most times a reviewer had a question that was answered by a clarification without code change.)

bjorn3 · 2024-03-12T14:40:26Z

src/bootstrap/src/core/builder.rs

@@ -2028,10 +2028,16 @@ impl<'a> Builder<'a> {
        // When we build Rust dylibs they're all intended for intermediate
        // usage, so make sure we pass the -Cprefer-dynamic flag instead of
        // linking all deps statically into the dylib.
-        if matches!(mode, Mode::Std | Mode::Rustc) {
+        if matches!(mode, Mode::Std) {


I think you will need to add Mode::Codegen here as -Cprefer-dynamic is no longer implied when linking librustc_driver.so

I don't see how there's any change to -C prefer-dynamic for other crates?

bors · 2024-08-11T06:25:42Z

📌 Commit b22253c has been approved by oli-obk,lqd,bjorn3,Kobzol

It is now in the queue for this repository.

Kobzol · 2024-08-11T07:00:26Z

This doesn't need to block this PR, but we should probably either ignore or modify these failing tests in the dist builders that run tests. It's wasteful to ship a 12 MiB file in our dist artifacts that is unused.

…obk,lqd,bjorn3,Kobzol Link `std` statically in `rustc_driver` This makes `rustc_driver` statically link to `std`. This is done by not passing `-C prefer-dynamic` when building `rustc_driver`. However building `rustc-main` won't work currently as it tries to dynamically link to both `rustc_driver` and `std` resulting in a crate graph with `std` duplicated. To fix that new command line option `-Z prefer_deps_of_dynamic` is added which prevents linking to a dylib if there's a static variant of it already statically linked into another dylib dependency. The main motivation for this change is to enable `#[global_allocator]` to be used in `rustc_driver` allowing overriding the allocator used in rustc on all platforms. --- Instead of adding `-Z prefer_deps_of_dynamic`, this PR is changed to crate opt-in to the linking change via the `rustc_private` feature instead, as that would be typically needed to link to `rustc_driver` anyway. --- try-job: aarch64-apple try-job: x86_64-msvc try-job: i686-mingw try-job: dist-x86_64-msvc try-job: aarch64-gnu

bors · 2024-08-11T08:25:17Z

⌛ Testing commit b22253c with merge d5dfd7a...

rust-log-analyzer · 2024-08-11T10:39:47Z

The job i686-mingw failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

---- [run-make] tests\run-make\dump-ice-to-disk stdout ----

error: rmake recipe failed to complete
status: exit code: 101
command: "C:\\a\\rust\\rust\\build\\i686-pc-windows-gnu\\test\\run-make\\dump-ice-to-disk\\rmake.exe"
--- stderr -------------------------------
thread 'main' panicked at C:\a\rust\rust\tests\run-make\dump-ice-to-disk\rmake.rs:63:5:
assertion `left == right` failed
  left: 60

bors · 2024-08-11T10:40:17Z

💔 Test failed - checks-actions

jieyouxu · 2024-08-11T10:42:50Z

Lol. @bors retry

bors · 2024-08-11T15:08:05Z

⌛ Testing commit b22253c with merge 9cb1998...

bors · 2024-08-11T17:32:26Z

☀️ Test successful - checks-actions
Approved by: oli-obk,lqd,bjorn3,Kobzol
Pushing 9cb1998 to master...

lqd · 2024-08-11T17:34:02Z

🎉

rust-timer · 2024-08-11T18:53:05Z

Finished benchmarking commit (9cb1998): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.0%	[-2.0%, -0.2%]	229
Improvements ✅ (secondary)	-1.0%	[-2.0%, -0.2%]	222
All ❌✅ (primary)	-1.0%	[-2.0%, -0.2%]	229

Max RSS (memory usage)

Results (primary 1.8%, secondary 1.8%)

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)	1.8%	[1.0%, 2.5%]	2
Regressions ❌ (secondary)	2.9%	[1.5%, 6.0%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	1.8%	[1.0%, 2.5%]	2

Cycles

Results (primary -1.1%, secondary -2.4%)

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)	-	-	0
Improvements ✅ (primary)	-1.1%	[-1.8%, -0.8%]	15
Improvements ✅ (secondary)	-2.4%	[-3.3%, -1.7%]	8
All ❌✅ (primary)	-1.1%	[-1.8%, -0.8%]	15

Binary size

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

Bootstrap: 758.668s -> 756.29s (-0.31%)
Artifact size: 339.27 MiB -> 341.47 MiB (0.65%)

Now we need `#![feature(rustc_private)]` in the `main.rs` See the original PR rust-lang/rust#122362 And the PR in another tool that requires the same change trailofbits/dylint#1298 Update lock file too

…, r=onur-ozkan Do not copy libstd dynamic library to sysroot Since rust-lang#122362, rustc links statically to libstd.[so|dll]. Which means that the libstd.[so|dll] file no longer has to be in the rustc sysroot. However, we are currently still shipping this file, in every new release of Rust, for no reason, it's just wasted bytes. This PR removes the dynamic library file from the built sysroot. However, it is not yet performed on Windows, because stage0 incremental tests start failing there (see description of the issue [here](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Failing.20incr.20tests.20on.20Windows.20when.20std.2Edll.20is.20missing/near/474507064)). This is an extended version of rust-lang#128986. CC `@Zoxc`

…ozkan Do not copy libstd dynamic library to sysroot Since rust-lang/rust#122362, rustc links statically to libstd.[so|dll]. Which means that the libstd.[so|dll] file no longer has to be in the rustc sysroot. However, we are currently still shipping this file, in every new release of Rust, for no reason, it's just wasted bytes. This PR removes the dynamic library file from the built sysroot. However, it is not yet performed on Windows, because stage0 incremental tests start failing there (see description of the issue [here](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Failing.20incr.20tests.20on.20Windows.20when.20std.2Edll.20is.20missing/near/474507064)). This is an extended version of rust-lang/rust#128986. CC `@Zoxc`

rustbot assigned oli-obk Mar 12, 2024

Zoxc mentioned this pull request Mar 12, 2024

Redirect __rust_dealloc to sdallocx #122329

Closed

This comment has been minimized.

Sign in to view

Zoxc force-pushed the rustc_driver_static_std branch from d3ef302 to 9506132 Compare March 12, 2024 05:50

This comment has been minimized.

Sign in to view

Zoxc mentioned this pull request Mar 12, 2024

Use mimalloc as global_allocator for rustc_driver #122369

Closed

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 12, 2024

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 12, 2024

oli-obk reviewed Mar 12, 2024

View reviewed changes

src/bootstrap/src/core/builder.rs Outdated Show resolved Hide resolved

oli-obk reviewed Mar 12, 2024

View reviewed changes

src/bootstrap/src/bin/rustc.rs Outdated Show resolved Hide resolved

bjorn3 reviewed Mar 12, 2024

View reviewed changes

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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 11, 2024

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 11, 2024

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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 11, 2024

jieyouxu mentioned this pull request Aug 11, 2024

Tidy up dump-ice-to-disk and make assertion failures dump ICE messages #128916

Merged

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 11, 2024

bors merged commit 9cb1998 into rust-lang:master Aug 11, 2024
7 checks passed

rustbot added this to the 1.82.0 milestone Aug 11, 2024

bors mentioned this pull request Aug 11, 2024

Show paths relative to the source root for diagnostics in sub-workspaces #128904

Open

Zoxc mentioned this pull request Aug 11, 2024

Remove rustc's std copy #128986

Closed

Zoxc deleted the rustc_driver_static_std branch August 11, 2024 20:35

smoelius mentioned this pull request Aug 19, 2024

Add #![feature(rustc_private)] to driver's main.rs trailofbits/dylint#1298

Merged

Kobzol mentioned this pull request Oct 3, 2024

Do not copy libstd dynamic library to sysroot #131188

Merged

Kobzol mentioned this pull request Oct 12, 2024

Remove std.dll from dist archives on Windows #131587

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Link `std` statically in `rustc_driver` #122362

Link `std` statically in `rustc_driver` #122362

Zoxc commented Mar 12, 2024 •

edited by jieyouxu

Loading

rustbot commented Mar 12, 2024

This comment has been minimized.

oli-obk commented Mar 12, 2024

Noratrieb commented Mar 12, 2024

This comment has been minimized.

Zoxc commented Mar 12, 2024

Zoxc commented Mar 12, 2024

Kobzol commented Mar 12, 2024

This comment has been minimized.

bors commented Mar 12, 2024

bors commented Mar 12, 2024

This comment has been minimized.

rust-timer commented Mar 12, 2024

bjorn3 Mar 12, 2024

Zoxc Mar 14, 2024

bjorn3 Mar 14, 2024

Zoxc Mar 15, 2024

bjorn3 Mar 12, 2024

Zoxc Mar 14, 2024

bjorn3 Mar 12, 2024

Zoxc Mar 14, 2024

RalfJung Mar 25, 2024

bjorn3 Mar 12, 2024

Zoxc Mar 14, 2024

bors commented Aug 11, 2024

Kobzol commented Aug 11, 2024 •

edited

Loading

bors commented Aug 11, 2024

rust-log-analyzer commented Aug 11, 2024

bors commented Aug 11, 2024

jieyouxu commented Aug 11, 2024

bors commented Aug 11, 2024

bors commented Aug 11, 2024

lqd commented Aug 11, 2024

rust-timer commented Aug 11, 2024

Link std statically in rustc_driver #122362

Link std statically in rustc_driver #122362

Conversation

Zoxc commented Mar 12, 2024 • edited by jieyouxu Loading

rustbot commented Mar 12, 2024

This comment has been minimized.

oli-obk commented Mar 12, 2024

Noratrieb commented Mar 12, 2024

This comment has been minimized.

Zoxc commented Mar 12, 2024

Zoxc commented Mar 12, 2024

Kobzol commented Mar 12, 2024

This comment has been minimized.

bors commented Mar 12, 2024

bors commented Mar 12, 2024

This comment has been minimized.

rust-timer commented Mar 12, 2024

Overall result: ✅ improvements - no action needed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Aug 11, 2024

Kobzol commented Aug 11, 2024 • edited Loading

bors commented Aug 11, 2024

rust-log-analyzer commented Aug 11, 2024

bors commented Aug 11, 2024

jieyouxu commented Aug 11, 2024

bors commented Aug 11, 2024

bors commented Aug 11, 2024

lqd commented Aug 11, 2024

rust-timer commented Aug 11, 2024

Overall result: ✅ improvements - no action needed

Link `std` statically in `rustc_driver` #122362

Link `std` statically in `rustc_driver` #122362

Zoxc commented Mar 12, 2024 •

edited by jieyouxu

Loading

Kobzol commented Aug 11, 2024 •

edited

Loading