[PATCH] D88336: [ubsan] nullability-arg: Fix crash on C++ member function pointers

[Vedant Kumar via Phabricator via cfe-commits]

vsk created this revision.
vsk added reviewers: jkorous, ahatanak.
Herald added a subscriber: dexonsmith.
Herald added a project: clang.
vsk requested review of this revision.

Extend -fsanitize=nullability-arg to handle call sites which accept C++
member function pointers.

rdar://62476022


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88336

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenCXX/ubsan-nullability-arg.cpp


Index: clang/test/CodeGenCXX/ubsan-nullability-arg.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/ubsan-nullability-arg.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -x c++ -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=nullability-arg | FileCheck %s -check-prefix=ITANIUM
+// RUN: %clang_cc1 -x c++ -triple x86_64-pc-windows-msvc -emit-llvm -o - %s -fsanitize=nullability-arg | FileCheck %s -check-prefix=MSVC
+
+namespace method_ptr {
+
+struct S0 {
+  void foo1();
+};
+
+void foo1(void (S0::*_Nonnull f)());
+
+// ITANIUM-LABEL: @_ZN10method_ptr5test1Ev(){{.*}} {
+// ITANIUM: br i1 icmp ne (i64 ptrtoint (void (%"struct.method_ptr::S0"*)* @_ZN10method_ptr2S04foo1Ev to i64), i64 0), label %[[CONT:.*]], label %[[FAIL:[^,]*]]
+// ITANIUM-EMPTY:
+// ITANIUM-NEXT: [[FAIL]]:
+// ITANIUM-NEXT:   call void @__ubsan_handle_nullability_arg
+
+// MSVC-LABEL: @"?test1 at method_ptr@@YAXXZ"(){{.*}} {
+// MSVC: br i1 true, label %[[CONT:.*]], label %[[FAIL:[^,]*]]
+// MSVC-EMPTY:
+// MSVC-NEXT: [[FAIL]]:
+// MSVC-NEXT:   call void @__ubsan_handle_nullability_arg
+void test1() {
+  foo1(&S0::foo1);
+}
+
+} // namespace method_ptr
Index: clang/lib/CodeGen/CGCall.cpp
===================================================================
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -3741,6 +3741,20 @@
   }
 }
 
+/// Extract an integer or pointer argument from an RValue.
+static llvm::Value *getIntOrPtrArg(RValue RV, QualType ArgType) {
+  assert(RV.isScalar() && "Cannot convert non-scalar value to int/ptr type");
+  llvm::Value *Scalar = RV.getScalarVal();
+
+  // Under the Itanium ABI, if the argument has member pointer type, it's a
+  // pair containing the member pointer and the required adjustment to `this`.
+  if (ArgType->isMemberPointerType() && Scalar->getType()->isStructTy())
+    Scalar = cast<llvm::User>(Scalar)->getOperand(0);
+
+  assert(Scalar->getType()->isIntOrPtrTy() && "Must be an integer or pointer");
+  return Scalar;
+}
+
 void CodeGenFunction::EmitNonNullArgCheck(RValue RV, QualType ArgType,
                                           SourceLocation ArgLoc,
                                           AbstractCallee AC,
@@ -3783,8 +3797,7 @@
   }
 
   SanitizerScope SanScope(this);
-  assert(RV.isScalar());
-  llvm::Value *V = RV.getScalarVal();
+  llvm::Value *V = getIntOrPtrArg(RV, ArgType);
   llvm::Value *Cond =
       Builder.CreateICmpNE(V, llvm::Constant::getNullValue(V->getType()));
   llvm::Constant *StaticData[] = {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88336.294409.patch
Type: text/x-patch
Size: 2559 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200925/4a021ae5/attachment.bin>