Remove shadowing "size" field from classes that inherit from SyntheticSection

[Sterling-Augustine]

A field-named 'size' already available and perfectly usable via inheritance from InputSection, and these variables shadow it for no good reason.

The only interesting change here is in PaddingSection, because a parent's field cannot be initialized via a constructor initializer list, setting it needs to be done inside the constructor body.

[llvmbot]

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: None (Sterling-Augustine)

Changes

A field-named 'size' already available and perfectly usable via inheritance from InputSection, and these variables shadow it for no good reason.

The only interesting change here is in PaddingSection, because a parent's field cannot be initialized via a constructor initializer list, setting it needs to be done inside the constructor body.

---

Full diff: https://github.com/llvm/llvm-project/pull/166323.diff

2 Files Affected:

• (modified) lld/ELF/SyntheticSections.cpp (+3-3)
• (modified) lld/ELF/SyntheticSections.h (+1-12)

diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index a4150ebfa1653..a27f5339a3d4d 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -2749,9 +2749,9 @@ RelroPaddingSection::RelroPaddingSection(Ctx &ctx)
     : SyntheticSection(ctx, ".relro_padding", SHT_NOBITS, SHF_ALLOC | SHF_WRITE,
                        1) {}
 
-PaddingSection::PaddingSection(Ctx &ctx, uint64_t size, OutputSection *parent)
-    : SyntheticSection(ctx, ".padding", SHT_PROGBITS, SHF_ALLOC, 1),
-      size(size) {
+PaddingSection::PaddingSection(Ctx &ctx, uint64_t amount, OutputSection *parent)
+    : SyntheticSection(ctx, ".padding", SHT_PROGBITS, SHF_ALLOC, 1) {
+  size = amount;
   this->parent = parent;
 }
 
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index 38e68110e4bc0..66c866d7e8cde 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -78,8 +78,6 @@ class EhFrameSection final : public SyntheticSection {
   // allocating one for each EhInputSection.
   llvm::DenseMap<size_t, CieRecord *> offsetToCie;
 
-  uint64_t size = 0;
-
   template <llvm::endianness E> void addRecords(EhInputSection *s);
   template <class ELFT>
   void iterateFDEWithLSDAAux(EhInputSection &sec,
@@ -127,7 +125,6 @@ class GotSection final : public SyntheticSection {
 protected:
   size_t numEntries = 0;
   uint32_t tlsIndexOff = -1;
-  uint64_t size = 0;
   struct AuthEntryInfo {
     size_t offset;
     bool isSymbolFunc;
@@ -182,7 +179,6 @@ class BssSection final : public SyntheticSection {
   static bool classof(const SectionBase *s) {
     return isa<SyntheticSection>(s) && cast<SyntheticSection>(s)->bss;
   }
-  uint64_t size;
 };
 
 class MipsGotSection final : public SyntheticSection {
@@ -312,8 +308,6 @@ class MipsGotSection final : public SyntheticSection {
   // Number of "Header" entries.
   static const unsigned headerEntriesNum = 2;
 
-  uint64_t size = 0;
-
   // Symbol and addend.
   using GotEntry = std::pair<Symbol *, int64_t>;
 
@@ -407,8 +401,6 @@ class StringTableSection final : public SyntheticSection {
 private:
   const bool dynamic;
 
-  uint64_t size = 0;
-
   llvm::DenseMap<llvm::CachedHashStringRef, unsigned> stringMap;
   SmallVector<StringRef, 0> strings;
 };
@@ -475,7 +467,6 @@ template <class ELFT> class DynamicSection final : public SyntheticSection {
 
 private:
   std::vector<std::pair<int32_t, uint64_t>> computeContents();
-  uint64_t size = 0;
 };
 
 class RelocationBaseSection : public SyntheticSection {
@@ -780,10 +771,8 @@ class RelroPaddingSection final : public SyntheticSection {
 };
 
 class PaddingSection final : public SyntheticSection {
-  uint64_t size;
-
 public:
-  PaddingSection(Ctx &ctx, uint64_t size, OutputSection *parent);
+  PaddingSection(Ctx &ctx, uint64_t amount, OutputSection *parent);
   size_t getSize() const override { return size; }
   void writeTo(uint8_t *buf) override;
 };

[Sterling-Augustine]

I want to explicitly note here that github did not recommend maskray as a reviewer, but I added him as a courtesy regardless.

[arichardson]

Since all of the ci checks pass this LGTM but I'd wait for @MaskRay approval before merging since I am not sure why this extra field was added initially.

[Sterling-Augustine]

Ping on this.

[smithp35]

Change LGTM.

From looking at the history there was a series of refactorings in this area, where uncompressedSize was renamed to size field went from being a protected: int64_t for use to public uint64_t in 2022. For example: 53b674e , 565b3b0 and e8fafaf

The use of size fields in the SyntheticSections will have predated that refactoring.

[smithp35]

Forgot to add approval

[MaskRay]

Thanks for fixing this.