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

std::any uses mismatched allocation and deallocation #44444

Closed
tkoeppe opened this issue Mar 4, 2020 · 6 comments
Closed

std::any uses mismatched allocation and deallocation #44444

tkoeppe opened this issue Mar 4, 2020 · 6 comments
Assignees
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@tkoeppe
Copy link
Contributor

tkoeppe commented Mar 4, 2020

Bugzilla Link 45099
Resolution FIXED
Resolved on Sep 15, 2020 09:42
Version unspecified
OS All
CC @ldionne,@mclow
Fixed by commit(s) 39c8795

Extended Description

The constructors of std::any that create an object of type T use std::allocator to allocate memory and "::new ((T*)p)" placement new to construct the object. (That is, memory allocation subject only to specializations of std::allocator, and construction only to overloads of global operator new for user pointer types.)

By contrast, destruction uses "delete (T*)p". This means that overloaded operators T::operator delete will be used.

(In the above, the cast notation "(T*)" doesn't appear literally in the code, but is just my way of indicating the argument type. Specifically, note that there is no cast to void* in the placement new.)

This mismatch results in undefined behaviour whenever a suitable amount of user customization is present for T. Examples can be constructed to exercise all aspects of this (specializing std::allocator, adding a ::operator new(std::size_t, T*) overload, or adding overloaded operators T::operator delete).

One example:

================

#include
#include
#include

struct U {
int big[128];
static void* operator new(std::size_t n) { return std::malloc(n); }
static void operator delete(void* p) { std::free(p); }
};

int main() {
std::any a(std::in_place_type);
return std::any_cast<U*>(a) != nullptr; // avoid optimizing everything away
}

================

The generated code clearly shows one call to "operator new" and one to "free".

One solution would be to make the construction use an unqualified new expression "new _Tp(...)"; this means that standard library code will call user-defined functions for internal bookkeeping. An alternative solution would be to also use std::allocator to deallocate the allocation.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Mar 4, 2020

assigned to @ldionne

@mclow
Copy link
Contributor

mclow commented Jun 3, 2020

There's more to this than the reported issue.

The any implementation uses placement new to construct the objects.
This should really be allocator_traits<allocator>::construct(...).

And T::~T should be allocator_traits<allocator>::destroy(...).

But yes, the issue as reported is correct.

@mclow
Copy link
Contributor

mclow commented Jun 4, 2020

reading up on this some more, I note that (https://wg21.link/allocator.members):

std::allocator::allocate is required to get its storage from ::operator new, and
std::allocator::delete is required to use ::operator delete

but there's certainly some wiggle room in those specifications. (Deliberately, I'm sure)

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jun 4, 2020

reading up on this some more, I note that
(https://wg21.link/allocator.members):

std::allocator::allocate is required to get its storage from ::operator new, and
std::allocator::delete is required to use ::operator delete

but there's certainly some wiggle room in those specifications.
(Deliberately, I'm sure)

Yes, this is true, but the way I read this is merely as a guarantee that, say, replacements of "::operator new" are observed by (any specialization of) std::allocator. It doesn't preclude specializations from doing additional, custom work (so the allocator should still be called in matching pairs, and not freely interchanged with direct calls to "::operator new/delete").

@ldionne
Copy link
Member

ldionne commented Sep 15, 2020

Took over potential solution in https://reviews.llvm.org/D81133.

@ldionne
Copy link
Member

ldionne commented Sep 15, 2020

Fixed by 39c8795

@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 libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

3 participants