search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.86k stars 11.48k forks source link

[PAC][clang] Fix address discrimination for type_info vtable pointers #101716

Open kovdan01 opened 1 month ago

kovdan01 commented 1 month ago

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 is missing. Particularly, in ItaniumRTTIBuilder::BuildVTablePointer (clang/lib/CodeGen/ItaniumCXXABI.cpp), there is the following piece of code:

  if (auto &Schema = CGM.getCodeGenOpts().PointerAuth.CXXTypeInfoVTablePointer)
    VTable = CGM.getConstantSignedPointer(VTable, Schema, nullptr, GlobalDecl(),
                                          QualType(Ty, 0));

Here, nullptr is used as StorageAddress unconditionally, so, address discrimination is not actually enabled even if requested. It caused test-suite failures in several EH-related tests.

I was able to fix that locally by just using a dummy ptr inttoptr (i64 1 to ptr) value as StorageAddress (just like I did with init/fini, see https://github.com/llvm/llvm-project/pull/96478#issuecomment-2196819332), and tests became passing. I'm not sure how to get a proper StorageAddress here, so I've used that dummy placeholder which actually seems to do the job.

An existing test clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp shows such incorrect behavior. Particularly, in line 55, we have

ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 [[STDTYPEINFO_DISC]])

This has constant discriminator, but does not have address discrimination, while it should be enabled with -fptrauth-type-info-vtable-pointer-discrimination. The correct output should be smth like (if we use a placeholder value ptr inttoptr (i64 1 to ptr) as storage address)

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))
kovdan01 commented 1 month ago

Tagging @asl @ojhunt

llvmbot commented 1 month ago

@llvm/issue-subscribers-clang-driver

Author: Daniil Kovalev (kovdan01)

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 is missing. Particularly, in `ItaniumRTTIBuilder::BuildVTablePointer` (clang/lib/CodeGen/ItaniumCXXABI.cpp), there is the following piece of code: ``` if (auto &Schema = CGM.getCodeGenOpts().PointerAuth.CXXTypeInfoVTablePointer) VTable = CGM.getConstantSignedPointer(VTable, Schema, nullptr, GlobalDecl(), QualType(Ty, 0)); ``` Here, `nullptr` is used as `StorageAddress` unconditionally, so, address discrimination is not actually enabled even if requested. It caused test-suite failures in several EH-related tests. I was able to fix that locally by just using a dummy `ptr inttoptr (i64 1 to ptr)` value as `StorageAddress` (just like I did with init/fini, see https://github.com/llvm/llvm-project/pull/96478#issuecomment-2196819332), and tests became passing. I'm not sure how to get a proper `StorageAddress` here, so I've used that dummy placeholder which actually seems to do the job. An existing test clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp shows such incorrect behavior. Particularly, in line 55, we have ``` ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 [[STDTYPEINFO_DISC]]) ``` This has constant discriminator, but does not have address discrimination, while it should be enabled with `-fptrauth-type-info-vtable-pointer-discrimination`. The correct output should be smth like (if we use a placeholder value `ptr inttoptr (i64 1 to ptr)` as storage address) ``` 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)) ```
llvmbot commented 1 month ago

@llvm/issue-subscribers-bug

Author: Daniil Kovalev (kovdan01)

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 is missing. Particularly, in `ItaniumRTTIBuilder::BuildVTablePointer` (clang/lib/CodeGen/ItaniumCXXABI.cpp), there is the following piece of code: ``` if (auto &Schema = CGM.getCodeGenOpts().PointerAuth.CXXTypeInfoVTablePointer) VTable = CGM.getConstantSignedPointer(VTable, Schema, nullptr, GlobalDecl(), QualType(Ty, 0)); ``` Here, `nullptr` is used as `StorageAddress` unconditionally, so, address discrimination is not actually enabled even if requested. It caused test-suite failures in several EH-related tests. I was able to fix that locally by just using a dummy `ptr inttoptr (i64 1 to ptr)` value as `StorageAddress` (just like I did with init/fini, see https://github.com/llvm/llvm-project/pull/96478#issuecomment-2196819332), and tests became passing. I'm not sure how to get a proper `StorageAddress` here, so I've used that dummy placeholder which actually seems to do the job. An existing test clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp shows such incorrect behavior. Particularly, in line 55, we have ``` ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 [[STDTYPEINFO_DISC]]) ``` This has constant discriminator, but does not have address discrimination, while it should be enabled with `-fptrauth-type-info-vtable-pointer-discrimination`. The correct output should be smth like (if we use a placeholder value `ptr inttoptr (i64 1 to ptr)` as storage address) ``` 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)) ```
llvmbot commented 1 month ago

@llvm/issue-subscribers-clang-codegen

Author: Daniil Kovalev (kovdan01)

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 is missing. Particularly, in `ItaniumRTTIBuilder::BuildVTablePointer` (clang/lib/CodeGen/ItaniumCXXABI.cpp), there is the following piece of code: ``` if (auto &Schema = CGM.getCodeGenOpts().PointerAuth.CXXTypeInfoVTablePointer) VTable = CGM.getConstantSignedPointer(VTable, Schema, nullptr, GlobalDecl(), QualType(Ty, 0)); ``` Here, `nullptr` is used as `StorageAddress` unconditionally, so, address discrimination is not actually enabled even if requested. It caused test-suite failures in several EH-related tests. I was able to fix that locally by just using a dummy `ptr inttoptr (i64 1 to ptr)` value as `StorageAddress` (just like I did with init/fini, see https://github.com/llvm/llvm-project/pull/96478#issuecomment-2196819332), and tests became passing. I'm not sure how to get a proper `StorageAddress` here, so I've used that dummy placeholder which actually seems to do the job. An existing test clang/test/CodeGenCXX/ptrauth-type-info-vtable.cpp shows such incorrect behavior. Particularly, in line 55, we have ``` ptr ptrauth (ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv117__class_type_infoE, i64 2), i32 2, i64 [[STDTYPEINFO_DISC]]) ``` This has constant discriminator, but does not have address discrimination, while it should be enabled with `-fptrauth-type-info-vtable-pointer-discrimination`. The correct output should be smth like (if we use a placeholder value `ptr inttoptr (i64 1 to ptr)` as storage address) ``` 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)) ```