llvm dev - Nov 2017 - question about xray tls data initialization

On Wed, Nov 22, 2017 at 10:37 AM, Dean Michael Berris
<dean.berris at gmail.com> wrote:
>
> On 22 Nov 2017, at 02:32, comic fans <comicfans44 at gmail.com>
wrote:
>
> with some dirty hack , I've made xray runtime  'built' on
windows ,
>
>
> \o/
with more test, I've found that trampoline didn't got built for windows
:/
currently cmake didn't generate build rule for asm so its silently
ignored(with msvc ide, but not ninja).
we must have enable_language(ASM_MASM) to use masm, and trampoline
also need ports.
>
> If you're alright with it, maybe you can send some patches to review,
> preferably through the LLVM Phabricator instance? You can have me or Reid
> (who knows more about COFF and the Windows stuff) as reviewers.
>
> in AsmPrinter, copy/paster xray for coff target
>
> InstMap = OutContext.getCOFFSection("xray_instr_map", 0,
> SectionKind::getReadOnlyWithRel());
> FnSledIndex = OutContext.getCOFFSection("xray_fn_idx",
> 0,SectionKind::getReadOnlyWithRel());
>
> in XRayArgs , allow windows platform to use xray args. with this,
> generated code seems have sled and xray parts.
>
>
> Nice, I suspect we can make this change with tests as well, which we can
> build on incrementally.
where can I find some examples to test this xray part in llvm ?

> in xray runtime,
> bool atomic_compare_exchange_strong(volatile atomic_sint32_t *a,
>                                           s32 *cmp,
>                                           s32 xchg,
>                                           memory_order mo)
> is missed for MSVC , I take atomic_uint32_t implementation
>
>
> This is in compiler-rt/lib/sanitizer_common/... right?
yes, sanitizer_atomic_msvc.h didn't provide this override. according
to msdn of interlockedcompareexchange,  implementation for
atomic_uint32_t should also works for atomic_sint32_t. this is a
copy/paste but I think its short enough. any better suggestion ?
>
> FunctionRecord pack , __attribute__((packed)) =>  #pragma
> pack(push,1),  msvc also requires bitfields to be same type to pack
> them together( all types => uint32_t)
>
>
> Are you able to test this on other platforms?
I've tested this on linux64 (with clang) and it pass check-xray , but
I don't have mac to test. if changing all attribute to pragma is
desirable , I can submit a patch for that .

> On 23 Nov 2017, at 23:34, comic fans <comicfans44 at gmail.com>
wrote:
> 
> On Wed, Nov 22, 2017 at 10:37 AM, Dean Michael Berris
> <dean.berris at gmail.com> wrote:
>> 
>> On 22 Nov 2017, at 02:32, comic fans <comicfans44 at gmail.com>
wrote:
>> 
>> with some dirty hack , I've made xray runtime  'built' on
windows ,
>> 
>> 
>> \o/
> 
> with more test, I've found that trampoline didn't got built for
windows :/
> currently cmake didn't generate build rule for asm so its silently
> ignored(with msvc ide, but not ninja).
> we must have enable_language(ASM_MASM) to use masm, and trampoline
> also need ports.
> 
Right -- this is similar to issues we've run into trying to make XRay work /
get built for Darwin too.
>> 
>> If you're alright with it, maybe you can send some patches to
review,
>> preferably through the LLVM Phabricator instance? You can have me or
Reid
>> (who knows more about COFF and the Windows stuff) as reviewers.
>> 
>> in AsmPrinter, copy/paster xray for coff target
>> 
>> InstMap = OutContext.getCOFFSection("xray_instr_map", 0,
>> SectionKind::getReadOnlyWithRel());
>> FnSledIndex = OutContext.getCOFFSection("xray_fn_idx",
>> 0,SectionKind::getReadOnlyWithRel());
>> 
>> in XRayArgs , allow windows platform to use xray args. with this,
>> generated code seems have sled and xray parts.
>> 
>> 
>> Nice, I suspect we can make this change with tests as well, which we
can
>> build on incrementally.
> 
> where can I find some examples to test this xray part in llvm ?
> 
Those are in the llvm/test/CodeGen/X86/... -- in particular, searching for
'xray_' in the files there will be the best way of finding examples of
what we're looking for to verify.
> 
>> in xray runtime,
>> bool atomic_compare_exchange_strong(volatile atomic_sint32_t *a,
>>                                          s32 *cmp,
>>                                          s32 xchg,
>>                                          memory_order mo)
>> is missed for MSVC , I take atomic_uint32_t implementation
>> 
>> 
>> This is in compiler-rt/lib/sanitizer_common/... right?
> 
> yes, sanitizer_atomic_msvc.h didn't provide this override. according
> to msdn of interlockedcompareexchange,  implementation for
> atomic_uint32_t should also works for atomic_sint32_t. this is a
> copy/paste but I think its short enough. any better suggestion ?
> 
I'm sure adding an implementation for atomic_sint32_t will be nice to have
across platforms. :)
>> 
>> FunctionRecord pack , __attribute__((packed)) =>  #pragma
>> pack(push,1),  msvc also requires bitfields to be same type to pack
>> them together( all types => uint32_t)
>> 
>> 
>> Are you able to test this on other platforms?
> 
> I've tested this on linux64 (with clang) and it pass check-xray , but
> I don't have mac to test. if changing all attribute to pragma is
> desirable , I can submit a patch for that .

We're still working on getting XRay to build right and work on macOS so that
shouldn't be a barrier. :)

A patch would be good there too.

Thanks again!

-- Dean

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20171124/4ae73c01/attachment.html>

I wonder if I can build xray with clang/llvm-as on windows, seems that
is a little easier and requires less changes. but I've not tried yet.

On Fri, Nov 24, 2017 at 8:22 AM, Dean Michael Berris
<dean.berris at gmail.com> wrote:
>
> On 23 Nov 2017, at 23:34, comic fans <comicfans44 at gmail.com>
wrote:
>
> On Wed, Nov 22, 2017 at 10:37 AM, Dean Michael Berris
> <dean.berris at gmail.com> wrote:
>
>
> On 22 Nov 2017, at 02:32, comic fans <comicfans44 at gmail.com>
wrote:
>
> with some dirty hack , I've made xray runtime  'built' on
windows ,
>
>
> \o/
>
>
> with more test, I've found that trampoline didn't got built for
windows :/
> currently cmake didn't generate build rule for asm so its silently
> ignored(with msvc ide, but not ninja).
> we must have enable_language(ASM_MASM) to use masm, and trampoline
> also need ports.
>
>
> Right -- this is similar to issues we've run into trying to make XRay
work /
> get built for Darwin too.
>
>
> If you're alright with it, maybe you can send some patches to review,
> preferably through the LLVM Phabricator instance? You can have me or Reid
> (who knows more about COFF and the Windows stuff) as reviewers.
>
> in AsmPrinter, copy/paster xray for coff target
>
> InstMap = OutContext.getCOFFSection("xray_instr_map", 0,
> SectionKind::getReadOnlyWithRel());
> FnSledIndex = OutContext.getCOFFSection("xray_fn_idx",
> 0,SectionKind::getReadOnlyWithRel());
>
> in XRayArgs , allow windows platform to use xray args. with this,
> generated code seems have sled and xray parts.
>
>
> Nice, I suspect we can make this change with tests as well, which we can
> build on incrementally.
>
>
> where can I find some examples to test this xray part in llvm ?
>
>
> Those are in the llvm/test/CodeGen/X86/... -- in particular, searching for
> 'xray_' in the files there will be the best way of finding examples
of what
> we're looking for to verify.
>
>
> in xray runtime,
> bool atomic_compare_exchange_strong(volatile atomic_sint32_t *a,
>                                          s32 *cmp,
>                                          s32 xchg,
>                                          memory_order mo)
> is missed for MSVC , I take atomic_uint32_t implementation
>
>
> This is in compiler-rt/lib/sanitizer_common/... right?
>
>
> yes, sanitizer_atomic_msvc.h didn't provide this override. according
> to msdn of interlockedcompareexchange,  implementation for
> atomic_uint32_t should also works for atomic_sint32_t. this is a
> copy/paste but I think its short enough. any better suggestion ?
>
>
> I'm sure adding an implementation for atomic_sint32_t will be nice to
have
> across platforms. :)
>
>
> FunctionRecord pack , __attribute__((packed)) =>  #pragma
> pack(push,1),  msvc also requires bitfields to be same type to pack
> them together( all types => uint32_t)
>
>
> Are you able to test this on other platforms?
>
>
> I've tested this on linux64 (with clang) and it pass check-xray , but
> I don't have mac to test. if changing all attribute to pragma is
> desirable , I can submit a patch for that .
>
>
> We're still working on getting XRay to build right and work on macOS so
that
> shouldn't be a barrier. :)
>
> A patch would be good there too.
>
> Thanks again!
>
> -- Dean
>