samitolvanen created this revision.
samitolvanen added reviewers: aaron.ballman, pcc, nickdesaulniers, ardb, kees, 
joaomoreira.
Herald added a project: All.
samitolvanen requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

With `-fsanitize=kcfi`, disabling indirect call checking using
the `no_sanitize` attribute is not fine-grained enough in cases
where we want to disable checking only for specific calls. The
`__builtin_call_kcfi_unchecked` builtin accepts an indirect call
expression and emits the call without KCFI type checking.

Depends on D119296 <https://reviews.llvm.org/D119296>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124211

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-call-kcfi-unchecked.cpp
  clang/test/SemaCXX/builtins.cpp

Index: clang/test/SemaCXX/builtins.cpp
===================================================================
--- clang/test/SemaCXX/builtins.cpp
+++ clang/test/SemaCXX/builtins.cpp
@@ -43,6 +43,35 @@
   S *ptmp = __builtin_addressof(S{}); // expected-error {{taking the address of a temporary}}
 }
 
+namespace call_kcfi_unchecked {
+int a(void) { return 0; }
+struct S {
+  int f(void) { return 1; }
+  static int g(void) { return 2; }
+} s;
+static int n;
+int &b(int (*p)(void)) {
+  n += p();
+  return n;
+}
+int (*p1)(void) = a;
+int (*p2)(void) = &S::g;
+int &(*p3)(int (*)(void)) = b;
+
+void test() {
+  __builtin_call_kcfi_unchecked(p1());
+  __builtin_call_kcfi_unchecked(p2());
+  __builtin_call_kcfi_unchecked(p3(p1));
+  __builtin_call_kcfi_unchecked(p1);     // expected-error {{argument to __builtin_call_kcfi_unchecked must be an indirect call expression}}
+  __builtin_call_kcfi_unchecked(p1, p2); // expected-error {{too many arguments to function call, expected 1, have 2}}
+  __builtin_call_kcfi_unchecked(a());    // expected-error {{argument to __builtin_call_kcfi_unchecked must be an indirect call expression}}
+  __builtin_call_kcfi_unchecked(s.f());  // expected-error {{argument to __builtin_call_kcfi_unchecked must be an indirect call expression}}
+  __builtin_call_kcfi_unchecked(S::g()); // expected-error {{argument to __builtin_call_kcfi_unchecked must be an indirect call expression}}
+  static_assert(__is_same(decltype(__builtin_call_kcfi_unchecked(p1())), int), "");
+  static_assert(__is_same(decltype(__builtin_call_kcfi_unchecked(p3(p1))), int &), "");
+}
+} // namespace call_kcfi_unchecked
+
 namespace function_start {
 void a(void) {}
 int n;
Index: clang/test/CodeGen/builtin-call-kcfi-unchecked.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGen/builtin-call-kcfi-unchecked.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck %s
+
+#if !__has_builtin(__builtin_call_kcfi_unchecked)
+#error "missing __builtin_call_kcfi_unchecked"
+#endif
+
+// CHECK: define{{.*}} i32 @_Z1av(){{.*}} prefix i32 [[#%d,HASH1:]]
+int a(void) { return 0; }
+
+class A {
+public:
+  static void a();
+};
+
+// CHECK: define{{.*}} void @_ZN1A1aEv(){{.*}} prefix i32 [[#%d,HASH2:]]
+void A::a() {}
+
+void h(void) {
+  // CHECK: store ptr @_Z1av, ptr %
+  // CHECK: %[[#P1:]] = load ptr, ptr %
+  // CHECK: call void @llvm.kcfi.check(ptr %[[#P1]], i32 [[#HASH1]])
+  // CHECK: call{{.*}} i32 %[[#P1]]()
+  ({ a; })();
+
+  // CHECK: store ptr @_Z1av, ptr %
+  // CHECK: %[[#P2:]] = load ptr, ptr %
+  // CHECK-NOT: call void @llvm.kcfi.check
+  // CHECK: call{{.*}} i32 %[[#P2]]()
+  __builtin_call_kcfi_unchecked(({ a; })());
+
+  // CHECK: store ptr @_ZN1A1aEv, ptr %
+  // CHECK: %[[#P3:]] = load ptr, ptr %
+  // CHECK: call void @llvm.kcfi.check(ptr %[[#P3]], i32 [[#HASH2]])
+  // CHECK: call void %[[#P3]]()
+  ({ &A::a; })();
+
+  // CHECK: store ptr @_ZN1A1aEv, ptr %
+  // CHECK: %[[#P4:]] = load ptr, ptr %
+  // CHECK-NOT: call void @llvm.kcfi.check
+  // CHECK: call void %[[#P4]]()
+  __builtin_call_kcfi_unchecked(({ &A::a; })());
+}
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -429,6 +429,39 @@
   return false;
 }
 
+static bool SemaBuiltinCallKCFIUnchecked(Sema &S, CallExpr *BuiltinCall) {
+  if (checkArgCount(S, BuiltinCall, 1))
+    return true;
+
+  auto Call = dyn_cast_or_null<CallExpr>(BuiltinCall->getArg(0));
+
+  if (!Call || !Call->getCallee()
+                    ->IgnoreImpCasts()
+                    ->getType()
+                    ->isFunctionPointerType()) {
+    S.Diag(BuiltinCall->getBeginLoc(), diag::err_kcfi_unchecked_argument)
+        << BuiltinCall->getSourceRange();
+    return true;
+  }
+
+  QualType ReturnTy = Call->getCallReturnType(S.Context);
+  QualType BuiltinTy = S.Context.getFunctionType(
+      ReturnTy, {ReturnTy}, FunctionProtoType::ExtProtoInfo());
+
+  auto Builtin = S.ImpCastExprToType(BuiltinCall->getCallee()->IgnoreImpCasts(),
+                                     S.Context.getPointerType(BuiltinTy),
+                                     CK_BuiltinFnToFnPtr)
+                     .get();
+
+  BuiltinCall->setType(Call->getType());
+  BuiltinCall->setValueKind(Call->getValueKind());
+  BuiltinCall->setObjectKind(Call->getObjectKind());
+  BuiltinCall->setCallee(Builtin);
+  Call->setKCFIUnchecked();
+
+  return false;
+}
+
 namespace {
 
 class ScanfDiagnosticFormatHandler
@@ -2106,6 +2139,10 @@
     if (SemaBuiltinCallWithStaticChain(*this, TheCall))
       return ExprError();
     break;
+  case Builtin::BI__builtin_call_kcfi_unchecked:
+    if (SemaBuiltinCallKCFIUnchecked(*this, TheCall))
+      return ExprError();
+    break;
   case Builtin::BI__exception_code:
   case Builtin::BI_exception_code:
     if (SemaBuiltinSEHScopeCheck(*this, TheCall, Scope::SEHExceptScope,
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5397,7 +5397,8 @@
     }
   }
 
-  if (SanOpts.has(SanitizerKind::KCFI) && IsIndirectCall)
+  if (SanOpts.has(SanitizerKind::KCFI) && IsIndirectCall &&
+      !E->isKCFIUnchecked())
     EmitKCFICheck(Callee.getFunctionPointer(),
                   CGM.CreateKCFITypeId(QualType(FnType, 0)));
 
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -4598,6 +4598,13 @@
                     EmitCallee(Call->getCallee()), Call, ReturnValue,
                     EmitScalarExpr(Chain));
   }
+
+  case Builtin::BI__builtin_call_kcfi_unchecked: {
+    const CallExpr *Call = cast<CallExpr>(E->getArg(0));
+    CGCallee Callee = EmitCallee(Call->getCallee());
+    return EmitCall(Call->getCallee()->getType(), Callee, Call, ReturnValue);
+  }
+
   case Builtin::BI_InterlockedExchange8:
   case Builtin::BI_InterlockedExchange16:
   case Builtin::BI_InterlockedExchange:
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9864,6 +9864,9 @@
 def err_second_argument_to_cwsc_not_pointer : Error<
   "second argument to __builtin_call_with_static_chain must be of pointer type">;
 
+def err_kcfi_unchecked_argument : Error<
+  "argument to __builtin_call_kcfi_unchecked must be an indirect call expression">;
+
 def err_vector_incorrect_num_initializers : Error<
   "%select{too many|too few}0 elements in vector initialization (expected %1 elements, have %2)">;
 def err_altivec_empty_initializer : Error<"expected initializer">;
Index: clang/include/clang/Basic/Builtins.def
===================================================================
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -1604,6 +1604,7 @@
 BUILTIN(__builtin_char_memchr, "c*cC*iz", "n")
 BUILTIN(__builtin_dump_struct, "ivC*v*", "tn")
 BUILTIN(__builtin_preserve_access_index, "v.", "t")
+BUILTIN(__builtin_call_kcfi_unchecked, "v.", "nt")
 
 // Alignment builtins (uses custom parsing to support pointers and integers)
 BUILTIN(__builtin_is_aligned, "bvC*z", "nct")
Index: clang/include/clang/AST/Stmt.h
===================================================================
--- clang/include/clang/AST/Stmt.h
+++ clang/include/clang/AST/Stmt.h
@@ -468,8 +468,11 @@
     /// True if the call expression has some floating-point features.
     unsigned HasFPFeatures : 1;
 
+    /// True if a KCFI check should be omitted for the call expression
+    unsigned IsKCFIUnchecked : 1;
+
     /// Padding used to align OffsetToTrailingObjects to a byte multiple.
-    unsigned : 24 - 3 - NumExprBits;
+    unsigned : 24 - 4 - NumExprBits;
 
     /// The offset in bytes from the this pointer to the start of the
     /// trailing objects belonging to CallExpr. Intentionally byte sized
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -2962,6 +2962,10 @@
 
   bool hasStoredFPFeatures() const { return CallExprBits.HasFPFeatures; }
 
+  void setKCFIUnchecked(bool V = true) { CallExprBits.IsKCFIUnchecked = V; }
+
+  bool isKCFIUnchecked() const { return CallExprBits.IsKCFIUnchecked; }
+
   Decl *getCalleeDecl() { return getCallee()->getReferencedDeclOfCallee(); }
   const Decl *getCalleeDecl() const {
     return getCallee()->getReferencedDeclOfCallee();
Index: clang/docs/LanguageExtensions.rst
===================================================================
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2327,6 +2327,38 @@
 
 Query for this feature with ``__has_builtin(__builtin_call_with_static_chain)``.
 
+``__builtin_call_kcfi_unchecked``
+------------------------------------
+
+``__builtin_call_kcfi_unchecked`` is used to perform an indirect function call
+without :ref:`KCFI<kcfi>` type checking when the program is compiled with
+``-fsanitize=kcfi``.
+
+**Syntax**:
+
+.. code-block:: c++
+
+  T __builtin_call_kcfi_unchecked(T expr)
+
+**Example of Use**:
+
+.. code-block:: c++
+
+  int f1(void) { ... }
+
+  int (*p)(void) = f1;
+  int n;
+
+  n = p(); // checked
+  n = __builtin_call_kcfi_unchecked(p()); // unchecked
+
+**Description**:
+
+This builtin returns ``expr`` after checking that ``expr`` is an indirect call
+expression, without emitting type checks for the call.
+
+Query for this feature with ``__has_builtin(__builtin_call_kcfi_unchecked)``.
+
 ``__builtin_readcyclecounter``
 ------------------------------
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to