[llvm] [BOLT][AArch64] Fix static binary patching for ELF. (PR #97710)

[Paschalis Mpeis via llvm-commits]

https://github.com/paschalis-mpeis updated https://github.com/llvm/llvm-project/pull/97710

>From a9962ed5890f1ae0b374b4e8113cb0c6092cb9ec Mon Sep 17 00:00:00 2001
From: Paschalis Mpeis <Paschalis.Mpeis at arm.com>
Date: Tue, 2 Jul 2024 15:28:09 +0100
Subject: [PATCH 1/2] Statically linked AArch64 binaries crash at loading time.

Executing such a binary would result in:
> Unexpected reloc type in static binary.
> [1]    1234 abort (core dumped)
---
 bolt/test/AArch64/patch-elfstatic-libc.test | 15 +++++++++++++++
 bolt/test/Inputs/main.c                     |  3 +++
 2 files changed, 18 insertions(+)
 create mode 100644 bolt/test/AArch64/patch-elfstatic-libc.test
 create mode 100644 bolt/test/Inputs/main.c

diff --git a/bolt/test/AArch64/patch-elfstatic-libc.test b/bolt/test/AArch64/patch-elfstatic-libc.test
new file mode 100644
index 00000000000000..07dc13ab096d82
--- /dev/null
+++ b/bolt/test/AArch64/patch-elfstatic-libc.test
@@ -0,0 +1,15 @@
+// Tests that statically linked AArch64 binaries do not crash at binary loading
+// time.
+//
+// Functions like `_init` do not appear in the `.text` section of the original
+// binary. For those cases, the BOLT'ed binary creates an alias (ie, `.init`),
+// which  matches the address of the relevant function in the original binary.
+// If such function appears in the GOT and is patched, it leads to a runtime
+// crash.
+
+REQUIRES: system-linux
+
+RUN: %clang %p/../Inputs/main.c -o %t -Wl,-q -static
+RUN: llvm-bolt %t -o %t.bolt > /dev/null 2>&1
+RUN: not --crash %t.bolt 2>&1 | FileCheck %s
+CHECK: Unexpected reloc type in static binary.
diff --git a/bolt/test/Inputs/main.c b/bolt/test/Inputs/main.c
new file mode 100644
index 00000000000000..7bfe92adb92d3d
--- /dev/null
+++ b/bolt/test/Inputs/main.c
@@ -0,0 +1,3 @@
+// dummy function just for emitting relocations to the linker.
+int foo() { return 0; }
+int main(int argc, char **argv) { return foo() + 1; }

>From fe4d176f261536c948bc8164be967944fc9f25fd Mon Sep 17 00:00:00 2001
From: Paschalis Mpeis <Paschalis.Mpeis at arm.com>
Date: Mon, 8 Jul 2024 16:36:54 +0100
Subject: [PATCH 2/2] [WORKAROUND][BOLT][AArch64] Static binary patching for
 ELF.

When patching statically linked binaries, avoid patching GOT entries
that did not belong to the original text section and had an alias.

One such special case is the '_init' function that belongs to the '.init'
section. It has '.init' as an alias, which points to the same address of
'_init' in the original binary.

This was observed with GNU linker. BOLT normally rejects these cases.
See issue:
https://github.com/llvm/llvm-project/issues/100096
---
 bolt/lib/Rewrite/RewriteInstance.cpp        | 28 +++++++++++++++++++++
 bolt/test/AArch64/patch-elfstatic-libc.test |  5 ++--
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index ded2f577237fef..0ded964f6de3dc 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -5265,6 +5265,34 @@ void RewriteInstance::patchELFGOT(ELFObjectFile<ELFT> *File) {
                                                      GOTContents.size());
        ++GOTEntry) {
     if (uint64_t NewAddress = getNewFunctionAddress(*GOTEntry)) {
+      auto *Function = BC->getBinaryFunctionAtAddress(*GOTEntry);
+
+      // WORKAROUND:
+      // Background:
+      // Static binaries generated with GNU linker have GOT entries, which is
+      // wrong. LLD does not suffer from this. See related discussion:
+      // https://github.com/llvm/llvm-project/issues/100096
+      //
+      // The current approach is for BOLT to abort when a static binary contains
+      // such entries. However, this patch may serve as a 'Workaround' in case
+      // someone has encountered such a binary. ATTOW it is unclear if/when GNU
+      // linker will have this fixed.
+      //
+      // The workaround:
+      // On static binaries, avoid patching the "_init" got entry. It also
+      // checks that it does not belong to the original text section and that it
+      // an alias. This function actually aliases '.init', belongs to the
+      // '.init' section, and points to the same address of '_init' in the
+      // original binary.
+      if (BC->IsStaticExecutable && !Function->Aliases.empty() &&
+          Function->getOriginSectionName() != ".bolt.org.text" &&
+          (Function->getOneName() == "_init")) {
+        LLVM_DEBUG(dbgs() << "BOLT-DEBUG: ignoring GOT entry 0x"
+                          << Twine::utohexstr(*GOTEntry) << " for '"
+                          << Function->getOneName() << "'" << '\n');
+        continue;
+      }
+
       LLVM_DEBUG(dbgs() << "BOLT-DEBUG: patching GOT entry 0x"
                         << Twine::utohexstr(*GOTEntry) << " with 0x"
                         << Twine::utohexstr(NewAddress) << '\n');
diff --git a/bolt/test/AArch64/patch-elfstatic-libc.test b/bolt/test/AArch64/patch-elfstatic-libc.test
index 07dc13ab096d82..66c8be8aed683b 100644
--- a/bolt/test/AArch64/patch-elfstatic-libc.test
+++ b/bolt/test/AArch64/patch-elfstatic-libc.test
@@ -11,5 +11,6 @@ REQUIRES: system-linux
 
 RUN: %clang %p/../Inputs/main.c -o %t -Wl,-q -static
 RUN: llvm-bolt %t -o %t.bolt > /dev/null 2>&1
-RUN: not --crash %t.bolt 2>&1 | FileCheck %s
-CHECK: Unexpected reloc type in static binary.
+RUN: not %t.bolt 2>&1 | FileCheck %s --allow-empty
+
+CHECK-NOT: Unexpected reloc type in static binary.