[PATCH] D28612: [ELF] - Added support for --emit-relocs.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/InputFiles.cpp:409
+      if (!isa<InputSection<ELFT>>(Target))
+        fatal(toString(this) + ": --emit-relocs for relocations sections "
+                               "pointing to .eh_frame is not supported");
----------------
grimar wrote:
> I am going to remove that fatal error in following patch.
> It is needed for linux kernel, but since it involves move DependentSection from InputSection<ELFT> to somewhere and also adding more additional tests probably, I think it is better not to do that change in this patch.
> Hope it's ok.
 I think what you really need to do is to make it a vector just like Rafael did in https://reviews.llvm.org/D28626.


================
Comment at: ELF/InputSection.cpp:45-56
+// Returns if GC should be enabled for section. We do not run GC for
+// non-allocatable sections, which typically contain debugging information.
+// REL[A] sections are not allocatable, but we do GC for them, because them are
+// dependent on section they relocate, we do not want them be live
+// unconditionally. That is used for --emit-relocs.
+bool lld::shouldGC(uint64_t Flags, uint32_t Type) {
+  if (!Config->GcSections)
----------------
Exporting this as a global function is probably not a good idea. Whether a section could be discarded or never be discarded can be decided when the section is instantiated (because the decision depends only on `Sec->Type`, `Config->GcSections` and its section name, all of which are available at the constructor.)

So, the right way of doing it is to set `Live` bit in the ctor if it should never be GC'ed.


================
Comment at: ELF/LinkerScript.cpp:265-270
+
+    // If we discard a section, we also should discard a dependent section.
+    InputSection<ELFT> *IS = dyn_cast<InputSection<ELFT>>(S);
+    if (!IS || !IS->DependentSection || !IS->DependentSection->Live)
+      continue;
+    discard({IS->DependentSection});
----------------
I don't think this is correct. You shouldn't add any code this function, or there's a bug in GC.

If section S depends on section T, and if S is marked as dead, T must be marked as dead too before the control reaches this function. So both S and T will be discarded without doing anything special.


https://reviews.llvm.org/D28612