David Blaikie via llvm-dev
2017-Mar-15 01:19 UTC
[llvm-dev] Use of the C++ standard library in XRay compiler-rt
On Tue, Mar 14, 2017 at 5:34 PM Dean Michael Berris <dean.berris at gmail.com> wrote:> 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> > wrote: > > > > On 9 Mar 2017, at 09:32, David Blaikie via llvm-dev < > 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. >Not much work - Clang discovers existing GCC installations and picks the newest. So at the very least, if a user installs a newer GCC, they get a newer libstdc++ and Clang implicitly picks that up without any further work. Also Clang distribution might be available for several platforms which may have different system compilers/std libraries, the distribution is statically linked - so libstdc++ doesn't have to match or be present on the destination system, I think?> > 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 they won't be aware that they're mismatched - they take the LLVM distribution, compile their code on a system with a libstdc++ different from the one on the system where the LLVM distribution was built... and badness.> But if the constraints are different for runtimes in compiler-rt, then I'm > alright with abiding by those. :) >*nod* so far as I know, the runtimes have historically not depended on libstdc++ for these reasons (& the added bonus that the sanitizers need to sit below such libraries)> > > 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/e37c30e3/attachment.html>
Dean Michael Berris via llvm-dev
2017-Mar-15 01:23 UTC
[llvm-dev] Use of the C++ standard library in XRay compiler-rt
> On 15 Mar 2017, at 12:19, David Blaikie <dblaikie at gmail.com> wrote: > > > > On Tue, Mar 14, 2017 at 5:34 PM Dean Michael Berris <dean.berris at gmail.com <mailto:dean.berris at gmail.com>> wrote: >> On 13 Mar 2017, at 15:39, David Blaikie <dblaikie at gmail.com <mailto: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. > > Not much work - Clang discovers existing GCC installations and picks the newest. So at the very least, if a user installs a newer GCC, they get a newer libstdc++ and Clang implicitly picks that up without any further work. > > Also Clang distribution might be available for several platforms which may have different system compilers/std libraries, the distribution is statically linked - so libstdc++ doesn't have to match or be present on the destination system, I think? >Ah, for *binary* distributions of LLVM/compiler-rt, then yes this would be bad.> >> 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 they won't be aware that they're mismatched - they take the LLVM distribution, compile their code on a system with a libstdc++ different from the one on the system where the LLVM distribution was built... and badness. > > But if the constraints are different for runtimes in compiler-rt, then I'm alright with abiding by those. :) > > *nod* so far as I know, the runtimes have historically not depended on libstdc++ for these reasons (& the added bonus that the sanitizers need to sit below such libraries) >SGTM. I'll add this to the list of priorities we have for tracking. Probably worth filing a bug on bugs.llvm.org <http://bugs.llvm.org/> too, against XRay runtime assigned to me. :) Cheers -- Dean -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170315/0c088195/attachment-0001.html>
Dean Michael Berris via llvm-dev
2017-Mar-15 02:31 UTC
[llvm-dev] Use of the C++ standard library in XRay compiler-rt
> On 15 Mar 2017, at 12:23, Dean Michael Berris <dean.berris at gmail.com> wrote: > >> >> On 15 Mar 2017, at 12:19, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote: >> >> >> >> On Tue, Mar 14, 2017 at 5:34 PM Dean Michael Berris <dean.berris at gmail.com <mailto:dean.berris at gmail.com>> wrote: >>> On 13 Mar 2017, at 15:39, David Blaikie <dblaikie at gmail.com <mailto: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. >> >> Not much work - Clang discovers existing GCC installations and picks the newest. So at the very least, if a user installs a newer GCC, they get a newer libstdc++ and Clang implicitly picks that up without any further work. >> >> Also Clang distribution might be available for several platforms which may have different system compilers/std libraries, the distribution is statically linked - so libstdc++ doesn't have to match or be present on the destination system, I think? >> > > Ah, for *binary* distributions of LLVM/compiler-rt, then yes this would be bad. > >> >>> 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 they won't be aware that they're mismatched - they take the LLVM distribution, compile their code on a system with a libstdc++ different from the one on the system where the LLVM distribution was built... and badness. >> >> But if the constraints are different for runtimes in compiler-rt, then I'm alright with abiding by those. :) >> >> *nod* so far as I know, the runtimes have historically not depended on libstdc++ for these reasons (& the added bonus that the sanitizers need to sit below such libraries) >> > > SGTM. I'll add this to the list of priorities we have for tracking. > > Probably worth filing a bug on bugs.llvm.org <http://bugs.llvm.org/> too, against XRay runtime assigned to me. :) >This is now http://bugs.llvm.org/show_bug.cgi?id=32274 <http://bugs.llvm.org/show_bug.cgi?id=32274> Cheers -- Dean -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170315/3f617f56/attachment.html>