[clang] Ensure pointers passed to runtime support functions are correctly signed

[ojhunt]

Updates codegen for global destructors and raising exceptions to ensure that the function pointers being passed are signed using the correct schema.

Notably this requires that CodeGenFunction::createAtExitStub to return an opaque Constant* rather than a Function* as the value being emitted is no longer necessarily a raw function pointer depending on the configured ABI.

Co-Authored-By: Akira Hatanaka [email protected]
Co-Authored-By: John McCall [email protected]

[llvmbot]

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Oliver Hunt (ojhunt)

Changes

Updates codegen for global destructors and raising exceptions to ensure that the function pointers being passed are signed using the correct schema.

Notably this requires that CodeGenFunction::createAtExitStub to return an opaque Constant* rather than a Function* as the value being emitted is no longer necessarily a raw function pointer depending on the configured ABI.

Co-Authored-By: Akira Hatanaka <[email protected]>
Co-Authored-By: John McCall <[email protected]>

---

Full diff: https://github.com/llvm/llvm-project/pull/98276.diff

5 Files Affected:

• (modified) clang/lib/CodeGen/CGDeclCXX.cpp (+9-3)
• (modified) clang/lib/CodeGen/CodeGenFunction.h (+1-1)
• (modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+19-2)
• (added) clang/test/CodeGenCXX/ptrauth-static-destructors.cpp (+24)
• (added) clang/test/CodeGenCXX/ptrauth-throw.cpp (+20)

diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 05dd7ddb86fa6..2f56355cff90e 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -232,7 +232,7 @@ void CodeGenFunction::EmitCXXGlobalVarDeclInit(const VarDecl &D,
 
 /// Create a stub function, suitable for being passed to atexit,
 /// which passes the given address to the given destructor function.
-llvm::Function *CodeGenFunction::createAtExitStub(const VarDecl &VD,
+llvm::Constant *CodeGenFunction::createAtExitStub(const VarDecl &VD,
                                                   llvm::FunctionCallee dtor,
                                                   llvm::Constant *addr) {
   // Get the destructor function type, void(*)(void).
@@ -264,7 +264,12 @@ llvm::Function *CodeGenFunction::createAtExitStub(const VarDecl &VD,
 
   CGF.FinishFunction();
 
-  return fn;
+  // Get a proper function pointer.
+  FunctionProtoType::ExtProtoInfo EPI(getContext().getDefaultCallingConvention(
+      /*IsVariadic=*/false, /*IsCXXMethod=*/false));
+  QualType fnType = getContext().getFunctionType(getContext().VoidTy,
+                                                 {getContext().VoidPtrTy}, EPI);
+  return CGM.getFunctionPointer(fn, fnType);
 }
 
 /// Create a stub function, suitable for being passed to __pt_atexit_np,
@@ -333,7 +338,8 @@ void CodeGenFunction::registerGlobalDtorWithLLVM(const VarDecl &VD,
                                                  llvm::FunctionCallee Dtor,
                                                  llvm::Constant *Addr) {
   // Create a function which calls the destructor.
-  llvm::Function *dtorStub = createAtExitStub(VD, Dtor, Addr);
+  llvm::Function *dtorStub =
+      cast<llvm::Function>(createAtExitStub(VD, Dtor, Addr));
   CGM.AddGlobalDtor(dtorStub);
 }
 
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 13f12b5d878a6..e33267c4787fd 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -4862,7 +4862,7 @@ class CodeGenFunction : public CodeGenTypeCache {
   void EmitCXXGlobalVarDeclInit(const VarDecl &D, llvm::GlobalVariable *GV,
                                 bool PerformInit);
 
-  llvm::Function *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee Dtor,
+  llvm::Constant *createAtExitStub(const VarDecl &VD, llvm::FunctionCallee Dtor,
                                    llvm::Constant *Addr);
 
   llvm::Function *createTLSAtExitStub(const VarDecl &VD,
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index e1d056765a866..489051e95953e 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1321,8 +1321,16 @@ void ItaniumCXXABI::emitThrow(CodeGenFunction &CGF, const CXXThrowExpr *E) {
   if (const RecordType *RecordTy = ThrowType->getAs<RecordType>()) {
     CXXRecordDecl *Record = cast<CXXRecordDecl>(RecordTy->getDecl());
     if (!Record->hasTrivialDestructor()) {
+      // __cxa_throw is declared to take its destructor as void (*)(void *). We
+      // must match that if function pointers can be authenticated with a
+      // discriminator based on their type.
+      ASTContext &Ctx = getContext();
+      QualType DtorTy = Ctx.getFunctionType(Ctx.VoidTy, {Ctx.VoidPtrTy},
+                                            FunctionProtoType::ExtProtoInfo());
+
       CXXDestructorDecl *DtorD = Record->getDestructor();
       Dtor = CGM.getAddrOfCXXStructor(GlobalDecl(DtorD, Dtor_Complete));
+      Dtor = CGM.getFunctionPointer(Dtor, DtorTy);
     }
   }
   if (!Dtor) Dtor = llvm::Constant::getNullValue(CGM.Int8PtrTy);
@@ -2699,6 +2707,14 @@ static void emitGlobalDtorWithCXAAtExit(CodeGenFunction &CGF,
   if (llvm::Function *fn = dyn_cast<llvm::Function>(atexit.getCallee()))
     fn->setDoesNotThrow();
 
+  auto &Context = CGF.CGM.getContext();
+  FunctionProtoType::ExtProtoInfo EPI(Context.getDefaultCallingConvention(
+      /*IsVariadic=*/false, /*IsCXXMethod=*/false));
+  QualType fnType =
+      Context.getFunctionType(Context.VoidTy, {Context.VoidPtrTy}, EPI);
+  llvm::Constant *dtorCallee = cast<llvm::Constant>(dtor.getCallee());
+  dtorCallee = CGF.CGM.getFunctionPointer(dtorCallee, fnType);
+
   if (!addr)
     // addr is null when we are trying to register a dtor annotated with
     // __attribute__((destructor)) in a constructor function. Using null here is
@@ -2706,7 +2722,7 @@ static void emitGlobalDtorWithCXAAtExit(CodeGenFunction &CGF,
     // function.
     addr = llvm::Constant::getNullValue(CGF.Int8PtrTy);
 
-  llvm::Value *args[] = {dtor.getCallee(), addr, handle};
+  llvm::Value *args[] = {dtorCallee, addr, handle};
   CGF.EmitNounwindRuntimeCall(atexit, args);
 }
 
@@ -4907,7 +4923,8 @@ void XLCXXABI::registerGlobalDtor(CodeGenFunction &CGF, const VarDecl &D,
   }
 
   // Create __dtor function for the var decl.
-  llvm::Function *DtorStub = CGF.createAtExitStub(D, Dtor, Addr);
+  llvm::Function *DtorStub =
+      cast<llvm::Function>(CGF.createAtExitStub(D, Dtor, Addr));
 
   // Register above __dtor with atexit().
   CGF.registerGlobalDtorWithAtExit(DtorStub);
diff --git a/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
new file mode 100644
index 0000000000000..cad43dc0746df
--- /dev/null
+++ b/clang/test/CodeGenCXX/ptrauth-static-destructors.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm -std=c++11 %s -o - \
+// RUN:  | FileCheck %s --check-prefix=CXAATEXIT
+
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -emit-llvm -std=c++11 %s -o - \
+// RUN:    -fno-use-cxa-atexit \
+// RUN:  | FileCheck %s --check-prefix=ATEXIT
+
+class Foo {
+ public:
+  ~Foo() {
+  }
+};
+
+Foo global;
+
+// CXAATEXIT: define internal void @__cxx_global_var_init()
+// CXAATEXIT:   call i32 @__cxa_atexit(ptr ptrauth (ptr @_ZN3FooD1Ev, i32 0), ptr @global, ptr @__dso_handle)
+
+
+// ATEXIT: define internal void @__cxx_global_var_init()
+// ATEXIT:   %{{.*}} = call i32 @atexit(ptr ptrauth (ptr @__dtor_global, i32 0))
+
+// ATEXIT: define internal void @__dtor_global() {{.*}} section "__TEXT,__StaticInit,regular,pure_instructions" {
+// ATEXIT:   %{{.*}} = call ptr @_ZN3FooD1Ev(ptr @global)
diff --git a/clang/test/CodeGenCXX/ptrauth-throw.cpp b/clang/test/CodeGenCXX/ptrauth-throw.cpp
new file mode 100644
index 0000000000000..0e143433ee5a3
--- /dev/null
+++ b/clang/test/CodeGenCXX/ptrauth-throw.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios -fptrauth-calls -fcxx-exceptions -emit-llvm %s -o - | FileCheck %s
+
+class Foo {
+ public:
+  ~Foo() {
+  }
+};
+
+// CHECK-LABEL: define void @_Z1fv()
+// CHECK:  call void @__cxa_throw(ptr %{{.*}}, ptr @_ZTI3Foo, ptr ptrauth (ptr @_ZN3FooD1Ev, i32 0))
+void f() {
+  throw Foo();
+}
+
+// __cxa_throw is defined to take its destructor as "void (*)(void *)" in the ABI.
+// CHECK-LABEL: define void @__cxa_throw({{.*}})
+// CHECK:  call void {{%.*}}(ptr noundef {{%.*}}) [ "ptrauth"(i32 0, i64 0) ]
+extern "C" void __cxa_throw(void *exception, void *, void (*dtor)(void *)) {
+  dtor(exception);
+}

[ojhunt]

I notice the r+, but I haven't fully addressed this feedback - are you suggesting corrections in a follow up patch, or just conceptually approving with this PR? :D

[asl]

I notice the r+, but I haven't fully addressed this feedback - are you suggesting corrections in a follow up patch, or just conceptually approving with this PR? :D

I think this patch is fine to go as-is. We can re-consider that MaybeSigned abstraction further on if it could appear in multiple places.