[PATCH] D48247: lld: add experimental support for SHT_RELR sections.

[Rahul Chaudhry via Phabricator via llvm-commits]

rahulchaudhry added inline comments.


================
Comment at: ELF/Relocations.cpp:717-723
+                             RelType Type) {
+  // Add a relative relocation. If RelrDyn section is enabled, and the
+  // relocation offset is guaranteed to be even, add the relocation to
+  // the RelrDyn section, otherwise add it to the RelaDyn section.
+  // RelrDyn sections don't support odd offsets. Also, RelrDyn sections
+  // don't store the addend values, so we must write it to the relocated
+  // address.
----------------
ruiu wrote:
> Well, this comment explains what you are doing in this function, but that's what I can find by reading this function, so I don't find this very helpful. I'd explain the intention instead. For example, this comment should answer to the question why the offset must be aligned to 2, what is RelrDyn in the first place, etc.
> 
> Now that I understood the entire patch, I'll rewrite this patch after you commit this change so that we can proceed, though.
I cannot commit this change as I do not have write access.
I won't ask you to commit it either, since it is clear that you are not happy with this change.
You've indicated a couple of times that you would like to rewrite this patch.
I would suggest that you re-implement the functionality to your liking, and simply commit that instead.
Unfortunately, I will not be able to spend any more time on this.
Thanks for your time,
Rahul



Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D48247