[PATCH] D133268: [tsan] Replace mem intrinsics with calls to interceptors

[Marco Elver via Phabricator via llvm-commits]

melver requested changes to this revision.
melver added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp:344
   MemmoveFn =
-      M.getOrInsertFunction("memmove", Attr, IRB.getInt8PtrTy(),
+      M.getOrInsertFunction("__interceptor_memmove", Attr, IRB.getInt8PtrTy(),
                             IRB.getInt8PtrTy(), IRB.getInt8PtrTy(), IntptrTy);
----------------
This will cause problems for non-TSan runtimes, like KCSAN (on Linux, but also Fuchsia, FreeBSD, etc.).

I also don't understand why this happens in the first place, isn't ThreadSanitizer pass supposed to run after such transforms?

Some options in order of preference:

1. Is there another way to suppress this "optimization" back to an intrinsic? InstCombine has some exceptions when not to touch a call: https://github.com/llvm/llvm-project/blob/c4a174be9190b20eff0334415b5db7b2e2e204aa/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp#L2808
So the function calls could be 'notail' AFAIK (I think llvm/test/Transforms/InstCombine/memcpy-1.ll is missing a test for that, it's only testing 'musttail'). Or a new attribute that's added to tryOptimizeCall().

2. Make the prefix configurable with an option, so we can just set it to "" or `"__tsan_"` in the kernel (because `__interceptor_` seems too generic). It might be nice to mirror what ASan and MSan does, and actually call them `__tsan_mem*`, but it requires a runtime change as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133268