[llvm-dev] Use of the C++ standard library in XRay compiler-rt

[Dean Michael Berris via llvm-dev]

> On 13 Mar 2017, at 15:39, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Sun, Mar 12, 2017, 4:10 PM Dean Michael Berris <dean.berris at gmail.com <mailto:dean.berris at gmail.com>> wrote:
> 
> > On 9 Mar 2017, at 09:32, David Blaikie via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> >
> > I agree that we should clean up the standard library usage even just for consistency.
> >
> 
> +1 -- now that I think about it, it should be fairly doable (also happy to help with reviews if that helps).
> 
> > Searching the xray directory for dependencies:
> > ...compiler-rt/lib/xray % grep '#include <[^>.]*>' -oh `find . -type f|grep -v 'tests'` | sort | uniq -c
> >       1 #include <algorithm>
> >      10 #include <atomic>
> >       1 #include <bitset>
> >       6 #include <cassert>
> >       1 #include <cerrno>
> >       1 #include <cstddef>
> >       7 #include <cstdint>
> >       2 #include <cstdio>
> >       1 #include <cstdlib>
> >       2 #include <cstring>
> >       1 #include <deque>
> >       2 #include <iterator>
> >       2 #include <limits>
> >       2 #include <memory>
> >       4 #include <mutex>
> >       1 #include <system_error>
> >       1 #include <thread>
> >       2 #include <tuple>
> >       1 #include <unordered_map>
> >       1 #include <unordered_set>
> >       3 #include <utility>
> > I think the biggest part is containers, and they are mostly in ./xray_buffer_queue.h and ./xray_fdr_logging.cc.
> 
> Yes, buffer_queue can definitely live without using system_error, unordered_map, and unordered_set. It might make it a bit more complex (we'd need to implement a correct and fairly efficient hash set) but if it means the deployment model is simpler then I'm happy with that trade-off. When we were implementing this, we made a decision to make it so that the "mismatch of standard library implementations" was treated as a lower priority issue -- something we don't think comes up as often, and is easily solvable by re-building the runtime with the standard library the end application/binary will be using anyway. Since XRay is only ever statically-linked (we don't have a dynamic version of it), I thought the rebuild option is slightly simpler than trying to implement the whole XRay runtime in a constrained version of C++ and libc-only functions.
> 
> Except that's not how llvm is distributed. In releases it will ship with the compiler and runtime libraries but can be used with any c++ standard library.
> 

That's fair. I was mostly thinking of the majority cases where I assumed that the user would be using clang/llvm/compiler-rt with the system-provided standard library anyway, and any exotic set-ups (not system libraries used) would require more work from the user anyway.

> This isn't a quality if implementation thing, this is more a correctness issue.

I'm less convinced of that, mostly because I've assumed that users using libraries and runtimes will be aware of the potential ABI incompatibilities that arise when using different standard libraries to build and then link. But if the constraints are different for runtimes in compiler-rt, then I'm alright with abiding by those. :)

> 
> One thing we rely on heavily on in the FDR mode implementation is C++'s `thread_local` keyword. I'm not sure what that entails runtime-wise (does it need pthreads? or something else?) but I'm sure a functional replacement would be alright too.
> 
> No doubt we can find some common API for that, I'd guess tsan probably has already had to figure out things like that.
> 

I suspect I'm more worried about costs rather than APIs. I've learned a while back that system-provided TLS functionality isn't as good as compiler-provided (and system/platform-specific) TLS implementations.

> 
> >
> >
> > (this came up for me due to what's probably a bug in the way compiler-rt is built - where the lib itself is built with the host compiler but the tests are built/linked with the just-bulit clang. My host compiler uses libstdc++ 6, whereas the just-built clang will use libstdc++ 4.8. So it fails to link due to this mismatch)
> 
> How hard would it be to fix that bug?
> 
> I've started a separate thread on that, but even if that's fixed it's still necessary to fix the dependency/distribution model here.
> 

Agreed.

Cheers

-- Dean

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170315/8e6d6286/attachment.html>