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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/Driver.cpp:166-169
+    if (Config->EmitRelocs) {
+      error("--emit-reloc does not allow link against dynamic object " + Path);
+      return;
+    }
----------------
ruiu wrote:
> Why do you need this restriction?
Like I mentioned in patch description, currently we use Symbol->DynsymIndex to store symbol index in .symtab for -relocatable output.
That works because -r means static linking, so .dynsym not exist.
We need to know symtab symbol index when perform fixup of relocations in InputSection<ELFT>::copyRelocations(),
because .rela.text is linked with .symtab.

This patch reuses Symbol->DynsymIndex for the same purpose for --emit-reloc.
That should work for linux kerner I think. So I had to add the same restrictions for now.

That restrictions probably can be fixed with adding one more variable to Symbol, like SymtabIndex instead of reusing
DynsymIndex. But I am not sure if it is worst to do ?


================
Comment at: ELF/Driver.cpp:227-229
+
+  if (Config->EmitRelocs && Config->Pic)
+    error("-pie/-shared and --emit-relocs may not be used together");
----------------
ruiu wrote:
> Why?
Reasons in comment above,
Out condition for creating .dynsymtab is:
```
bool HasDynSymTab = !Symtab<ELFT>::X->getSharedFiles().empty() || Config->Pic;
```


================
Comment at: ELF/InputFiles.cpp:403-404
+
+    return Config->EmitRelocs ? make<InputSection<ELFT>>(this, &Sec, Name)
+                              : nullptr;
   }
----------------
ruiu wrote:
> Use `if` instead of ?:.
Done.


https://reviews.llvm.org/D28612