[PATCH] D14382: [ELF2] - Basic implementation of -r/--relocatable

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 22 14:03:43 PST 2016

ruiu added inline comments.

Comment at: ELF/InputFiles.cpp:230
@@ +229,3 @@
+      if (Config->Relocatable) {
+        // If relocatable output is choosen, relocation sections are handled as
+        // regular input sections, because no relocations are applied during link.
choosen -> chosen

Comment at: ELF/InputSection.cpp:109
@@ +108,3 @@
+template <bool isRela>
+void InputSectionBase<ELFT>::copyRelocations(uint8_t *Buf, uint8_t *BufEnd,
+                                             RelIteratorRange<isRela> Rels) {
You are not using BufEnd. Please remove.

Comment at: ELF/InputSection.cpp:112-113
@@ +111,4 @@
+  typedef Elf_Rel_Impl<ELFT, isRela> RelTy;
+  size_t Num = Rels.end() - Rels.begin();
+  for (size_t I = 0; I < Num; ++I) {
+    const RelTy &Rel = *(Rels.begin() + I);
Why don't you use range-based for?

Comment at: ELF/OutputSections.cpp:763
@@ +762,3 @@
+  else if (RelocOutSec != Sec->RelocatingSection->OutSec)
+    error("Sections to which the relocations applies doesn`t match");
grimar wrote:
> rafael wrote:
> > Can you get here? If so, add a test, if not, use an assert.
> I just assumed that users who will try linker probably will use the release version. So if them meet that error situation, there is no way they see the assert, it will not be debug build for them.
> error() is universal way to prompt here. I would use it or fatal() here.
So, the point is that

 - if this is not reachable except by programmer's error (by a bug), assert is better
 - if this is reachable by feeding incompatible files, giving incorrect combination of flags, etc, error is appropriate

Comment at: ELF/Writer.cpp:844
@@ -836,2 +843,3 @@
   case InputSectionBase<ELFT>::Regular:
-    Sec = new OutputSection<ELFT>(Key.Name, Key.Type, Key.Flags);
+    if (C->RelocatingSection)
+      Sec = new StaticRelocSection<ELFT>(Key.Name, Key.Type, Key.Flags,
Checking for `RelocatingSection` to see if an input section is a relocation section is an obscure way to distinguish two different kind of sections. Maybe we should define RelInputSection for relocation sections?


More information about the llvm-commits mailing list