<br><br><div class="gmail_quote"><div dir="ltr">On Sun, Mar 12, 2017, 4:10 PM Dean Michael Berris <<a href="mailto:dean.berris@gmail.com">dean.berris@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br class="gmail_msg">
> On 9 Mar 2017, at 09:32, David Blaikie via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" class="gmail_msg" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
> I agree that we should clean up the standard library usage even just for consistency.<br class="gmail_msg">
><br class="gmail_msg">
<br class="gmail_msg">
+1 -- now that I think about it, it should be fairly doable (also happy to help with reviews if that helps).<br class="gmail_msg">
<br class="gmail_msg">
> Searching the xray directory for dependencies:<br class="gmail_msg">
> ...compiler-rt/lib/xray % grep '#include <[^>.]*>' -oh `find . -type f|grep -v 'tests'` | sort | uniq -c<br class="gmail_msg">
> 1 #include <algorithm><br class="gmail_msg">
> 10 #include <atomic><br class="gmail_msg">
> 1 #include <bitset><br class="gmail_msg">
> 6 #include <cassert><br class="gmail_msg">
> 1 #include <cerrno><br class="gmail_msg">
> 1 #include <cstddef><br class="gmail_msg">
> 7 #include <cstdint><br class="gmail_msg">
> 2 #include <cstdio><br class="gmail_msg">
> 1 #include <cstdlib><br class="gmail_msg">
> 2 #include <cstring><br class="gmail_msg">
> 1 #include <deque><br class="gmail_msg">
> 2 #include <iterator><br class="gmail_msg">
> 2 #include <limits><br class="gmail_msg">
> 2 #include <memory><br class="gmail_msg">
> 4 #include <mutex><br class="gmail_msg">
> 1 #include <system_error><br class="gmail_msg">
> 1 #include <thread><br class="gmail_msg">
> 2 #include <tuple><br class="gmail_msg">
> 1 #include <unordered_map><br class="gmail_msg">
> 1 #include <unordered_set><br class="gmail_msg">
> 3 #include <utility><br class="gmail_msg">
> I think the biggest part is containers, and they are mostly in ./xray_buffer_queue.h and ./xray_fdr_logging.cc.<br class="gmail_msg">
<br class="gmail_msg">
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.<br class="gmail_msg"></blockquote></div><div><br></div><div>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.</div><div><br></div><div>This isn't a quality if implementation thing, this is more a correctness issue.</div><div><br></div><div class="gmail_quote"><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">
> dependencies without buffer queue and fdr logging:<br class="gmail_msg">
> ...compiler-rt/lib/xray % grep '#include <[^>.]*>' -oh `find . -type f|egrep -v 'tests|buffer|fdr'` | sort | uniq -c<br class="gmail_msg">
> 9 #include <atomic><br class="gmail_msg">
> 4 #include <cassert><br class="gmail_msg">
> 1 #include <cerrno><br class="gmail_msg">
> 1 #include <cstddef><br class="gmail_msg">
> 6 #include <cstdint><br class="gmail_msg">
> 2 #include <cstdio><br class="gmail_msg">
> 1 #include <cstring><br class="gmail_msg">
> 2 #include <iterator><br class="gmail_msg">
> 2 #include <limits><br class="gmail_msg">
> 1 #include <memory><br class="gmail_msg">
> 3 #include <mutex><br class="gmail_msg">
> 1 #include <thread><br class="gmail_msg">
> 2 #include <tuple><br class="gmail_msg">
> 2 #include <utility><br class="gmail_msg">
> I believe that this is relatively easy to cleanup. I can do that.<br class="gmail_msg">
><br class="gmail_msg">
> I don't know how hard it is to rewrite buffer queue and fdr logging using compiler_rt infrastructure.<br class="gmail_msg">
><br class="gmail_msg">
<br class="gmail_msg">
The crucial things we need in FDR mode logging are:<br class="gmail_msg">
<br class="gmail_msg">
- Aligned storage (I suspect this could be done without std::aligned_storage<...>)<br class="gmail_msg">
- memcpy (we use std::memcpy there, but probably didn't need to)<br class="gmail_msg">
- thread-local storage (using C++'s `thread_local` keyword)<br class="gmail_msg">
<br class="gmail_msg">
The buffer queue can be re-written to not use std::system_error in the APIs (use XRay-specific enums instead), internally not use std::atomic<...> types, and have to implement the FIFO queue, a lookup table for ownership, etc. -- while none of these seem hard, they didn't seem worth it at the time of implementation.<br class="gmail_msg">
<br class="gmail_msg">
> I think buffer_queue's probably sufficiently well bounded that it shouldn't be drastically hard to replace it with a custom implementation. Haven't looked at fdr_logging.<br class="gmail_msg">
><br class="gmail_msg">
> Maps/dictionary-like things might be a bit of a pain in particular. Not sure if the sanitizers already have some reusable idioms/libraries for that.<br class="gmail_msg">
><br class="gmail_msg">
> I'm also not really clear on where the boundary is - which headers or language features ('new'?) can be used, and which can't. Can't say I've ever tried to make code library agnostic.<br class="gmail_msg">
><br class="gmail_msg">
<br class="gmail_msg">
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.<br class="gmail_msg"></blockquote></div><div><br></div><div>No doubt we can find some common API for that, I'd guess tsan probably has already had to figure out things like that.</div><div><br></div><div class="gmail_quote"><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">
> (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)<br class="gmail_msg">
<br class="gmail_msg">
How hard would it be to fix that bug?<br class="gmail_msg"></blockquote></div><div><br></div><div>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.</div><div><br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
I would think this error would come up in the wild too, but could be remedied by just rebuilding XRay with the right C++ standard library.<br class="gmail_msg">
<br class="gmail_msg">
-- Dean<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div>