[PATCH] D150460: [gcov] Add nosanitize metadata to memory access instructions inserted by emitProfileNotes()

Tue May 16 21:02:05 PDT 2023

MaskRay added a comment.

In D150460#4348564 <https://reviews.llvm.org/D150460#4348564>, @Enna1 wrote:

>> Though, I guess you wouldn't ship coverage enabled in production.
>
> Sure.
>
>> I guess I'm worried about __llvm_gcov_ctr becoming some kind of vector for attack.
>
> __llvm_gcov_ctr global variable is not instrumented by asan, see https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp#L1184
>
>> Hmm...that I'm less comfortable with approving.
>
> It's ok, if so, I can abandon this patch :) 
> Or maybe we can add an option gcov-nosanitize?

I think this patch is fine. `__llvm_gcov_ctr` is an internal variable and accessing doesn't use indirection, so no risk of hijection.
I don't recommend mixing gcov and sanitizers (e.g. `--coverage -fsanitize=hwaddress`), but if a user does this, disabling instrumentation for `__llvm_gcov_ctr` provides some size benefit.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150460