[sanitizer] Simplify and future-proof maybeMarkSanitizerLibraryCallNoBuiltin().

[eugenis]

Summary

Sanitizers set nobuiltin attribute on certain library functions to
avoid a situation where such function is neither instrumented nor
intercepted.

At the moment the list of interesting functions is hardcoded. This
change replaces it with logic based on
TargetLibraryInfo::hasOptimizedCodegen and the presense of readnone
function attribute (sanitizers are generally interested in memory
behavior of library functions).

This is expected to be a no-op change: the new logic matches exactly
the same set of functions.

r276771 (currently reverted) added mempcpy() to the list, breaking
MSan tests. With this change, r276771 can be safely re-landed.

[Sunita_Marathe]

Hi Evgeniy,

Thank you for adding support for mempcpy to resolve the MSan fails caused by r276771.
Your change looks right to me.

memcpy() and memmov(), which are represented in the IR as intrinsics, would also, I think, be getting marked with Attribute::NoBuiltin for the purposes of MSan testing. They appear to be handled in a different way from the LibFunc functions.

Thanks
Sunita

Thanks
Sunita

[eugenis]

Right, memcpy/memmove _intrinsics_ are handled specially in sanitizer instrumentation passes. Memcpy/memmove functions are not; and neither are they inlined in the codegen (at least I hope so, otherwise we have a problem). They are not mentioned in hasOptimizedCodeGen, too, so this change does not affect them.

[Sunita_Marathe]

Thank you Evgeniy, yes my comment was about the memcpy/memov intrinsics.
Sunita

[vitalybuka]

lgtm

[eugenis]

r277086

lib/Transforms/Utils/Local.cpp

bool llvm::recognizeBSwapOrBitReverseIdiom(
   return true;
 }
 
 // CodeGen has special handling for some string functions that may replace
 // them with target-specific intrinsics.  Since that'd skip our interceptors
 // in ASan/MSan/TSan/DFSan, and thus make us miss some memory accesses,
 // we mark affected calls as NoBuiltin, which will disable optimization
 // in CodeGen.
-void llvm::maybeMarkSanitizerLibraryCallNoBuiltin(CallInst *CI,
-                                          const TargetLibraryInfo *TLI) {
+void llvm::maybeMarkSanitizerLibraryCallNoBuiltin(
+    CallInst *CI, const TargetLibraryInfo *TLI) {
   Function *F = CI->getCalledFunction();
   LibFunc::Func Func;
-  if (!F || F->hasLocalLinkage() || !F->hasName() ||
-      !TLI->getLibFunc(F->getName(), Func))
-    return;
-  switch (Func) {
-    default: break;
-    case LibFunc::memcmp:
-    case LibFunc::memchr:
-    case LibFunc::strcpy:
-    case LibFunc::stpcpy:
-    case LibFunc::strcmp:
-    case LibFunc::strlen:
-    case LibFunc::strnlen:
-      CI->addAttribute(AttributeSet::FunctionIndex, Attribute::NoBuiltin);
-      break;
-  }
+  if (F && !F->hasLocalLinkage() && F->hasName() &&
+      TLI->getLibFunc(F->getName(), Func) && TLI->hasOptimizedCodeGen(Func) &&
+      !F->doesNotAccessMemory())
 }