Skip to content

Pass -pg to linker when using -Zinstrument-mcount#152457

Merged
rust-bors[bot] merged 3 commits intorust-lang:mainfrom
pmur:murp/mcount-link-pg
Mar 27, 2026
Merged

Pass -pg to linker when using -Zinstrument-mcount#152457
rust-bors[bot] merged 3 commits intorust-lang:mainfrom
pmur:murp/mcount-link-pg

Conversation

@pmur
Copy link
Copy Markdown
Contributor

@pmur pmur commented Feb 10, 2026

View all comments

This selects a slightly different crt on gnu targets which enables the profiler within glibc.

This makes using gprof a little easier with Rust binaries. Otherwise, rustc must be passed -Clink-args=-pg to ensure the correct startup code is linked.

@rustbot 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 Feb 10, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Feb 10, 2026

r? @davidtwco

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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: codegen, compiler
  • codegen, compiler expanded to 66 candidates
  • Random selection from 13 candidates

@mati865
Copy link
Copy Markdown
Member

mati865 commented Feb 11, 2026

The change itself is correct, but perhaps it deserves a test?

g(r)crt1.o enabled by -pg is a startup code for the c runtime. Is it right to call it the crt?

@bjorn3
Copy link
Copy Markdown
Member

bjorn3 commented Feb 11, 2026

Is this also correct for musl and other targets that use a gnu toolchain?

@mati865
Copy link
Copy Markdown
Member

mati865 commented Feb 11, 2026

Good question.
This flag is supported by GCC and Clang, so any target that uses them should accept it. If GCC/Clang does a wrong thing for a specific target, this is arguably not our fault, yet we still might want to add workarounds.

@pmur
Copy link
Copy Markdown
Contributor Author

pmur commented Feb 11, 2026

g(r)crt1.o enabled by -pg is a startup code for the c runtime. Is it right to call it the crt?

That's maybe a detail about the glibc specific implementation which can be redacted? non-gnu targets seem to have a little more varied behavior.

Is this also correct for musl and other targets that use a gnu toolchain?

musl, no. Linking should fail with unresolved mcount-ish symbols. Looking around at Clang, it does seem a few other platforms should respect this option. @mati865, I think you're right. If the target lacks support for this option, it will likely fail similar to musl. Removing the gnu check is likely helpful for other targets.

@pmur pmur force-pushed the murp/mcount-link-pg branch from 78a8cde to a4a5208 Compare February 11, 2026 17:46
@rustbot rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label Feb 11, 2026
@mati865
Copy link
Copy Markdown
Member

mati865 commented Feb 12, 2026

Just to double check we don't regress here.
Musl (and likely other libc like uclibc) failed to before and still do after this PR, albeit with different errors, right?

FreeBSD seems to support it: https://man.freebsd.org/cgi/man.cgi?query=moncontrol&sektion=3&apropos=0&manpath=freebsd

Even mingw-w64 provides the necessary objects.

@pmur
Copy link
Copy Markdown
Contributor Author

pmur commented Feb 12, 2026

Just to double check we don't regress here.
Musl (and likely other libc like uclibc) failed to before and still do after this PR, albeit with different errors, right?

I verified musl will fail to compile in the same way with and without -pg. It fails to link due to the missing mcount symbol. I think uclibc will fail similarly, it appears to lack an mcount implementation too.

@pmur
Copy link
Copy Markdown
Contributor Author

pmur commented Feb 25, 2026

ping

@davidtwco
Copy link
Copy Markdown
Member

@bors r+

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Mar 10, 2026

📌 Commit a4a5208 has been approved by davidtwco

It is now in the queue for this repository.

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

@rust-bors rust-bors bot 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 Mar 10, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 10, 2026
Pass -pg to linker when using -Zinstrument-mcount

This selects a slightly different crt on gnu targets which enables the profiler within glibc.

This makes using gprof a little easier with Rust binaries. Otherwise, rustc must be passed `-Clink-args=-pg` to ensure the correct startup code is linked.
rust-bors bot pushed a commit that referenced this pull request Mar 10, 2026
…uwer

Rollup of 8 pull requests

Successful merges:

 - #149130 (Implement coercions between `&pin (mut|const) T` and `&(mut) T` when `T: Unpin`)
 - #152457 (Pass -pg to linker when using -Zinstrument-mcount)
 - #153143 (Allow `./x test` to run tests without doc tests and without benchmarks)
 - #153653 (scalable vector: type renames and simple checks)
 - #152302 (fix: don't suggest replacing `env!("CARGO_BIN_NAME")` with itself)
 - #153641 (Move `Spanned`.)
 - #153643 (Avoid projection-only suggestions for inherent assoc types)
 - #153657 (triagebot: remove myself from some mention groups)
rust-bors bot pushed a commit that referenced this pull request Mar 10, 2026
…uwer

Rollup of 8 pull requests

Successful merges:

 - #149130 (Implement coercions between `&pin (mut|const) T` and `&(mut) T` when `T: Unpin`)
 - #152457 (Pass -pg to linker when using -Zinstrument-mcount)
 - #153143 (Allow `./x test` to run tests without doc tests and without benchmarks)
 - #153653 (scalable vector: type renames and simple checks)
 - #152302 (fix: don't suggest replacing `env!("CARGO_BIN_NAME")` with itself)
 - #153641 (Move `Spanned`.)
 - #153643 (Avoid projection-only suggestions for inherent assoc types)
 - #153657 (triagebot: remove myself from some mention groups)
rust-bors bot pushed a commit that referenced this pull request Mar 10, 2026
…uwer

Rollup of 8 pull requests

Successful merges:

 - #149130 (Implement coercions between `&pin (mut|const) T` and `&(mut) T` when `T: Unpin`)
 - #152457 (Pass -pg to linker when using -Zinstrument-mcount)
 - #153143 (Allow `./x test` to run tests without doc tests and without benchmarks)
 - #153653 (scalable vector: type renames and simple checks)
 - #152302 (fix: don't suggest replacing `env!("CARGO_BIN_NAME")` with itself)
 - #153641 (Move `Spanned`.)
 - #153643 (Avoid projection-only suggestions for inherent assoc types)
 - #153657 (triagebot: remove myself from some mention groups)
rust-bors bot pushed a commit that referenced this pull request Mar 10, 2026
…uwer

Rollup of 8 pull requests

Successful merges:

 - #149130 (Implement coercions between `&pin (mut|const) T` and `&(mut) T` when `T: Unpin`)
 - #152457 (Pass -pg to linker when using -Zinstrument-mcount)
 - #153143 (Allow `./x test` to run tests without doc tests and without benchmarks)
 - #153653 (scalable vector: type renames and simple checks)
 - #152302 (fix: don't suggest replacing `env!("CARGO_BIN_NAME")` with itself)
 - #153641 (Move `Spanned`.)
 - #153643 (Avoid projection-only suggestions for inherent assoc types)
 - #153657 (triagebot: remove myself from some mention groups)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 10, 2026
Pass -pg to linker when using -Zinstrument-mcount

This selects a slightly different crt on gnu targets which enables the profiler within glibc.

This makes using gprof a little easier with Rust binaries. Otherwise, rustc must be passed `-Clink-args=-pg` to ensure the correct startup code is linked.
rust-bors bot pushed a commit that referenced this pull request Mar 10, 2026
…uwer

Rollup of 13 pull requests

Successful merges:

 - #149130 (Implement coercions between `&pin (mut|const) T` and `&(mut) T` when `T: Unpin`)
 - #152457 (Pass -pg to linker when using -Zinstrument-mcount)
 - #153143 (Allow `./x test` to run tests without doc tests and without benchmarks)
 - #153471 (Refactor `ActiveJobGuard`)
 - #153595 (`QueryLatch` cleanups)
 - #153653 (scalable vector: type renames and simple checks)
 - #152302 (fix: don't suggest replacing `env!("CARGO_BIN_NAME")` with itself)
 - #153479 (Add rationale for intentional potential_query_instability allows)
 - #153600 (add test for proc-macros with custom panic payloads)
 - #153641 (Move `Spanned`.)
 - #153643 (Avoid projection-only suggestions for inherent assoc types)
 - #153657 (triagebot: remove myself from some mention groups)
 - #153659 (Mark an unreachable match arm as such)
rust-bors bot pushed a commit that referenced this pull request Mar 10, 2026
…uwer

Rollup of 13 pull requests

Successful merges:

 - #149130 (Implement coercions between `&pin (mut|const) T` and `&(mut) T` when `T: Unpin`)
 - #152457 (Pass -pg to linker when using -Zinstrument-mcount)
 - #153143 (Allow `./x test` to run tests without doc tests and without benchmarks)
 - #153471 (Refactor `ActiveJobGuard`)
 - #153595 (`QueryLatch` cleanups)
 - #153653 (scalable vector: type renames and simple checks)
 - #152302 (fix: don't suggest replacing `env!("CARGO_BIN_NAME")` with itself)
 - #153479 (Add rationale for intentional potential_query_instability allows)
 - #153600 (add test for proc-macros with custom panic payloads)
 - #153641 (Move `Spanned`.)
 - #153643 (Avoid projection-only suggestions for inherent assoc types)
 - #153657 (triagebot: remove myself from some mention groups)
 - #153659 (Mark an unreachable match arm as such)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 10, 2026
Pass -pg to linker when using -Zinstrument-mcount

This selects a slightly different crt on gnu targets which enables the profiler within glibc.

This makes using gprof a little easier with Rust binaries. Otherwise, rustc must be passed `-Clink-args=-pg` to ensure the correct startup code is linked.
@pmur
Copy link
Copy Markdown
Contributor Author

pmur commented Mar 11, 2026

It seems only-gnu also applies to non-glibc targets.

only-gnu and only-linux compiletest directives should only test a glibc target?

@mati865
Copy link
Copy Markdown
Member

mati865 commented Mar 12, 2026

Mingw-w64 provides the symbols it failed on in libgmon, so it should have worked.

Some of the symbols that linker couldnt find: https://github.com/mingw-w64/mingw-w64/blob/59cd5ce805b12cdefced08ce567d3518c6053b40/mingw-w64-crt/profile/gmon.c#L88 (other symbols are in different files).

This is then assembled to libgmon: https://github.com/mingw-w64/mingw-w64/blob/59cd5ce805b12cdefced08ce567d3518c6053b40/mingw-w64-crt/Makefile.am#L2049

@pmur
Copy link
Copy Markdown
Contributor Author

pmur commented Mar 12, 2026

Mingw-w64 provides the symbols it failed on in libgmon, so it should have worked.

Thanks for noting that. Investigating it, mingw expects the symbol to be named _mcount. That's a quick fix.

@mati865
Copy link
Copy Markdown
Member

mati865 commented Mar 12, 2026

Yeah. The other one is missing -lgmon because of Rust using -nodefaultlibs:

❯ x86_64-w64-mingw32-gcc ~/tmp/hello.c -pg -### 2>&1 | tr ' ' '\n' | rg gmon
"-lgmon"

❯ x86_64-w64-mingw32-gcc ~/tmp/hello.c -pg -### -nodefaultlibs 2>&1 | tr ' ' '\n' | rg gmon

So, I guess Rust should add -lgmon as well for all *-windows-gnu* targets.

@pmur pmur force-pushed the murp/mcount-link-pg branch from f47f0f0 to 40eb8d1 Compare March 12, 2026 16:58
@pmur
Copy link
Copy Markdown
Contributor Author

pmur commented Mar 12, 2026

So, I guess Rust should add -lgmon as well for all *-windows-gnu* targets.

Indeed, and that required readding a couple other libs too. I verified it links a simple gprof instrumented binary.

self.cc_arg("-pg");
// On windows-gnu targets, libgmon also needs to be linked, and this
// requires readding libraries to satisfy its dependencies.
if self.sess.target.os == Os::Windows {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can check self.sess.target.is_like_windows here instead.

@mati865
Copy link
Copy Markdown
Member

mati865 commented Mar 12, 2026

Indeed, and that required readding a couple other libs too. I verified it links a simple gprof instrumented binary.

Right, I often forget how annoying ld.bfd is.

Thanks for noting that. Investigating it, mingw expects the symbol to be named _mcount. That's a quick fix.

Do you want to update base/windows_gnu_llvm.rs too (in the same commit as the other one)? If not, I can do it separately.

pmur added 2 commits March 12, 2026 14:39
mingw exposes the `_mcount` symbol for profiling.
libgmon needs to be linked. This also requires readding a few
other system libraries to satisfy its dependencies.
@pmur
Copy link
Copy Markdown
Contributor Author

pmur commented Mar 12, 2026

Do you want to update base/windows_gnu_llvm.rs too (in the same commit as the other one)? If not, I can do it separately.

I'll update the patch with both changes you've mentioned. Thanks.

@pmur pmur force-pushed the murp/mcount-link-pg branch from 40eb8d1 to 87c2552 Compare March 12, 2026 19:53
@pmur
Copy link
Copy Markdown
Contributor Author

pmur commented Mar 24, 2026

Ping. This issue found in the rollup should be fixed.

@davidtwco
Copy link
Copy Markdown
Member

@bors r+

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Mar 26, 2026

📌 Commit 87c2552 has been approved by davidtwco

It is now in the queue for this repository.

@rust-bors rust-bors bot 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 Mar 26, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Mar 26, 2026
Pass -pg to linker when using -Zinstrument-mcount

This selects a slightly different crt on gnu targets which enables the profiler within glibc.

This makes using gprof a little easier with Rust binaries. Otherwise, rustc must be passed `-Clink-args=-pg` to ensure the correct startup code is linked.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 26, 2026
Pass -pg to linker when using -Zinstrument-mcount

This selects a slightly different crt on gnu targets which enables the profiler within glibc.

This makes using gprof a little easier with Rust binaries. Otherwise, rustc must be passed `-Clink-args=-pg` to ensure the correct startup code is linked.
rust-bors bot pushed a commit that referenced this pull request Mar 27, 2026
Rollup of 7 pull requests

Successful merges:

 - #152457 (Pass -pg to linker when using -Zinstrument-mcount)
 - #154031 (Remove divergence check from check_expr_array)
 - #154418 (move many tests out of `ui/unsafe`)
 - #153662 (Suggest fully qualified path on method name collision)
 - #153675 (simd_add/sub/mul/neg: document overflow behavior)
 - #154110 (Change "error finalizing incremental compilation" text and emit it as a note, not a warning)
 - #154430 (Create GPU target notification group)
Zalathar added a commit to Zalathar/rust that referenced this pull request Mar 27, 2026
Pass -pg to linker when using -Zinstrument-mcount

This selects a slightly different crt on gnu targets which enables the profiler within glibc.

This makes using gprof a little easier with Rust binaries. Otherwise, rustc must be passed `-Clink-args=-pg` to ensure the correct startup code is linked.
rust-bors bot pushed a commit that referenced this pull request Mar 27, 2026
Rollup of 7 pull requests

Successful merges:

 - #152457 (Pass -pg to linker when using -Zinstrument-mcount)
 - #154031 (Remove divergence check from check_expr_array)
 - #154418 (move many tests out of `ui/unsafe`)
 - #154441 (bootstrap: force a CI LLVM stamp bump)
 - #153662 (Suggest fully qualified path on method name collision)
 - #153675 (simd_add/sub/mul/neg: document overflow behavior)
 - #154430 (Create GPU target notification group)
rust-bors bot pushed a commit that referenced this pull request Mar 27, 2026
Rollup of 6 pull requests

Successful merges:

 - #152457 (Pass -pg to linker when using -Zinstrument-mcount)
 - #154031 (Remove divergence check from check_expr_array)
 - #154418 (move many tests out of `ui/unsafe`)
 - #154441 (bootstrap: force a CI LLVM stamp bump)
 - #153675 (simd_add/sub/mul/neg: document overflow behavior)
 - #154430 (Create GPU target notification group)
@rust-bors rust-bors bot merged commit 433e106 into rust-lang:main Mar 27, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants