[PATCH] D105217: [LLD] Adding support for RELA for CG Profile.

[Di Mo via Phabricator via llvm-commits]

modimo added a comment.

Thanks for the fix. Please add a test case of what a stripped binary would look like (yaml2obj probably easiest way to do this) to make sure this works. Also are there llvm-readobj changes that need to be made?



================
Comment at: lld/ELF/Driver.cpp:63
 #include "llvm/Support/raw_ostream.h"
+#include <cstdint>
 #include <cstdlib>
----------------
Needed?


================
Comment at: lld/ELF/Driver.cpp:869
 template <class ELFT> static void readCallGraphsFromObjectFiles() {
-  auto getIndex = [&](ObjFile<ELFT> *obj, uint32_t index) {
-    const Elf_Rel_Impl<ELFT, false> &rel = obj->cgProfileRel[index];
-    return rel.getSymbol(config->isMips64EL);
-  };
+  SmallVector<uint32_t, 128> symbolIndices;
 
----------------
I don't think you need this vector, the getIndex lambda can be generalized as a templated lambda/function based on ArrayRef


================
Comment at: lld/ELF/Driver.cpp:873
     auto *obj = cast<ObjFile<ELFT>>(file);
-    if (obj->cgProfileRel.empty())
+    if (obj->cgProfileRel.empty() && obj->cgProfileRela.empty())
       continue;
----------------
Add a warning + bailout if a binary every has both for some weird reason?


================
Comment at: lld/ELF/InputFiles.h:254
   ArrayRef<Elf_CGProfile> cgProfile;
   // SHT_LLVM_CALL_GRAPH_PROFILE relocations, always in the REL format.
   ArrayRef<Elf_Rel> cgProfileRel;
----------------
Fix up comments to reflect that there is "always" here now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105217/new/

https://reviews.llvm.org/D105217