[clang] [llvm] [PAC] Fix address discrimination for type info vtable pointers (PR #102199)

Daniil Kovalev via cfe-commits cfe-commits at lists.llvm.org
Sun Sep 29 22:25:09 PDT 2024


https://github.com/kovdan01 updated https://github.com/llvm/llvm-project/pull/102199

>From 0c20bcdf35f3c15024986da50cafb2a8c155e3cf Mon Sep 17 00:00:00 2001
From: Daniil Kovalev <dkovalev at accesssoftek.com>
Date: Tue, 6 Aug 2024 20:48:02 +0300
Subject: [PATCH 1/3] [PAC] Fix address discrimination for type info vtable
 pointers

In #99726, `-fptrauth-type-info-vtable-pointer-discrimination` was introduced,
which is intended to enable type and address discrimination for type_info
vtable pointers. However, some codegen logic for actually enabling address
discrimination was missing. This patch addresses the issue.

Fixes #101716
---
 clang/lib/CodeGen/ItaniumCXXABI.cpp           | 20 ++++++++++++++++---
 .../CodeGenCXX/ptrauth-type-info-vtable.cpp   |  2 +-
 llvm/include/llvm/IR/Constants.h              | 18 +++++++++++------
 .../AArch64/ptrauth-type-info-vptr-discr.ll   | 20 +++++++++++++++++++
 4 files changed, 50 insertions(+), 10 deletions(-)
 create mode 100644 llvm/test/CodeGen/AArch64/ptrauth-type-info-vptr-discr.ll

diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 9b5227772125b2..13aeb3e75d6843 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -3955,9 +3955,23 @@ void ItaniumRTTIBuilder::BuildVTablePointer(const Type *Ty) {
                                                           VTable, Two);
   }
 
-  if (auto &Schema = CGM.getCodeGenOpts().PointerAuth.CXXTypeInfoVTablePointer)
-    VTable = CGM.getConstantSignedPointer(VTable, Schema, nullptr, GlobalDecl(),
-                                          QualType(Ty, 0));
+  if (const auto &Schema =
+          CGM.getCodeGenOpts().PointerAuth.CXXTypeInfoVTablePointer) {
+    llvm::PointerType *PtrTy = llvm::PointerType::get(
+        CGM.getLLVMContext(),
+        CGM.getModule().getDataLayout().getProgramAddressSpace());
+    llvm::Constant *StorageAddress =
+        (Schema.isAddressDiscriminated()
+             ? llvm::ConstantExpr::getIntToPtr(
+                   llvm::ConstantInt::get(
+                       CGM.IntPtrTy,
+                       llvm::ConstantPtrAuth::
+                           AddrDiscriminator_CXXTypeInfoVTablePointer),
+                   PtrTy)
+             : nullptr);
+    VTable = CGM.getConstantSignedPointer(VTable, Schema, StorageAddress,
+                                          GlobalDecl(), QualType(Ty, 0));
+  }
 
   Fields.push_back(VTable);
 }
diff --git a/clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp b/clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp
index 174aeda89d1755..61eef73b5be2a4 100644
--- a/clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp
+++ b/clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp
@@ -65,7 +65,7 @@ extern "C" int disc_std_type_info = __builtin_ptrauth_string_discriminator("_ZTV
 
 // NODISC: @_ZTI10TestStruct = constant { ptr, ptr } { ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2), ptr @_ZTS10TestStruct }, align 8
 
-// DISC: @_ZTI10TestStruct = constant { ptr, ptr } { ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 [[STDTYPEINFO_DISC]]), ptr @_ZTS10TestStruct }, align 8
+// DISC: @_ZTI10TestStruct = constant { ptr, ptr } { ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 [[STDTYPEINFO_DISC]], ptr inttoptr (i64 1 to ptr)), ptr @_ZTS10TestStruct }, align 8
 
 struct TestStruct {
   virtual ~TestStruct();
diff --git a/llvm/include/llvm/IR/Constants.h b/llvm/include/llvm/IR/Constants.h
index 2788751e8b62a1..aaa1c197651a66 100644
--- a/llvm/include/llvm/IR/Constants.h
+++ b/llvm/include/llvm/IR/Constants.h
@@ -1056,12 +1056,18 @@ class ConstantPtrAuth final : public Constant {
     return !getAddrDiscriminator()->isNullValue();
   }
 
-  /// A constant value for the address discriminator which has special
-  /// significance to ctors/dtors lowering. Regular address discrimination can't
-  /// be applied for them since uses of llvm.global_{c|d}tors are disallowed
-  /// (see Verifier::visitGlobalVariable) and we can't emit getelementptr
-  /// expressions referencing these special arrays.
-  enum { AddrDiscriminator_CtorsDtors = 1 };
+  /// Constant values for the address discriminator which have special
+  /// significance to lowering in some contexts.
+  /// - For ctors/dtors, regular address discrimination can't
+  ///   be applied for them since uses of llvm.global_{c|d}tors are disallowed
+  ///   (see Verifier::visitGlobalVariable) and we can't emit getelementptr
+  ///   expressions referencing these special arrays.
+  /// - For vtable pointers of std::type_info and classes derived from it,
+  ///   we do not know the storage address when emitting ptrauth constant.
+  enum {
+    AddrDiscriminator_CtorsDtors = 1,
+    AddrDiscriminator_CXXTypeInfoVTablePointer = 1
+  };
 
   /// Whether the address uses a special address discriminator.
   /// These discriminators can't be used in real pointer-auth values; they
diff --git a/llvm/test/CodeGen/AArch64/ptrauth-type-info-vptr-discr.ll b/llvm/test/CodeGen/AArch64/ptrauth-type-info-vptr-discr.ll
new file mode 100644
index 00000000000000..b25a32f856b7ab
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/ptrauth-type-info-vptr-discr.ll
@@ -0,0 +1,20 @@
+; RUN: llc -mtriple aarch64-linux-gnu    -mattr=+pauth -filetype=asm -o - %s | FileCheck --check-prefix=ELF %s
+; RUN: llc -mtriple aarch64-apple-darwin -mattr=+pauth -filetype=asm -o - %s | FileCheck --check-prefix=MACHO %s
+
+; ELF-LABEL:   _ZTI10Disc:
+; ELF-NEXT:      .xword  (_ZTVN10__cxxabiv117__class_type_infoE+16)@AUTH(da,45546,addr)
+; ELF-LABEL:   _ZTI10NoDisc:
+; ELF-NEXT:      .xword  (_ZTVN10__cxxabiv117__class_type_infoE+16)@AUTH(da,45546)
+
+; MACHO-LABEL: __ZTI10Disc:
+; MACHO-NEXT:    .quad   (__ZTVN10__cxxabiv117__class_type_infoE+16)@AUTH(da,45546,addr)
+; MACHO-LABEL: __ZTI10NoDisc:
+; MACHO-NEXT:    .quad   (__ZTVN10__cxxabiv117__class_type_infoE+16)@AUTH(da,45546)
+
+ at _ZTVN10__cxxabiv117__class_type_infoE = external global [0 x ptr]
+
+ at _ZTS10Disc   = constant [4 x i8] c"Disc", align 1
+ at _ZTI10Disc   = constant { ptr, ptr } { ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 45546, ptr inttoptr (i64 1 to ptr)), ptr @_ZTS10Disc }, align 8
+
+ at _ZTS10NoDisc = constant [6 x i8] c"NoDisc", align 1
+ at _ZTI10NoDisc = constant { ptr, ptr } { ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 45546), ptr @_ZTS10NoDisc }, align 8

>From 130ff558e146a0da1ed542b62b9cb46b4d102327 Mon Sep 17 00:00:00 2001
From: Daniil Kovalev <dkovalev at accesssoftek.com>
Date: Mon, 16 Sep 2024 18:19:25 +0300
Subject: [PATCH 2/3] Use real addr instead of placeholder for signed type info
 vt ptrs

---
 clang/lib/CodeGen/ItaniumCXXABI.cpp           | 43 +++++++++++++------
 .../CodeGenCXX/ptrauth-type-info-vtable.cpp   |  2 +-
 llvm/include/llvm/IR/Constants.h              | 18 +++-----
 .../AArch64/ptrauth-type-info-vptr-discr.ll   |  2 +-
 4 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 21168cd485ef12..ed253be1ad257a 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -3975,18 +3975,9 @@ void ItaniumRTTIBuilder::BuildVTablePointer(const Type *Ty) {
 
   if (const auto &Schema =
           CGM.getCodeGenOpts().PointerAuth.CXXTypeInfoVTablePointer) {
-    llvm::PointerType *PtrTy = llvm::PointerType::get(
-        CGM.getLLVMContext(),
-        CGM.getModule().getDataLayout().getProgramAddressSpace());
-    llvm::Constant *StorageAddress =
-        (Schema.isAddressDiscriminated()
-             ? llvm::ConstantExpr::getIntToPtr(
-                   llvm::ConstantInt::get(
-                       CGM.IntPtrTy,
-                       llvm::ConstantPtrAuth::
-                           AddrDiscriminator_CXXTypeInfoVTablePointer),
-                   PtrTy)
-             : nullptr);
+    // If address discrimination is enabled, we'll re-write that to actual
+    // storage address later in ItaniumRTTIBuilder::BuildTypeInfo.
+    llvm::Constant *StorageAddress = nullptr;
     VTable = CGM.getConstantSignedPointer(VTable, Schema, StorageAddress,
                                           GlobalDecl(), QualType(Ty, 0));
   }
@@ -4107,6 +4098,7 @@ llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(
       llvm::GlobalValue::DLLStorageClassTypes DLLStorageClass) {
   // Add the vtable pointer.
   BuildVTablePointer(cast<Type>(Ty));
+  size_t VTablePointerIdx = Fields.size() - 1;
 
   // And the name.
   llvm::GlobalVariable *TypeName = GetAddrOfTypeName(Ty, Linkage);
@@ -4222,7 +4214,6 @@ llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(
   }
 
   llvm::Constant *Init = llvm::ConstantStruct::getAnon(Fields);
-
   SmallString<256> Name;
   llvm::raw_svector_ostream Out(Name);
   CGM.getCXXABI().getMangleContext().mangleCXXRTTI(Ty, Out);
@@ -4231,6 +4222,32 @@ llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(
   llvm::GlobalVariable *GV =
       new llvm::GlobalVariable(M, Init->getType(),
                                /*isConstant=*/true, Linkage, Init, Name);
+  if (const auto &Schema =
+          CGM.getCodeGenOpts().PointerAuth.CXXTypeInfoVTablePointer) {
+    if (Schema.isAddressDiscriminated()) {
+      // If type info vtable pointer is signed with address discrimination
+      // enabled, we need to place actual storage address (which was unknown
+      // during construction in ItaniumRTTIBuilder::BuildVTablePointer) in the
+      // corresponding field.
+      ConstantInitBuilder Builder(CGM);
+      auto InitBuilder = Builder.beginStruct();
+      for (size_t I = 0; I < Fields.size(); ++I) {
+        if (I != VTablePointerIdx) {
+          InitBuilder.add(Fields[I]);
+          continue;
+        }
+        auto *SignedVTablePointer = cast<llvm::ConstantPtrAuth>(Fields[I]);
+        llvm::Constant *UnsignedVtablePointer =
+            SignedVTablePointer->getPointer();
+        llvm::Constant *StorageAddress =
+            InitBuilder.getAddrOfCurrentPosition(CGM.UnqualPtrTy);
+        InitBuilder.add(CGM.getConstantSignedPointer(
+            UnsignedVtablePointer, Schema, StorageAddress, GlobalDecl(),
+            QualType(cast<Type>(Ty), 0)));
+      }
+      InitBuilder.finishAndSetAsInitializer(GV);
+    }
+  }
 
   // Export the typeinfo in the same circumstances as the vtable is exported.
   auto GVDLLStorageClass = DLLStorageClass;
diff --git a/clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp b/clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp
index 61eef73b5be2a4..bb692484c94ece 100644
--- a/clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp
+++ b/clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp
@@ -65,7 +65,7 @@ extern "C" int disc_std_type_info = __builtin_ptrauth_string_discriminator("_ZTV
 
 // NODISC: @_ZTI10TestStruct = constant { ptr, ptr } { ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2), ptr @_ZTS10TestStruct }, align 8
 
-// DISC: @_ZTI10TestStruct = constant { ptr, ptr } { ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 [[STDTYPEINFO_DISC]], ptr inttoptr (i64 1 to ptr)), ptr @_ZTS10TestStruct }, align 8
+// DISC: @_ZTI10TestStruct = constant { ptr, ptr } { ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 [[STDTYPEINFO_DISC]], ptr @_ZTI10TestStruct), ptr @_ZTS10TestStruct }, align 8
 
 struct TestStruct {
   virtual ~TestStruct();
diff --git a/llvm/include/llvm/IR/Constants.h b/llvm/include/llvm/IR/Constants.h
index a49d13b6568696..3b16aa039a5087 100644
--- a/llvm/include/llvm/IR/Constants.h
+++ b/llvm/include/llvm/IR/Constants.h
@@ -1067,18 +1067,12 @@ class ConstantPtrAuth final : public Constant {
     return !getAddrDiscriminator()->isNullValue();
   }
 
-  /// Constant values for the address discriminator which have special
-  /// significance to lowering in some contexts.
-  /// - For ctors/dtors, regular address discrimination can't
-  ///   be applied for them since uses of llvm.global_{c|d}tors are disallowed
-  ///   (see Verifier::visitGlobalVariable) and we can't emit getelementptr
-  ///   expressions referencing these special arrays.
-  /// - For vtable pointers of std::type_info and classes derived from it,
-  ///   we do not know the storage address when emitting ptrauth constant.
-  enum {
-    AddrDiscriminator_CtorsDtors = 1,
-    AddrDiscriminator_CXXTypeInfoVTablePointer = 1
-  };
+  /// A constant value for the address discriminator which has special
+  /// significance to ctors/dtors lowering. Regular address discrimination can't
+  /// be applied for them since uses of llvm.global_{c|d}tors are disallowed
+  /// (see Verifier::visitGlobalVariable) and we can't emit getelementptr
+  /// expressions referencing these special arrays.
+  enum { AddrDiscriminator_CtorsDtors = 1 };
 
   /// Whether the address uses a special address discriminator.
   /// These discriminators can't be used in real pointer-auth values; they
diff --git a/llvm/test/CodeGen/AArch64/ptrauth-type-info-vptr-discr.ll b/llvm/test/CodeGen/AArch64/ptrauth-type-info-vptr-discr.ll
index b25a32f856b7ab..704cccdc1107a1 100644
--- a/llvm/test/CodeGen/AArch64/ptrauth-type-info-vptr-discr.ll
+++ b/llvm/test/CodeGen/AArch64/ptrauth-type-info-vptr-discr.ll
@@ -14,7 +14,7 @@
 @_ZTVN10__cxxabiv117__class_type_infoE = external global [0 x ptr]
 
 @_ZTS10Disc   = constant [4 x i8] c"Disc", align 1
- at _ZTI10Disc   = constant { ptr, ptr } { ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 45546, ptr inttoptr (i64 1 to ptr)), ptr @_ZTS10Disc }, align 8
+ at _ZTI10Disc   = constant { ptr, ptr } { ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 45546, ptr @_ZTI10Disc), ptr @_ZTS10Disc }, align 8
 
 @_ZTS10NoDisc = constant [6 x i8] c"NoDisc", align 1
 @_ZTI10NoDisc = constant { ptr, ptr } { ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 45546), ptr @_ZTS10NoDisc }, align 8

>From 55fc2c822f8f543d335e443a13860ae72fb5ce6d Mon Sep 17 00:00:00 2001
From: Daniil Kovalev <dkovalev at accesssoftek.com>
Date: Mon, 30 Sep 2024 07:51:09 +0300
Subject: [PATCH 3/3] Address review comments

---
 clang/lib/CodeGen/ItaniumCXXABI.cpp | 35 +++++++++++------------------
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index ed253be1ad257a..43a4a22abc7b2f 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -4098,7 +4098,8 @@ llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(
       llvm::GlobalValue::DLLStorageClassTypes DLLStorageClass) {
   // Add the vtable pointer.
   BuildVTablePointer(cast<Type>(Ty));
-  size_t VTablePointerIdx = Fields.size() - 1;
+  assert(Fields.size() == 1);
+  size_t VTablePointerIdx = 0;
 
   // And the name.
   llvm::GlobalVariable *TypeName = GetAddrOfTypeName(Ty, Linkage);
@@ -4213,15 +4214,14 @@ llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(
     break;
   }
 
-  llvm::Constant *Init = llvm::ConstantStruct::getAnon(Fields);
   SmallString<256> Name;
   llvm::raw_svector_ostream Out(Name);
   CGM.getCXXABI().getMangleContext().mangleCXXRTTI(Ty, Out);
   llvm::Module &M = CGM.getModule();
   llvm::GlobalVariable *OldGV = M.getNamedGlobal(Name);
-  llvm::GlobalVariable *GV =
-      new llvm::GlobalVariable(M, Init->getType(),
-                               /*isConstant=*/true, Linkage, Init, Name);
+  llvm::GlobalVariable *GV = new llvm::GlobalVariable(
+      M, llvm::ConstantStruct::getTypeForElements(Fields),
+      /*isConstant=*/true, Linkage, /*Initializer=*/nullptr, Name);
   if (const auto &Schema =
           CGM.getCodeGenOpts().PointerAuth.CXXTypeInfoVTablePointer) {
     if (Schema.isAddressDiscriminated()) {
@@ -4229,25 +4229,16 @@ llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(
       // enabled, we need to place actual storage address (which was unknown
       // during construction in ItaniumRTTIBuilder::BuildVTablePointer) in the
       // corresponding field.
-      ConstantInitBuilder Builder(CGM);
-      auto InitBuilder = Builder.beginStruct();
-      for (size_t I = 0; I < Fields.size(); ++I) {
-        if (I != VTablePointerIdx) {
-          InitBuilder.add(Fields[I]);
-          continue;
-        }
-        auto *SignedVTablePointer = cast<llvm::ConstantPtrAuth>(Fields[I]);
-        llvm::Constant *UnsignedVtablePointer =
-            SignedVTablePointer->getPointer();
-        llvm::Constant *StorageAddress =
-            InitBuilder.getAddrOfCurrentPosition(CGM.UnqualPtrTy);
-        InitBuilder.add(CGM.getConstantSignedPointer(
-            UnsignedVtablePointer, Schema, StorageAddress, GlobalDecl(),
-            QualType(cast<Type>(Ty), 0)));
-      }
-      InitBuilder.finishAndSetAsInitializer(GV);
+      llvm::Constant *UnsignedVtablePointer =
+          cast<llvm::ConstantPtrAuth>(Fields[VTablePointerIdx])->getPointer();
+      assert(VTablePointerIdx == 0 && "Expected 0 offset for StorageAddress");
+      llvm::Constant *StorageAddress = GV;
+      Fields[VTablePointerIdx] = CGM.getConstantSignedPointer(
+          UnsignedVtablePointer, Schema, StorageAddress, GlobalDecl(),
+          QualType(cast<Type>(Ty), 0));
     }
   }
+  GV->replaceInitializer(llvm::ConstantStruct::getAnon(Fields));
 
   // Export the typeinfo in the same circumstances as the vtable is exported.
   auto GVDLLStorageClass = DLLStorageClass;



More information about the cfe-commits mailing list