[PATCH] D15191: [ELF] Support PHDRS command

[Rui Ueyama via llvm-commits]

ruiu added a comment.

I'm sorry that this code review was slipped from my inbox.


================
Comment at: ELF/Writer.cpp:43
@@ -42,1 +42,3 @@
 private:
+  typedef llvm::SmallVector<size_t, 4> IndicesVector;
+
----------------
Please use std::vector if there's no strong reason to use SmallVector.

================
Comment at: ELF/Writer.cpp:103-104
@@ -93,2 +102,4 @@
   llvm::StringMap<llvm::StringRef> InputToOutputSection;
+  llvm::StringMap<std::vector<llvm::StringRef>> OutputSectionToPhdrs;
+  llvm::SmallMapVector<llvm::StringRef, IndicesVector, 8> PhdrNameToIndices;
 };
----------------
These fields need a brief comment that they are for the linker script's phdr command.

================
Comment at: ELF/Writer.cpp:869
@@ -839,6 +868,3 @@
 
-// Visits all sections to create PHDRs and to assign incremental,
-// non-overlapping addresses to output sections.
-template <class ELFT> void Writer<ELFT>::assignAddresses() {
-  uintX_t VA = Target->getVAStart() + sizeof(Elf_Ehdr);
-  uintX_t FileOff = sizeof(Elf_Ehdr);
+template <class ELFT>
+bool Writer<ELFT>::updatePhdrsForSection(std::vector<StringRef> &PhdrNames,
----------------
Please add a function comment.

================
Comment at: ELF/Writer.cpp:873
@@ +872,3 @@
+  bool NewPhdrs = false;
+  const std::vector<StringRef> &NewPhdrNames =
+      OutputSectionToPhdrs.lookup(SecName);
----------------
Instead of const std::vector<X> &, you want to use ArrayRef<X>.

================
Comment at: ELF/Writer.cpp:889
@@ -850,5 +888,3 @@
 
-  // The first phdr entry is PT_PHDR which describes the program header itself.
-  setPhdr(&Phdrs[0], PT_PHDR, PF_R, FileOff, VA, PhdrSize, /*Align=*/8);
-  FileOff += PhdrSize;
-  VA += PhdrSize;
+template <class ELFT>
+typename Writer<ELFT>::IndicesVector
----------------
This also needs a function comment.

================
Comment at: ELF/Writer.cpp:905
@@ -855,6 +904,3 @@
 
-  // PT_INTERP must be the second entry if exists.
-  int PhdrIdx = 0;
-  Elf_Phdr *Interp = nullptr;
-  if (needsInterpSection())
-    Interp = &Phdrs[++PhdrIdx];
+template <class ELFT>
+void Writer<ELFT>::fillPhdrs(const IndicesVector &PhdrInds) {
----------------
This needs a comment too. They are all non-trivial, so they need some descriptions to help reader understand what they are supposed to do.


http://reviews.llvm.org/D15191