<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Fri, May 5, 2017 at 9:57 AM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, May 4, 2017 at 4:53 PM Dean Michael Berris via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dberris marked an inline comment as done.<br>
dberris added a comment.<br>
<br>
In <a href="https://reviews.llvm.org/D32846#746274" rel="noreferrer" target="_blank">https://reviews.llvm.org/D32846#746274</a>, @dblaikie wrote:<br>
<br>
> Slightly curious to me that function IDs are > 0, but use a signed type - but no big deal, I guess there's probably a reason.<br>
<br>
<br>
I agonised over this for a while. There's no real reason why it's signed, but changing it now might be too late (unless we start doing XRay versioning of the sleds). It's hard to assume that when we're laying down bytes in the binary with the compiler that signed numbers and unsigned numbers have the same representation (maybe they are for one platform, while not in another).<br>
<br>
I suspect we can change it. Probably should do it a later time though. :)<br>
<br>
<br>
<br>
================<br>
Comment at: test/xray/TestCases/Linux/func-id-utils.cc:26-28<br>
+  // CHECK-LABEL: addresses:<br>
+  // CHECK: #[[MAX]] -> @[[ADDR:.*]]<br>
+  // CHECK-NOT: #0 -> @{{.*}}<br>
----------------<br>
dblaikie wrote:<br>
> What does this test?<br>
><br>
> Should the test case print the &foo and &bar, or assert(xray_function_address(1) == &foo), etc?<br>
It does look a little goofy, but it's testing that:<br>
<br>
- We do have functions instrumented (i.e. __xray_max_function_id() != 0).<br>
- The maximum function id has an address associated with it.<br>
- We don't have a function id 0 that has an address. :)<br>
<br>
While it's tempting to print the addresses of function id's and check, unfortunately we cannot assume that the compiler/linker will lay out the functions in a stable manner -- and thus function id's won't be guaranteed to always be mapped to specific function name.<br></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><div><br>Sort them?<br><br></div></div></div></blockquote><div><br></div><div>That might work.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>But also I was thinking more having the code itself check, rather than print and check with FileCheck?<br><br>  std::set<void*> original = {&foo, &bar};<br>  // build similar set from the xray_function_address calls<br>  if original != xray<br>    return 1;<br><br>that sort of thing.<br> </div></div></div></blockquote><div><br></div><div>Right. I can do both, let me update the patch. :)</div></div></div>