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

[libc++] undefined behaviour in std::set/std::map/std::unordered_set/std::unordered_map #102547

Open
hanickadot opened this issue Aug 8, 2024 · 4 comments
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@hanickadot
Copy link
Contributor

hanickadot commented Aug 8, 2024

I was implementing P3372 (constexpr containers and adaptors) in libc++ and found UB which fails evaluation in ExprConstant.cpp.

It's in these two files:
libcxx/include/__tree
libcxx/include/__hash_table

(link is to the locations of problems)

These are similar issue where a node / bucket is allocated, but NOT constructed, and then a subobject is constructed.

@github-actions github-actions bot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Aug 8, 2024
@hanickadot
Copy link
Contributor Author

@hanickadot
Copy link
Contributor Author

Screenshot 2024-08-08 at 22 43 43 Screenshot 2024-08-08 at 22 48 03

@hanickadot
Copy link
Contributor Author

(lines can be a little different in screenshots as it's my constexpr branch)

@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Aug 9, 2024

Possibly off-topic: I attempted to add constexpr to MSVC STL's (multi)map, and then found that

  • the manner of construction of the node needs to be fixed, while the fix is simple, and
  • node-handle needs to be reworked to use a proper union, but
  • the const-dropping key() function of node-handle is not yet fixable.

It's not yet possible to avoid core UB in modification of the key which is a const subobject. CWG2514 is related.


Maybe related: [class.cdtor]/1 and CWG1517.

Perhaps we should to start the lifetime of the node first and wrap the element into a union at this moment.

I don't understand why can't we refer to these subobjects (evaluate the member access expression?) before construction even if virtual base class is not involved.


BTW, you can use permanent links to show the code in the Github UI.

typename __tree<_Tp, _Compare, _Allocator>::__node_holder
__tree<_Tp, _Compare, _Allocator>::__construct_node(_Args&&... __args) {
static_assert(!__is_tree_value_type<_Args...>::value, "Cannot construct from __value_type");
__node_allocator& __na = __node_alloc();
__node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
__node_traits::construct(__na, _NodeTypes::__get_ptr(__h->__value_), std::forward<_Args>(__args)...);
__h.get_deleter().__value_constructed = true;
return __h;

__hash_table<_Tp, _Hash, _Equal, _Alloc>::__construct_node_hash(size_t __hash, _First&& __f, _Rest&&... __rest) {
static_assert(!__is_hash_value_type<_First, _Rest...>::value, "Construct cannot be called with a hash value type");
__node_allocator& __na = __node_alloc();
__node_holder __h(__node_traits::allocate(__na, 1), _Dp(__na));
std::__construct_at(std::addressof(*__h), /* next = */ nullptr, /* hash = */ __hash);
__node_traits::construct(
__na, _NodeTypes::__get_ptr(__h->__get_value()), std::forward<_First>(__f), std::forward<_Rest>(__rest)...);
__h.get_deleter().__value_constructed = true;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

2 participants