[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/Driver.cpp:192
@@ -189,1 +191,3 @@
+    else if (S == "norelro")
+      Config->ZRelro = false;
   }
----------------
davide wrote:
> grimar wrote:
> > davide wrote:
> > > You already assign ZRelro to false so the last two lines (I think) can be removed, no?
> > I am not sure about that.  What will be if we specify both ones in the command line (-z relro -z norelro ... -z relro) ? I guess that processing both ones can also be helpfull here since that can be feature that is already used and works for overriding the option for somebody. 
> > 
> > But if first argument is not fine then it is still questionable which exactly lines should be removed then.
> > Like notes mentioned: looks like ld has ZRelro set to true by default. 
> > So I need to know should I do the same in this patch or not. If yes then not the last ones, but two lines before the last two are becoming redundant.
> I think -zrelro -znorelro -zlrelro is a fairly weird way of specifying options on a cmdline  (probably we shouldn't really care about that at this stage of development) but just out of curiosity, can you please check what gold and ld do in this case? My experience is that the option to enable "wins" but they're not always consistent with themselves.
I would agree that we should not care, but not for current case when it is just 2 lines of code that are already written and keeps consistenty with both ld/gold:

gcc -fuse-ld=bfd -Wl,-z,norelro,-z,relro,-z,norelro -fPIC maintest.c -o mainro
gcc -fuse-ld=gold -Wl,-z,norelro,-z,relro,-z,norelro -fPIC maintest.c -o mainro
Both ld/gold does not generate GNU_RELRO headers in that cases above. 

Adding one more -z relro makes header to appear in output:
gcc -fuse-ld=bfd -Wl,-z,norelro,-z,relro,-z,norelro,-z,relro -fPIC maintest.c -o mainro


http://reviews.llvm.org/D14218