[PATCH] D60305: ELF: De-template SharedFile. NFCI.

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 5 13:52:05 PDT 2019


pcc added inline comments.


================
Comment at: lld/ELF/InputFiles.cpp:985
 
-  return Verdefs;
-}
-
-// We do not usually care about alignments of data in shared object
-// files because the loader takes care of it. However, if we promote a
-// DSO symbol to point to .bss due to copy relocation, we need to keep
-// the original alignment requirements. We infer it in this function.
-template <class ELFT>
-uint32_t SharedFile<ELFT>::getAlignment(ArrayRef<Elf_Shdr> Sections,
-                                        const Elf_Sym &Sym) {
-  uint64_t Ret = UINT64_MAX;
-  if (Sym.st_value)
-    Ret = 1ULL << countTrailingZeros((uint64_t)Sym.st_value);
-  if (0 < Sym.st_shndx && Sym.st_shndx < Sections.size())
-    Ret = std::min<uint64_t>(Ret, Sections[Sym.st_shndx].sh_addralign);
-  return (Ret > UINT32_MAX) ? 0 : Ret;
-}
+  // Parse ".gnu.version" section which is a parallel array for the symbol
+  // table. If a given file doesn't have a ".gnu.version" section, we use
----------------
ruiu wrote:
> Even though this function is not logically complicated, I'd split it into smaller pieces just because it's a bit too long.
Okay, I split out a couple of functions that don't need a lot of state from the parse() function. It seemed awkward to split out more than that because of the amount of state that would need to be passed in, but maybe you can give it a try.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60305





More information about the llvm-commits mailing list