I believe everything is addressed but I don't have permission to commit. Can push this @eugenis? Otherwise I will wait for @ldionne to get back.

Friendly ping. Is there any feedback on the annotation approach?

I changed the patch to be more annotation-oriented while trying to avoid just tacking _LIBCPP_NO_CFI onto every function. PTAL, thanks!

In D82627#2155962, @ldionne wrote:

In D82627#2154623, @btolsch wrote:

What are the annotations that could be used instead of this change?

I don't know -- I learned about CFI just now. But my point stands that if this is a tool flagging something that we know is safe as unsafe, then it's just noise and modifying the code to workaround that noise is not the right option.

Reading at https://clang.llvm.org/docs/ControlFlowIntegrity.html, it looks like it's possible to have blacklists for CFI: https://clang.llvm.org/docs/ControlFlowIntegrity.html#cfi-blacklist.

Can you pull in someone from the sanitizers in this review so we can talk about the proper way of blacklisting this function for all users (so that users don't have to do it by themselves each time)?

I don't think any of this is actually UB, for the same reason you stated. What are the annotations that could be used instead of this change?

Replace temporary variable use with in-place cast.

Friendly ping. Is there something else I should do or someone else I should add to get this reviewed?

In D82627#2117120, @ldionne wrote:

Can you provide a link to the CI failure you're referring to?

I attached a small repro case, but I don't know if there's a way to make this into a unit test. LMK if that's something you want and where I would add it. Also, I wasn't able to get any of the clang-format tools to work, and just running clang-format with the included .clang-format of the repo completely changed the whole file. I think I kept the diff in the existing style though. Lastly, I ran make check-cxx and that passed.