[PATCH] D18183: [ELF] - -pie/--pic-executable option implemented

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 16 02:49:12 PDT 2016


grimar added inline comments.

================
Comment at: ELF/Driver.cpp:150-151
@@ +149,4 @@
+      error("-shared and -pie may not be used together");
+    if (Config->Static)
+      error("-static and -pie may not be used together");
+  }
----------------
ruiu wrote:
> Why is -pie incompatible with -static?
It was done in bfd/gold to be consistent with gcc driver:
https://gcc.gnu.org/ml/gcc/2014-01/msg00125.html
My motivation was to be consistent with gold here.

================
Comment at: ELF/SymbolTable.cpp:21
@@ -20,2 +20,3 @@
 #include "Symbols.h"
+#include "Target.h"
 #include "llvm/Bitcode/ReaderWriter.h"
----------------
ruiu wrote:
> isPIO is a single line function. I don't think including the entire header file just for that function is a good idea.
I can't agree here, because using of declarations instead of including header in cpp files does not look clean for me.
Fortunately we don't need that after introducing Config->Pic anymore.

================
Comment at: ELF/SymbolTable.cpp:127
@@ -124,3 +126,3 @@
   TargetOptions Options;
-  Reloc::Model R = Config->Shared ? Reloc::PIC_ : Reloc::Static;
+  Reloc::Model R = isPIO() ? Reloc::PIC_ : Reloc::Static;
   std::unique_ptr<TargetMachine> TM(
----------------
ruiu wrote:
> What does PIO stand for?
> 
> Anyways, I think this function is probably a bit too much. How about this: add Config->Pic and set it to true if either Config->Pie or Config->Shared.
It was Position Independent Output. I saw that term in gold sources.
Applied your suggestion about Config->Pic, that should work.


http://reviews.llvm.org/D18183





More information about the llvm-commits mailing list