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

Set path of the compile unit to the source directory #82102

Merged
merged 3 commits into from Feb 23, 2021

Conversation

nagisa
Copy link
Member

@nagisa nagisa commented Feb 14, 2021

As part of the effort to implement split dwarf debug info, we ended up
setting the compile unit location to the output directory rather than
the source directory. Furthermore, it seems like we failed to remap the
prefixes for this as well!

The desired behaviour is to instead set the DW_AT_GNU_dwo_name to a
path relative to compiler's working directory. This still allows
debuggers to find the split dwarf files, while not changing the
behaviour of the code that is compiling with regular debug info, and not
changing the compiler's behaviour with regards to reproducibility.

Fixes #82074

cc @alexcrichton @davidtwco

As part of the effort to implement split dwarf debug info, we ended up
setting the compile unit location to the output directory rather than
the source directory. Furthermore, it seems like we failed to remap the
prefixes for this as well!

The desired behaviour is to instead set the `DW_AT_GNU_dwo_name` to a
path relative to compiler's working directory. This still allows
debuggers to find the split dwarf files, while not changing the
behaviour of the code that is compiling with regular debug info, and not
changing the compiler's behaviour with regards to reproducibility.

Fixes rust-lang#82074
@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(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 Feb 14, 2021
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@davidtwco
Copy link
Member

r? @davidtwco

@davidtwco
Copy link
Member

r=me unless fixing the test failure requires sufficient changes to warrant another look.

@nagisa
Copy link
Member Author

nagisa commented Feb 14, 2021

Huh, so what's happening is that llvm-dwp is naively concatenating DW_AT_comp_dir + DW_AT_GNU_dwo_name from DW_TAG_compile_unit. This issue is also reproducible with clang and gcc:

$ clang -gsplit-dwarf /tmp/banana/test.c -c -o /tmp/outdir/foo.o
$ clang outdir/foo.o -o outdir/hm
$ llvm-dwarfdump outdir/hm | grep -C5 foo.dwo

0x00002114: DW_TAG_compile_unit
              DW_AT_stmt_list	(0x000005ae)
              DW_AT_comp_dir	("/tmp")
              DW_AT_GNU_pubnames	(true)
              DW_AT_GNU_dwo_name	("/tmp/outdir/foo.dwo")
              DW_AT_GNU_dwo_id	(0xde4d396f3bf0e257)
              DW_AT_low_pc	(0x0000000000401100)
              DW_AT_high_pc	(0x0000000000401103)
              DW_AT_GNU_addr_base	(0x00000000)
0x00002139: Compile Unit: length = 0x0000001e, format = DWARF32, version = 0x0002, abbr_offset = 0x03c0, addr_size = 0x08 (next unit at 0x0000215b)
$ strace -o trace llvm-dwp -e outdir/hm -o outdir/hm.dwp
error: No such file or directory
$ cat trace | grep foo.dwo
openat(AT_FDCWD, "/tmp/tmp/outdir/foo.dwo", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

From what I can tell the right place to solve this would be in the llvm-dwp tool itself.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@nagisa
Copy link
Member Author

nagisa commented Feb 17, 2021

@bors r=davidtwco rollup=never

@bors
Copy link
Contributor

bors commented Feb 17, 2021

📌 Commit 956b5ce7a12ba82163dc835a43421d5056800578 has been approved by davidtwco

@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 Feb 17, 2021
@bors
Copy link
Contributor

bors commented Feb 18, 2021

⌛ Testing commit 956b5ce7a12ba82163dc835a43421d5056800578 with merge aaa3ae5fd54761939253c04f4f1e0c238045480c...

@jethrogb
Copy link
Contributor

Please r-! This is pointing the llvm-project submodule at a commit that does not exist in any branches on https://github.com/rust-lang/llvm-project/

@nagisa
Copy link
Member Author

nagisa commented Feb 18, 2021

@bors r-

@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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 18, 2021
@nagisa

This comment has been minimized.

@bors

This comment has been minimized.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 18, 2021
@nagisa
Copy link
Member Author

nagisa commented Feb 18, 2021

@bors r=davidtwco

@bors

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 18, 2021

📌 Commit beb3f1e885c619f7964a093d72d56446e9c0bdae has been approved by davidtwco

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 19, 2021

⌛ Testing commit beb3f1e885c619f7964a093d72d56446e9c0bdae with merge 79516cd72305b88a97aefb8071e57e71693a02a4...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Feb 19, 2021

💔 Test failed - checks-actions

@bors 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 Feb 19, 2021
Make sure that we don't regress setting of the CU directory to the
working directory.
@nagisa
Copy link
Member Author

nagisa commented Feb 20, 2021

@bors r=davidtwco

@bors
Copy link
Contributor

bors commented Feb 20, 2021

📌 Commit 925ed48 has been approved by davidtwco

@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 Feb 20, 2021
@bors
Copy link
Contributor

bors commented Feb 23, 2021

⌛ Testing commit 925ed48 with merge 446d453...

@bors
Copy link
Contributor

bors commented Feb 23, 2021

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing 446d453 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 23, 2021
@bors bors merged commit 446d453 into rust-lang:master Feb 23, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 23, 2021
@bors bors mentioned this pull request Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

--remap-path-prefix behavior regression resulting in bad paths in 1.50.0
8 participants