Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable probestack when GCOV profiling is being used #51666

Merged
merged 1 commit into from
Jun 22, 2018

Conversation

marco-c
Copy link
Contributor

@marco-c marco-c commented Jun 20, 2018

If I compile Firefox with gcov profiling enabled, Firefox crashes at startup because of probestack.
Since it's disabled for PGO, I think it makes sense to disable it for gcov too.

@rust-highfive
Copy link
Collaborator

r? @oli-obk

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 20, 2018
@marco-c
Copy link
Contributor Author

marco-c commented Jun 20, 2018

CC @kennytm

@nagisa
Copy link
Member

nagisa commented Jun 21, 2018

The idea seems sound to me. It is a shame that it has to be disabled, as profiling no longer reflects the call overhead accurately, though.

@bors r+

@bors
Copy link
Contributor

bors commented Jun 21, 2018

📌 Commit e9aacfd has been approved by nagisa

@bors 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 Jun 21, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Jun 22, 2018
Disable probestack when GCOV profiling is being used

If I compile Firefox with gcov profiling enabled, Firefox crashes at startup because of probestack.
Since it's disabled for PGO, I think it makes sense to disable it for gcov too.
bors added a commit that referenced this pull request Jun 22, 2018
Rollup of 6 pull requests

Successful merges:

 - #51158 (Mention spec and indented blocks in doctest docs)
 - #51629 (Do not consume semicolon twice while parsing local statement)
 - #51637 (Update zx_cprng_draw_new on Fuchsia)
 - #51664 (make more libsyntax methods public)
 - #51666 (Disable probestack when GCOV profiling is being used)
 - #51703 (Recognize the extra "LLVM tools versions" argument to build-manifest.)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Jun 22, 2018

⌛ Testing commit e9aacfd with merge 4dc2d74...

@bors bors merged commit e9aacfd into rust-lang:master Jun 22, 2018
@marco-c marco-c deleted the disable_probestack branch June 23, 2018 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants