[libcxx-commits] [PATCH] D95827: Avoid cast<T*> before T is constructed to pacify CFI checks

[Reid Kleckner via Phabricator via libcxx-commits]

rnk planned changes to this revision.
rnk added a comment.

In D95827#2535177 <https://reviews.llvm.org/D95827#2535177>, @ldionne wrote:

> Ah, I just commented on the bug report, but your patch confirms what I thought. Thanks for the patch. So this is basically a CFI false positive, do we agree?

I think the code is fine, but I'm not 100% sure it isn't technically UB. I mean, I'd write it.

> Then, we can use it as `__get_elem<void*>()` for the placement-new use case, and just `__get_elem()` for the normal case. What do you think?

That works, I tried it, but I realized the C++17+ cases are still broken.



================
Comment at: libcxx/include/memory:2594
         _TpAlloc __tmp(*__get_alloc());
         allocator_traits<_TpAlloc>::construct(__tmp, __get_elem(), _VSTD::forward<_Args>(__args)...);
 #else
----------------
I realize that the same issue exists here. However, the prorotype of C++17 `std::construct_at` expects a `_Tp*` argument, not `void*`, so this change is really fighting uphill against future standards versions, and that makes me reconsider the whole effort. I guess I'll hold off on this change for now and go forward with the exclusion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95827/new/

https://reviews.llvm.org/D95827