[PATCH] D32846: [XRay][compiler-rt] Add function id utilities for XRay

Dean Michael Berris via llvm-commits llvm-commits at lists.llvm.org
Thu May 4 17:04:46 PDT 2017


On Fri, May 5, 2017 at 9:57 AM David Blaikie <dblaikie at gmail.com> wrote:

> On Thu, May 4, 2017 at 4:53 PM Dean Michael Berris via Phabricator <
> reviews at reviews.llvm.org> wrote:
>
>> dberris marked an inline comment as done.
>> dberris added a comment.
>>
>> In https://reviews.llvm.org/D32846#746274, @dblaikie wrote:
>>
>> > 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.
>>
>>
>> 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).
>>
>> I suspect we can change it. Probably should do it a later time though. :)
>>
>>
>>
>> ================
>> Comment at: test/xray/TestCases/Linux/func-id-utils.cc:26-28
>> +  // CHECK-LABEL: addresses:
>> +  // CHECK: #[[MAX]] -> @[[ADDR:.*]]
>> +  // CHECK-NOT: #0 -> @{{.*}}
>> ----------------
>> dblaikie wrote:
>> > What does this test?
>> >
>> > Should the test case print the &foo and &bar, or
>> assert(xray_function_address(1) == &foo), etc?
>> It does look a little goofy, but it's testing that:
>>
>> - We do have functions instrumented (i.e. __xray_max_function_id() != 0).
>> - The maximum function id has an address associated with it.
>> - We don't have a function id 0 that has an address. :)
>>
>> 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.
>>
>
> Sort them?
>
>
That might work.


> But also I was thinking more having the code itself check, rather than
> print and check with FileCheck?
>
>   std::set<void*> original = {&foo, &bar};
>   // build similar set from the xray_function_address calls
>   if original != xray
>     return 1;
>
> that sort of thing.
>
>

Right. I can do both, let me update the patch. :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170505/2d3207d8/attachment.html>


More information about the llvm-commits mailing list