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

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 6 11:22:17 PDT 2018


On Fri, Jul 6, 2018 at 10:45 AM Rahul Chaudhry via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>

My apologies. I meant I would update this "comment" and not the "patch".
Rewriting the entire patch doesn't make sense at all, that's just a silly
typo, and my intention is to save your time. My typo unfortunately made my
comment arrogant. I'm sorry, I didn't mean it.


>
> Repository:
>   rLLD LLVM Linker
>
> https://reviews.llvm.org/D48247
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180706/3968a7b4/attachment.html>


More information about the llvm-commits mailing list