Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XRay should not use C++ standard libraries internally #31622

Closed
llvmbot opened this issue Mar 15, 2017 · 6 comments
Closed

XRay should not use C++ standard libraries internally #31622

llvmbot opened this issue Mar 15, 2017 · 6 comments
Labels
bugzilla Issues migrated from bugzilla xray

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 15, 2017

Bugzilla Link 32274
Resolution FIXED
Resolved on Jun 07, 2018 21:41
Version unspecified
OS All
Reporter LLVM Bugzilla Contributor
CC @dwblaikie

Extended Description

As reported by David Blaikie in llvm-dev@ (see http://lists.llvm.org/pipermail/llvm-dev/2017-March/110835.html)

Action items:

  • Remove dependency on standard C++ library symbols for XRay's runtime
  • Use functional equivalents to std::atomic<...>, containers, and C++11+'s thread_local keyword
@llvmbot
Copy link
Collaborator Author

llvmbot commented Mar 28, 2017

Fixed as of https://reviews.llvm.org/rL298835.

Not only have we removed the runtime dependency on the C++ standard library runtime, but also removed some uses of std::atomic<...> that might prove problematic in some cases.

@dwblaikie
Copy link
Collaborator

Fixed as of https://reviews.llvm.org/rL298835.

Not only have we removed the runtime dependency on the C++ standard library
runtime,

That's not guaranteed though, is it? If you use any C++ library features, they might rely on runtime support from some library that needs to be linked in. The one(s) that've been used so far don't appear to, but it doesn't make this solidly portable (granted, there's no guarantee that some compiler doesn't produce binaries that need a C++ runtime support library even if they use no library features at all - but that's perhaps a bit less likely).

I'm not too fussed personally - but suspect that it's probably still right/proper to remove all C++ library dependencies unless there's a way to do what Chandler had mentioned a while back - statically linking in a privately mangled version of the standard library.

but also removed some uses of std::atomic<...> that might prove
problematic in some cases.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Oct 23, 2017

Re-opening to reflect the most recent state: we should not be using anything from the C++ standard library. This is based on the conversation we've had in llvm-dev@:

http://lists.llvm.org/pipermail/llvm-dev/2017-October/118126.html

In particular:

"""
The sanitizers don't use any of the C++ std library.
More than that, they don't include any system headers in most of the
sources (exception is some OS-dependent .cc files).

This rule is somewhat documented, e.g.
tsan/rtl/tsan_rtl.h:

// - No system headers included in header files ().
// - Platform specific headres included only into platform-specific files
(
).
"""

http://lists.llvm.org/pipermail/llvm-dev/2017-October/118132.html

@llvmbot
Copy link
Collaborator Author

llvmbot commented Mar 16, 2018

Recent work on this has removed all dependencies to standard library dependencies.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Apr 13, 2018

Unfortunately, we're still relying on some C++ specific functionality, some of which are provided by libcxxabi, but I'd like to explore the possibility of removing those first/instead.

This came up in a discussion with why we're failing to link C programs with XRay instrumentation. Reopening this bug so that we can address this in either the runtime, or even in Clang.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Jun 8, 2018

This is now fixed by https://reviews.llvm.org/rL334262 (r334262).

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla xray
Projects
None yet
Development

No branches or pull requests

2 participants