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 <dkova...@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) + +@_ZTVN10__cxxabiv117__class_type_infoE = external global [0 x ptr] + +@_ZTS10Disc = constant [4 x i8] c"Disc", align 1 +@_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 + +@_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 130ff558e146a0da1ed542b62b9cb46b4d102327 Mon Sep 17 00:00:00 2001 From: Daniil Kovalev <dkova...@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 -@_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 +@_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 <dkova...@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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits