[llvm-dwp] Get the DWO file from relative path if the absolute path is not valid

[steven.zhang]

Summary

We are now getting the DWO using absolute path(Comp_dir + dwo_name). It will have problems if the objects are built with distribution system(i.e. distcc or buildfarm). Because they are likely built in other workspace which is NOT valid.

i.e.

<13a>   DW_AT_comp_dir    : (indirect string, offset: 0x3a9c2e): /opt/bazel-buildfarm/default/operations/9b0734e1-599d-4563-a3a1-76620b3b8cd3 // remote workspace
 <13e>   DW_AT_GNU_pubnames: 1
 <13e>   DW_AT_GNU_dwo_name: (indirect string, offset: 0x5faf0b): build64/not_interest_constants.cpp.dwo

What we got is build64/not_interest_constants.cpp.dwo from network, and current workspace is not /opt/bazel-buildfarm/default/operations/9b0734e1-599d-4563-a3a1-76620b3b8cd3.

So, we need to change it with relative path as what gcc dwp does.

[dblaikie]

I don't think this is correct - the dwp tool might be run from a different directory than the equivalent comp_dir and so treating these as cwd-of-dwp-relative paths seems problematic.

I think the correct thing to do in the situation described is for the compilation to use -fdebug-compilation-dir to make the debug info portable - this will also ensure that a debugger can find the source and find the .dwo files even if they aren't packaged into a .dwp.

[steven.zhang]

In D133480#3777059, @dblaikie wrote:

I don't think this is correct - the dwp tool might be run from a different directory than the equivalent comp_dir and so treating these as cwd-of-dwp-relative paths seems problematic.

I think the correct thing to do in the situation described is for the compilation to use -fdebug-compilation-dir to make the debug info portable - this will also ensure that a debugger can find the source and find the .dwo files even if they aren't packaged into a .dwp.

gdb can work well as it seems that, it also refers the relative path, except the llvm-dwp. And we require user to specify an extra option to make it work.

Another issue is that, building in different workspace we need specify the different -fdebug-compilation-dir=xxx options, and produce different objects(comp_dir changed), which completely pollute the remote cache.

[steven.zhang]

In D133480#3777059, @dblaikie wrote:

I don't think this is correct - the dwp tool might be run from a different directory than the equivalent comp_dir and so treating these as cwd-of-dwp-relative paths seems problematic.

I think the correct thing to do in the situation described is for the compilation to use -fdebug-compilation-dir to make the debug info portable - this will also ensure that a debugger can find the source and find the .dwo files even if they aren't packaged into a .dwp.

Can we do it as what gdb did ? That is, getting the dwo from absolute path first, and if failed, get it from relative path ?

[dblaikie]

In D133480#3777196, @steven.zhang wrote:

In D133480#3777059, @dblaikie wrote:

I don't think this is correct - the dwp tool might be run from a different directory than the equivalent comp_dir and so treating these as cwd-of-dwp-relative paths seems problematic.

I think the correct thing to do in the situation described is for the compilation to use -fdebug-compilation-dir to make the debug info portable - this will also ensure that a debugger can find the source and find the .dwo files even if they aren't packaged into a .dwp.

Can we do it as what gdb did ? That is, getting the dwo from absolute path first, and if failed, get it from relative path ?

I'm somewhat open to that, though I'd really encourage you to switch your build system/build flags so you don't have to keep adding functionality like that into other debug info consuming tools.

[steven.zhang]

In D133480#3778054, @dblaikie wrote:

In D133480#3777196, @steven.zhang wrote:

In D133480#3777059, @dblaikie wrote:

I don't think this is correct - the dwp tool might be run from a different directory than the equivalent comp_dir and so treating these as cwd-of-dwp-relative paths seems problematic.

I think the correct thing to do in the situation described is for the compilation to use -fdebug-compilation-dir to make the debug info portable - this will also ensure that a debugger can find the source and find the .dwo files even if they aren't packaged into a .dwp.

Can we do it as what gdb did ? That is, getting the dwo from absolute path first, and if failed, get it from relative path ?

I'm somewhat open to that, though I'd really encourage you to switch your build system/build flags so you don't have to keep adding functionality like that into other debug info consuming tools.

Thank for the suggestion, and I appreciate it. The issue of adding the build flags is that, it will pollute the remote cache. For example:

• building in workspace A: adding compilation option -fdebug-compilation-dir=path_to_workspace_A, and the objects will be cached in the remote build server.

• building in workspace B: adding compilation option -fdebug-compilation-dir=path_to_workspace_B, we cannot get the objects from remote cache though all the source code didn't change. We have to rebuild it as the compilation option changed.

If with relative path, we don't need to rebuild the objects as everything is unchanged. Remote cache is important to large project to speed up the building.

I will extend current implementation to support relative path.

[steven.zhang]

Address comments.

[dblaikie]

In D133480#3778767, @steven.zhang wrote:

In D133480#3778054, @dblaikie wrote:

In D133480#3777196, @steven.zhang wrote:

In D133480#3777059, @dblaikie wrote:

I don't think this is correct - the dwp tool might be run from a different directory than the equivalent comp_dir and so treating these as cwd-of-dwp-relative paths seems problematic.

I think the correct thing to do in the situation described is for the compilation to use -fdebug-compilation-dir to make the debug info portable - this will also ensure that a debugger can find the source and find the .dwo files even if they aren't packaged into a .dwp.

Can we do it as what gdb did ? That is, getting the dwo from absolute path first, and if failed, get it from relative path ?

I'm somewhat open to that, though I'd really encourage you to switch your build system/build flags so you don't have to keep adding functionality like that into other debug info consuming tools.

Thank for the suggestion, and I appreciate it. The issue of adding the build flags is that, it will pollute the remote cache. For example:

• building in workspace A: adding compilation option -fdebug-compilation-dir=path_to_workspace_A, and the objects will be cached in the remote build server.

• building in workspace B: adding compilation option -fdebug-compilation-dir=path_to_workspace_B, we cannot get the objects from remote cache though all the source code didn't change. We have to rebuild it as the compilation option changed.

If with relative path, we don't need to rebuild the objects as everything is unchanged. Remote cache is important to large project to speed up the building.

I will extend current implementation to support relative path.

Ah, sorry I didn't fully explain - yes (for context: I work at Google/on the compiler we use in the internal version of Bazel - so I'm familiar with the complexities of distributed and reusable build caches). Specifically, if the comp_dir is basically unusable (because if it's accurate/absolute for a local user, it's useless to another user, and it can't be whatever's on the build node either) you can use -fdebug-compilation-dir=. (I think at Google we use /proc/cwd to be precise) and then you'll get the current-working-directory relative behavior without consumers having to implement any special case.

[dblaikie]

Chromium does something similar here: https://crrev.com/4ea122b62d8d7887a4d653c73bdc8ba50c890c2e/build/config/compiler/BUILD.gn#521

[steven.zhang]

In D133480#3779118, @dblaikie wrote:

In D133480#3778767, @steven.zhang wrote:

In D133480#3778054, @dblaikie wrote:

In D133480#3777196, @steven.zhang wrote:

In D133480#3777059, @dblaikie wrote:

I don't think this is correct - the dwp tool might be run from a different directory than the equivalent comp_dir and so treating these as cwd-of-dwp-relative paths seems problematic.

I think the correct thing to do in the situation described is for the compilation to use -fdebug-compilation-dir to make the debug info portable - this will also ensure that a debugger can find the source and find the .dwo files even if they aren't packaged into a .dwp.

Can we do it as what gdb did ? That is, getting the dwo from absolute path first, and if failed, get it from relative path ?

I'm somewhat open to that, though I'd really encourage you to switch your build system/build flags so you don't have to keep adding functionality like that into other debug info consuming tools.

Thank for the suggestion, and I appreciate it. The issue of adding the build flags is that, it will pollute the remote cache. For example:

• building in workspace A: adding compilation option -fdebug-compilation-dir=path_to_workspace_A, and the objects will be cached in the remote build server.

• building in workspace B: adding compilation option -fdebug-compilation-dir=path_to_workspace_B, we cannot get the objects from remote cache though all the source code didn't change. We have to rebuild it as the compilation option changed.

If with relative path, we don't need to rebuild the objects as everything is unchanged. Remote cache is important to large project to speed up the building.

I will extend current implementation to support relative path.

Ah, sorry I didn't fully explain - yes (for context: I work at Google/on the compiler we use in the internal version of Bazel - so I'm familiar with the complexities of distributed and reusable build caches). Specifically, if the comp_dir is basically unusable (because if it's accurate/absolute for a local user, it's useless to another user, and it can't be whatever's on the build node either) you can use -fdebug-compilation-dir=. (I think at Google we use /proc/cwd to be precise) and then you'll get the current-working-directory relative behavior without consumers having to implement any special case.

Hmm, get your point. -fdebug-compilation-dir=. should work, and I will try with that. Thank you. But I think, it is also NOT bad to do another attempt if the absolute path is not valid for those cases that someone didn't add this option. What do you think ?

[dblaikie]

fair enough, if this is gold/binutils dwp's behavior (can you provide a pastebin/show this is the behavior there, maybe even point to the code that implements it?) doesn't seem like the worst thing to have in llvm-dwp, though this will need a test case

[steven.zhang]

In D133480#3780651, @dblaikie wrote:

fair enough, if this is gold/binutils dwp's behavior (can you provide a pastebin/show this is the behavior there, maybe even point to the code that implements it?) doesn't seem like the worst thing to have in llvm-dwp, though this will need a test case

binutils dwp ONLY check the relative path while llvm-dwp now ONLY check the absolute path.

$ clang++ a.cpp -g -gsplit-dwarf
$ cd ..
$ ls
test
$ dwp -e test/a.out
dwp: error: cannot open a.dwo: No such file or directory
dwp: fatal error: a.dwo: can't open

$ llvm-dwp -e test/a.out -o test/a.out.dwp

We are trying to make both way work. The way GDB search the dwo is both way. It searches the absolute path first, and then, the relative path.

$ ls test/
a.cpp  a.dwo  a.out
$ touch a.dwo             # invalid dwo in current dir
$ gdb test/a.out < a.dbx  # works fine, so, it picks the dwo in test instead of current dir
$ mv test/a.dwo dwo       # move the dwo to current dir
$ gdb test/a.out < a.dbx  # still works fine, so, it now picks the dwo in current dir.

[steven.zhang]

Add tests.

[dblaikie]

Sounds OK - some minor test case improvements that might be useful.

llvm/test/tools/llvm-dwp/X86/search_dwos.test

13

maybe use a less-likely-to-accidentally-exist path rather than /tmp (/non-existent or something?) (& any ideas how this works if the test runs on Windows? does the absolute path confuse things? maybe using a relative comp-dir like non-existent (without the leading /) would be better? Not sure how the other tests work already

[steven.zhang]

llvm/test/tools/llvm-dwp/X86/search_dwos.test

13

Good point. path-not-exists I will use.

[aeubanks]

llvm/test/tools/llvm-dwp/X86/search_dwos.test

13

this test is failing on my machine because I have a /tmp/a.dwo with a different dwarf version. I think the checked in binaries are still using /tmp, could that be fixed?

[dblaikie]

llvm/test/tools/llvm-dwp/X86/search_dwos.test

13

Yeah, this probably should be recompiled with -fdebug-info-compilation-dir=some_relative_path (that way it's more readily portable to Windows, and would work reliably in the local test directory where we control what files exist/don't exist)

@steven.zhang could you please update the patch & post it for review along with the llvm-dwarfdump of the contents? (it's also possible this test could be modified to use assembly and llvm-mc to assemble it, rather than using checked in binary files, perhaps?)

[steven.zhang]

llvm/test/tools/llvm-dwp/X86/search_dwos.test

13

I will do it after my vacation （Oct 9）or someone could do it as it is a bit long time.

[aeubanks]

llvm/test/tools/llvm-dwp/X86/search_dwos.test

13

(next week is fine, I removed /tmp/a.dwo locally and now the test passes, just something that should eventually be fixed)

llvm/test/tools/llvm-dwp/X86/search_dwos.test

+RUN: rm -rf %t
+RUN: mkdir %t
+RUN: cd %t
+RUN: cp %p/../Inputs/search_dwos/a.dwo a.dwo
+RUN: cp %p/../Inputs/search_dwos/b.dwo b.dwo
+RUN: cp %p/../Inputs/search_dwos/main main
+RUN: llvm-dwp -e main -o %t.dwp 
+
+Search the DWO from relative path if absolute path is not valid.
+Build commands for the test binaries:
+
+clang++ -Xclang -fdebug-compilation-dir -Xclang "path-not-exists" -g -O0 -gsplit-dwarf a.cpp b.cpp -o main
+
+sources:
+a.cpp:
+  void a() {}
+
+b.cpp:
+  void b() {}
+  int main() {
+     return 0;
+  }

llvm/tools/llvm-dwp/llvm-dwp.cpp

std::string DWOName = dwarf::toString(
         Die.find({dwarf::DW_AT_dwo_name, dwarf::DW_AT_GNU_dwo_name}), "");
     if (DWOName.empty())
       continue;
     std::string DWOCompDir =
         dwarf::toString(Die.find(dwarf::DW_AT_comp_dir), "");
     if (!DWOCompDir.empty()) {
       SmallString<16> DWOPath(std::move(DWOName));
       sys::fs::make_absolute(DWOCompDir, DWOPath);
+      if (!sys::fs::exists(DWOPath) && sys::fs::exists(DWOName))
+        DWOPaths.push_back(std::move(DWOName));
+      else
         DWOPaths.emplace_back(DWOPath.data(), DWOPath.size());
     } else {
       DWOPaths.push_back(std::move(DWOName));
     }
   }
   return std::move(DWOPaths);
 }
 
 static int error(const Twine &Error, const Twine &Context) {