-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add some basic guidelines around using C++ standard headers now that our build allows us to #101088
Conversation
…our build allows us to.
Tagging subscribers to this area: @dotnet/area-meta |
|
||
### <a name="2.11.2"></a> 2.11.2 Do not use C++ standard exceptions | ||
|
||
The exception handling mechanisms in CoreCLR only handle `Exception`-derived types and `PAL_SEHException`. As a result, standard C++ exceptions, derived from `std::exception`, will cause runtime instability and should never be used. This includes using standard collection types with the default `std::allocator<T>` allocator. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Linux, we build against ROOTFS generated from Linux version that is out of support. We have convinced ourselves that it is ok given that we are using just prototypes and no actual code from ROOTFS.
Do C++ standard headers come from the ROOTFS? C++ headers do contain actual code (e.g. collection types). If we start including C++ headers with actual code from ROOTFS, our reasoning for why the ROOTFS is ok falls apart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do use some code from the ROOTFS today. We get some object files linked in from the relevant libc in the ROOTFS. This does give us more exposure to the C++ standard library version from the ROOTFS though, which is worth considering with respect to security issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been investigating this further based on our offline conversations, and I've noticed that tools like createdump are already using C++ headers with the template types. I'd assume that we'd have the same concern there as well as in the hosting layer (which uses C++ standard collections and other template types as well).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dotnet/arcade#14728 is the first step towards ensuring we're using maintained/supported C++ standard headers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as long as @jkotas's concerns about building from a ROOTFS are addressed.
|
||
### <a name="2.11.2"></a> 2.11.2 Do not use C++ Standard-defined exceptions | ||
|
||
The exception handling mechanisms in CoreCLR only handle `Exception`-derived types and `PAL_SEHException`. As a result, standard C++ exceptions, derived from `std::exception`, will cause runtime instability and should never be used. There is one standard C++ exception type the CoreCLR infrastructure supports, `std::bad_alloc`. Since CoreCLR supports `std::bad_alloc`, the standard container allocators, `std::allocator<T>` and the standard C++ containers, can be used as long as only the non-throwing members are used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as long as only the non-throwing members are used
What is an example of throwing member that cannot be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::vector<T>::at()
is the first one that comes to mind. I'll add it as an example (and mention to look at cppreference.com
or the C++ standard to check the contracts for members).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is bad_alloc
support something new or existed before? Can STL containers be used more to replace custom containers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add it as an example (and mention to look at cppreference.com or the C++ standard to check the contracts for members).
This is not going to be sufficient. There are many more concerns with these collections and their use in the coreclr codebase. I appreciate we are starting to document this, but the bad_alloc
case is the tip of the iceberg here. We have no support in our system's contracts, these have caught innumerable bugs at dev time, so we need a conversation on that. Functions that are marked noexcept
and encounter an exception will now result in a C++ run-time's "fail fast" occurring - this would be very new behavior that must be carefully considered across all standard implementations since currently our contracts would find it or it could be caught by some EX_TRY
.
My point here is this guidance will need a lot more consideration until we use any C++ collection. Introduction of this kind of use in the .NET 9 release should be avoided or only in the most trivial and appropriate cases. We've a lot to learn in this area and accomodate in the current code base. My recommendation is we approach this cautiously.
Can STL containers be used more to replace custom containers?
This is going to be a long term goal and not something we are likely to accept from community members until the core team develops more guidelines on use. Really appreciate the enthusiasm and look forward to help in this area, but for .NET 9 and likely .NET 10 our use of these collections is going to be narrow and very limited.
… what throws and what doesn't.
Circling back to this: Trying to use libc++ with libstdc++ as the ABI doesn't work for Ubuntu 16.04. As a result, I've updated the guidance to recommend against template types for now. We may revisit these rules in .NET 10, but these rules should be a good starting point. |
Just putting some basic guidelines in our docs as a starting point.