[libcxx-commits] [PATCH] D82627: Fix CFI issues in <future>

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: dexonsmith.

This mostly LGTM. Definitely not the kind of issue one would find without a tool.

I'm wondering though, is that *actually* UB? I don't think so, because we never access the pointer to `__packaged_task_base` as a `__packaged_task_base` before it is initialized. If that's correct, then we're basically removing useful documentation (through type annotations) to satisfy a tool. Instead, it would be better to add an annotation to tell the tool that the code is OK.



================
Comment at: libcxx/include/future:1966
         __f_ = (__base*)&__buf_;
-        __t->__move_to((__base*)&__f.__buf_);
+        __t->__move_to(&__f.__buf_);
         __t->destroy();
----------------
Can you please use `static_cast<__base*>(&__tempbuf)->__move_to(...)` here instead (and the same for the `destroy()` call)? I find it very confusing to have both `__t` and `__f_` interleaved, especially since the two letters look almost the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82627