[CodeGen] Ensure relative vtables use llvm.type.checked.load.relative #126785

PiJoules · 2025-02-11T19:31:48Z

This intrinsic is used when whole program vtables is used in conjunction with either CFI or virtual function elimination. The llvm.type.checked.load is unconditionally used, but we need to use the relative intrinsic for WPD and CFI to work correctly.

llvmbot · 2025-02-11T19:32:24Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: None (PiJoules)

Changes

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

4 Files Affected:

(modified) clang/lib/CodeGen/CGClass.cpp (+5-1)
(modified) clang/lib/CodeGen/CGVTables.h (+6-4)
(modified) clang/lib/CodeGen/ItaniumCXXABI.cpp (+8-6)
(modified) clang/test/CodeGenCXX/type-metadata.cpp (+11-6)

diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 7a1096fcbca82..e4e1b640460bf 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -2937,9 +2937,13 @@ llvm::Value *CodeGenFunction::EmitVTableTypeCheckedLoad(
       CGM.CreateMetadataIdentifierForType(QualType(RD->getTypeForDecl(), 0));
   llvm::Value *TypeId = llvm::MetadataAsValue::get(CGM.getLLVMContext(), MD);
 
+  auto checked_load = CGM.getVTables().useRelativeLayout()
+                          ? llvm::Intrinsic::type_checked_load_relative
+                          : llvm::Intrinsic::type_checked_load;
   llvm::Value *CheckedLoad = Builder.CreateCall(
-      CGM.getIntrinsic(llvm::Intrinsic::type_checked_load),
+      CGM.getIntrinsic(checked_load),
       {VTable, llvm::ConstantInt::get(Int32Ty, VTableByteOffset), TypeId});
+
   llvm::Value *CheckResult = Builder.CreateExtractValue(CheckedLoad, 1);
 
   std::string TypeName = RD->getQualifiedNameAsString();
diff --git a/clang/lib/CodeGen/CGVTables.h b/clang/lib/CodeGen/CGVTables.h
index c06bf7a525d9f..5c45e355fb145 100644
--- a/clang/lib/CodeGen/CGVTables.h
+++ b/clang/lib/CodeGen/CGVTables.h
@@ -75,10 +75,6 @@ class CodeGenVTables {
                             bool vtableHasLocalLinkage,
                             bool isCompleteDtor) const;
 
-  bool useRelativeLayout() const;
-
-  llvm::Type *getVTableComponentType() const;
-
 public:
   /// Add vtable components for the given vtable layout to the given
   /// global initializer.
@@ -151,6 +147,12 @@ class CodeGenVTables {
 
   /// Specify a global should not be instrumented with hwasan.
   void RemoveHwasanMetadata(llvm::GlobalValue *GV) const;
+
+  /// Return the type used as components for a vtable.
+  llvm::Type *getVTableComponentType() const;
+
+  /// Return true if the relative vtable layout is used.
+  bool useRelativeLayout() const;
 };
 
 } // end namespace CodeGen
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 7375a511809b9..181f90f7d14fc 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2187,12 +2187,14 @@ CGCallee ItaniumCXXABI::getVirtualFunctionPointer(CodeGenFunction &CGF,
   uint64_t VTableIndex = CGM.getItaniumVTableContext().getMethodVTableIndex(GD);
   llvm::Value *VFunc, *VTableSlotPtr = nullptr;
   auto &Schema = CGM.getCodeGenOpts().PointerAuth.CXXVirtualFunctionPointers;
+
+  llvm::Type *ComponentTy = CGM.getVTables().getVTableComponentType();
+  uint64_t ByteOffset =
+      VTableIndex * CGM.getDataLayout().getTypeSizeInBits(ComponentTy) / 8;
+
   if (!Schema && CGF.ShouldEmitVTableTypeCheckedLoad(MethodDecl->getParent())) {
-    VFunc = CGF.EmitVTableTypeCheckedLoad(
-        MethodDecl->getParent(), VTable, PtrTy,
-        VTableIndex *
-            CGM.getContext().getTargetInfo().getPointerWidth(LangAS::Default) /
-            8);
+    VFunc = CGF.EmitVTableTypeCheckedLoad(MethodDecl->getParent(), VTable,
+                                          PtrTy, ByteOffset);
   } else {
     CGF.EmitTypeMetadataCodeForVCall(MethodDecl->getParent(), VTable, Loc);
 
@@ -2200,7 +2202,7 @@ CGCallee ItaniumCXXABI::getVirtualFunctionPointer(CodeGenFunction &CGF,
     if (CGM.getItaniumVTableContext().isRelativeLayout()) {
       VFuncLoad = CGF.Builder.CreateCall(
           CGM.getIntrinsic(llvm::Intrinsic::load_relative, {CGM.Int32Ty}),
-          {VTable, llvm::ConstantInt::get(CGM.Int32Ty, 4 * VTableIndex)});
+          {VTable, llvm::ConstantInt::get(CGM.Int32Ty, ByteOffset)});
     } else {
       VTableSlotPtr = CGF.Builder.CreateConstInBoundsGEP1_64(
           PtrTy, VTable, VTableIndex, "vfn");
diff --git a/clang/test/CodeGenCXX/type-metadata.cpp b/clang/test/CodeGenCXX/type-metadata.cpp
index 4ddfc3c2e3a8f..1cb2fed8db3e6 100644
--- a/clang/test/CodeGenCXX/type-metadata.cpp
+++ b/clang/test/CodeGenCXX/type-metadata.cpp
@@ -30,7 +30,7 @@
 // RUN: %clang_cc1 -fexperimental-relative-c++-abi-vtables -O2 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM-OPT --check-prefix=RV-OPT-LAYOUT %s
 
 // Tests for cfi + whole-program-vtables:
-// RUN: %clang_cc1 -fexperimental-relative-c++-abi-vtables -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-VT --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-COMMON-MD --check-prefix=RV-MD --check-prefix=TC-ITANIUM %s
+// RUN: %clang_cc1 -fexperimental-relative-c++-abi-vtables -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-VT --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-COMMON-MD --check-prefix=RV-MD --check-prefix=TC-ITANIUM-RV %s
 
 // Tests that type metadata are annotated on vtables with `-profile-instrument=llvm` (which is equivalent to clang driver option `-fprofile-generate` without `-fcs-profile-generate`):
 // - In clang driver, `-fprofile-instrument` cc1 option is set to 'llvm' iff clang driver option `-fprofile-generate{,=}` is taking effect.
@@ -178,7 +178,8 @@ void af(A *a) {
   // TT-ITANIUM-HIDDEN: [[P:%[^ ]*]] = call i1 @llvm.type.test(ptr [[VT:%[^ ]*]], metadata !"_ZTS1A")
   // TT-ITANIUM-DEFAULT: [[P:%[^ ]*]] = call i1 @llvm.public.type.test(ptr [[VT:%[^ ]*]], metadata !"_ZTS1A")
   // TT-MS: [[P:%[^ ]*]] = call i1 @llvm.type.test(ptr [[VT:%[^ ]*]], metadata !"?AUA@@")
-  // TC-ITANIUM: [[PAIR:%[^ ]*]] = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 0, metadata !"_ZTS1A")
+  // TC-ITANIUM:    [[PAIR:%[^ ]*]] = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 0, metadata !"_ZTS1A")
+  // TC-ITANIUM-RV: [[PAIR:%[^ ]*]] = call { ptr, i1 } @llvm.type.checked.load.relative(ptr {{%[^ ]*}}, i32 0, metadata !"_ZTS1A")
   // TC-MS: [[PAIR:%[^ ]*]] = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 0, metadata !"?AUA@@")
   // CFI-VT: [[P:%[^ ]*]] = extractvalue { ptr, i1 } [[PAIR]], 1
   // DIAG-NEXT: [[VTVALID0:%[^ ]*]] = call i1 @llvm.type.test(ptr [[VT]], metadata !"all-vtables")
@@ -211,7 +212,8 @@ void df1(D *d) {
   // TT-ITANIUM-HIDDEN: {{%[^ ]*}} = call i1 @llvm.type.test(ptr {{%[^ ]*}}, metadata ![[DTYPE:[0-9]+]])
   // TT-ITANIUM-DEFAULT: {{%[^ ]*}} = call i1 @llvm.type.test(ptr {{%[^ ]*}}, metadata ![[DTYPE:[0-9]+]])
   // TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(ptr {{%[^ ]*}}, metadata !"?AUA@@")
-  // TC-ITANIUM: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 0, metadata ![[DTYPE:[0-9]+]])
+  // TC-ITANIUM:    {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 0, metadata ![[DTYPE:[0-9]+]])
+  // TC-ITANIUM-RV: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load.relative(ptr {{%[^ ]*}}, i32 0, metadata ![[DTYPE:[0-9]+]])
   // TC-MS: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 0, metadata !"?AUA@@")
   d->f();
 }
@@ -222,7 +224,8 @@ void dg1(D *d) {
   // TT-ITANIUM-HIDDEN: {{%[^ ]*}} = call i1 @llvm.type.test(ptr {{%[^ ]*}}, metadata !"_ZTS1B")
   // TT-ITANIUM-DEFAULT: {{%[^ ]*}} = call i1 @llvm.public.type.test(ptr {{%[^ ]*}}, metadata !"_ZTS1B")
   // TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(ptr {{%[^ ]*}}, metadata !"?AUB@@")
-  // TC-ITANIUM: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 8, metadata !"_ZTS1B")
+  // TC-ITANIUM:    {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 8, metadata !"_ZTS1B")
+  // TC-ITANIUM-RV: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load.relative(ptr {{%[^ ]*}}, i32 4, metadata !"_ZTS1B")
   // TC-MS: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 0, metadata !"?AUB@@")
   d->g();
 }
@@ -233,7 +236,8 @@ void dh1(D *d) {
   // TT-ITANIUM-HIDDEN: {{%[^ ]*}} = call i1 @llvm.type.test(ptr {{%[^ ]*}}, metadata ![[DTYPE]])
   // TT-ITANIUM-DEFAULT: {{%[^ ]*}} = call i1 @llvm.type.test(ptr {{%[^ ]*}}, metadata ![[DTYPE]])
   // TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(ptr {{%[^ ]*}}, metadata ![[DTYPE:[0-9]+]])
-  // TC-ITANIUM: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 16, metadata ![[DTYPE]])
+  // TC-ITANIUM:    {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 16, metadata ![[DTYPE]])
+  // TC-ITANIUM-RV: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load.relative(ptr {{%[^ ]*}}, i32 8, metadata ![[DTYPE]])
   // TC-MS: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 8, metadata ![[DTYPE:[0-9]+]])
   d->h();
 }
@@ -293,7 +297,8 @@ void f(D *d) {
   // TT-ITANIUM-HIDDEN: {{%[^ ]*}} = call i1 @llvm.type.test(ptr {{%[^ ]*}}, metadata !"_ZTSN5test21DE")
   // TT-ITANIUM-DEFAULT: {{%[^ ]*}} = call i1 @llvm.public.type.test(ptr {{%[^ ]*}}, metadata !"_ZTSN5test21DE")
   // TT-MS: {{%[^ ]*}} = call i1 @llvm.type.test(ptr {{%[^ ]*}}, metadata !"?AUA@test2@@")
-  // TC-ITANIUM: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 8, metadata !"_ZTSN5test21DE")
+  // TC-ITANIUM:    {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 8, metadata !"_ZTSN5test21DE")
+  // TC-ITANIUM-RV: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load.relative(ptr {{%[^ ]*}}, i32 4, metadata !"_ZTSN5test21DE")
   // TC-MS: {{%[^ ]*}} = call { ptr, i1 } @llvm.type.checked.load(ptr {{%[^ ]*}}, i32 0, metadata !"?AUA@test2@@")
   d->m_fn1();
 }

PiJoules · 2025-02-19T18:57:04Z

ping for reviews

pcc · 2025-02-20T03:18:46Z

clang/lib/CodeGen/CGClass.cpp

@@ -2937,9 +2937,13 @@ llvm::Value *CodeGenFunction::EmitVTableTypeCheckedLoad(
      CGM.CreateMetadataIdentifierForType(QualType(RD->getTypeForDecl(), 0));
  llvm::Value *TypeId = llvm::MetadataAsValue::get(CGM.getLLVMContext(), MD);

+  auto checked_load = CGM.getVTables().useRelativeLayout()


CheckedLoad?

It doesn't need to be in this change but it would be nice to simplify things here a bit and instead of having three accessors for the relative vtables bit (CGM.getLangOpts().RelativeCXXABIVTables, CGM.getItaniumVTableContext().isRelativeLayout(), CGM.getVTables().useRelativeLayout()), just have one. IMO that should be the one on LangOpts and the driver should make sure it isn't set to true with the MS ABI.

CheckedLoad?

Done

It doesn't need to be in this change but it would be nice to simplify things here a bit and instead of having three accessors for the relative vtables bit (CGM.getLangOpts().RelativeCXXABIVTables, CGM.getItaniumVTableContext().isRelativeLayout(), CGM.getVTables().useRelativeLayout()), just have one. IMO that should be the one on LangOpts and the driver should make sure it isn't set to true with the MS ABI.

I agree. I'll make a change for this after this lands

github-actions · 2025-02-20T19:55:59Z

✅ With the latest revision this PR passed the C/C++ code formatter.

petrhosek

Can you update the PR description and explain why this is desirable?

PiJoules · 2025-02-26T23:04:03Z

Can you update the PR description and explain why this is desirable?

Done. I'll give it a few days for others to chime in before landing.

…llvm#126785) This intrinsic is used when whole program vtables is used in conjunction with either CFI or virtual function elimination. The `llvm.type.checked.load` is unconditionally used, but we need to use the relative intrinsic for WPD and CFI to work correctly.

llvmbot added clang Clang issues not falling into any other category clang:codegen IR generation bugs: mangling, exceptions, etc. labels Feb 11, 2025

PiJoules requested review from zygoloid, pcc and rjmccall February 11, 2025 19:32

PiJoules requested a review from MaskRay February 13, 2025 19:54

PiJoules force-pushed the relative-vtables-checked.load.relative branch from faf20c7 to 4642bdb Compare February 18, 2025 20:22

pcc reviewed Feb 20, 2025

View reviewed changes

PiJoules force-pushed the relative-vtables-checked.load.relative branch from 4642bdb to 3f31a63 Compare February 20, 2025 19:52

PiJoules requested a review from pcc February 20, 2025 19:53

PiJoules force-pushed the relative-vtables-checked.load.relative branch from 3f31a63 to 864b0a0 Compare February 20, 2025 20:00

[CodeGen] Ensure relative vtables use llvm.type.checked.load.relative

07c7c06

PiJoules force-pushed the relative-vtables-checked.load.relative branch from 864b0a0 to 07c7c06 Compare February 20, 2025 22:08

petrhosek approved these changes Feb 26, 2025

View reviewed changes

Merge branch 'main' into relative-vtables-checked.load.relative

c88fdc1

PiJoules merged commit d9edca4 into llvm:main Feb 28, 2025
11 checks passed

PiJoules deleted the relative-vtables-checked.load.relative branch February 28, 2025 20:26

PiJoules mentioned this pull request May 9, 2025

[clang] Consolidate the relative vtable layout getters #139315

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[CodeGen] Ensure relative vtables use llvm.type.checked.load.relative #126785

[CodeGen] Ensure relative vtables use llvm.type.checked.load.relative #126785

Uh oh!

PiJoules commented Feb 11, 2025 •

edited

Loading

Uh oh!

llvmbot commented Feb 11, 2025 •

edited

Loading

Uh oh!

PiJoules commented Feb 19, 2025

Uh oh!

pcc Feb 20, 2025

Uh oh!

PiJoules Feb 20, 2025

Uh oh!

github-actions bot commented Feb 20, 2025 •

edited

Loading

Uh oh!

petrhosek left a comment

Uh oh!

PiJoules commented Feb 26, 2025

Uh oh!

Uh oh!

Uh oh!

[CodeGen] Ensure relative vtables use llvm.type.checked.load.relative #126785

[CodeGen] Ensure relative vtables use llvm.type.checked.load.relative #126785

Uh oh!

Conversation

PiJoules commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PiJoules commented Feb 19, 2025

Uh oh!

pcc Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

PiJoules Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrhosek left a comment

Choose a reason for hiding this comment

Uh oh!

PiJoules commented Feb 26, 2025

Uh oh!

Uh oh!

Uh oh!

PiJoules commented Feb 11, 2025 •

edited

Loading

llvmbot commented Feb 11, 2025 •

edited

Loading

github-actions bot commented Feb 20, 2025 •

edited

Loading