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

Add an abs_path member to FileMap, use it when writing debug info. #34187

Merged
merged 1 commit into from
Jun 16, 2016

Conversation

luser
Copy link
Contributor

@luser luser commented Jun 9, 2016

Fixes #34179.

When items are inlined from extern crates, the filename in the debug info
is taken from the FileMap that's serialized in the rlib metadata.
Currently this is just FileMap.name, which is whatever path is passed to rustc.
Since libcore and libstd are built by invoking rustc with relative paths,
they wind up with relative paths in the rlib, and when linked into a binary
the debug info uses relative paths for the names, but since the compilation
directory for the final binary, tools trying to read source filenames
will wind up with bad paths. We noticed this in Firefox with source
filenames from libcore/libstd having bad paths.

This change stores an absolute path in FileMap.abs_path, and uses that
if available for writing debug info. This is not going to magically make
debuggers able to find the source, but it will at least provide sensible
paths.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@alexcrichton
Copy link
Member

r? @michaelwoerister

@luser
Copy link
Contributor Author

luser commented Jun 10, 2016

I hadn't had a clean make check run locally because apparently I had an old copy of libev still sitting in my Rust checkout from who-knows-when, so I fixed that and hit a few style errors with some of the lines I changed being too long. With those fixed make check is clean for me locally on Linux x86-64.

pub fn new_filemap_and_lines(&self, filename: &str, abs_path: Option<&str>,
src: &str) -> Rc<FileMap> {
let fm = self.new_filemap(filename.to_string(),
abs_path.map(|s| s.to_owned()),
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 I would change this, but you could also write this as abs_path.map(str::to_owned).

@michaelwoerister
Copy link
Member

@bors r+

Thanks for the PR. Let's see if LLDB has any problems with it.

@bors
Copy link
Contributor

bors commented Jun 10, 2016

📌 Commit 4a08c70 has been approved by michaelwoerister

@bors
Copy link
Contributor

bors commented Jun 11, 2016

🔒 Merge conflict

When items are inlined from extern crates, the filename in the debug info
is taken from the FileMap that's serialized in the rlib metadata.
Currently this is just FileMap.name, which is whatever path is passed to rustc.
Since libcore and libstd are built by invoking rustc with relative paths,
they wind up with relative paths in the rlib, and when linked into a binary
the debug info uses relative paths for the names, but since the compilation
directory for the final binary, tools trying to read source filenames
will wind up with bad paths. We noticed this in Firefox with source
filenames from libcore/libstd having bad paths.

This change stores an absolute path in FileMap.abs_path, and uses that
if available for writing debug info. This is not going to magically make
debuggers able to find the source, but it will at least provide sensible
paths.
@luser
Copy link
Contributor Author

luser commented Jun 16, 2016

I rebased onto master and squashed the fixup commit.

@michaelwoerister
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 16, 2016

📌 Commit 24e7491 has been approved by michaelwoerister

@bors
Copy link
Contributor

bors commented Jun 16, 2016

⌛ Testing commit 24e7491 with merge 18f2871...

bors added a commit that referenced this pull request Jun 16, 2016
Add an abs_path member to FileMap, use it when writing debug info.

Fixes #34179.

When items are inlined from extern crates, the filename in the debug info
is taken from the FileMap that's serialized in the rlib metadata.
Currently this is just FileMap.name, which is whatever path is passed to rustc.
Since libcore and libstd are built by invoking rustc with relative paths,
they wind up with relative paths in the rlib, and when linked into a binary
the debug info uses relative paths for the names, but since the compilation
directory for the final binary, tools trying to read source filenames
will wind up with bad paths. We noticed this in Firefox with source
filenames from libcore/libstd having bad paths.

This change stores an absolute path in FileMap.abs_path, and uses that
if available for writing debug info. This is not going to magically make
debuggers able to find the source, but it will at least provide sensible
paths.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants