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

[llvm-dwp] Join dwo paths correctly when DWOPath is absolute (12.0) #91

Merged

Conversation

nagisa
Copy link
Member

@nagisa nagisa commented Feb 17, 2021

When the DWOPath is absolute, we want to use DWOPath as is, without prepending any other
components to the path. The sys::path::append does not join, but rather unconditionally appends
the paths, so something like sys::path::append("/tmp", "/tmp/banana") will result in
/tmp/tmp/banana rather than the desired /tmp/banana.

This then causes llvm-dwp to fail in a following situation:

$ 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 -C2 foo.dwo
                  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)
$ 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)

Reviewed By: dblaikie

Differential Revision: https://reviews.llvm.org/D96678

Backport of: 6ffcb29

When the `DWOPath` is absolute, we want to use `DWOPath` as is, without prepending any other
components to the path. The `sys::path::append` does not join, but rather unconditionally appends
the paths, so something like `sys::path::append("/tmp", "/tmp/banana")` will result in
`/tmp/tmp/banana` rather than the desired `/tmp/banana`.

This then causes `llvm-dwp` to fail in a following situation:

```
$ 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 -C2 foo.dwo
                  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)
$ 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)
```

Reviewed By: dblaikie

Differential Revision: https://reviews.llvm.org/D96678

Backport of: 6ffcb29
@nikic
Copy link

nikic commented Feb 17, 2021

Did you request a backport to the release/12.x branch for this change?

@nagisa
Copy link
Member Author

nagisa commented Feb 17, 2021

I did not.

@nikic
Copy link

nikic commented Feb 17, 2021

Okay, I'll file one then. Is there any context on what this fixes on the rustc side? I couldn't find a relevant issue.

@nagisa
Copy link
Member Author

nagisa commented Feb 17, 2021

Ah, rust-lang/rust#82102 is the PR that this is necessary for.

@nagisa
Copy link
Member Author

nagisa commented Feb 17, 2021

But also see https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/llvm-dwp.20is.20not.20recommended/near/226567575 which potentially means we'll look to migrate away from this tool in some way or another.

@nikic
Copy link

nikic commented Feb 17, 2021

Thanks! I've opened https://bugs.llvm.org/show_bug.cgi?id=49229 for the backport.

@nikic nikic merged commit efa7d85 into rust-lang:rustc/12.0-2021-02-03 Feb 17, 2021
vext01 added a commit to vext01/llvm-project that referenced this pull request Oct 13, 2023
91: Give branches some attention. r=ltratt a=vext01



Co-authored-by: Edd Barrett <[email protected]>
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.

2 participants