Page MenuHomePhabricator

btolsch (Brandon Tolsch)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 26 2020, 12:45 AM (35 w, 12 h)

Recent Activity

Aug 5 2020

btolsch added a comment to D82627: Fix CFI issues in <future>.

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.

Aug 5 2020, 3:41 PM · Restricted Project
btolsch updated the diff for D82627: Fix CFI issues in <future>.
Aug 5 2020, 3:22 PM · Restricted Project
btolsch added a comment to D82627: Fix CFI issues in <future>.

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

Aug 5 2020, 12:02 AM · Restricted Project

Jul 29 2020

btolsch updated the diff for D82627: Fix CFI issues in <future>.

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

Jul 29 2020, 10:19 AM · Restricted Project

Jul 16 2020

btolsch added a comment to D82627: Fix CFI issues in <future>.

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)?

Jul 16 2020, 10:49 AM · Restricted Project

Jul 15 2020

btolsch added a comment to D82627: Fix CFI issues in <future>.

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?

Jul 15 2020, 5:11 PM · Restricted Project
btolsch updated the diff for D82627: Fix CFI issues in <future>.

Replace temporary variable use with in-place cast.

Jul 15 2020, 5:08 PM · Restricted Project

Jul 8 2020

btolsch added a comment to D82627: Fix CFI issues in <future>.

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

Jul 8 2020, 9:01 AM · Restricted Project

Jun 26 2020

btolsch added a comment to D82627: Fix CFI issues in <future>.

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

Jun 26 2020, 9:17 AM · Restricted Project
btolsch added a comment to D82627: Fix CFI issues in <future>.

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.

Jun 26 2020, 1:36 AM · Restricted Project
btolsch updated the summary of D82627: Fix CFI issues in <future>.
Jun 26 2020, 1:36 AM · Restricted Project
btolsch created D82627: Fix CFI issues in <future>.
Jun 26 2020, 1:36 AM · Restricted Project