[llvm-dwp] Add command line option "-e"

[alexander-shaposhnikov]

Summary

The binutils utility dwp has an option "-e" https://gcc.gnu.org/wiki/DebugFissionDWP
to pass an executable / library (to get the list of dwo files from it).
This option is particularly useful when someone runs the tool manually outside of a build system
(passing the full list of dwo files is not convenient).
This diff adds an implementation "-e" to llvm-dwp.

Test plan: make check-all

[alexander-shaposhnikov]

Apply clang-format.

[grimar]

Few comments below.

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

7

Please specify compiler and command line used.

tools/llvm-dwp/llvm-dwp.cpp

474

I would add few empty lines to this function to separate code blocks.

490

I think SmallString<16> would be more common.

491

I believe it is excessive to reserve here.

493

Here is a bug. DWOPatch.data() will return pointer to internal buffer, but not a string.
It can be not nul-terminated.

You may use following to fix probably:

DWOPaths.emplace_back(DWOPath.data(), DWOPath.size());

718

It seems this will fail if you run tests with LLVM_ENABLE_ABI_BREAKING_CHECKS.
You need to check Expected<>:

auto DWOs = getDWOFilenames(ExecFilename);
if (!DWOs) {
...

Otherwise if should fail:
https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Support/Error.h#L507
https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Support/Error.h#L636.

[alexander-shaposhnikov]

address code review comments,
fix a bug.

[alexander-shaposhnikov]

@grimar - many thanks for the review - good catches ! (+ i was too sleepy)

[dblaikie]

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

5

Yeah, it's difficult/not quite possible to stream out ELF files (because the header has to point to the section headers, which are emitted at the end of the file so they can contain all the section sizes... (you could do a prepass to compute all these things then you could stream output)) - Clang (& maybe the llvm tools like llvm-mc, etc - not sure, haven't checked them) avoids this by writing to a temp file then streaming the temp file out through stdout or whatnot. llvm-dwp doesn't have that functionality, but could be taught it, maybe. (might be useful if it were optional, for those who don't want to burn more file IO/space, etc)

tools/llvm-dwp/llvm-dwp.cpp

57–60

Wonder if this even needs a separate argument - would it be reasonable to detect which case this is in? (probably not by file extension, but by contents? Not sure... maybe that'd be hard/annoying/not much use)

725–727

Seems weird to ignore all the other inputs silently if -e is specified. Maybe error out if there are both?

Alternatively could support both, maybe - more like what I mentioned above, even. Could detect that something is an executable/contains skeleton units - use those to find DWOs, and treat that as another source of DWOs (so you could have arbitrarily many executables and DWOs on the command line and they'd all be read and smooshed into a DWP).

*shrug* Dunno.

What does binutils dwp do if you have both -e and .dwo files? Error?

[alexander-shaposhnikov]

tools/llvm-dwp/llvm-dwp.cpp

725–727

yeah,
i've just looked at binutils dwp ( 2.25.1-22.base.el7).
Actually binutils dwp behaves differently, it takes everything, i.e.

[[e-mail]1 ~/fission/build] dwp c.dwo -e out/libFoo.so -o test.dwp
[[e-mail]1 ~/fission/build] cat c.cpp
struct XXXXXX {
  int YYYYYY;
 };

 int ZZZZZZ() {
   XXXXXX x;
   return x.YYYYYY;
 }
 [[e-mail]1 ~/fission/build] strings test.dwp | grep ZZZZZZ
 ZZZZZZ
 _Z6ZZZZZZv

additionally, binutils dwp allows to pass -e multiple times.

so yes, we can try to replicate this behavior (that looks easy),
alternatively, we can try to avoid introducing this new flag and try to extract the necessary information from the files themselves (i agree, this probably would be cleaner, on the down side - requires more work/a bit more complex logic + switching back and forth from bintutils version to llvm will require changing the invocation) @dblaikie - how should we proceed ?

[alexander-shaposhnikov]

Allow using multiple sources of dwos

[dblaikie]

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

2

Should probably include a direct .dwo file in this test as well? (maybe multiple .dwo and executables, ideally - to demonstrate/test that they're both supported in plurals)

tools/llvm-dwp/llvm-dwp.cpp

715–727

Prefer:

std::vector<std::string> DWOFilenames = InputFiles;

(if that compiles... ) rather than direct init.

718–721

Does binutils dwp error on this, or ignore the executable?

[alexander-shaposhnikov]

tools/llvm-dwp/llvm-dwp.cpp

718–721

[[e-mail]1 ~/fission/build] dwp c.dwo -e out/libFooDoesNotExist.so -o test.dwp
dwp: error: cannot open out/libFooDoesNotExist.so: No such file or directory
dwp: fatal error: out/libFooDoesNotExist.so: can't open

binutils dwp reports an error on this.

[alexander-shaposhnikov]

Update the test.

[dblaikie]

Some things to tidy up in the test, some optional.

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

17–19

You could simplify these a bit by just making them void:

void a() {}

etc.

22–27

Could have main here and drop 'b', I'd expect.

44–69

Is it worth checking the abbrevs? Seems like it might be enough to just check the DW_AT_names?

77

This seems wrong - you're not using the DWOA value, and you're rematching it on every dwo_id? Probably skip that.

[alexander-shaposhnikov]

Simplify the test a bit.

[alexander-shaposhnikov]

@dblaikie - many thanks for the code review

[alexander-shaposhnikov]

Update the binaries used by the test and document the hack used to generate "portable" binaries.

I have not committed this diff yet and will keep this open for some time since the problem with the test
might be of independent interest.
Notes:

1. obj2yaml + sed + yaml2obj doesn't seem to be an option at this point

(we need to edit the debug info, obj2yaml dumps dwarf just as hex).

2. running the compiler + linker doesn't seem to be an option (since this is an llvm test).
3. the approach chosen below (while being hacky) only requires some efforts at the data preparation step and is quite simple,

thus it seems to be the smaller evil than other options are,
however i assume i could have missed something.

[alexander-shaposhnikov]

fix typo

[alexander-shaposhnikov]

Rebuild the test binaries,
make use of -fdebug-compilation-dir.
Rerun the tests.

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

+FIXME: for the reasons explained below 
+(see the description of how the thest binaries were generated) 
+we have to do a few hacky things to make this test portable. 
+It would be wonderful to find a better and simpler approach.
+
+REQUIRES: shell
+
+RUN: rm -rf %t
+RUN: mkdir %t
+RUN: cd %t
+RUN: cp %p/../Inputs/dwos_list_from_exec/a.dwo a.dwo
+RUN: cp %p/../Inputs/dwos_list_from_exec/b.dwo b.dwo
+RUN: cp %p/../Inputs/dwos_list_from_exec/c.dwo c.dwo
+RUN: cp %p/../Inputs/dwos_list_from_exec/d.dwo d.dwo
+RUN: cp %p/../Inputs/dwos_list_from_exec/e.dwo e.dwo
+RUN: cp %p/../Inputs/dwos_list_from_exec/main main
+RUN: cp %p/../Inputs/dwos_list_from_exec/libd.so libd.so
+RUN: llvm-dwp c.dwo e.dwo -e main -e libd.so -o pkg.dwp
+RUN: llvm-dwarfdump pkg.dwp | FileCheck %s
+
+Build commands for test data:
+
+mkdir <build directory> && cd <build directory>
+clang++ -g -O0 -gsplit-dwarf a.cpp b.cpp -o main
+clang++ -g -O0 -gsplit-dwarf -c c.cpp -o c.o
+clang++ -g -O0 -gsplit-dwarf -fPIC -shared d.cpp -o libd.so
+clang++ -g -O0 -gsplit-dwarf -c e.cpp -o e.o
+Note, that at this stage the binaries main and libd.so
+contain dwarf::DW_AT_comp_dir with the absolute path
+to the <build directory>, thus they are not suitable 
+for portable tests. To overcome this difficulty we apply
+the "binary patching" script convert.py to main and libd.so.
+
+convert.py:
+
+#!/usr/bin/env python
+import sys
+# Read the binary data from stdin
+content = sys.stdin.read()
+# The right-hand side should be the actual value of <build directory>
+old_value="<build directory>"
+new_value="./" + "/" * (len(old_value) - 2)
+assert(len(old_value) == len(new_value))
+# Replace the absolute path to the build directory
+# with the string of the same length of the following form: "./////.../".
+# Since the length has not changed this transformation doesn't break 
+# the binary format.
+new_content = content.replace(old_value, new_value)
+sys.stdout.write(new_content)
+
+
+sources:
+a.cpp:
+  void a() {}
+
+b.cpp:
+  void b() {}
+  int main() {
+     return 0;
+  }
+
+c.cpp:
+  void c() {}
+
+d.cpp:
+  void d() {}
+
+e.cpp:
+  void e() {}
+
+CHECK-LABEL: .debug_abbrev.dwo contents:
+
+CHECK-LABEL: Abbrev table for offset:
+CHECK: DW_TAG_compile_unit
+CHECK: DW_TAG_subprogram
+
+CHECK-LABEL: Abbrev table for offset:
+CHECK: DW_TAG_compile_unit
+CHECK: DW_TAG_subprogram
+
+CHECK-LABEL: Abbrev table for offset:
+CHECK: DW_TAG_compile_unit
+CHECK: DW_TAG_subprogram
+
+CHECK-LABEL: Abbrev table for offset:
+CHECK: DW_TAG_compile_unit
+CHECK: DW_TAG_subprogram
+
+CHECK-LABEL: Abbrev table for offset:
+CHECK: DW_TAG_compile_unit
+CHECK: DW_TAG_subprogram
+
+CHECK: .debug_info.dwo contents:
+CHECK: [[AOFF:0x[0-9a-f]*]]:
+
+CHECK-LABEL: Compile Unit: length = {{.*}} version = 0x0004
+CHECK: DW_TAG_compile_unit
+CHECK:   DW_AT_name {{.*}} "c.cpp"
+CHECK:   DW_TAG_subprogram
+CHECK:     DW_AT_name {{.*}} "c"
+
+CHECK-LABEL: Compile Unit: length = {{.*}} version = 0x0004
+CHECK: DW_TAG_compile_unit
+CHECK:   DW_AT_name {{.*}} "e.cpp"
+CHECK:   DW_TAG_subprogram
+CHECK:     DW_AT_name {{.*}} "e"
+
+CHECK-LABEL: Compile Unit: length = {{.*}} version = 0x0004
+CHECK: DW_TAG_compile_unit
+CHECK:   DW_AT_name {{.*}} "a.cpp"
+CHECK:   DW_TAG_subprogram
+CHECK:     DW_AT_name {{.*}} "a"
+
+CHECK-LABEL: Compile Unit: length = {{.*}} version = 0x0004
+CHECK: DW_TAG_compile_unit
+CHECK:   DW_AT_name {{.*}} "b.cpp"
+CHECK:   DW_TAG_subprogram
+CHECK:     DW_AT_name {{.*}} "b"
+CHECK:   DW_TAG_subprogram
+CHECK:     DW_AT_name {{.*}} "main"
+
+CHECK-LABEL: Compile Unit: length = {{.*}} version = 0x0004
+CHECK: DW_TAG_compile_unit
+CHECK:   DW_AT_name {{.*}} "d.cpp"
+CHECK:   DW_TAG_subprogram
+CHECK:     DW_AT_name {{.*}} "d"

tools/llvm-dwp/llvm-dwp.cpp

 //
 //===----------------------------------------------------------------------===//
 #include "DWPError.h"
 #include "DWPStringPool.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/CodeGen/AsmPrinter.h"
+#include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFFormValue.h"
 #include "llvm/DebugInfo/DWARF/DWARFUnitIndex.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCRegisterInfo.h"
 #include "llvm/MC/MCSectionELF.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCTargetOptionsCommandFlags.h"
 #include "llvm/Object/Decompressor.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/DataExtractor.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Options.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include <deque>
 #include <iostream>
 #include <memory>
 
 using namespace llvm;
 using namespace llvm::object;
 using namespace cl;
 
 OptionCategory DwpCategory("Specific Options");
-static list<std::string> InputFiles(Positional, OneOrMore,
+static list<std::string> InputFiles(Positional, ZeroOrMore,
                                     desc("<input files>"), cat(DwpCategory));
 
+static list<std::string> ExecFilenames(
+    "e", ZeroOrMore,
+    desc("Specify the executable/library files to get the list of *.dwo from"),
+    value_desc("filename"), cat(DwpCategory));
+
 static opt<std::string> OutputFilename(Required, "o",
                                        desc("Specify the output file."),
                                        value_desc("filename"),
                                        cat(DwpCategory));
 
 static void writeStringsAndOffsets(MCStreamer &Out, DWPStringPool &Strings,
                                    MCSection *StrOffsetSection,
                                    StringRef CurStrSection,

 
 struct CompileUnitIdentifiers {
   uint64_t Signature = 0;
   const char *Name = "";
   const char *DWOName = "";
 };
 
 static Expected<const char *>
-getIndexedString(dwarf::Form Form, DataExtractor InfoData, 
+getIndexedString(dwarf::Form Form, DataExtractor InfoData,
                  uint32_t &InfoOffset, StringRef StrOffsets, StringRef Str) {
   if (Form == dwarf::DW_FORM_string)
     return InfoData.getCStr(&InfoOffset);
   if (Form != dwarf::DW_FORM_GNU_str_index)
     return make_error<DWPError>(
         "string field encoded without DW_FORM_string or DW_FORM_GNU_str_index");
   auto StrIndex = InfoData.getULEB128(&InfoOffset);
   DataExtractor StrOffsetsData(StrOffsets, true, 0);
buildDuplicateError(const std::pair<uint64_t, UnitIndexEntry> &PrevE,
                     const CompileUnitIdentifiers &ID, StringRef DWPName) {
   return make_error<DWPError>(
       std::string("Duplicate DWO ID (") + utohexstr(PrevE.first) + ") in " +
       buildDWODescription(PrevE.second.Name, PrevE.second.DWPName,
                           PrevE.second.DWOName) +
       " and " + buildDWODescription(ID.Name, DWPName, ID.DWOName));
 }
 
+static Expected<SmallVector<std::string, 16>>
+getDWOFilenames(StringRef ExecFilename) {
+  auto ErrOrObj = object::ObjectFile::createObjectFile(ExecFilename);
+  if (!ErrOrObj)
+    return ErrOrObj.takeError();
+
+  const ObjectFile &Obj = *ErrOrObj.get().getBinary();
+  std::unique_ptr<DWARFContext> DWARFCtx = DWARFContext::create(Obj);
+
+  SmallVector<std::string, 16> DWOPaths;
+  for (const auto &CU : DWARFCtx->compile_units()) {
+    const DWARFDie &Die = CU->getUnitDIE();
+    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;
+      sys::path::append(DWOPath, DWOCompDir, DWOName);
+      DWOPaths.emplace_back(DWOPath.data(), DWOPath.size());
+    } else {
+      DWOPaths.push_back(std::move(DWOName));
+    }
+  }
+  return std::move(DWOPaths);
+}
+
 static Error write(MCStreamer &Out, ArrayRef<std::string> Inputs) {
   const auto &MCOFI = *Out.getContext().getObjectFileInfo();
   MCSection *const StrSection = MCOFI.getDwarfStrDWOSection();
   MCSection *const StrOffsetSection = MCOFI.getDwarfStrOffDWOSection();
   MCSection *const TypesSection = MCOFI.getDwarfTypesDWOSection();
   MCSection *const CUIndexSection = MCOFI.getDwarfCUIndexSection();
   MCSection *const TUIndexSection = MCOFI.getDwarfTUIndexSection();
   const StringMap<std::pair<MCSection *, DWARFSectionKind>> KnownSections = {
int main(int argc, char **argv) {
   MCTargetOptions MCOptions = InitMCTargetOptionsFromFlags();
   std::unique_ptr<MCStreamer> MS(TheTarget->createMCObjectStreamer(
       TheTriple, MC, *MAB, OutFile, MCE, *MSTI, MCOptions.MCRelaxAll,
       MCOptions.MCIncrementalLinkerCompatible,
       /*DWARFMustBeAtTheEnd*/ false));
   if (!MS)
     return error("no object streamer for target " + TripleName, Context);
 
-  if (auto Err = write(*MS, InputFiles)) {
+  std::vector<std::string> DWOFilenames = InputFiles;
+  for (const auto &ExecFilename : ExecFilenames) {
+    auto DWOs = getDWOFilenames(ExecFilename);
+    if (!DWOs) {
+      logAllUnhandledErrors(DWOs.takeError(), errs(), "error: ");
+      return 1;
+    }
+    DWOFilenames.insert(DWOFilenames.end(),
+                        std::make_move_iterator(DWOs->begin()),
+                        std::make_move_iterator(DWOs->end()));
+  }
+
+  if (auto Err = write(*MS, DWOFilenames)) {
     logAllUnhandledErrors(std::move(Err), errs(), "error: ");
     return 1;
   }
 
   MS->Finish();
 }