[libc-commits] [PATCH] D78612: [libc] Add sanitizer instrumentation to the SIMD strlen implementation.

[Mitch Phillips via Phabricator via libc-commits]

hctim added a comment.

I personally think this is too clever. The whole templating thing seems like a good overall QOL improvement - but the whole unsanitized `safe_check_word` that we have to manually sanitize seems challenging to maintain. You would need to do sanitizer instrumentation manually for ASan, MSan, UBSan, HWASan, MTE, DFSan, and any other fun toys we come up with. Each of these has a different interface, and different quirks. Why not just let the compiler do these quirks?

Instead, can't we just use the slow path always under sanitizers? i.e.:

  #if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
    static constexpr bool word_optimized = false;
  #else // ASan
    static constexpr bool word_optimized = Mask<sizeof(uintptr_t)>::opt;
  #endif

Then, the whole dancing-around-managing sanitizers is solved as the compiler just does normal instrumentation of the slow path.



================
Comment at: libc/test/src/string/strlen_test.cpp:70
+  const char hello[8] = {'h', 'e', 'l', 'l', 'o', '\0', '0', '0'};
+#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
+  __asan_poison_memory_region(hello + 6, 2);
----------------
Wouldn't this be simpler just as `const char *hello = "hello"`? We definitely shouldn't be manually doing asan-ish things in the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78612