[PATCH] D60593: [GwpAsan] Introduce GWP-ASan.
Vlad Tsyrklevich via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 18 10:55:35 PDT 2019
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
More information about the llvm-commits
mailing list