[PATCH] D25960: Read section headers upfront

[Rui Ueyama via llvm-commits]

ruiu added a comment.

Thank you for doing this. I agree that it should make things easier to create linker-synthesized input sections.



================
Comment at: ELF/InputSection.cpp:36
   if (!File || Hdr->sh_type == SHT_NOBITS)
-    return {};
+    return makeArrayRef<uint8_t>(nullptr, Hdr->sh_size);
   return check(File->getObj().getSectionContents(Hdr));
----------------
What is this change for?


================
Comment at: ELF/InputSection.cpp:842
   }
+  ArrayRef<uint8_t> Data = makeArrayRef<uint8_t>(nullptr, Size);
+  InputSection Ret(SHF_ALLOC | SHF_WRITE, SHT_NOBITS, Alignment, Data);
----------------
Is it a common practice to create an ArrayRef with non-zero size and a nullptr? It seems odd to me.


================
Comment at: ELF/InputSection.h:88
 
+  uintX_t Flags;
+  uintX_t Entsize;
----------------
Please add a comment to note that they correspond to Elf_Shdr.


================
Comment at: ELF/InputSection.h:119-123
+  uintX_t getFlags() const { return Flags; }
+  uint32_t getType() const { return Type; }
+  uintX_t getEntsize() const { return Entsize; }
+  uint32_t getLink() const { return Link; }
+  uint32_t getInfo() const { return Info; }
----------------
Do you still need these accessors?


================
Comment at: ELF/InputSection.h:280-281
+  // Common symbols don't belong to any section. But it is easier for us
+  // to handle them as if they belong to some input section. So we defined
+  // this class. CommonInputSection is a virtual singleton class that
+  // "contains" all common symbols.
----------------
Please update the comment becasue "this class" no longer exists.


https://reviews.llvm.org/D25960