Skip to content

[BOLT] Support relative vtable #135449

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.

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 3 commits into from
Apr 14, 2025
Merged

[BOLT] Support relative vtable #135449

merged 3 commits into from
Apr 14, 2025

Conversation

yozhu
Copy link
Contributor

@yozhu yozhu commented Apr 11, 2025

To handle relative vftable, which is enabled with clang option
-fexperimental-relative-c++-abi-vtables, we look for PC relative
relocations whose fixup locations fall in vtable address ranges.
For such relocations, actual target is just virtual function itself,
and the addend is to record the distance between vtable slot for
target virtual function and the first virtual function slot in vtable,
which is to match generated code that calls virtual function. So
we can skip the logic of handling "function + offset" and directly
save such relocations for future fixup after new layout is known.

Summary:
To handle relative vftable (which is enabled with clang option
`-fexperimental-relative-c++-abi-vtables`), we look for PC relative
relocations whose fixup locations fall in vftable address ranges. For such
relocations actual target is just virtual function itself and the addend
is to record distance between vftable slot for target virtual function and
the first virtual function slot in vftable, which is to match generated code
calling virtual function. So we can skip the logic on handling
"function + offset" and directly record such relocations for future fixup
after new layout is known.
@llvmbot
Copy link
Member

llvmbot commented Apr 11, 2025

@llvm/pr-subscribers-bolt

Author: YongKang Zhu (yozhu)

Changes

To handle relative vftable, which is enabled with clang option
-fexperimental-relative-c++-abi-vtables, we look for PC relative
relocations whose fixup locations fall in vtable address ranges.
For such relocations, actual target is just virtual function itself,
and the addend is to record the distance between vtable slot for
target virtual function and the first virtual function slot in vtable,
which is to match generated code that calls virtual function. So
we can skip the logic of handling "function + offset" and directly
save such relocations for future fixup after new layout is known.


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

3 Files Affected:

  • (modified) bolt/lib/Core/Relocation.cpp (+4)
  • (modified) bolt/lib/Rewrite/RewriteInstance.cpp (+29-3)
  • (added) bolt/test/runtime/relative-vftable.cpp (+57)
diff --git a/bolt/lib/Core/Relocation.cpp b/bolt/lib/Core/Relocation.cpp
index 4696a1f1f0402..ff1681f823987 100644
--- a/bolt/lib/Core/Relocation.cpp
+++ b/bolt/lib/Core/Relocation.cpp
@@ -96,6 +96,7 @@ static bool isSupportedAArch64(uint32_t Type) {
   case ELF::R_AARCH64_MOVW_UABS_G2:
   case ELF::R_AARCH64_MOVW_UABS_G2_NC:
   case ELF::R_AARCH64_MOVW_UABS_G3:
+  case ELF::R_AARCH64_PLT32:
     return true;
   }
 }
@@ -202,6 +203,7 @@ static size_t getSizeForTypeAArch64(uint32_t Type) {
   case ELF::R_AARCH64_MOVW_UABS_G2_NC:
   case ELF::R_AARCH64_MOVW_UABS_G3:
   case ELF::R_AARCH64_ABS32:
+  case ELF::R_AARCH64_PLT32:
     return 4;
   case ELF::R_AARCH64_ABS64:
   case ELF::R_AARCH64_PREL64:
@@ -354,6 +356,7 @@ static uint64_t extractValueAArch64(uint32_t Type, uint64_t Contents,
   case ELF::R_AARCH64_PREL16:
     return static_cast<int64_t>(PC) + SignExtend64<16>(Contents & 0xffff);
   case ELF::R_AARCH64_PREL32:
+  case ELF::R_AARCH64_PLT32:
     return static_cast<int64_t>(PC) + SignExtend64<32>(Contents & 0xffffffff);
   case ELF::R_AARCH64_PREL64:
     return static_cast<int64_t>(PC) + Contents;
@@ -676,6 +679,7 @@ static bool isPCRelativeAArch64(uint32_t Type) {
   case ELF::R_AARCH64_PREL16:
   case ELF::R_AARCH64_PREL32:
   case ELF::R_AARCH64_PREL64:
+  case ELF::R_AARCH64_PLT32:
     return true;
   }
 }
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 23faa92642d01..70a175eec2900 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -2603,7 +2603,9 @@ void RewriteInstance::readRelocations(const SectionRef &Section) {
 void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection,
                                        const RelocationRef &Rel) {
   const bool IsAArch64 = BC->isAArch64();
+  const bool IsX86 = BC->isX86();
   const bool IsFromCode = RelocatedSection.isText();
+  const bool IsWritable = BinarySection(*BC, RelocatedSection).isWritable();
 
   SmallString<16> TypeName;
   Rel.getTypeName(TypeName);
@@ -2612,7 +2614,7 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection,
     return;
 
   // Adjust the relocation type as the linker might have skewed it.
-  if (BC->isX86() && (RType & ELF::R_X86_64_converted_reloc_bit)) {
+  if (IsX86 && (RType & ELF::R_X86_64_converted_reloc_bit)) {
     if (opts::Verbosity >= 1)
       dbgs() << "BOLT-WARNING: ignoring R_X86_64_converted_reloc_bit\n";
     RType &= ~ELF::R_X86_64_converted_reloc_bit;
@@ -2620,7 +2622,7 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection,
 
   if (Relocation::isTLS(RType)) {
     // No special handling required for TLS relocations on X86.
-    if (BC->isX86())
+    if (IsX86)
       return;
 
     // The non-got related TLS relocations on AArch64 and RISC-V also could be
@@ -2661,6 +2663,30 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection,
     return;
   }
 
+  if (!IsFromCode && !IsWritable && (IsX86 || IsAArch64) &&
+      Relocation::isPCRelative(RType)) {
+    BinaryData *BD = BC->getBinaryDataContainingAddress(Rel.getOffset());
+    if (BD && (BD->nameStartsWith("_ZTV") ||   // vtable
+               BD->nameStartsWith("_ZTCN"))) { // construction vtable
+      BinaryFunction *BF = BC->getBinaryFunctionContainingAddress(
+          SymbolAddress, /*CheckPastEnd*/ false, /*UseMaxSize*/ true);
+      if (!BF || BF->getAddress() != SymbolAddress) {
+        BC->errs()
+            << "BOLT-ERROR: the virtual function table entry at offset 0x"
+            << Twine::utohexstr(Rel.getOffset());
+        if (BF)
+          BC->errs() << " points to the middle of a function @ 0x"
+                     << Twine::utohexstr(BF->getAddress()) << "\n";
+        else
+          BC->errs() << " does not point to any function\n";
+        exit(1);
+      }
+      BC->addRelocation(Rel.getOffset(), BF->getSymbol(), RType, Addend,
+                        ExtractedValue);
+      return;
+    }
+  }
+
   const uint64_t Address = SymbolAddress + Addend;
 
   LLVM_DEBUG({
@@ -2724,7 +2750,7 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection,
   const bool IsToCode = ReferencedSection && ReferencedSection->isText();
 
   // Special handling of PC-relative relocations.
-  if (BC->isX86() && Relocation::isPCRelative(RType)) {
+  if (IsX86 && Relocation::isPCRelative(RType)) {
     if (!IsFromCode && IsToCode) {
       // PC-relative relocations from data to code are tricky since the
       // original information is typically lost after linking, even with
diff --git a/bolt/test/runtime/relative-vftable.cpp b/bolt/test/runtime/relative-vftable.cpp
new file mode 100644
index 0000000000000..f4c14c30f971f
--- /dev/null
+++ b/bolt/test/runtime/relative-vftable.cpp
@@ -0,0 +1,57 @@
+// Test bolt instrumentation is able to handle relative virtual function table,
+// i.e., when code is compiled with `-fexperimental-relative-c++-abi-vtables`.
+
+// REQUIRES: system-linux,bolt-runtime
+
+// RUN: split-file %s %t
+// RUN: %clang -fuse-ld=lld -o %t/main.so %t/tt.cpp %t/main.cpp -Wl,-q \
+// RUN:     -fno-rtti -fexperimental-relative-c++-abi-vtables
+// RUN: %t/main.so | FileCheck %s
+
+// CHECK: derived_foo
+// CHECK-NEXT: derived_bar
+// CHECK-NEXT: derived_goo
+
+// RUN: llvm-bolt --instrument %t/main.so -o %t/main.instr.so
+// RUN: %t/main.instr.so | FileCheck %s
+
+;--- tt.h
+#include <stdio.h>
+
+class Base {
+public:
+  virtual void foo();
+  virtual void bar();
+  virtual void goo();
+};
+
+class Derived : public Base {
+public:
+  virtual void foo() override;
+  virtual void bar() override;
+  virtual void goo() override;
+};
+
+;--- tt.cpp
+#include "tt.h"
+void Derived::goo() { printf("derived_goo\n"); }
+
+;--- main.cpp
+#include "tt.h"
+// #pragma clang optimize off
+
+void Base::foo() { printf("base_foo\n"); }
+void Base::bar() { printf("base_bar\n"); }
+void Base::goo() { printf("base_goo\n"); }
+
+void Derived::foo() { printf("derived_foo\n"); }
+void Derived::bar() { printf("derived_bar\n"); }
+
+int main() {
+  Derived D;
+  Base *ptr = &D;
+  ptr->foo();
+  ptr->bar();
+  ptr->goo();
+  return 0;
+}

Copy link

github-actions bot commented Apr 11, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff HEAD~1 HEAD --extensions cpp -- bolt/test/runtime/relative-vftable.cpp bolt/lib/Core/Relocation.cpp bolt/lib/Rewrite/RewriteInstance.cpp
View the diff from clang-format here.
diff --git a/bolt/test/runtime/relative-vftable.cpp b/bolt/test/runtime/relative-vftable.cpp
index 10de94a2e..1cca61256 100644
--- a/bolt/test/runtime/relative-vftable.cpp
+++ b/bolt/test/runtime/relative-vftable.cpp
@@ -15,10 +15,11 @@
 // RUN: llvm-bolt %t/main.so -o %t/main.bolted.so --trap-old-code
 // RUN: %t/main.bolted.so | FileCheck %s
 
-;--- tt.h
+;
+-- -tt.h
 #include <stdio.h>
 
-class Base {
+    class Base {
 public:
   virtual void foo();
   virtual void bar();
@@ -32,15 +33,23 @@ public:
   virtual void goo() override;
 };
 
-;--- tt.cpp
+;
+-- -tt.cpp
 #include "tt.h"
-void Derived::goo() { printf("derived_goo\n"); }
+    void
+    Derived::goo() {
+  printf("derived_goo\n");
+}
 
-;--- main.cpp
+;
+-- -main.cpp
 #include "tt.h"
 #pragma clang optimize off
 
-void Base::foo() { printf("base_foo\n"); }
+    void
+    Base::foo() {
+  printf("base_foo\n");
+}
 void Base::bar() { printf("base_bar\n"); }
 void Base::goo() { printf("base_goo\n"); }
 

Copy link
Contributor

@maksfb maksfb left a comment

Choose a reason for hiding this comment

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

Couple of nits for the test case. Otherwise LGTM. Thanks!

// Test bolt instrumentation is able to handle relative virtual function table,
// i.e., when code is compiled with `-fexperimental-relative-c++-abi-vtables`.

// REQUIRES: system-linux,bolt-runtime
Copy link
Contributor

Choose a reason for hiding this comment

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

Can drop bolt-runtime without instrumentation.

@@ -0,0 +1,57 @@
// Test bolt instrumentation is able to handle relative virtual function table,
Copy link
Contributor

Choose a reason for hiding this comment

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

s/instrumentation/processing/

@yozhu yozhu merged commit 2a83c0c into llvm:main Apr 14, 2025
9 of 10 checks passed
@yozhu yozhu deleted the relvtab branch April 14, 2025 17:26
bcardosolopes added a commit to bcardosolopes/llvm-project that referenced this pull request Apr 14, 2025
* origin/main: (199 commits)
  [NFC][AsmPrinter] Refactor AsmPrinter and AArch64AsmPrinter to prepare for jump table partitions on aarch64 (llvm#125993)
  [HEXAGON] Fix corner cases for hwloops pass (llvm#135439)
  [flang] Handle volatility in lowering and codegen (llvm#135311)
  [MLIR][Shape] Support >2 args in `shape.broadcast` folder (llvm#126808)
  [DirectX] Use scalar arguments for @llvm.dx.dot intrinsics (llvm#134570)
  Remove the redundant check for "WeakPtr" in isSmartPtrClass to fix the issue 135612. (llvm#135629)
  [BOLT] Support relative vtable (llvm#135449)
  [flang] Fix linking to libMLIR (llvm#135483)
  [AsmPrinter] Link .section_sizes to the correct section (llvm#135583)
  [ctxprof] Handle instrumenting functions with `musttail` calls (llvm#135121)
  [SystemZ] Consider VST/VL as SimpleBDXStore/Load (llvm#135623)
  [libc++][CI] Pin the XCode version. (llvm#135412)
  [lldb-dap] Fix win32 build. (llvm#135638)
  [Interp] Mark inline-virtual.cpp as unsupported with ASan (llvm#135402)
  [libc++] Removes the _LIBCPP_VERBOSE_ABORT_NOT_NOEXCEPT macro. (llvm#135494)
  [CVP] Add tests for ucmp/scmp with switch (NFC)
  [mlir][tosa] Align AbsOp example variable names (llvm#135268)
  [mlir][tosa] Align AddOp examples to spec (llvm#135266)
  [mlir][tosa] Align RFFT2d and FFT2d operator examples (llvm#135261)
  [flang][OpenMP][HLFIR] Support vector subscripted array sections for DEPEND (llvm#133892)
  ...
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
To handle relative vftable, which is enabled with clang option
`-fexperimental-relative-c++-abi-vtables`, we look for PC relative
relocations whose fixup locations fall in vtable address ranges.
For such relocations, actual target is just virtual function itself,
and the addend is to record the distance between vtable slot for
target virtual function and the first virtual function slot in vtable,
which is to match generated code that calls virtual function. So
we can skip the logic of handling "function + offset" and directly
save such relocations for future fixup after new layout is known.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants