LLD: Remove sectionPosition attribute

[ruiu]

Summary

This code is simply dead. No one is using it.

[davide]

I looked at this one week ago or such and I thought about removing it as well, so, I'm fine with this patch. I would double-check with Nick Kledzik but, considering this code has been sitting unused in the tree for a while, I would rather remove it and re-introduce again in case there's some utility in it.

[shankarke]

Can you please add reviewers to the code review request next time, its possible people would miss reviewing this.

For ELF this is required to support start and end linker generated variables automatically, if you refer to it. More information on this feature below :-

http://mgalgs.github.io/2013/05/10/hacking-your-ELF-for-fun-and-profit.html

[ruiu]

You can add them back when you need it. As usual, keeping code "just in
case" is a bad engineering practice. Let's keep it simple and clean.
Can you please add reviewers to the code review request next time, its
possible people would miss reviewing this.

For ELF this is required to support start and end linker generated
variables automatically, if you refer to it. More information on this
feature below :-

http://mgalgs.github.io/2013/05/10/hacking-your-ELF-for-fun-and-profit.html

http://reviews.llvm.org/D8125

EMAIL PREFERENCES

http://reviews.llvm.org/settings/panel/emailpreferences/

include/lld/Core/DefinedAtom.h

public:
   };
 
   enum SectionChoice {
     sectionBasedOnContent,  // linker infers final section based on content
     sectionCustomPreferred, // linker may place in specific section
     sectionCustomRequired   // linker must place in specific section
   };
 
-  enum SectionPosition {
-    sectionPositionStart,   // atom must be at start of section (and zero size)
-    sectionPositionEarly,   // atom should be near start of section
-    sectionPositionAny,     // atom can be anywhere in section
-    sectionPositionEnd      // atom must be at end of section (and zero size)
-  };
-
   enum DeadStripKind {
     deadStripNormal,        // linker may dead strip this atom
     deadStripNever,         // linker must never dead strip this atom
     deadStripAlways         // linker must remove this atom if unused
   };
 
   enum DynamicExport {
     /// \brief The linker may or may not export this atom dynamically depending
public:
   /// contentType().
   virtual SectionChoice sectionChoice() const = 0;
 
   /// \brief If sectionChoice() != sectionBasedOnContent, then this return the
   /// name of the section the atom should be placed into.
   virtual StringRef customSectionName() const = 0;
 
   /// \brief constraints on whether the linker may dead strip away this atom.
-  virtual SectionPosition sectionPosition() const = 0;
-
-  /// \brief constraints on whether the linker may dead strip away this atom.
   virtual DeadStripKind deadStrip() const = 0;
 
   /// \brief Under which conditions should this atom be dynamically exported.
   virtual DynamicExport dynamicExport() const {
     return dynamicExportNormal;
   }
 
   /// \brief Code model used by the atom.

include/lld/Core/Simple.h

public:
   Merge merge() const override { return DefinedAtom::mergeNo; }
 
   Alignment alignment() const override { return Alignment(0, 0); }
 
   SectionChoice sectionChoice() const override {
     return DefinedAtom::sectionBasedOnContent;
   }
 
-  SectionPosition sectionPosition() const override {
-    return DefinedAtom::sectionPositionAny;
-  }
-
   StringRef customSectionName() const override { return StringRef(); }
   DeadStripKind deadStrip() const override {
     return DefinedAtom::deadStripNormal;
   }
 
   DefinedAtom::reference_iterator begin() const override {
     const void *it = reinterpret_cast<const void *>(&*_references.begin());
     return reference_iterator(*this, it);

lib/Core/Resolver.cpp

DEBUG_WITH_TYPE("resolver", llvm::dbgs()
                     << ", file=#"
                     << atom.file().ordinal()
                     << ", atom=#"
                     << atom.ordinal()
                     << ", name="
                     << atom.name()
                     << "\n");
 
-  // Verify on zero-size atoms are pinned to start or end of section.
-  assert((atom.sectionPosition() != DefinedAtom::sectionPositionStart &&
-          atom.sectionPosition() != DefinedAtom::sectionPositionEnd) ||
-         atom.size() == 0);
-
   // add to list of known atoms
   _atoms.push_back(&atom);
 
   if (atom.isGroupParent()) {
     maybeAddSectionGroupOrGnuLinkOnce(atom);
   } else {
     _symbolTable.add(atom);
   }

lib/ReaderWriter/CoreLinkingContext.cpp

public:
   Alignment alignment() const override { return Alignment(0, 0); }
 
   SectionChoice sectionChoice() const override {
     return DefinedAtom::sectionBasedOnContent;
   }
 
   StringRef customSectionName() const override { return StringRef(); }
 
-  SectionPosition sectionPosition() const override { return sectionPositionAny; }
-
   DeadStripKind deadStrip() const override {
     return DefinedAtom::deadStripNormal;
   }
 
   ContentPermissions permissions() const override {
     return DefinedAtom::permR_X;
   }
 
public:
   Alignment alignment() const override { return Alignment(3, 0); }
 
   SectionChoice sectionChoice() const override {
     return DefinedAtom::sectionBasedOnContent;
   }
 
   StringRef customSectionName() const override { return StringRef(); }
 
-  SectionPosition sectionPosition() const override { return sectionPositionAny; }
-
   DeadStripKind deadStrip() const override {
     return DefinedAtom::deadStripNormal;
   }
 
   ContentPermissions permissions() const override {
     return DefinedAtom::permRW_;
   }
 

lib/ReaderWriter/ELF/Atoms.h

public:
 
   StringRef customSectionName() const override {
     if ((contentType() == typeZeroFill) ||
         (_symbol && _symbol->st_shndx == llvm::ELF::SHN_COMMON))
       return ".bss";
     return _sectionName;
   }
 
-  SectionPosition sectionPosition() const override {
-    return sectionPositionAny;
-  }
-
   // It isn't clear that __attribute__((used)) is transmitted to the ELF object
   // file.
   DeadStripKind deadStrip() const override { return deadStripNormal; }
 
   ContentPermissions permissions() const override {
     if (_permissions != permUnknown)
       return _permissions;
 
public:
   Alignment alignment() const override {
     return Alignment(llvm::Log2_64(_section->sh_addralign));
   }
 
   SectionChoice sectionChoice() const override { return sectionCustomRequired; }
 
   StringRef customSectionName() const override { return _sectionName; }
 
-  SectionPosition sectionPosition() const override {
-    return sectionPositionAny;
-  }
-
   DeadStripKind deadStrip() const override { return deadStripNormal; }
 
   ContentPermissions permissions() const override { return permR__; }
 
   ArrayRef<uint8_t> rawContent() const override { return _contentData; }
 
   DefinedAtom::reference_iterator begin() const override {
     uintptr_t index = 0;
public:
   Alignment alignment() const override {
     return Alignment(llvm::Log2_64(_symbol->st_value));
   }
 
   SectionChoice sectionChoice() const override { return sectionBasedOnContent; }
 
   StringRef customSectionName() const override { return ".bss"; }
 
-  SectionPosition sectionPosition() const override {
-    return sectionPositionAny;
-  }
-
   DeadStripKind deadStrip() const override { return deadStripNormal; }
 
   ContentPermissions permissions() const override { return permRW_; }
 
   ArrayRef<uint8_t> rawContent() const override { return ArrayRef<uint8_t>(); }
 
   DefinedAtom::reference_iterator begin() const override {
     uintptr_t index = 0;

lib/ReaderWriter/MachO/LayoutPass.cpp

void LayoutPass::checkFollowonChain(MutableFile::DefinedAtomRange &range) {
   for (const auto &ai : _followOnNexts) {
     checkReachabilityFromRoot(_followOnRoots, ai.first);
     checkReachabilityFromRoot(_followOnRoots, ai.second);
   }
 }
 #endif // #ifndef NDEBUG
 
 /// The function compares atoms by sorting atoms in the following order
-/// a) Sorts atoms by Section position preference
-/// b) Sorts atoms by their ordinal overrides (layout-after/ingroup)
-/// c) Sorts atoms by their permissions
-/// d) Sorts atoms by their content
-/// e) Sorts atoms by custom sorter
-/// f) Sorts atoms on how they appear using File Ordinality
-/// g) Sorts atoms on how they appear within the File
+/// a) Sorts atoms by their ordinal overrides (layout-after/ingroup)
+/// b) Sorts atoms by their permissions
+/// c) Sorts atoms by their content
+/// d) Sorts atoms by custom sorter
+/// e) Sorts atoms on how they appear using File Ordinality
+/// f) Sorts atoms on how they appear within the File
 static bool compareAtomsSub(const LayoutPass::SortKey &lc,
                             const LayoutPass::SortKey &rc,
                             LayoutPass::SortOverride customSorter,
                             std::string &reason) {
   const DefinedAtom *left = lc._atom;
   const DefinedAtom *right = rc._atom;
   if (left == right) {
     reason = "same";
     return false;
   }
 
-  // Sort by section position preference.
-  DefinedAtom::SectionPosition leftPos = left->sectionPosition();
-  DefinedAtom::SectionPosition rightPos = right->sectionPosition();
-
-  bool leftSpecialPos = (leftPos != DefinedAtom::sectionPositionAny);
-  bool rightSpecialPos = (rightPos != DefinedAtom::sectionPositionAny);
-  if (leftSpecialPos || rightSpecialPos) {
-    if (leftPos != rightPos) {
-      DEBUG(reason = formatReason("sectionPos", (int)leftPos, (int)rightPos));
-      return leftPos < rightPos;
-    }
-  }
-
   // Find the root of the chain if it is a part of a follow-on chain.
   const DefinedAtom *leftRoot = lc._root;
   const DefinedAtom *rightRoot = rc._root;
 
   // Sort atoms by their ordinal overrides only if they fall in the same
   // chain.
   if (leftRoot == rightRoot) {
     DEBUG(reason = formatReason("override", lc._override, rc._override));

lib/ReaderWriter/Native/NativeFileFormat.h

 struct NativeAtomAttributesV1 {
   uint32_t  sectionNameOffset;
   uint16_t  align2;
   uint16_t  alignModulus;
   uint8_t   scope;
   uint8_t   interposable;
   uint8_t   merge;
   uint8_t   contentType;
-  uint8_t   sectionChoiceAndPosition; // high nibble is choice, low is position
+  uint8_t   sectionChoice;
   uint8_t   deadStrip;
   uint8_t   dynamicExport;
   uint8_t   permissions;
   uint8_t   alias;
   uint8_t   codeModel;
 };
 
 

lib/ReaderWriter/Native/ReaderNative.cpp

DefinedAtom::ContentType contentType() const override {
     return (DefinedAtom::ContentType)(attr.contentType);
   }
 
   DefinedAtom::Alignment alignment() const override {
     return DefinedAtom::Alignment(attributes().align2, attributes().alignModulus);
   }
 
   DefinedAtom::SectionChoice sectionChoice() const override {
-    return (DefinedAtom::SectionChoice)(
-        attributes().sectionChoiceAndPosition >> 4);
+    return (DefinedAtom::SectionChoice)(attributes().sectionChoice);
   }
 
   StringRef customSectionName() const override;
 
-  SectionPosition sectionPosition() const override {
-     return (DefinedAtom::SectionPosition)(
-        attributes().sectionChoiceAndPosition & 0xF);
-  }
-
   DefinedAtom::DeadStripKind deadStrip() const override {
      return (DefinedAtom::DeadStripKind)(attributes().deadStrip);
   }
 
   DynamicExport dynamicExport() const override {
     return (DynamicExport)attributes().dynamicExport;
   }
 

lib/ReaderWriter/Native/WriterNative.cpp

NativeAtomAttributesV1 computeAttributesV1(const DefinedAtom& atom) {
     NativeAtomAttributesV1 attrs;
     attrs.sectionNameOffset = sectionNameOffset(atom);
     attrs.align2            = atom.alignment().powerOf2;
     attrs.alignModulus      = atom.alignment().modulus;
     attrs.scope             = atom.scope();
     attrs.interposable      = atom.interposable();
     attrs.merge             = atom.merge();
     attrs.contentType       = atom.contentType();
-    attrs.sectionChoiceAndPosition
-                          = atom.sectionChoice() << 4 | atom.sectionPosition();
+    attrs.sectionChoice     = atom.sectionChoice();
     attrs.deadStrip         = atom.deadStrip();
     attrs.dynamicExport     = atom.dynamicExport();
     attrs.codeModel         = atom.codeModel();
     attrs.permissions       = atom.permissions();
     return attrs;
   }
 
   NativeAtomAttributesV1 computeAbsoluteAttributes(const AbsoluteAtom& atom) {

lib/ReaderWriter/PECOFF/Atoms.h

public:
   };
 
   const File &file() const override { return _file; }
   StringRef name() const override { return _name; }
   Interposable interposable() const override { return interposeNo; }
   Merge merge() const override { return mergeNo; }
   Alignment alignment() const override { return Alignment(0); }
   StringRef customSectionName() const override { return ""; }
-  SectionPosition sectionPosition() const override {
-    return sectionPositionAny;
-  }
   DeadStripKind deadStrip() const override { return deadStripNormal; }
 
   Kind getKind() const { return _kind; }
 
   void addReference(std::unique_ptr<COFFReference> reference) {
     _references.push_back(std::move(reference));
   }
 

lib/ReaderWriter/YAML/ReaderWriterYAML.cpp

template <> struct ScalarEnumerationTraits<lld::DefinedAtom::SectionChoice> {
   static void enumeration(IO &io, lld::DefinedAtom::SectionChoice &value) {
     io.enumCase(value, "content", lld::DefinedAtom::sectionBasedOnContent);
     io.enumCase(value, "custom",  lld::DefinedAtom::sectionCustomPreferred);
     io.enumCase(value, "custom-required",
                                  lld::DefinedAtom::sectionCustomRequired);
   }
 };
 
-template <> struct ScalarEnumerationTraits<lld::DefinedAtom::SectionPosition> {
-  static void enumeration(IO &io, lld::DefinedAtom::SectionPosition &value) {
-    io.enumCase(value, "start", lld::DefinedAtom::sectionPositionStart);
-    io.enumCase(value, "early", lld::DefinedAtom::sectionPositionEarly);
-    io.enumCase(value, "any",   lld::DefinedAtom::sectionPositionAny);
-    io.enumCase(value, "end",   lld::DefinedAtom::sectionPositionEnd);
-  }
-};
-
 template <> struct ScalarEnumerationTraits<lld::DefinedAtom::Interposable> {
   static void enumeration(IO &io, lld::DefinedAtom::Interposable &value) {
     io.enumCase(value, "no",           DefinedAtom::interposeNo);
     io.enumCase(value, "yes",          DefinedAtom::interposeYes);
     io.enumCase(value, "yes-and-weak", DefinedAtom::interposeYesAndRuntimeWeak);
   }
 };
 
NormalizedAtom(IO &io)
       static uint32_t ordinalCounter = 1;
       _ordinal = ordinalCounter++;
     }
     NormalizedAtom(IO &io, const lld::DefinedAtom *atom)
         : _file(fileFromContext(io)), _name(atom->name()), _refName(),
           _scope(atom->scope()), _interpose(atom->interposable()),
           _merge(atom->merge()), _contentType(atom->contentType()),
           _alignment(atom->alignment()), _sectionChoice(atom->sectionChoice()),
-          _sectionPosition(atom->sectionPosition()),
           _deadStrip(atom->deadStrip()), _dynamicExport(atom->dynamicExport()),
           _codeModel(atom->codeModel()),
           _permissions(atom->permissions()), _size(atom->size()),
           _sectionName(atom->customSectionName()),
           _sectionSize(atom->sectionSize()) {
       for (const lld::Reference *r : *atom)
         _references.push_back(r);
       if (!atom->occupiesDiskSpace())
public:
     Scope scope() const override { return _scope; }
     Interposable interposable() const override { return _interpose; }
     Merge merge() const override { return _merge; }
     ContentType contentType() const override { return _contentType; }
     Alignment alignment() const override { return _alignment; }
     SectionChoice sectionChoice() const override { return _sectionChoice; }
     StringRef customSectionName() const override { return _sectionName; }
     uint64_t sectionSize() const override { return _sectionSize; }
-    SectionPosition sectionPosition() const override { return _sectionPosition; }
     DeadStripKind deadStrip() const override { return _deadStrip; }
     DynamicExport dynamicExport() const override { return _dynamicExport; }
     CodeModel codeModel() const override { return _codeModel; }
     ContentPermissions permissions() const override { return _permissions; }
     void setGroupChild(bool val) { _isGroupChild = val; }
     bool isGroupChild() const { return _isGroupChild; }
     ArrayRef<uint8_t> rawContent() const override {
       if (!occupiesDiskSpace())
public:
     StringRef                           _name;
     StringRef                           _refName;
     Scope                               _scope;
     Interposable                        _interpose;
     Merge                               _merge;
     ContentType                         _contentType;
     Alignment                           _alignment;
     SectionChoice                       _sectionChoice;
-    SectionPosition                     _sectionPosition;
     DeadStripKind                       _deadStrip;
     DynamicExport                       _dynamicExport;
     CodeModel                           _codeModel;
     ContentPermissions                  _permissions;
     uint32_t                            _ordinal;
     std::vector<ImplicitHex8>           _content;
     uint64_t                            _size;
     StringRef                           _sectionName;
static void mapping(IO &io, const lld::DefinedAtom *&atom) {
     io.mapOptional("interposable",     keys->_interpose,
                                          DefinedAtom::interposeNo);
     io.mapOptional("merge",            keys->_merge, DefinedAtom::mergeNo);
     io.mapOptional("alignment",        keys->_alignment,
                                          DefinedAtom::Alignment(0));
     io.mapOptional("section-choice",   keys->_sectionChoice,
                                          DefinedAtom::sectionBasedOnContent);
     io.mapOptional("section-name",     keys->_sectionName, StringRef());
-    io.mapOptional("section-position", keys->_sectionPosition,
-                                         DefinedAtom::sectionPositionAny);
     io.mapOptional("section-size",     keys->_sectionSize, (uint64_t)0);
     io.mapOptional("dead-strip",       keys->_deadStrip,
                                          DefinedAtom::deadStripNormal);
     io.mapOptional("dynamic-export",   keys->_dynamicExport,
                                          DefinedAtom::dynamicExportNormal);
     io.mapOptional("code-model",       keys->_codeModel, DefinedAtom::codeNA);
     // default permissions based on content type
     io.mapOptional("permissions",      keys->_permissions,