[PATCH] D16575: [ELF] - redesign of program headers creation code.

Rafael Ávila de Espíndola via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 26 14:57:46 PST 2016


rafael added a comment.

The idea is that with linker script scanHeaders is not used and instead we use the data from the script to construct a mapping and the rest of the code is not changed? If so, I like the general direction.


================
Comment at: ELF/Writer.cpp:103
@@ +102,3 @@
+    unsigned Flags;
+    std::list<OutputSectionBase<ELFT> *> OutSections;
+  };
----------------
Why std::list and not std::vector?

================
Comment at: ELF/Writer.cpp:106
@@ +105,3 @@
+
+  using PhdrMap = std::list<std::unique_ptr<Phdr>>;
+  PhdrMap scanHeaders();
----------------
Thin can be a std::list<Phdr> or a std::vector<std::unique_ptr<Phdr>>, no?

================
Comment at: ELF/Writer.cpp:1052
@@ -1039,11 +1051,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);
-
-  // Calculate and reserve the space for the program header first so that
-  // the first section can start right after the program header.
-  Phdrs.resize(getPhdrsNum());
-  size_t PhdrSize = sizeof(Elf_Phdr) * Phdrs.size();
+template <class ELFT>
+typename Writer<ELFT>::PhdrMap Writer<ELFT>::scanHeaders() {
----------------
Add a small comment.

================
Comment at: ELF/Writer.cpp:1056
@@ +1055,3 @@
+  auto AddIf = [this, &Phdrs](unsigned Type, bool C, unsigned Flags = 0) {
+    if (!C)
+      return (Phdr *)nullptr;
----------------
Not sure if having this if in here is actually more readable.

================
Comment at: ELF/Writer.cpp:1143
@@ +1142,3 @@
+
+      bool CreateLoad = !FirstLoad;
+      bool isW = PHdr->Flags & PF_W;
----------------
Do you really need this flag? Looks like the only special case is that the first load starts at 0, not at the start of its first section.

Is that the same behaviour one gets with linker scripts?


http://reviews.llvm.org/D16575





More information about the llvm-commits mailing list