[PATCH] D14218: [ELF2] Implements -z relro: create an ELF "PT_GNU_RELRO" segment header in the object.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/Writer.cpp:327
@@ +326,3 @@
+    return Config->ZNow;
+  return (Sec->getName() != ".data" && Sec->getType() != SHT_NOBITS);
+}
----------------
rafael wrote:
> This has to be a whitelist.
> 
> If a .o file has a section name "foobar", we cannot assume that it is relro.
Done.

================
Comment at: ELF/Writer.cpp:692
@@ +691,3 @@
+                               OutputSectionBase<ELFT> *Sec, uintX_t VA) {
+
+  if (!Config->ZRelro || !(Cur->p_flags & PF_W))
----------------
rafael wrote:
> delete the empty line.
Done.

================
Comment at: ELF/Writer.cpp:693
@@ +692,3 @@
+
+  if (!Config->ZRelro || !(Cur->p_flags & PF_W))
+    return;
----------------
ruiu wrote:
>   if (!Config->ZRelro || !(Cur->p_flags & PF_W) || isRelroSection(Sec))
>     return;
Done.

================
Comment at: ELF/Writer.cpp:700
@@ +699,3 @@
+              VA - Cur->p_vaddr, 1 /*p_align*/);
+
+    GnuRelroPhdr->p_filesz = VA - Cur->p_vaddr;
----------------
ruiu wrote:
> Remove this blank line.
Done.

================
Comment at: ELF/Writer.cpp:839-840
@@ -800,2 +838,4 @@
     ++I;
+  if (Config->ZRelro)
+    ++I;
   return I;
----------------
ruiu wrote:
> Here you are using something different. Are they guaranteed to be the same?
Fixed


http://reviews.llvm.org/D14218