[HWASAN] Instrument memory intrinsics

[evgeny777]

Summary

Patch replaces memory intrinsics with corresponding libc calls when specific option is set. The memset and friends can be either hooked by the runtime or libc itself can be sanitized

The patch lacks test case - I'll implement one if the whole thing makes sense.

[eugenis]

Looks fine to me.

[kcc]

asan replaces memset with __asan_memset (same for memcpy and memmove), see AddressSanitizer::instrumentMemIntrinsic.
Would it be more flexible to use the same approach here?

(and yes, we need to do this, so please proceed with a test)

[evgeny777]

Would it be more flexible to use the same approach here?

Since r340216 HWASAN stopped intercepting common libc routines. As far as I understand the goal of this change is to sanitize libc.
My understanding is that if we do sanitize the whole libc then we don't need special versions of memset and friends in HWASAN runtime.

Even if one doesn't sanitize libc then it is always possible to hook memset/memcpy/memmove like it was previously done with strcpy and stuff.

[evgeny777]

Added test case

[kcc]

We do instrument libc on Android, but

• memset&co are still not instrumented because they are written in asm (eugenis@, please confirm)
• hwasan may be useful outside of Android in future. (we actually test it on arm64 linux, and even on x86_64 linux)

So, I still prefer __hwasan_memset (which means that we will also need a run-time test)
eugenis@, thoughts?

test/Instrumentation/HWAddressSanitizer/mem-intrinsics.ll

3 ↗

(On Diff #177164)

please place the checks after their respective "call void @llvm..." lines

[eugenis]

I think I prefer regular memset to hwasan_memset.
hwasan variants can be slightly faster because they don't need to check that the address is not a shadow address (from stack tagging), but that is probably not a big deal.
Also the hardware implementation will definitely not have (or need) __hwasan_memset.

[kcc]

How will do do the actual checking then, given the memset is implemented in asm?
extra code in libc?

[eugenis]

Yes, I think so.

[kcc]

ok, let's do it this way, I'll leave it to eugenis@ to stamp.
Also, at some point we'll need to flip the flag.

[eugenis]

In fact, I changed my mind - let's do __hwasan_memset.
Tag checking in memset is part of software emulation; as such, it belongs in libclang_rt.hwasan, and not in any system library.

[evgeny777]

In fact, I changed my mind - let's do __hwasan_memset.

What about kernel compilation mode?

[eugenis]

We should do the same thing in kernel mode, I think. Either way memintrinsic checking will require changes on the kernel side. @andreyknvl

[evgeny777]

Addressed review comments. Now the patch uses approach identical to ASAN.
ASAN doesn't add prefix in kernel compilation mode (my guess is that memset and friends are part of Linux kernel and are always instrumented in such scenario)

See also patch to runtime: D55554

[kcc]

LGTM
Next step is to add run-time tests and enable by default

lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

static cl::opt<int> ClMatchAllTag(
     cl::desc("don't report bad accesses via pointers with this tag"),
     cl::Hidden, cl::init(-1));
 
 static cl::opt<bool> ClEnableKhwasan(
     "hwasan-kernel",
     cl::desc("Enable KernelHWAddressSanitizer instrumentation"),
     cl::Hidden, cl::init(false));
 
+static cl::opt<bool>
+    ClInstrumentMemIntrinsics("hwasan-instrument-mem-intrinsics",
+                              cl::desc("instrument memory intrinsics"),
+                              cl::Hidden, cl::init(false));
+
 // These flags allow to change the shadow mapping and control how shadow memory
 // is accessed. The shadow mapping looks like:
 //    Shadow = (Mem >> scale) + offset
 
 static cl::opt<unsigned long long> ClMappingOffset(
     "hwasan-mapping-offset",
     cl::desc("HWASan shadow mapping offset [EXPERIMENTAL]"), cl::Hidden,
     cl::init(0));
public:
 
   Value *getDynamicShadowNonTls(IRBuilder<> &IRB);
 
   void untagPointerOperand(Instruction *I, Value *Addr);
   Value *memToShadow(Value *Shadow, Type *Ty, IRBuilder<> &IRB);
   void instrumentMemAccessInline(Value *PtrLong, bool IsWrite,
                                  unsigned AccessSizeIndex,
                                  Instruction *InsertBefore);
+  void instrumentMemIntrinsic(MemIntrinsic *MI);
   bool instrumentMemAccess(Instruction *I);
   Value *isInterestingMemoryAccess(Instruction *I, bool *IsWrite,
                                    uint64_t *TypeSize, unsigned *Alignment,
                                    Value **MaybeMask);
 
   bool isInterestingAlloca(const AllocaInst &AI);
   bool tagAlloca(IRBuilder<> &IRB, AllocaInst *AI, Value *Tag);
   Value *tagPointer(IRBuilder<> &IRB, Type *Ty, Value *PtrLong, Value *Tag);
case Triple::aarch64_be:
           /*hasSideEffects=*/true);
       break;
     default:
       report_fatal_error("unsupported architecture");
   }
   IRB.CreateCall(Asm, PtrLong);
 }
 
+void HWAddressSanitizer::instrumentMemIntrinsic(MemIntrinsic *MI) {
+  IRBuilder<> IRB(MI);
+  Module *M = MI->getParent()->getParent()->getParent();
+  if (isa<MemTransferInst>(MI)) {
+    auto *F = isa<MemMoveInst>(MI)
+                  ? M->getOrInsertFunction("memmove", IRB.getInt8PtrTy(),
+                                           IRB.getInt8PtrTy(),
+                                           IRB.getInt8PtrTy(), IntptrTy)
+                  : M->getOrInsertFunction("memcpy", IRB.getInt8PtrTy(),
+                                           IRB.getInt8PtrTy(),
+                                           IRB.getInt8PtrTy(), IntptrTy);
+    IRB.CreateCall(
+        F, {IRB.CreatePointerCast(MI->getOperand(0), IRB.getInt8PtrTy()),
+            IRB.CreatePointerCast(MI->getOperand(1), IRB.getInt8PtrTy()),
+            IRB.CreateIntCast(MI->getOperand(2), IntptrTy, false)});
+  } else if (isa<MemSetInst>(MI)) {
+    IRB.CreateCall(
+        M->getOrInsertFunction("memset", IRB.getInt8PtrTy(), IRB.getInt8PtrTy(),
+                               IRB.getInt32Ty(), IntptrTy),
+        {IRB.CreatePointerCast(MI->getOperand(0), IRB.getInt8PtrTy()),
+         IRB.CreateIntCast(MI->getOperand(1), IRB.getInt32Ty(), false),
+         IRB.CreateIntCast(MI->getOperand(2), IntptrTy, false)});
+  }
+  MI->eraseFromParent();
+}
+
 bool HWAddressSanitizer::instrumentMemAccess(Instruction *I) {
   LLVM_DEBUG(dbgs() << "Instrumenting: " << *I << "\n");
   bool IsWrite = false;
   unsigned Alignment = 0;
   uint64_t TypeSize = 0;
   Value *MaybeMask = nullptr;
+
+  if (ClInstrumentMemIntrinsics && isa<MemIntrinsic>(I)) {
+    instrumentMemIntrinsic(cast<MemIntrinsic>(I));
+    return true;
+  }
+
   Value *Addr =
       isInterestingMemoryAccess(I, &IsWrite, &TypeSize, &Alignment, &MaybeMask);
 
   if (!Addr)
     return false;
 
   if (MaybeMask)
     return false; //FIXME