Skip to content

[PAC] Fix address discrimination for type info vtable pointers #102199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Oct 18, 2024
Merged

[PAC] Fix address discrimination for type info vtable pointers #102199

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0c20bcd
[PAC] Fix address discrimination for type info vtable pointers
kovdan01 Aug 6, 2024
e0beb3f
Merge branch 'main' into pauth-type-info-addr-discr-fix
kovdan01 Sep 16, 2024
130ff55
Use real addr instead of placeholder for signed type info vt ptrs
kovdan01 Sep 16, 2024
969a877
Merge branch 'main' into pauth-type-info-addr-discr-fix
kovdan01 Sep 29, 2024
55fc2c8
Address review comments
kovdan01 Sep 30, 2024
90d6355
Merge branch 'main' into pauth-type-info-addr-discr-fix
kovdan01 Oct 17, 2024
b08172f
Construct GV before BuildVTablePointer call
kovdan01 Oct 17, 2024
c9d26dd
Fix tests
kovdan01 Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 30 additions & 8 deletions clang/lib/CodeGen/ItaniumCXXABI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3973,9 +3973,14 @@ 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) {
// 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));
}

Fields.push_back(VTable);
}
Expand Down Expand Up @@ -4093,6 +4098,8 @@ llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(
llvm::GlobalValue::DLLStorageClassTypes DLLStorageClass) {
// Add the vtable pointer.
BuildVTablePointer(cast<Type>(Ty));
assert(Fields.size() == 1);
size_t VTablePointerIdx = 0;

// And the name.
llvm::GlobalVariable *TypeName = GetAddrOfTypeName(Ty, Linkage);
Expand Down Expand Up @@ -4207,16 +4214,31 @@ 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),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're going to replace the initializer anyway, you don't need to call ConstantStruct::getTypeForElements here; you can just use an arbitrary type, like i8. That should allow you to pass the global into BuildVTablePointer(), instead of replacing it later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@efriedma-quic Thanks for suggestion! Followed this way in b08172f. This actually changes order of globals definitions in output IR and requires changing related tests correspondingly (see c9d26dd), but, as far as I understand, this should not cause any problems.

/*isConstant=*/true, Linkage, /*Initializer=*/nullptr, 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.
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;
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 @_ZTI10TestStruct), ptr @_ZTS10TestStruct }, align 8

struct TestStruct {
virtual ~TestStruct();
Expand Down
20 changes: 20 additions & 0 deletions llvm/test/CodeGen/AArch64/ptrauth-type-info-vptr-discr.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 @_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
Loading