[BOLT] Ignore special symbols as function aliases in updateELFSymbolTable

[aaupov]

Exempt special symbols (hot text/data and _end symbol) from normal
handling. We only need to set their value and make them absolute.

If these symbols are handled as normal symbols and if they alias
functions we may create non-sensical symbols, e.g. __hot_start.cold.

Test Plan: updated hot-end-symbol.s

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

If __hot_start marker aliases split function, we create non-sensical
__hot_start.cold symbols. Similar to how hot markers are ignored when
reading symbol table, explicitly ignore them as function references in
updating the symbol table and only update their values.

Test Plan: updated hot-end-symbol.s

---

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

2 Files Affected:

• (modified) bolt/lib/Rewrite/RewriteInstance.cpp (+9-2)
• (modified) bolt/test/runtime/X86/hot-end-symbol.s (+3-2)

diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index fa5167490923c..0d11bdc46fa5a 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -4788,6 +4788,9 @@ void RewriteInstance::updateELFSymbolTable(
     if (!IsDynSym && shouldStrip(Symbol))
       continue;
 
+    Expected<StringRef> SymbolName = Symbol.getName(StringSection);
+    assert(SymbolName && "cannot get symbol name");
+
     const BinaryFunction *Function =
         BC->getBinaryFunctionAtAddress(Symbol.st_value);
     // Ignore false function references, e.g. when the section address matches
@@ -4795,6 +4798,12 @@ void RewriteInstance::updateELFSymbolTable(
     if (Function && Symbol.getType() == ELF::STT_SECTION)
       Function = nullptr;
 
+    // Ignore input hot markers as function aliases – markers are handled
+    // separately.
+    if (Function &&
+        (*SymbolName == "__hot_start" || *SymbolName == "__hot_end"))
+      Function = nullptr;
+
     // For non-dynamic symtab, make sure the symbol section matches that of
     // the function. It can mismatch e.g. if the symbol is a section marker
     // in which case we treat the symbol separately from the function.
@@ -4906,8 +4915,6 @@ void RewriteInstance::updateELFSymbolTable(
     }
 
     // Handle special symbols based on their name.
-    Expected<StringRef> SymbolName = Symbol.getName(StringSection);
-    assert(SymbolName && "cannot get symbol name");
 
     auto updateSymbolValue = [&](const StringRef Name,
                                  std::optional<uint64_t> Value = std::nullopt) {
diff --git a/bolt/test/runtime/X86/hot-end-symbol.s b/bolt/test/runtime/X86/hot-end-symbol.s
index e6d83d77167ac..f973013b1b3de 100755
--- a/bolt/test/runtime/X86/hot-end-symbol.s
+++ b/bolt/test/runtime/X86/hot-end-symbol.s
@@ -12,7 +12,8 @@
 # RUN: %clang %cflags -no-pie %t.o -o %t.exe -Wl,-q
 
 # RUN: llvm-bolt %t.exe --relocs=1 --hot-text --reorder-functions=hfsort \
-# RUN:    --data %t.fdata -o %t.out | FileCheck %s
+# RUN:    --data %t.fdata -o %t.out --split-functions --split-strategy=all \
+# RUN:    | FileCheck %s
 
 # RUN: %t.out 1
 
@@ -30,12 +31,12 @@
 # CHECK-OUTPUT:       __hot_start
 # CHECK-OUTPUT-NEXT:  main
 # CHECK-OUTPUT-NEXT:  __hot_end
+# CHECK-OUTPUT-NOT:   __hot_start.cold
 
   .text
   .globl  main
   .type main, %function
   .globl  __hot_start
-  .type __hot_start, %object
   .p2align  4
 main:
 __hot_start:

[dcci]

Some comments, thanks for working o this.

[dcci]

is this assert needed? if Expected returns null, it will blow up, no?

[aaupov]

Not really. Let me drop it. Yes, Expected will blow up upon null access, and the assertion message is not more insightful than SymbolName variable itself.

[aaupov]

Expected values in success mode must still be checked prior to being destroyed

So we must keep the assertion.

[dcci]

SG

[dcci]

can you expand this comment -- explaining why we ignore input hot markers?

[dcci]

why are you removing this?

[aaupov]

If __hot_start has %object type, it will be an absolute symbol and won't belong to .text section. When updating symbols, we check that symbol section matches that of the function. So we need hot_start to be a global symbol in .text section.

[dcci]

LG

[maksfb]

LGTM. Thanks.