[PATCH] D15423: [ELF] - Place RW sections that go after relro to another memory page.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/Writer.cpp:855
@@ -852,2 +854,3 @@
 template <class ELFT>
-void Writer<ELFT>::updateRelro(Elf_Phdr *Cur, Elf_Phdr *GnuRelroPhdr,
+bool Writer<ELFT>::isRelroEnded(Elf_Phdr *Cur, Elf_Phdr *RelroPhdr,
+                                OutputSectionBase<ELFT> *Sec) {
----------------
ruiu wrote:
> Can you rename this isFirstNonRelroSection? I think it conveys the meaning better.
Method unfortunately does not return is that is first non relro section or not. 
"unfortunately" because it seems not to be possible to know here without additional flags outside.
Please see my comment for  "bool RelroAligned = false;" below.
I renamed it to isPostRelroSection.

================
Comment at: ELF/Writer.cpp:859
@@ +858,3 @@
+    return false;
+  return RelroPhdr->p_type;
+}
----------------
ruiu wrote:
> Please add that as a comment.
> 
>   // p_type is zero if we have not created RELRO PHDR yet.
Done.

================
Comment at: ELF/Writer.cpp:901
@@ -891,1 +900,3 @@
+  Elf_Phdr RelroPhdr = {};
+  bool RelroAligned = false;
   Elf_Phdr TlsPhdr{};
----------------
ruiu wrote:
> Do you need this boolean variable? Because of the p_type check, I think isRelroEnded returns true only once.
It will return true for each section after relro. Thats why I need that flag.
Lets look again on conditions:

```
if (!Config->ZRelro || ((Cur->p_flags & PF_W) && isRelroSection(Sec)))
  return false;
return RelroPhdr->p_type;

```
If current section is not relro or not in writable segment it will return p_type, which is here can be:
1) zero - if we did not start proccessing the relro yet. So this flag was never set.
2) non zero - if we had relro and finished with it.

So because of second I need to know if current section is the **first** section after relro or not, that is for what flag is there,


http://reviews.llvm.org/D15423