[PATCH] D149541: [JITLink] Process null symbols

[Job Noorman via Phabricator via llvm-commits]

jobnoorman created this revision.
jobnoorman added a reviewer: lhames.
Herald added subscribers: asb, pmatos, luismarques, s.egerton, PkmX, simoncook, hiraditya, arichardson.
Herald added a project: All.
jobnoorman requested review of this revision.
Herald added subscribers: llvm-commits, pcwang-thead.
Herald added a project: LLVM.

Some relocations (e.g., R_RISCV_ALIGN) don't have a target symbol and
use a null symbol as a placeholder. These symbols were not processed
before making it impossible to create edges for them.

This patch tries to detect these null symbols and create absolute
symbols for them. Note that technically, these null symbols are UND in
the ELF file, not ABS, so it might make more consistent to create a new
symbol type for this (local undefined or so). However, since these
symbols are only used as placeholders (i.e., their values are never
used), I don't think it's worth the effort of doing this.

Also note that in the binaries that I have inspected, this null symbol
always has index 0. Could it make sense to add that to the test to avoid
accidentally adding unnecessary symbols? The reason I didn't do this
yet, is that I couldn't find any references in the specs that actually
guarantee this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149541

Files:
  llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
  llvm/lib/ExecutionEngine/JITLink/JITLink.cpp


Index: llvm/lib/ExecutionEngine/JITLink/JITLink.cpp
===================================================================
--- llvm/lib/ExecutionEngine/JITLink/JITLink.cpp
+++ llvm/lib/ExecutionEngine/JITLink/JITLink.cpp
@@ -133,7 +133,7 @@
   auto &TargetSym = E.getTarget();
   if (TargetSym.hasName())
     OS << TargetSym.getName();
-  else {
+  else if (TargetSym.isDefined()) {
     auto &TargetBlock = TargetSym.getBlock();
     auto &TargetSec = TargetBlock.getSection();
     orc::ExecutorAddr SecAddress(~uint64_t(0));
@@ -149,7 +149,8 @@
     if (TargetSym.getOffset())
       OS << " + " << formatv("{0:x}", TargetSym.getOffset());
     OS << ")";
-  }
+  } else
+    OS << formatv("{0:x}", TargetSym.getAddress());
 
   if (E.getAddend() != 0)
     OS << " + " << E.getAddend();
Index: llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
===================================================================
--- llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
+++ llvm/lib/ExecutionEngine/JITLink/ELFLinkGraphBuilder.h
@@ -540,6 +540,18 @@
       auto &GSym = G->addExternalSymbol(*Name, Sym.st_size,
                                         Sym.getBinding() == ELF::STB_WEAK);
       setGraphSymbol(SymIndex, GSym);
+    } else if (Sym.isUndefined() && Sym.st_value == 0 && Sym.st_size == 0 &&
+               Sym.getType() == ELF::STT_NOTYPE &&
+               Sym.getBinding() == ELF::STB_LOCAL && Name->empty()) {
+      // Some relocations (e.g., R_RISCV_ALIGN) don't have a target symbol and
+      // use this kind of null symbol as a placeholder.
+      LLVM_DEBUG({
+        dbgs() << "      " << SymIndex << ": Creating null graph symbol\n";
+      });
+
+      auto &GSym = G->addAbsoluteSymbol(*Name, orc::ExecutorAddr(0), 0,
+                                        Linkage::Strong, Scope::Local, false);
+      setGraphSymbol(SymIndex, GSym);
     } else {
       LLVM_DEBUG({
         dbgs() << "      " << SymIndex


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D149541.518282.patch
Type: text/x-patch
Size: 1949 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230430/377583a1/attachment.bin>