[HWASAN] Mark built-ins as not built-ins to prevent optimizations

[kstoimenov]

The other 3 sanitizers (ASAN, TSAN and MSAN) all use maybeMarkSanitizerLibraryCallNoBuiltin to make disable optimizations which inline functions like memcmp for example. The lack of this optimization was allowing ExpandMemCmpPass to convert a memcmp call to inlined assembly and cause a false negative in HWASAN.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Kirill Stoimenov (kstoimenov)

Changes

The other 3 sanitizers (ASAN, TSAN and MSAN) all use maybeMarkSanitizerLibraryCallNoBuiltin to make disable optimizations which inline functions like memcmp for example. The lack of this optimization was allowing ExpandMemCmpPass to convert a memcmp call to inlined assembly and cause a false negative in HWASAN.

---

Full diff: https://github.com/llvm/llvm-project/pull/68936.diff

2 Files Affected:

• (modified) compiler-rt/test/hwasan/TestCases/memcmp.cpp (+2-2)
• (modified) llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp (+9-3)

diff --git a/compiler-rt/test/hwasan/TestCases/memcmp.cpp b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
index c6a2b42b54d271c..5f8a93f62a44a1d 100644
--- a/compiler-rt/test/hwasan/TestCases/memcmp.cpp
+++ b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
@@ -11,8 +11,8 @@
 int main(int argc, char **argv) {
   __hwasan_enable_allocator_tagging();
   char a[] = {static_cast<char>(argc), 2, 3, 4};
-  volatile int size = sizeof(a);
-  char *volatile p = (char *)malloc(size);
+  int size = sizeof(a);
+  char *p = (char *)malloc(size);
   memcpy(p, a, size);
   free(p);
   return memcmp(p, a, size);
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index fd7c641ccf4b2f3..e194b96475481d5 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -21,6 +21,7 @@
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/Analysis/PostDominators.h"
 #include "llvm/Analysis/StackSafetyAnalysis.h"
+#include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/BinaryFormat/ELF.h"
@@ -52,6 +53,7 @@
 #include "llvm/TargetParser/Triple.h"
 #include "llvm/Transforms/Instrumentation/AddressSanitizerCommon.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
+#include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Utils/MemoryTaggingSupport.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include "llvm/Transforms/Utils/PromoteMemToReg.h"
@@ -320,7 +322,8 @@ class HWAddressSanitizer {
                            LoopInfo *LI);
   bool ignoreAccess(Instruction *Inst, Value *Ptr);
   void getInterestingMemoryOperands(
-      Instruction *I, SmallVectorImpl<InterestingMemoryOperand> &Interesting);
+      Instruction *I, const TargetLibraryInfo &TLI,
+      SmallVectorImpl<InterestingMemoryOperand> &Interesting);
 
   void tagAlloca(IRBuilder<> &IRB, AllocaInst *AI, Value *Tag, size_t Size);
   Value *tagPointer(IRBuilder<> &IRB, Type *Ty, Value *PtrLong, Value *Tag);
@@ -779,7 +782,8 @@ bool HWAddressSanitizer::ignoreAccess(Instruction *Inst, Value *Ptr) {
 }
 
 void HWAddressSanitizer::getInterestingMemoryOperands(
-    Instruction *I, SmallVectorImpl<InterestingMemoryOperand> &Interesting) {
+    Instruction *I, const TargetLibraryInfo &TLI,
+    SmallVectorImpl<InterestingMemoryOperand> &Interesting) {
   // Skip memory accesses inserted by another instrumentation.
   if (I->hasMetadata(LLVMContext::MD_nosanitize))
     return;
@@ -817,6 +821,7 @@ void HWAddressSanitizer::getInterestingMemoryOperands(
       Type *Ty = CI->getParamByValType(ArgNo);
       Interesting.emplace_back(I, ArgNo, false, Ty, Align(1));
     }
+    maybeMarkSanitizerLibraryCallNoBuiltin(CI, &TLI);
   }
 }
 
@@ -1493,6 +1498,7 @@ void HWAddressSanitizer::sanitizeFunction(Function &F,
   SmallVector<InterestingMemoryOperand, 16> OperandsToInstrument;
   SmallVector<MemIntrinsic *, 16> IntrinToInstrument;
   SmallVector<Instruction *, 8> LandingPadVec;
+  const TargetLibraryInfo &TLI = FAM.getResult<TargetLibraryAnalysis>(F);
 
   memtag::StackInfoBuilder SIB(SSI);
   for (auto &Inst : instructions(F)) {
@@ -1503,7 +1509,7 @@ void HWAddressSanitizer::sanitizeFunction(Function &F,
     if (InstrumentLandingPads && isa<LandingPadInst>(Inst))
       LandingPadVec.push_back(&Inst);
 
-    getInterestingMemoryOperands(&Inst, OperandsToInstrument);
+    getInterestingMemoryOperands(&Inst, TLI, OperandsToInstrument);
 
     if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(&Inst))
       if (!ignoreMemIntrinsic(MI))

[vitalybuka]

Check LGTM, but if we change llvm transformation, we should also update/add llvm test cases

[vitalybuka]

please add/update some llvm/ tests

[vitalybuka]

-passes=hwasan ?

[kstoimenov]

Done.