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

[David Blaikie via llvm-commits]

On Mon, Oct 24, 2016 at 10:17 PM Dean Michael Berris <dberris at google.com>
wrote:

> 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?
>

Hex editor or temporarily modify the generation code to produce broken
output, generate the output then test.

I think that's what I did for some dwp tests.

- Dave


>
>
>
> ================
> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161025/680cd7e1/attachment.html>