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

Rahul Chaudhry via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 3 14:24:19 PDT 2018


rahulchaudhry added inline comments.


================
Comment at: ELF/SyntheticSections.cpp:1783
+
+    Elf_Relr R;
+    if (Bits == 0) {
----------------
pcc wrote:
> rahulchaudhry wrote:
> > ruiu wrote:
> > > Please use `typename ELFT::uint` instead of Elf_Relr.
> > See above.
> I guess uint would work here because the value is only stored temporarily in `R`.
> 
> That said, it looks like you could simplify the code a little by changing the code in the two branches to push values onto `RelrRelocs` directly instead of storing them in `R`.
The assignment to R is doing an implicit conversion from uint to packed<uint>.
If I try to do RelrRelocs.push_back(Current), build fails with "no matching function for call to ...".
We'll need explicit conversion, i.e.
  RelrRelocs.push_back(Elf_Relr(Current));
and
  RelrRelocs.push_back(Elf_Relr((Bits << 1) | 1));
It makes the conversion explicit (maybe that's better), and it has to be done at two places.

Alternately, I can make RelrRelocs a vector<uint> as Rui suggested, then modify writeTo to iterate over entries and convert each before writing instead of using a single memcpy to write all of them.



Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D48247





More information about the llvm-commits mailing list