[MSan] Fix calling pointers to varargs functions on SystemZ

[iii]

Summary

VarArgSystemZHelper.visitCallBase() checks whether the callee has the
"use-soft-float" attribute, but if the callee is a function pointer, a
null pointer dereference happens.

Fix by checking this attribute on the current function. Alternatively,
one could try the callee first, but this is pointless, since one should
not be mixing hardfloat and softfloat code anyway.

[uweigand]

Fix by checking this attribute on the current function. Alternatively, one could try the callee first, but this is pointless, since one should not be mixing hardfloat and softfloat code anyway.

Agreed. If the current function is compiled for soft-float, it cannot use any FP registers and therefore could not set up arguments for a hard-float routine.

The patch LGTM.

llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

struct VarArgSystemZHelper : public VarArgHelper {
   static const unsigned SystemZOverflowOffset = 160;
   static const unsigned SystemZVAListTagSize = 32;
   static const unsigned SystemZOverflowArgAreaPtrOffset = 16;
   static const unsigned SystemZRegSaveAreaPtrOffset = 24;
 
   Function &F;
   MemorySanitizer &MS;
   MemorySanitizerVisitor &MSV;
+  bool IsSoftFloatABI;
   Value *VAArgTLSCopy = nullptr;
   Value *VAArgTLSOriginCopy = nullptr;
   Value *VAArgOverflowSize = nullptr;
 
   SmallVector<CallInst *, 16> VAStartInstrumentationList;
 
   enum class ArgKind {
     GeneralPurpose,
     FloatingPoint,
     Vector,
     Memory,
     Indirect,
   };
 
   enum class ShadowExtension { None, Zero, Sign };
 
   VarArgSystemZHelper(Function &F, MemorySanitizer &MS,
                       MemorySanitizerVisitor &MSV)
-      : F(F), MS(MS), MSV(MSV) {}
+      : F(F), MS(MS), MSV(MSV),
+        IsSoftFloatABI(F.getFnAttribute("use-soft-float").getValueAsBool()) {}
 
-  ArgKind classifyArgument(Type *T, bool IsSoftFloatABI) {
+  ArgKind classifyArgument(Type *T) {
     // T is a SystemZABIInfo::classifyArgumentType() output, and there are
     // only a few possibilities of what it can be. In particular, enums, single
     // element structs and large types have already been taken care of.
 
     // Some i128 and fp128 arguments are converted to pointers only in the
     // back end.
     if (T->isIntegerTy(128) || T->isFP128Ty())
       return ArgKind::Indirect;
ShadowExtension getShadowExtension(const CallBase &CB, unsigned ArgNo) {
     if (SExt) {
       assert(!ZExt);
       return ShadowExtension::Sign;
     }
     return ShadowExtension::None;
   }
 
   void visitCallBase(CallBase &CB, IRBuilder<> &IRB) override {
-    bool IsSoftFloatABI = CB.getCalledFunction()
-                              ->getFnAttribute("use-soft-float")
-                              .getValueAsBool();
     unsigned GpOffset = SystemZGpOffset;
     unsigned FpOffset = SystemZFpOffset;
     unsigned VrIndex = 0;
     unsigned OverflowOffset = SystemZOverflowOffset;
     const DataLayout &DL = F.getParent()->getDataLayout();
     for (const auto &[ArgNo, A] : llvm::enumerate(CB.args())) {
       bool IsFixed = ArgNo < CB.getFunctionType()->getNumParams();
       // SystemZABIInfo does not produce ByVal parameters.
       assert(!CB.paramHasAttr(ArgNo, Attribute::ByVal));
       Type *T = A->getType();
-      ArgKind AK = classifyArgument(T, IsSoftFloatABI);
+      ArgKind AK = classifyArgument(T);
       if (AK == ArgKind::Indirect) {
         T = PointerType::get(T, 0);
         AK = ArgKind::GeneralPurpose;
       }
       if (AK == ArgKind::GeneralPurpose && GpOffset >= SystemZGpEndOffset)
         AK = ArgKind::Memory;
       if (AK == ArgKind::FloatingPoint && FpOffset >= SystemZFpEndOffset)
         AK = ArgKind::Memory;

llvm/test/Instrumentation/MemorySanitizer/SystemZ/vararg-kernel.ll

define i64 @bar() #1 {
   %1 = call i64 (i64, ...) @foo(i64 1, i32 zeroext %arg2, float %arg3,
                                 i32 signext %arg4, double %arg5, i64 %arg6,
                                 i64 7, double 8.0, i32 zeroext %arg9,
                                 double 10.0, float %arg11, i32 signext %arg12,
                                 double %arg13, i64 %arg14)
   ret i64 %1
 }
 
-attributes #1 = { sanitize_memory }
+attributes #1 = { sanitize_memory "target-features"="+soft-float" "use-soft-float"="true" }
 
 ; In kernel the floating point values are passed in GPRs:
 ; - r2@16              == i64 1            - skipped, because it's fixed
 ; - r3@24              == i32 zext %arg2   - shadow is zero-extended
 ; - r4@(32 + 4)        == float %arg3      - right-justified, shadow is 32-bit
 ; - r5@40              == i32 sext %arg4   - shadow is sign-extended
 ; - r6@48              == double %arg5     - straightforward
 ; - overflow@160       == i64 %arg6        - straightforward

llvm/test/Instrumentation/MemorySanitizer/SystemZ/vararg.ll

 ; CHECK: declare void @__msan_maybe_warning_1(i8 zeroext, i32 zeroext)
 ; CHECK: declare void @__msan_maybe_store_origin_1(i8 zeroext, ptr, i32 zeroext)
 ; CHECK: declare void @__msan_maybe_warning_2(i16 zeroext, i32 zeroext)
 ; CHECK: declare void @__msan_maybe_store_origin_2(i16 zeroext, ptr, i32 zeroext)
 ; CHECK: declare void @__msan_maybe_warning_4(i32 zeroext, i32 zeroext)
 ; CHECK: declare void @__msan_maybe_store_origin_4(i32 zeroext, ptr, i32 zeroext)
 ; CHECK: declare void @__msan_maybe_warning_8(i64 zeroext, i32 zeroext)
 ; CHECK: declare void @__msan_maybe_store_origin_8(i64 zeroext, ptr, i32 zeroext)
+
+; Test vararg function pointers.
+;
+; void (*ptr)(int, ...);
+; void call_ptr(void) { ptr(0); }
+
+@ptr = dso_local global ptr null, align 8
+
+define dso_local void @call_ptr() {
+  %1 = load ptr, ptr @ptr, align 8
+  call void (i32, ...) %1(i32 noundef signext 0)
+  ret void
+}