[AddressSanitizer] Instrument byval call arguments

[thejh]

Summary

In the LLVM IR, "call" instructions read memory for each byval operand.
For example:

$ cat blah.c
struct foo { void *a, *b, *c; };
struct bar { struct foo foo; };
void func1(const struct foo);
void func2(struct bar *bar) { func1(bar->foo); }
$ [...]/bin/clang -S -flto -c blah.c -O2 ; cat blah.s
[...]
define dso_local void @func2(%struct.bar* %bar) local_unnamed_addr #0 {
entry:
  %foo = getelementptr inbounds %struct.bar, %struct.bar* %bar, i64 0, i32 0
  tail call void @func1(%struct.foo* byval(%struct.foo) align 8 %foo) #2
  ret void
}
[...]
$ [...]/bin/clang -S -c blah.c -O2 ; cat blah.s
[...]
func2:                                  # @func2
[...]
        subq    $24, %rsp
[...]
        movq    16(%rdi), %rax
        movq    %rax, 16(%rsp)
        movups  (%rdi), %xmm0
        movups  %xmm0, (%rsp)
        callq   func1
        addq    $24, %rsp
[...]
        retq

Let ASAN instrument these hidden memory accesses.

This is patch 4/4 of a patch series:
https://reviews.llvm.org/D77616 [PATCH 1/4] [AddressSanitizer] Refactor ClDebug{Min,Max} handling
https://reviews.llvm.org/D77617 [PATCH 2/4] [AddressSanitizer] Split out memory intrinsic handling
https://reviews.llvm.org/D77618 [PATCH 3/4] [AddressSanitizer] Refactor: Permit >1 interesting operands per instruction
https://reviews.llvm.org/D77619 [PATCH 4/4] [AddressSanitizer] Instrument byval call arguments

[thejh]

fix clang-format warnings

[thejh]

rebase onto changed preceding patches

[thejh]

I missed that Kostya commented a while back that it'd be better to have a flag to gate the new functionality... adding that now.

@glider I noticed that you marked the three preceding patches as Accepted, but not this one - with this change, is this series ready to land?

[thejh]

Ugh... so the build failed on this patch because actually a preceding patch no longer applies cleanly and LLVM's build infrastructure doesn't have base-commit support and therefore can't do an automatic 3-way merge?

Sigh, I'll rebase and re-send the whole thing...

[thejh]

rebase to make harbormaster happy

[thejh]

resend patch to trigger new build

[glider]

Rebase

[glider]

rebase

[glider]

fix what I broke

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

static cl::opt<bool> ClInstrumentWrites(
     "asan-instrument-writes", cl::desc("instrument write instructions"),
     cl::Hidden, cl::init(true));
 
 static cl::opt<bool> ClInstrumentAtomics(
     "asan-instrument-atomics",
     cl::desc("instrument atomic instructions (rmw, cmpxchg)"), cl::Hidden,
     cl::init(true));
 
+static cl::opt<bool>
+    ClInstrumentByval("asan-instrument-byval",
+                      cl::desc("instrument byval call arguments"), cl::Hidden,
+                      cl::init(true));
+
 static cl::opt<bool> ClAlwaysSlowPath(
     "asan-always-slow-path",
     cl::desc("use instrumentation with slow path for all accesses"), cl::Hidden,
     cl::init(false));
 
 static cl::opt<bool> ClForceDynamicShadow(
     "asan-force-dynamic-shadow",
     cl::desc("Load shadow address into a local variable for each function"),
if (F && (F->getName().startswith("llvm.masked.load.") ||
       auto Ty = cast<PointerType>(BasePtr->getType())->getElementType();
       unsigned Alignment = 1;
       // Otherwise no alignment guarantees. We probably got Undef.
       if (auto AlignmentConstant =
               dyn_cast<ConstantInt>(CI->getOperand(1 + OpOffset)))
         Alignment = (unsigned)AlignmentConstant->getZExtValue();
       Value *Mask = CI->getOperand(2 + OpOffset);
       Interesting.emplace_back(I, OpOffset, IsWrite, Ty, Alignment, Mask);
+    } else {
+      for (unsigned ArgNo = 0; ArgNo < CI->getNumArgOperands(); ArgNo++) {
+        if (!ClInstrumentByval || !CI->isByValArgument(ArgNo) ||
+            ignoreAccess(CI->getArgOperand(ArgNo)))
+          continue;
+        Type *Ty = CI->getParamByValType(ArgNo);
+        Interesting.emplace_back(I, ArgNo, false, Ty, 1);
+      }
     }
   }
 }
 
 static bool isPointerOperand(Value *V) {
   return V->getType()->isPointerTy() || isa<PtrToIntInst>(V);
 }
 

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

static cl::opt<bool> ClInstrumentWrites(
     "hwasan-instrument-writes", cl::desc("instrument write instructions"),
     cl::Hidden, cl::init(true));
 
 static cl::opt<bool> ClInstrumentAtomics(
     "hwasan-instrument-atomics",
     cl::desc("instrument atomic instructions (rmw, cmpxchg)"), cl::Hidden,
     cl::init(true));
 
+static cl::opt<bool> ClInstrumentByval("hwasan-instrument-byval",
+                                       cl::desc("instrument byval arguments"),
+                                       cl::Hidden, cl::init(true));
+
 static cl::opt<bool> ClRecover(
     "hwasan-recover",
     cl::desc("Enable recovery mode (continue-after-error)."),
     cl::Hidden, cl::init(false));
 
 static cl::opt<bool> ClInstrumentStack("hwasan-instrument-stack",
                                        cl::desc("instrument stack (allocas)"),
                                        cl::Hidden, cl::init(true));
if (!ClInstrumentAtomics || ignoreAccess(RMW->getPointerOperand()))
       return;
     Interesting.emplace_back(I, RMW->getPointerOperandIndex(), true,
                              RMW->getValOperand()->getType(), 0);
   } else if (AtomicCmpXchgInst *XCHG = dyn_cast<AtomicCmpXchgInst>(I)) {
     if (!ClInstrumentAtomics || ignoreAccess(XCHG->getPointerOperand()))
       return;
     Interesting.emplace_back(I, XCHG->getPointerOperandIndex(), true,
                              XCHG->getCompareOperand()->getType(), 0);
+  } else if (auto CI = dyn_cast<CallInst>(I)) {
+    for (unsigned ArgNo = 0; ArgNo < CI->getNumArgOperands(); ArgNo++) {
+      if (!ClInstrumentByval || !CI->isByValArgument(ArgNo) ||
+          ignoreAccess(CI->getArgOperand(ArgNo)))
+        continue;
+      Type *Ty = CI->getParamByValType(ArgNo);
+      Interesting.emplace_back(I, ArgNo, false, Ty, 1);
+    }
   }
 }
 
 static unsigned getPointerOperandIndex(Instruction *I) {
   if (LoadInst *LI = dyn_cast<LoadInst>(I))
     return LI->getPointerOperandIndex();
   if (StoreInst *SI = dyn_cast<StoreInst>(I))
     return SI->getPointerOperandIndex();

llvm/test/Instrumentation/AddressSanitizer/byval-args.ll

+; RUN: opt < %s -asan -S | FileCheck %s
+; Test that for call instructions, the by-value arguments are instrumented.
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.bar = type { %struct.foo }
+%struct.foo = type { i8*, i8*, i8* }
+define dso_local void @func2(%struct.foo* %foo) sanitize_address {
+; CHECK-LABEL: @func2
+  tail call void @func1(%struct.foo* byval(%struct.foo) align 8 %foo) #2
+; CHECK: call void @__asan_report_load
+  ret void
+; CHECK: ret void
+}
+declare dso_local void @func1(%struct.foo* byval(%struct.foo) align 8)
+
+!0 = !{i32 1, !"wchar_size", i32 4}