Add __builtin_kcfi_call_unchecked

[samitolvanen]

Summary

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_kcfi_call_unchecked builtin accepts an indirect call
expression and emits the call without KCFI type checking.

Depends on D119296

[nickdesaulniers]

Can you link to the lore thread on discussions around the builtin?

clang/lib/Sema/SemaChecking.cpp

436

auto *Call =
https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto

451

auto *Builtin =

though I would perhaps not use auto here.

[nickdesaulniers]

clang/lib/Sema/SemaChecking.cpp

436

Also, can this simply be a dyn_cast rather than dyn_cast_or_null? Don't you already check the arg count above?

[samitolvanen]

In D124211#3480652, @nickdesaulniers wrote:

Can you link to the lore thread on discussions around the builtin?

There's no LKML discussion about the built-in, but there were earlier proposals of using an attribute to accomplish the same thing. @pcc suggested using a built-in instead, and the reviewers in D122673 agreed that it's a more appropriate solution. In the kernel, the indirect static_call trampoline calls that are patched into direct calls at boot are the primary reason we need a finer-grained method of disabling CFI checks.

[nickdesaulniers]

In D124211#3480709, @samitolvanen wrote:

In D124211#3480652, @nickdesaulniers wrote:

Can you link to the lore thread on discussions around the builtin?

There's no LKML discussion about the built-in, but there were earlier proposals of using an attribute to accomplish the same thing. @pcc suggested using a built-in instead, and the reviewers in D122673 agreed that it's a more appropriate solution. In the kernel, the indirect static_call trampoline calls that are patched into direct calls at boot are the primary reason we need a finer-grained method of disabling CFI checks.

Perhaps it's time to start a discussion on LKML then, before landing this?

[samitolvanen]

In D124211#3480773, @nickdesaulniers wrote:

Perhaps it's time to start a discussion on LKML then, before landing this?

I don't intend to land these patches until there's a consensus on the correct approach from both projects, and I was hoping to get a confirmation that this API looks reasonable from the LLVM side before sending an RFC series to LKML. Since nobody has objected, I suppose there's no reason to delay that.

[rjmccall]

My only comment about the design is that maybe it should be __builtin_kcfi_call_unchecked, which does not seem like something you need to hold up LKML discussion for.

[samitolvanen]

• Renamed the builtin.
• Addressed Nick's comments.

[samitolvanen]

LKML RFC: https://lore.kernel.org/lkml/[e-mail]/

[samitolvanen]

OK, I confirmed that we won't need this after all. I'll abandon this patch and revisit if it becomes necessary in future.

clang/docs/LanguageExtensions.rst

 This builtin returns ``expr`` after checking that ``expr`` is a non-member
 static call expression. The call to that expression is made while using ``ptr``
 as a function pointer stored in a dedicated register to implement *static chain*
 calling convention, as used by some language to implement closures or nested
 functions.
 
 Query for this feature with ``__has_builtin(__builtin_call_with_static_chain)``.
 
+``__builtin_kcfi_call_unchecked``
+------------------------------------
+
+``__builtin_kcfi_call_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_kcfi_call_unchecked(T expr)
+
+**Example of Use**:
+
+.. code-block:: c++
+
+  int f1(void) { ... }
+
+  int (*p)(void) = f1;
+  int n;
+
+  n = p(); // checked
+  n = __builtin_kcfi_call_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_kcfi_call_unchecked)``.
+
 ``__builtin_readcyclecounter``
 ------------------------------
 
 ``__builtin_readcyclecounter`` is used to access the cycle counter register (or
 a similar low-latency, high-accuracy clock) on those targets that support it.
 
 **Syntax**:
 

clang/include/clang/AST/Expr.h

public:
   }
   void setADLCallKind(ADLCallKind V = UsesADL) {
     CallExprBits.UsesADL = static_cast<bool>(V);
   }
   bool usesADL() const { return getADLCallKind() == UsesADL; }
 
   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();
   }
 
   /// If the callee is a FunctionDecl, return it. Otherwise return null.
   FunctionDecl *getDirectCallee() {
     return dyn_cast_or_null<FunctionDecl>(getCalleeDecl());

clang/include/clang/AST/Stmt.h

class CallExprBitfields {
     unsigned NumPreArgs : 1;
 
     /// True if the callee of the call expression was found using ADL.
     unsigned UsesADL : 1;
 
     /// 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
     /// for faster access.
     unsigned OffsetToTrailingObjects : 8;
   };
   enum { NumCallExprBits = 32 };
 

clang/include/clang/Basic/Builtins.def

 // Clang builtins (not available in GCC).
 BUILTIN(__builtin_addressof, "v*v&", "nct")
 BUILTIN(__builtin_function_start, "v*v&", "nct")
 BUILTIN(__builtin_operator_new, "v*z", "tc")
 BUILTIN(__builtin_operator_delete, "vv*", "tn")
 BUILTIN(__builtin_char_memchr, "c*cC*iz", "n")
 BUILTIN(__builtin_dump_struct, "ivC*v*", "tn")
 BUILTIN(__builtin_preserve_access_index, "v.", "t")
+BUILTIN(__builtin_kcfi_call_unchecked, "v.", "nt")
 
 // Alignment builtins (uses custom parsing to support pointers and integers)
 BUILTIN(__builtin_is_aligned, "bvC*z", "nct")
 BUILTIN(__builtin_align_up, "v*vC*z", "nct")
 BUILTIN(__builtin_align_down, "v*vC*z", "nct")
 
 // Safestack builtins
 BUILTIN(__builtin___get_unsafe_stack_start, "v*", "Fn")

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_first_argument_to_cwsc_block_call : Error<
   "first argument to __builtin_call_with_static_chain must not be a block call">;
 def err_first_argument_to_cwsc_builtin_call : Error<
   "first argument to __builtin_call_with_static_chain must not be a builtin call">;
 def err_first_argument_to_cwsc_pdtor_call : Error<
   "first argument to __builtin_call_with_static_chain must not be a pseudo-destructor call">;
 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_kcfi_call_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">;
 
 def err_invalid_neon_type_code : Error<
   "incompatible constant for this __builtin_neon function">;
 def err_argument_invalid_range : Error<
   "argument value %0 is outside the valid range [%1, %2]">;

clang/lib/CodeGen/CGBuiltin.cpp

case Builtin::BI__noop:
     return RValue::get(ConstantInt::get(IntTy, 0));
   case Builtin::BI__builtin_call_with_static_chain: {
     const CallExpr *Call = cast<CallExpr>(E->getArg(0));
     const Expr *Chain = E->getArg(1);
     return EmitCall(Call->getCallee()->getType(),
                     EmitCallee(Call->getCallee()), Call, ReturnValue,
                     EmitScalarExpr(Chain));
   }
+
+  case Builtin::BI__builtin_kcfi_call_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:
   case Builtin::BI_InterlockedExchangePointer:
     return RValue::get(
         EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedExchange, E));
   case Builtin::BI_InterlockedCompareExchangePointer:
   case Builtin::BI_InterlockedCompareExchangePointer_nf: {

clang/lib/CodeGen/CGExpr.cpp

if (CGM.getCodeGenOpts().SanitizeCfiCrossDso && CrossDsoTypeId) {
                            CastedCallee, StaticData);
     } else {
       EmitCheck(std::make_pair(TypeTest, SanitizerKind::CFIICall),
                 SanitizerHandler::CFICheckFail, StaticData,
                 {CastedCallee, llvm::UndefValue::get(IntPtrTy)});
     }
   }
 
-  if (SanOpts.has(SanitizerKind::KCFI) && IsIndirectCall)
+  if (SanOpts.has(SanitizerKind::KCFI) && IsIndirectCall &&
+      !E->isKCFIUnchecked())
     EmitKCFICheck(Callee.getFunctionPointer(),
                   CGM.CreateKCFITypeId(QualType(FnType, 0)));
 
   CallArgList Args;
   if (Chain)
     Args.add(RValue::get(Builder.CreateBitCast(Chain, CGM.VoidPtrTy)),
              CGM.getContext().VoidPtrTy);
 

clang/lib/Sema/SemaChecking.cpp

static bool SemaBuiltinCallWithStaticChain(Sema &S, CallExpr *BuiltinCall) {
   BuiltinCall->setValueKind(CE->getValueKind());
   BuiltinCall->setObjectKind(CE->getObjectKind());
   BuiltinCall->setCallee(Builtin);
   BuiltinCall->setArg(1, ChainResult.get());
 
   return false;
 }
 
+static bool SemaBuiltinKCFICallUnchecked(Sema &S, CallExpr *BuiltinCall) {
+  if (checkArgCount(S, BuiltinCall, 1))
+    return true;
+
+  auto *Call = dyn_cast<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());
+
+  Expr *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
     : public analyze_format_string::FormatStringHandler {
   // Accepts the argument index (relative to the first destination index) of the
   // argument whose size we want.
   using ComputeSizeFunction =
       llvm::function_ref<Optional<llvm::APSInt>(unsigned)>;
#include "clang/Basic/Builtins.def"
   case Builtin::BI__builtin_preserve_access_index:
     if (SemaBuiltinPreserveAI(*this, TheCall))
       return ExprError();
     break;
   case Builtin::BI__builtin_call_with_static_chain:
     if (SemaBuiltinCallWithStaticChain(*this, TheCall))
       return ExprError();
     break;
+  case Builtin::BI__builtin_kcfi_call_unchecked:
+    if (SemaBuiltinKCFICallUnchecked(*this, TheCall))
+      return ExprError();
+    break;
   case Builtin::BI__exception_code:
   case Builtin::BI_exception_code:
     if (SemaBuiltinSEHScopeCheck(*this, TheCall, Scope::SEHExceptScope,
                                  diag::err_seh___except_block))
       return ExprError();
     break;
   case Builtin::BI__exception_info:
   case Builtin::BI_exception_info:

clang/test/CodeGen/builtin-kcfi-call-unchecked.cpp

+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=kcfi -o - %s | FileCheck %s
+
+#if !__has_builtin(__builtin_kcfi_call_unchecked)
+#error "missing __builtin_kcfi_call_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_kcfi_call_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_kcfi_call_unchecked(({ &A::a; })());
+}

clang/test/SemaCXX/builtins.cpp

namespace addressof {
   static_assert(&pt->n == &t.n, "");
 
   struct U { int n : 5; } u;
   int *pbf = __builtin_addressof(u.n); // expected-error {{address of bit-field requested}}
 
   S *ptmp = __builtin_addressof(S{}); // expected-error {{taking the address of a temporary}}
 }
 
+namespace kcfi_call_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_kcfi_call_unchecked(p1());
+  __builtin_kcfi_call_unchecked(p2());
+  __builtin_kcfi_call_unchecked(p3(p1));
+  __builtin_kcfi_call_unchecked(p1);     // expected-error {{argument to __builtin_kcfi_call_unchecked must be an indirect call expression}}
+  __builtin_kcfi_call_unchecked(p1, p2); // expected-error {{too many arguments to function call, expected 1, have 2}}
+  __builtin_kcfi_call_unchecked(a());    // expected-error {{argument to __builtin_kcfi_call_unchecked must be an indirect call expression}}
+  __builtin_kcfi_call_unchecked(s.f());  // expected-error {{argument to __builtin_kcfi_call_unchecked must be an indirect call expression}}
+  __builtin_kcfi_call_unchecked(S::g()); // expected-error {{argument to __builtin_kcfi_call_unchecked must be an indirect call expression}}
+  static_assert(__is_same(decltype(__builtin_kcfi_call_unchecked(p1())), int), "");
+  static_assert(__is_same(decltype(__builtin_kcfi_call_unchecked(p3(p1))), int &), "");
+}
+} // namespace kcfi_call_unchecked
+
 namespace function_start {
 void a(void) {}
 int n;
 void *p = __builtin_function_start(n);               // expected-error {{argument must be a function}}
 static_assert(__builtin_function_start(a) == a, ""); // expected-error {{static_assert expression is not an integral constant expression}}
 } // namespace function_start
 
 void no_ms_builtins() {