[PATCH] D71519: [ELF] Add IpltSection

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 16 08:34:40 PST 2019


MaskRay marked 6 inline comments as done.
MaskRay added inline comments.


================
Comment at: lld/ELF/SyntheticSections.cpp:2510
+    name = ".glink";
+  }
+}
----------------
grimar wrote:
> You do not need curly bracers around a single line.
https://reviews.llvm.org/D71520 will add another line here. So the curly brace is added beforehand to minimize diff:)


================
Comment at: lld/ELF/SyntheticSections.cpp:2537
+    off += target->pltEntrySize;
+  }
+}
----------------
grimar wrote:
> Should it be `target->ipltEntrySize`? How about adding a helper to
> remove the duplication we have in `PltSection::addSymbols()` and here?
> 
> 
Thanks for catching this!

ipltEntrySize = pltEntrySize on all targets which we support Iplt before https://reviews.llvm.org/D71509

Looks like I forgot to attach a test there and caught the issue.

Regarding the duplication with PltSection::addSymbols. The former has an addition call to  addPltHeaderSymbols. The functions are simply anyway, and may not be simple to deduplicate.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71519/new/

https://reviews.llvm.org/D71519





More information about the llvm-commits mailing list