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

[Davide Italiano via llvm-commits]

davide added inline comments.

================
Comment at: ELF/Driver.cpp:192
@@ -189,1 +191,3 @@
+    else if (S == "norelro")
+      Config->ZRelro = false;
   }
----------------
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.


http://reviews.llvm.org/D14218