[PATCH] D30892: [ELF] - Make Bss and BssRelRo sections to be synthetic (#3).

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Relocations.cpp:483
+  BssSection<ELFT> *Sec = IsReadOnly ? In<ELFT>::BssRelRo : In<ELFT>::Bss;
+  size_t Off = Sec->reserveSpace(SS->getAlignment<ELFT>(), SymSize);
 
----------------
I remember that last time I had to change this type from size_t to uintX_t because your previous change broke buildbots.


================
Comment at: ELF/SyntheticSections.h:156-157
 
-// For each copy relocation, we create an instance of this class to
-// reserve space in .bss or .bss.rel.ro.
-template <class ELFT> class CopyRelSection final : public SyntheticSection {
+// We create two BssRelSection sections now: .bss for read-write copy
+// relocations, .bss.rel.ro for read-only copy relocations.
+template <class ELFT> class BssSection final : public SyntheticSection {
----------------
BssRelSection sections? This needs updating.

"BssSection is used to reserve space for copy relocations. We create two instances of this class for .bss and .bss.rel.ro. .bss is use for writable symbols, and .bss.rel.ro is used for read-only symbols."


================
Comment at: ELF/SyntheticSections.h:165
+  bool empty() const override { return getSize() == 0; }
+  size_t reserveSpace(uintX_t AddrAlign, size_t Size);
   size_t getSize() const override { return Size; }
----------------
Rafael made a change to rename AddrAlign Alignment. Please take a look at that change and make your change consistent with that.


https://reviews.llvm.org/D30892