EricWF added a comment.
> This also could be fixed in a different way by replacing C-style
> casts with reinterpret_cast<>, which, from my reading of the
> standard, is allowed in this context.
I agree that using `void*` to represent raw memory is the better approach than
`reinterpret_cast<>()`.
However I'm concerned that changing the signature (and mangling) of `virtual
void __clone(...)` could cause ABI problems.
I *think* this should be "safe" because the VTable's mangled name doesn't
change. but if I'm wrong we must use `reinterpret_cast<>` for calls to
`__clone(...)`.
The parts of the patch that don't affect `__clone(...)` LGTM. You can commit
them separably if you want.
> That would not help with CFI
> though, which still flags such casts as invalid (yes, it is stricter that
> the standard).
I'm sure there are alternative ways to make CFI shut up. Perhaps we could do
the `Buffer* -> Base*` conversion inside a blacklisted function (akin to
std::launder)?
It would also be nice to have "`__attribute__((__no_sanitize__("cfi")))`.
================
Comment at: include/functional:1443
@@ -1442,3 +1442,3 @@
virtual __base* __clone() const = 0;
- virtual void __clone(__base*) const = 0;
+ virtual void __clone(void*) const = 0;
virtual void destroy() _NOEXCEPT = 0;
----------------
Doesn't this break the ABI because it changes the mangling of a virtual
function? I understand the vtable should remain layout compatible but I suspect
the mangling change is an issue.
================
Comment at: include/functional:1739
@@ -1738,2 +1738,3 @@
{
+ ::new (&__buf_) _FF(_VSTD::move(__f));
__f_ = (__base*)&__buf_;
----------------
`__f_ = ::new((void*)&__buf_) _FF(_VSTD::move(__f));`
Repository:
rL LLVM
http://reviews.llvm.org/D16738
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
