[PATCH] D21987: [XRay] Implement `llvm-xray extract`, start of the llvm-xray tool

[Dean Michael Berris via llvm-commits]

dberris marked 4 inline comments as done.
dberris added a comment.

In https://reviews.llvm.org/D21987#576626, @dblaikie wrote:

> In https://reviews.llvm.org/D21987#576231, @dberris wrote:
>
> > This is ready for another look @dblaikie -- added some failure tests that were easy to catch (not found, no instrumentation map) and provided some sample binaries to use instead of creating one on the fly (as it seemed harder to generate a final linked executable from a .ll).
> >
> > If you have ideas about how to make the sequence:
> >
> >   llc ...; <link the .o into a final binary> ;  llvm-xray extract <final binary>
> >   
> >
> > work in a `.ll` test, I'd be very interested to try.
>
>
> Could you explain why you need a linked binary rather than just an object file? What changes? Relocations, I guess? All the addresses look like zero?


The addresses are basically zero on a .o :/

> Yeah, doubt there's much to do about that - no way to produce linked binaries with the LLVM tools available to the test suite, and no point implementing support in the tool to handle unapplied relocations (unlike dwarfdump which does handle them, and there's more reason to run dwarfdump on unlinked objects, so it's worth having that support for more than just testing).

Yep, I think the binary should be fine here.

However, I'm having some trouble generating malformed binaries without resorting to hex-editing some existing files. Any recommendations for faking "bad" inputs to test the error conditions?



================
Comment at: tools/llvm-xray/xray-extract.cc:89-92
+  if (Filename.empty())
+    return make_error<StringError>(
+        "Provided filename is empty.",
+        std::make_error_code(std::errc::no_such_file_or_directory));
----------------
dblaikie wrote:
> Untested (do we need this test? Presumably we just won't be able to open the file)
Good point, removed.


================
Comment at: tools/llvm-xray/xray-extract.cc:97-98
+  // FIXME: Maybe support other ELF formats. For now, 64-bit Little Endian only.
+  if (!ObjectFile)
+    return ObjectFile.takeError();
+
----------------
dblaikie wrote:
> I'm guessing this is the path we take for "no such file or directory"? (ie: covered by the test for that)
Yes. that's correct.


https://reviews.llvm.org/D21987