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

rustc: Add the ability to not run dsymutil #47784

Merged
merged 1 commit into from
Feb 15, 2018

Conversation

alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Jan 26, 2018

This commit adds the ability for rustc to not run dsymutil by default
on OSX. A new codegen option, -Z run-dsymutil=no, was added to specify
that dsymutil should not run and instead the compiler should
unconditionally keep the object files around in a compilation if
necessary for debug information.

cc #47240

@rust-highfive
Copy link
Collaborator

r? @estebank

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

/// Returns a boolean indicating whether we should preserve the object files on
/// the filesystem for their debug information. This is often useful with
/// split-dwarf like schemes.
fn preserve_objects_for_their_debuginfo(sess: &Session) -> bool {
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm sort of hoping that we can use a function like this to enable split dwarf on Linux over time as well!

@alexcrichton
Copy link
Member Author

r? @michaelwoerister

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 26, 2018
@kennytm
Copy link
Member

kennytm commented Jan 26, 2018

Would -C run-dsymutil=no cause RUST_BACKTRACE=1 to not have any line numbers?

@alexcrichton
Copy link
Member Author

A good question and one I forgot to verify! Looks like the answer is "no" though which is a bummer!

@JohnColanduoni, out of curiosity, do you know if there's a way to get libbacktrace to relatively easily do the same logic that lldb is doing? (looking for object files in addition to the dSYM)

@JohnColanduoni
Copy link
Contributor

@alexchrichton Not easily, it would probably make more sense to add this feature as part of a pure-Rust rewrite of libbacktrace. I didn’t know lldb had this functionality on Mac, I’ve had issues where lldb refuses to give me line numbers if Spotlight hasn’t yet indexed tbe dSYM.

@alexcrichton
Copy link
Member Author

Ah ok, thanks for the info @JohnColanduoni! In light of that I've now switched this to a -Z flag instead of a -C flag so we can continue to tweak it. I think that we'll want this option though to play around with over time still with incremental compilation.

@@ -77,7 +77,7 @@ pub fn get_linker(sess: &Session) -> (PathBuf, Command, Vec<(OsString, OsString)
Command::new(linker)
};

if let Some(ref linker) = sess.opts.cg.linker {
if let Some(ref linker) = sess.opts.debugging_opts.linker {
Copy link
Member

Choose a reason for hiding this comment

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

I think you've changed the wrong variable...

[00:18:57] error[E0609]: no field `linker` on type `rustc::session::config::DebuggingOptions`
[00:18:57]   --> librustc_trans/back/link.rs:80:56
[00:18:57]    |
[00:18:57] 80 |     if let Some(ref linker) = sess.opts.debugging_opts.linker {
[00:18:57]    |                                                        ^^^^^^ unknown field
[00:18:57]    |
[00:18:57]    = note: available fields are: `codegen_backend`, `verbose`, `span_free_formats`, `identify_regions`, `emit_end_regions` ... and 89 others

Copy link
Member Author

Choose a reason for hiding this comment

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

er oops!

// *not* running dsymutil then the object files are the only source of truth
// for debug information, so we must preserve them.
if sess.target.target.options.is_like_osx {
match sess.opts.cg.run_dsymutil {
Copy link
Member

Choose a reason for hiding this comment

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

... the cgdebugging_opts should be applied here 😄

[00:18:57] error[E0609]: no field `run_dsymutil` on type `rustc::session::config::CodegenOptions`
[00:18:57]    --> librustc_trans/back/link.rs:224:28
[00:18:57]     |
[00:18:57] 224 |         match sess.opts.cg.run_dsymutil {
[00:18:57]     |                            ^^^^^^^^^^^^ unknown field
[00:18:57]     |
[00:18:57]     = note: available fields are: `ar`, `linker`, `link_arg`, `link_args`, `link_dead_code` ... and 28 others

@BatmanAoD
Copy link
Member

@kennytm @alexcrichton Something seems odd here; GitHub isn't displaying the commits in which kennytm's review comments were resolved, if indeed they were resolved. What is the status on this? (Also, does @michaelwoerister still need to review, since kennytm already did?)

@alexcrichton
Copy link
Member Author

Ah I think we're just waiting on review! @michaelwoerister I believe is on leave right now, but this isn't a particularly pressing PR so I'd be fine waiting.

@bors
Copy link
Contributor

bors commented Feb 7, 2018

☔ The latest upstream changes (presumably #48053) made this pull request unmergeable. Please resolve the merge conflicts.

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 7, 2018
@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 9, 2018
Copy link
Member

@michaelwoerister michaelwoerister left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks, @alexcrichton!

@@ -168,7 +168,9 @@ pub(crate) fn link_binary(sess: &Session,
if !sess.opts.cg.save_temps {
if sess.opts.output_types.should_trans() {
for obj in trans.modules.iter().filter_map(|m| m.object.as_ref()) {
remove(sess, obj);
if !preserve_objects_for_their_debuginfo(sess) {
Copy link
Member

Choose a reason for hiding this comment

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

Not that it makes much of a difference but this if could be pulled out of the loop, I think.

This commit adds the ability for rustc to not run `dsymutil` by default
on OSX. A new codegen option, `-Z run-dsymutil=no`, was added to specify
that `dsymutil` should *not* run and instead the compiler should
unconditionally keep the object files around in a compilation if
necessary for debug information.

cc rust-lang#47240
@alexcrichton
Copy link
Member Author

@bors: r=michaelwoerister

@bors
Copy link
Contributor

bors commented Feb 12, 2018

📌 Commit 0f16eee has been approved by michaelwoerister

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 12, 2018
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 12, 2018
@alexcrichton
Copy link
Member Author

@bors: rollup

kennytm added a commit to kennytm/rust that referenced this pull request Feb 13, 2018
…elwoerister

rustc: Add the ability to not run dsymutil

This commit adds the ability for rustc to not run `dsymutil` by default
on OSX. A new codegen option, `-Z run-dsymutil=no`, was added to specify
that `dsymutil` should *not* run and instead the compiler should
unconditionally keep the object files around in a compilation if
necessary for debug information.

cc rust-lang#47240
bors added a commit that referenced this pull request Feb 13, 2018
Rollup of 14 pull requests

- Successful merges: #47784, #47846, #48033, #48083, #48087, #48114, #48126, #48130, #48133, #48151, #48154, #48163, #48165, #48167
- Failed merges:
kennytm added a commit to kennytm/rust that referenced this pull request Feb 14, 2018
…elwoerister

rustc: Add the ability to not run dsymutil

This commit adds the ability for rustc to not run `dsymutil` by default
on OSX. A new codegen option, `-Z run-dsymutil=no`, was added to specify
that `dsymutil` should *not* run and instead the compiler should
unconditionally keep the object files around in a compilation if
necessary for debug information.

cc rust-lang#47240
bors added a commit that referenced this pull request Feb 14, 2018
bors added a commit that referenced this pull request Feb 15, 2018
bors added a commit that referenced this pull request Feb 15, 2018
@kennytm kennytm merged commit 0f16eee into rust-lang:master Feb 15, 2018
@alexcrichton alexcrichton deleted the less-dsymutil branch February 26, 2018 23:54
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.

8 participants