<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Oct 24, 2016 at 10:17 PM Dean Michael Berris <<a href="mailto:dberris@google.com">dberris@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dberris marked 4 inline comments as done.<br class="gmail_msg">
dberris added a comment.<br class="gmail_msg">
<br class="gmail_msg">
In <a href="https://reviews.llvm.org/D21987#576626" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D21987#576626</a>, @dblaikie wrote:<br class="gmail_msg">
<br class="gmail_msg">
> In <a href="https://reviews.llvm.org/D21987#576231" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D21987#576231</a>, @dberris wrote:<br class="gmail_msg">
><br class="gmail_msg">
> > 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).<br class="gmail_msg">
> ><br class="gmail_msg">
> > If you have ideas about how to make the sequence:<br class="gmail_msg">
> ><br class="gmail_msg">
> >   llc ...; <link the .o into a final binary> ;  llvm-xray extract <final binary><br class="gmail_msg">
> ><br class="gmail_msg">
> ><br class="gmail_msg">
> > work in a `.ll` test, I'd be very interested to try.<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> 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?<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
The addresses are basically zero on a .o :/<br class="gmail_msg">
<br class="gmail_msg">
> 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).<br class="gmail_msg">
<br class="gmail_msg">
Yep, I think the binary should be fine here.<br class="gmail_msg">
<br class="gmail_msg">
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?<br class="gmail_msg"></blockquote><div><br>Hex editor or temporarily modify the generation code to produce broken output, generate the output then test.<br><br>I think that's what I did for some dwp tests.<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: tools/llvm-xray/xray-extract.cc:89-92<br class="gmail_msg">
+  if (Filename.empty())<br class="gmail_msg">
+    return make_error<StringError>(<br class="gmail_msg">
+        "Provided filename is empty.",<br class="gmail_msg">
+        std::make_error_code(std::errc::no_such_file_or_directory));<br class="gmail_msg">
----------------<br class="gmail_msg">
dblaikie wrote:<br class="gmail_msg">
> Untested (do we need this test? Presumably we just won't be able to open the file)<br class="gmail_msg">
Good point, removed.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: tools/llvm-xray/xray-extract.cc:97-98<br class="gmail_msg">
+  // FIXME: Maybe support other ELF formats. For now, 64-bit Little Endian only.<br class="gmail_msg">
+  if (!ObjectFile)<br class="gmail_msg">
+    return ObjectFile.takeError();<br class="gmail_msg">
+<br class="gmail_msg">
----------------<br class="gmail_msg">
dblaikie wrote:<br class="gmail_msg">
> I'm guessing this is the path we take for "no such file or directory"? (ie: covered by the test for that)<br class="gmail_msg">
Yes. that's correct.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D21987" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D21987</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></div>