[PATCH] D60593: [GwpAsan] Introduce GWP-ASan.

[Vlad Tsyrklevich via Phabricator via llvm-commits]

vlad.tsyrklevich added a comment.

In D60593#1470991 <https://reviews.llvm.org/D60593#1470991>, @hctim wrote:

> | Option                                  | Median Time (with 20 runs) | Runtime increase relative to CT-Disabled |
> | Compile-time disabled                   | 3.5715s                    | 0.0000%                                  |
> | Runtime disabled (init check in ret)    | 3.6490s                    | 2.1239%                                  |
> | Runtime disabled (init check in branch) | 3.6350s                    | 1.7469%                                  |
> | Runtime enabled (init check in ret)     | 3.6435s                    | 1.9761%                                  |
> | Runtime enabled (init check in branch)  | 3.6500s                    | 2.1507%                                  |
> |


I'm struggling to understand why adding an additional branch on the fast path (e.g. init check in ret) would be faster. With the init check in the NextSampleCounter==0 branch we should be performing one fewer load/compare on the fast path in the enabled case. Could you show the code you used for the init check in ret/branch cases? I'm surprised by the results.



================
Comment at: compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp:182
+
+  bool ShouldRightAlign = (*Size % 2 == 1);
+
----------------
In Chrome we left- or right-align based on whether the (randomly chosen) slot is even/odd, that means an allocation will test for both under- and over-flows across different runs. However in this case we always left-/right-align based on the size which means a given type will only ever be tested one way (and I'd expect we have a lot more even allocations than odd allocations.)


================
Comment at: compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp:205
+std::size_t GuardedPoolAllocator::getNearestSlot(uintptr_t Ptr) const {
+  if (Ptr <= GuardedPagePool + PageSize / 2)
+    return 0;
----------------
You can simplify this to just + PageSize if you wanted.


================
Comment at: compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp:207
+    return 0;
+  else if (Ptr > GuardedPagePoolEnd + PageSize / 2)
+    return NumGuardedSlots - 1;
----------------
Ditto the simplification and this should be - PageSize


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60593