hwasan: fix inline instrumentation

[andreyknvl]

Summary

This patch changes hwasan inline instrumentation:

1. Fixes address untagging for shadow address calculation (use 0xFF instead of 0x00 for the top byte).
2. Emits brk instruction instead of hlt for the kernel and user space.
3. Use 0x900 instead of 0x100 for brk immediate (0x100 - 0x800 are unavailable in the kernel).
4. Fixes and adds appropriate tests.

[kcc]

please add a lit test.

[eugenis]

LGTM with a test.
Please don't copy all the tests for different hlt# values, just one would be enough to capture the difference in 0x900.

[eugenis]

Andrey, do you mind switching userspace to brk as well? You'd need to change the decoding in hwasan_linux.cc as well, but it could be even less work than writing a kernel-only lit test.

[andreyknvl]

Switched user space hwasan to brk.
Fixed and added tests.

[andreyknvl]

PTAL

[andreyknvl]

It seems that hwasan_linux.cc is in a different repo (compiler-rt) and is not picked up by svn diff. Do I need to submit that change as a separate patch?

[eugenis]

You can pass multiple repositories as arguments to svn diff, or upload a separate revision, or use git monorepo.

lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

314

Could we use 0x900 in the userspace as well? I assume 0x100 is not available in the kernel for some reason. There are no ABI concerns yet.

[andreyknvl]

Use 0x900 instead of 0x100 for brk immediate (0x100 - 0x800 are unavailable in the kernel).

[andreyknvl]

Done, PTAL

[eugenis]

projects/compiler-rt/lib/hwasan/hwasan_linux.cc

191 ↗

(On Diff #135278)

0x9XY

255 ↗

(On Diff #135278)

I don't think we have a SIGTRAP handler.

[andreyknvl]

Install SIGTRAP handler.

[andreyknvl]

PTAL

[eugenis]

projects/compiler-rt/lib/sanitizer_common/sanitizer_posix_libcdep.cc

221 ↗

(On Diff #135292)

It won't do anything, see GetHandleSignalModeImpl

[andreyknvl]

Try harder to install SIGTRAP handler.

[andreyknvl]

PTAL

[eugenis]

Looks great, thank you!
I can land it for you.

[andreyknvl]

If to land means to commit, then yes, please :)

[eugenis]

r325711.
I had to fix userspace callbacks - they were still doing hlt with 0x100 instead of brk with 0x900.

lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

public:
   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);
+  Value *untagPointer(IRBuilder<> &IRB, Value *PtrLong);
   bool instrumentStack(SmallVectorImpl<AllocaInst *> &Allocas,
                        SmallVectorImpl<Instruction *> &RetVec);
   Value *getNextTagWithCall(IRBuilder<> &IRB);
   Value *getStackBaseTag(IRBuilder<> &IRB);
   Value *getAllocaTag(IRBuilder<> &IRB, Value *StackTag, AllocaInst *AI,
                      unsigned AllocaNo);
   Value *getUARTag(IRBuilder<> &IRB, Value *StackTag);
 
static size_t TypeSizeToSizeIndex(uint32_t TypeSize) {
   return Res;
 }
 
 void HWAddressSanitizer::instrumentMemAccessInline(Value *PtrLong, bool IsWrite,
                                                    unsigned AccessSizeIndex,
                                                    Instruction *InsertBefore) {
   IRBuilder<> IRB(InsertBefore);
   Value *PtrTag = IRB.CreateTrunc(IRB.CreateLShr(PtrLong, kPointerTagShift), IRB.getInt8Ty());
-  Value *AddrLong =
-      IRB.CreateAnd(PtrLong, ConstantInt::get(PtrLong->getType(),
-                                              ~(0xFFULL << kPointerTagShift)));
+  Value *AddrLong = untagPointer(IRB, PtrLong);
   Value *ShadowLong = IRB.CreateLShr(AddrLong, kShadowScale);
   if (ClMappingOffset)
     ShadowLong = IRB.CreateAdd(
         ShadowLong, ConstantInt::get(PtrLong->getType(), ClMappingOffset,
                                      /*isSigned=*/false));
   Value *MemTag =
       IRB.CreateLoad(IRB.CreateIntToPtr(ShadowLong, IRB.getInt8PtrTy()));
   Value *TagMismatch = IRB.CreateICmpNE(PtrTag, MemTag);
 
   TerminatorInst *CheckTerm =
       SplitBlockAndInsertIfThen(TagMismatch, InsertBefore, !Recover,
                                 MDBuilder(*C).createBranchWeights(1, 100000));
 
   IRB.SetInsertPoint(CheckTerm);
   // The signal handler will find the data address in x0.
   InlineAsm *Asm = InlineAsm::get(
       FunctionType::get(IRB.getVoidTy(), {PtrLong->getType()}, false),
-      "hlt #" +
-          itostr(0x100 + Recover * 0x20 + IsWrite * 0x10 + AccessSizeIndex),
+      (ClEnableKhwasan ? "brk #" : "hlt #") +
+          itostr((ClEnableKhwasan ? 0x900 : 0x100) + Recover * 0x20 +
+      IsWrite * 0x10 + AccessSizeIndex),
       "{x0}",
       /*hasSideEffects=*/true);
   IRB.CreateCall(Asm, PtrLong);
 }
 
 bool HWAddressSanitizer::instrumentMemAccess(Instruction *I) {
   DEBUG(dbgs() << "Instrumenting: " << *I << "\n");
   bool IsWrite = false;
Value *HWAddressSanitizer::tagPointer(IRBuilder<> &IRB, Type *Ty, Value *PtrLong,
   } else {
     // Userspace can simply do OR (tag << 56);
     Value *ShiftedTag = IRB.CreateShl(Tag, kPointerTagShift);
     TaggedPtrLong = IRB.CreateOr(PtrLong, ShiftedTag);
   }
   return IRB.CreateIntToPtr(TaggedPtrLong, Ty);
 }
 
+// Remove tag from an address.
+Value *HWAddressSanitizer::untagPointer(IRBuilder<> &IRB, Value *PtrLong) {
+  Value *UntaggedPtrLong;
+  if (ClEnableKhwasan) {
+    // Kernel addresses have 0xFF in the most significant byte.
+    UntaggedPtrLong = IRB.CreateOr(PtrLong,
+        ConstantInt::get(PtrLong->getType(), 0xFFULL << kPointerTagShift));
+  } else {
+    // Userspace addresses have 0x00.
+    UntaggedPtrLong = IRB.CreateAnd(PtrLong,
+        ConstantInt::get(PtrLong->getType(), ~(0xFFULL << kPointerTagShift)));
+  }
+  return UntaggedPtrLong;
+}
+
 bool HWAddressSanitizer::instrumentStack(
     SmallVectorImpl<AllocaInst *> &Allocas,
     SmallVectorImpl<Instruction *> &RetVec) {
   Function *F = Allocas[0]->getParent()->getParent();
   Instruction *InsertPt = &*F->getEntryBlock().begin();
   IRBuilder<> IRB(InsertPt);
 
   Value *StackTag = getStackBaseTag(IRB);