<div dir="ltr">I pointed out the same thing recently as a post-commit review for r220129. You can almost always separate cleanup patches from actual changes, so please do that. Small patches are always better.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 4, 2014 at 4:00 PM, Shankar Easwaran <span dir="ltr"><<a href="mailto:shankare@codeaurora.org" target="_blank">shankare@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thinking about this, the cleanup part of the change could have been done separately. Will try to take care of this for all future patches.<br>
<br>
Shankar Easwaran<span class=""><br>
<br>
On 11/4/2014 5:17 PM, Eric Christopher wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
Huh, why not?<br>
<br>
-eric<br>
<br>
On Tue Nov 04 2014 at 3:15:54 PM Shankar Easwaran <<a href="mailto:shankare@codeaurora.org" target="_blank">shankare@codeaurora.org</a>><br>
wrote:<br>
<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
  Hi Eric,<br>
<br>
For the complete fix to the issue, the Writer had to be cleanup as well. I<br>
wasnot able to separate the two functionalities.<br>
<br>
Shankar Easwaran<br>
<br>
<br>
On 11/4/2014 4:24 PM, Eric Christopher wrote:<br>
<br>
Hi Shankar,<br>
<br>
Can you please try to separate out distinct patches in the future? The<br>
"cleanup" aspects of this patch make it hard to determine what changed.<br>
<br>
-eric<br>
<br></span>
On Mon Nov 03 2014 at 8:10:35 PM Shankar Easwaran <<a href="mailto:shankarke@gmail.com" target="_blank">shankarke@gmail.com</a>> <<a href="mailto:shankarke@gmail.com" target="_blank">shankarke@gmail.com</a>><div><div class="h5"><br>
wrote:<br>
<br>
<br>
  Author: shankare<br>
Date: Mon Nov  3 21:57:04 2014<br>
New Revision: 221233<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=221233&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=221233&view=rev</a><br>
Log:<br>
[ELF] Fix program headers.<br>
<br>
The ELF writer creates a invalid binary for few cases with large filesize<br>
and<br>
memory size for segments. This patch addresses the functionality and<br>
updates the<br>
test. This patch also cleans up parts of the ELF writer for future<br>
enhancements<br>
to support Linker scripts.<br>
<br>
Modified:<br>
     lld/trunk/lib/ReaderWriter/<u></u>ELF/Chunk.h<br>
     lld/trunk/lib/ReaderWriter/<u></u>ELF/DefaultLayout.h<br>
     lld/trunk/lib/ReaderWriter/<u></u>ELF/HeaderChunks.h<br>
     lld/trunk/lib/ReaderWriter/<u></u>ELF/Layout.h<br>
     lld/trunk/lib/ReaderWriter/<u></u>ELF/OutputELFWriter.h<br>
     lld/trunk/lib/ReaderWriter/<u></u>ELF/SectionChunks.h<br>
     lld/trunk/lib/ReaderWriter/<u></u>ELF/SegmentChunks.h<br>
     lld/trunk/test/elf/phdr.test<br>
<br>
Modified: lld/trunk/lib/ReaderWriter/<u></u>ELF/Chunk.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/lld/trunk/lib/</a><br>
ReaderWriter/ELF/Chunk.h?rev=<u></u>221233&r1=221232&r2=221233&<u></u>view=diff<br>
==============================<u></u>==============================<br>
==================<br>
--- lld/trunk/lib/ReaderWriter/<u></u>ELF/Chunk.h (original)<br>
+++ lld/trunk/lib/ReaderWriter/<u></u>ELF/Chunk.h Mon Nov  3 21:57:04 2014<br>
@@ -52,9 +52,10 @@ public:<br>
    StringRef name() const { return _name; }<br>
    // Kind of chunk<br>
    Kind kind() const { return _kind; }<br>
-  uint64_t        fileSize() const { return _fsize; }<br>
-  void            setAlign(uint64_t align) { _align2 = align; }<br>
-  uint64_t        align2() const { return _align2; }<br>
+  virtual uint64_t fileSize() const { return _fsize; }<br>
+  virtual void setFileSize(uint64_t sz) { _fsize = sz; }<br>
+  virtual void setAlign(uint64_t align) { _align2 = align; }<br>
+  virtual uint64_t align2() const { return _align2; }<br>
<br>
    // The ordinal value of the chunk<br>
    uint64_t            ordinal() const { return _ordinal;}<br>
@@ -66,8 +67,8 @@ public:<br>
    uint64_t            fileOffset() const { return _fileoffset; }<br>
    void               setFileOffset(uint64_t offset) { _fileoffset =<br>
offset; }<br>
    // Output start address of the chunk<br>
-  void               setVAddr(uint64_t start) { _start = start; }<br>
-  uint64_t            virtualAddr() const { return _start; }<br>
+  virtual void setVirtualAddr(uint64_t start) { _start = start; }<br>
+  virtual uint64_t virtualAddr() const { return _start; }<br>
    // Memory size of the chunk<br>
    uint64_t memSize() const { return _msize; }<br>
    void setMemSize(uint64_t msize) { _msize = msize; }<br>
<br>
Modified: lld/trunk/lib/ReaderWriter/<u></u>ELF/DefaultLayout.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/lld/trunk/lib/<u></u>ReaderWriter/ELF/</a><br>
DefaultLayout.h?rev=221233&r1=<u></u>221232&r2=221233&view=diff<br>
==============================<u></u>==============================<br>
==================<br>
--- lld/trunk/lib/ReaderWriter/<u></u>ELF/DefaultLayout.h (original)<br>
+++ lld/trunk/lib/ReaderWriter/<u></u>ELF/DefaultLayout.h Mon Nov  3 21:57:04<br>
2014<br>
@@ -215,9 +215,7 @@ public:<br>
<br>
    void assignVirtualAddress() override;<br>
<br>
-  void assignOffsetsForMiscSections()<u></u>;<br>
-<br>
-  void assignFileOffsets() override;<br>
+  void assignFileOffsetsForMiscSectio<u></u>ns();<br>
<br>
    /// Inline functions<br>
    inline range<AbsoluteAtomIterT> absoluteAtoms() { return<br>
_absoluteAtoms; }<br>
@@ -734,32 +732,15 @@ template <class ELFT> void DefaultLayout<br>
    }<br>
  }<br>
<br>
-template <class ELFT> void DefaultLayout<ELFT>::<u></u>assignFileOffsets() {<br>
-  // TODO: Do we want to give a chance for the targetHandlers<br>
-  // to sort segments in an arbitrary order?<br>
-  std::sort(_segments.begin(), _segments.end(), Segment<ELFT>::<br>
compareSegments);<br>
-  int ordinal = 0;<br>
-  // Compute the number of segments that might be needed, so that the<br>
-  // size of the program header can be computed<br>
-  uint64_t offset = 0;<br>
-  for (auto si : _segments) {<br>
-    si->setOrdinal(++ordinal);<br>
-    // Don't assign offsets for segments that are not loadable<br>
-    if (si->segmentType() != llvm::ELF::PT_LOAD)<br>
-      continue;<br>
-    si->assignOffsets(offset);<br>
-    offset += si->fileSize();<br>
-  }<br>
-}<br>
-<br>
  template<class ELFT><br>
  void<br>
  DefaultLayout<ELFT>::<u></u>assignVirtualAddress() {<br>
    if (_segments.empty())<br>
      return;<br>
<br>
+  std::sort(_segments.begin(), _segments.end(), Segment<ELFT>::<br>
compareSegments);<br>
+<br>
    uint64_t virtualAddress = _context.getBaseAddress();<br>
-  ELFLinkingContext::OutputMagic outputMagic = _context.getOutputMagic();<br>
<br>
    // HACK: This is a super dirty hack. The elf header and program header<br>
are<br>
    // not part of a section, but we need them to be loaded at the base<br>
address<br>
@@ -777,6 +758,7 @@ DefaultLayout<ELFT>::<u></u>assignVirtualAddres<br>
    firstLoadSegment->prepend(_<u></u>programHeader);<br>
    firstLoadSegment->prepend(_<u></u>elfHeader);<br>
    bool newSegmentHeaderAdded = true;<br>
+  bool virtualAddressAssigned = false;<br>
    while (true) {<br>
      for (auto si : _segments) {<br>
        si->finalize();<br>
@@ -784,24 +766,10 @@ DefaultLayout<ELFT>::<u></u>assignVirtualAddres<br>
        if (si->segmentType() != llvm::ELF::PT_NULL)<br>
          newSegmentHeaderAdded = _programHeader->addSegment(si)<u></u>;<br>
      }<br>
-    if (!newSegmentHeaderAdded)<br>
+    if (!newSegmentHeaderAdded && virtualAddressAssigned)<br>
        break;<br>
-    uint64_t fileoffset = 0;<br>
+    virtualAddressAssigned = true;<br>
      uint64_t address = virtualAddress;<br>
-    // Fix the offsets after adding the program header<br>
-    for (auto &si : _segments) {<br>
-      if ((si->segmentType() != llvm::ELF::PT_LOAD) &&<br>
-          (si->segmentType() != llvm::ELF::PT_NULL))<br>
-        continue;<br>
-      // Align the segment to a page boundary only if the output mode is<br>
-      // not OutputMagic::NMAGIC/<u></u>OutputMagic::OMAGIC<br>
-      if (outputMagic != ELFLinkingContext::<u></u>OutputMagic::NMAGIC &&<br>
-          outputMagic != ELFLinkingContext::<u></u>OutputMagic::OMAGIC)<br>
-        fileoffset =<br>
-            llvm::RoundUpToAlignment(<u></u>fileoffset, _context.getPageSize());<br>
-      si->assignOffsets(fileoffset);<br>
-      fileoffset = si->fileOffset() + si->fileSize();<br>
-    }<br>
      // start assigning virtual addresses<br>
      for (auto &si : _segments) {<br>
        if ((si->segmentType() != llvm::ELF::PT_LOAD) &&<br>
@@ -809,22 +777,19 @@ DefaultLayout<ELFT>::<u></u>assignVirtualAddres<br>
          continue;<br>
<br>
        if (si->segmentType() == llvm::ELF::PT_NULL) {<br>
-        // Handle Non allocatable sections.<br>
-        uint64_t nonLoadableAddr = 0;<br>
-        si->setVAddr(nonLoadableAddr);<br>
-        si->assignVirtualAddress(<u></u>nonLoadableAddr);<br>
+        si->assignVirtualAddress(0 /*non loadable*/);<br>
        } else {<br>
-        si->setVAddr(virtualAddress);<br>
-        // The first segment has the virtualAddress set to the base<br>
address as<br>
-        // we have added the file header and the program header don't<br>
align the<br>
-        // first segment to the pagesize<br>
          si->assignVirtualAddress(<u></u>address);<br>
-        si->setMemSize(address - virtualAddress);<br>
-        if (outputMagic != ELFLinkingContext::<u></u>OutputMagic::NMAGIC &&<br>
-            outputMagic != ELFLinkingContext::<u></u>OutputMagic::OMAGIC)<br>
-          virtualAddress =<br>
-              llvm::RoundUpToAlignment(<u></u>address, _context.getPageSize());<br>
        }<br>
+      address = si->virtualAddr() + si->memSize();<br>
+    }<br>
+    uint64_t fileoffset = 0;<br>
+    for (auto &si : _segments) {<br>
+      if ((si->segmentType() != llvm::ELF::PT_LOAD) &&<br>
+          (si->segmentType() != llvm::ELF::PT_NULL))<br>
+        continue;<br>
+      si->assignFileOffsets(<u></u>fileoffset);<br>
+      fileoffset = si->fileOffset() + si->fileSize();<br>
      }<br>
      _programHeader-><u></u>resetProgramHeaders();<br>
    }<br>
@@ -833,7 +798,7 @@ DefaultLayout<ELFT>::<u></u>assignVirtualAddres<br>
    for (auto &si : _sections) {<br>
      section = dyn_cast<Section<ELFT>>(si);<br>
      if (section && DefaultLayout<ELFT>::<u></u>hasOutputSegment(section))<br>
-      section->assignOffsets(<u></u>section->fileOffset());<br>
+      section->assignFileOffsets(<u></u>section->fileOffset());<br>
    }<br>
    // Set the size of the merged Sections<br>
    for (auto msi : _mergedSections) {<br>
@@ -873,9 +838,8 @@ DefaultLayout<ELFT>::<u></u>assignVirtualAddres<br>
    }<br>
  }<br>
<br>
-template<class ELFT><br>
-void<br>
-DefaultLayout<ELFT>::<u></u>assignOffsetsForMiscSections() {<br>
+template <class ELFT><br>
+void DefaultLayout<ELFT>::<u></u>assignFileOffsetsForMiscSectio<u></u>ns() {<br>
    uint64_t fileoffset = 0;<br>
    uint64_t size = 0;<br>
    for (auto si : _segments) {<br>
@@ -894,7 +858,7 @@ DefaultLayout<ELFT>::<u></u>assignOffsetsForMis<br>
        continue;<br>
      fileoffset = llvm::RoundUpToAlignment(<u></u>fileoffset, si->align2());<br>
      si->setFileOffset(fileoffset);<br>
-    si->setVAddr(0);<br>
+    si->setVirtualAddr(0);<br>
      fileoffset += si->fileSize();<br>
    }<br>
  }<br>
<br>
Modified: lld/trunk/lib/ReaderWriter/<u></u>ELF/HeaderChunks.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/lld/trunk/lib/</a><br>
ReaderWriter/ELF/HeaderChunks.<u></u>h?rev=221233&r1=221232&r2=<u></u>221233&view=diff<br>
==============================<u></u>==============================<br>
==================<br>
--- lld/trunk/lib/ReaderWriter/<u></u>ELF/HeaderChunks.h (original)<br>
+++ lld/trunk/lib/ReaderWriter/<u></u>ELF/HeaderChunks.h Mon Nov  3 21:57:04 2014<br>
@@ -43,7 +43,7 @@ public:<br>
    void e_shentsize(uint16_t shentsize) { _eh.e_shentsize = shentsize; }<br>
    void e_shnum(uint16_t shnum)         { _eh.e_shnum = shnum; }<br>
    void e_shstrndx(uint16_t shstrndx)   { _eh.e_shstrndx = shstrndx; }<br>
-  uint64_t  fileSize()                 { return sizeof (Elf_Ehdr); }<br>
+  uint64_t fileSize() const { return sizeof(Elf_Ehdr); }<br>
<br>
    static inline bool classof(const Chunk<ELFT> *c) {<br>
      return c->Kind() == Chunk<ELFT>::Kind::ELFHeader;<br>
@@ -125,9 +125,7 @@ public:<br>
<br>
    void resetProgramHeaders() { _phi = _ph.begin(); }<br>
<br>
-  uint64_t  fileSize() {<br>
-    return sizeof(Elf_Phdr) * _ph.size();<br>
-  }<br>
+  uint64_t fileSize() const { return sizeof(Elf_Phdr) * _ph.size(); }<br>
<br>
    static inline bool classof(const Chunk<ELFT> *c) {<br>
      return c->Kind() == Chunk<ELFT>::Kind::<u></u>ProgramHeader;<br>
@@ -271,7 +269,7 @@ public:<br>
<br>
    void finalize() {}<br>
<br>
-  inline uint64_t fileSize() { return sizeof(Elf_Shdr) *<br>
_sectionInfo.size(); }<br>
+  uint64_t fileSize() const { return sizeof(Elf_Shdr) *<br>
_sectionInfo.size(); }<br>
<br>
    inline uint64_t entsize() {<br>
      return sizeof(Elf_Shdr);<br>
<br>
Modified: lld/trunk/lib/ReaderWriter/<u></u>ELF/Layout.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/lld/trunk/lib/</a><br>
ReaderWriter/ELF/Layout.h?rev=<u></u>221233&r1=221232&r2=221233&<u></u>view=diff<br>
==============================<u></u>==============================<br>
==================<br>
--- lld/trunk/lib/ReaderWriter/<u></u>ELF/Layout.h (original)<br>
+++ lld/trunk/lib/ReaderWriter/<u></u>ELF/Layout.h Mon Nov  3 21:57:04 2014<br>
@@ -47,8 +47,6 @@ public:<br>
    virtual void assignSectionsToSegments() = 0;<br>
    /// associates a virtual address to the segment, section, and the atom<br>
    virtual void assignVirtualAddress() = 0;<br>
-  /// associates a file offset to the segment, section and the atom<br>
-  virtual void assignFileOffsets() = 0;<br>
<br>
  public:<br>
    Layout() {}<br>
<br>
Modified: lld/trunk/lib/ReaderWriter/<u></u>ELF/OutputELFWriter.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/lld/trunk/lib/<u></u>ReaderWriter/ELF/</a><br>
OutputELFWriter.h?rev=221233&<u></u>r1=221232&r2=221233&view=diff<br>
==============================<u></u>==============================<br>
==================<br>
--- lld/trunk/lib/ReaderWriter/<u></u>ELF/OutputELFWriter.h (original)<br>
+++ lld/trunk/lib/ReaderWriter/<u></u>ELF/OutputELFWriter.h Mon Nov  3 21:57:04<br>
2014<br>
@@ -265,7 +265,7 @@ void OutputELFWriter<ELFT>::<u></u>assignSectio<br>
      if (!mergedSec->hasSegment())<br>
        _shdrtab->appendSection(<u></u>mergedSec);<br>
    }<br>
-  _layout.<u></u>assignOffsetsForMiscSections()<u></u>;<br>
+  _layout.<u></u>assignFileOffsetsForMiscSectio<u></u>ns();<br>
    for (auto sec : _layout.sections())<br>
      if (auto section = dyn_cast<Section<ELFT>>(sec))<br>
        if (!DefaultLayout<ELFT>::<u></u>hasOutputSegment(section))<br>
@@ -377,7 +377,6 @@ std::error_code OutputELFWriter<ELFT>::b<br>
    // contained in them, in anyway the targets may want<br>
    _layout.doPreFlight();<br>
<br>
-  _layout.assignFileOffsets();<br>
    _layout.assignVirtualAddress()<u></u>;<br>
<br>
    // Finalize the default value of symbols that the linker adds<br>
<br>
Modified: lld/trunk/lib/ReaderWriter/<u></u>ELF/SectionChunks.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/lld/trunk/lib/<u></u>ReaderWriter/ELF/</a><br>
SectionChunks.h?rev=221233&r1=<u></u>221232&r2=221233&view=diff<br>
==============================<u></u>==============================<br>
==================<br>
--- lld/trunk/lib/ReaderWriter/<u></u>ELF/SectionChunks.h (original)<br>
+++ lld/trunk/lib/ReaderWriter/<u></u>ELF/SectionChunks.h Mon Nov  3 21:57:04<br>
2014<br>
@@ -59,11 +59,10 @@ public:<br>
    virtual bool isLoadableSection() const { return false; }<br>
<br>
    /// \brief Assign file offsets starting at offset.<br>
-  virtual void assignOffsets(uint64_t offset) {}<br>
+  virtual void assignFileOffsets(uint64_t offset) {}<br>
<br>
-  /// \brief Assign virtual addresses starting at addr. Addr is modified<br>
to be<br>
-  /// the next available virtual address.<br>
-  virtual void assignVirtualAddress(uint64_t &addr) {}<br>
+  /// \brief Assign virtual addresses starting at addr.<br>
+  virtual void assignVirtualAddress(uint64_t addr) {}<br>
<br>
    uint64_t getFlags() const { return _flags; }<br>
    uint64_t getEntSize() const { return _entSize; }<br>
@@ -195,7 +194,7 @@ public:<br>
    /// \brief Set the virtual address of each Atom in the Section. This<br>
    /// routine gets called after the linker fixes up the virtual address<br>
    /// of the section<br>
-  virtual void assignVirtualAddress(uint64_t &addr) {<br>
+  virtual void assignVirtualAddress(uint64_t addr) {<br>
      for (auto &ai : _atoms) {<br>
        ai->_virtualAddr = addr + ai->_fileOffset;<br>
      }<br>
@@ -203,7 +202,7 @@ public:<br>
<br>
    /// \brief Set the file offset of each Atom in the section. This routine<br>
    /// gets called after the linker fixes up the section offset<br>
-  virtual void assignOffsets(uint64_t offset) {<br>
+  virtual void assignFileOffsets(uint64_t offset) {<br>
      for (auto &ai : _atoms) {<br>
        ai->_fileOffset = offset + ai->_fileOffset;<br>
      }<br>
<br>
Modified: lld/trunk/lib/ReaderWriter/<u></u>ELF/SegmentChunks.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/lld/trunk/lib/<u></u>ReaderWriter/ELF/</a><br>
SegmentChunks.h?rev=221233&r1=<u></u>221232&r2=221233&view=diff<br>
==============================<u></u>==============================<br>
==================<br>
--- lld/trunk/lib/ReaderWriter/<u></u>ELF/SegmentChunks.h (original)<br>
+++ lld/trunk/lib/ReaderWriter/<u></u>ELF/SegmentChunks.h Mon Nov  3 21:57:04<br>
2014<br>
@@ -41,14 +41,8 @@ public:<br>
<br>
    SegmentSlice() { }<br>
<br>
-  /// Set the segment slice so that it begins at the offset specified<br>
-  /// by file offset and set the start of the slice to be s and the end<br>
-  /// of the slice to be e<br>
-  void set(uint64_t fileoffset, int32_t s, int e) {<br>
-    _startSection = s;<br>
-    _endSection = e + 1;<br>
-    _offset = fileoffset;<br>
-  }<br>
+  /// Set the start of the slice.<br>
+  void setStart(int32_t s) { _startSection = s; }<br>
<br>
    // Set the segment slice start and end iterators. This is used to walk<br>
through<br>
    // the sections that are part of the Segment slice<br>
@@ -59,8 +53,12 @@ public:<br>
    // Return the fileOffset of the slice<br>
    inline uint64_t fileOffset() const { return _offset; }<br>
<br>
+  void setFileOffset(uint64_t offset) { _offset = offset; }<br>
+<br>
    // Return the size of the slice<br>
-  inline uint64_t fileSize() const { return _size; }<br>
+  inline uint64_t fileSize() const { return _fsize; }<br>
+<br>
+  void setFileSize(uint64_t filesz) { _fsize = filesz; }<br>
<br>
    // Return the start of the slice<br>
    inline int32_t startSection() const { return _startSection; }<br>
@@ -74,11 +72,9 @@ public:<br>
    // Return the alignment of the slice<br>
    inline uint64_t align2() const { return _align2; }<br>
<br>
-  inline void setSize(uint64_t sz) { _size = sz; }<br>
-<br>
    inline void setMemSize(uint64_t memsz) { _memSize = memsz; }<br>
<br>
-  inline void setVAddr(uint64_t addr) { _addr = addr; }<br>
+  inline void setVirtualAddr(uint64_t addr) { _addr = addr; }<br>
<br>
    inline void setAlign(uint64_t align) { _align2 = align; }<br>
<br>
@@ -91,13 +87,12 @@ public:<br>
    }<br>
<br>
  private:<br>
-  int32_t _startSection;<br>
-  int32_t _endSection;<br>
    range<SectionIter> _sections;<br>
+  int32_t _startSection;<br>
    uint64_t _addr;<br>
    uint64_t _offset;<br>
-  uint64_t _size;<br>
    uint64_t _align2;<br>
+  uint64_t _fsize;<br>
    uint64_t _memSize;<br>
  };<br>
<br>
@@ -151,10 +146,10 @@ public:<br>
    /// the newly computed offset is greater than a page, then we create a<br>
segment<br>
    /// slice, as it would be a waste of virtual memory just to be filled<br>
with<br>
    /// zeroes<br>
-  void assignOffsets(uint64_t startOffset);<br>
+  void assignFileOffsets(uint64_t startOffset);<br>
<br>
    /// \brief Assign virtual addresses to the slices<br>
-  void assignVirtualAddress(uint64_t &addr);<br>
+  void assignVirtualAddress(uint64_t addr);<br>
<br>
    // Write the Segment<br>
    void write(ELFWriter *writer, TargetLayout<ELFT> &layout,<br>
@@ -181,7 +176,7 @@ public:<br>
      // last section to the first section, especially for TLS because<br>
      // the TLS segment contains both .tdata/.tbss<br>
      this->setFileOffset(_sections.<u></u>front()->fileOffset());<br>
-    this->setVAddr(_sections.<u></u>front()->virtualAddr());<br>
+    this->setVirtualAddr(_<u></u>sections.front()->virtualAddr(<u></u>));<br>
      size_t startFileOffset = _sections.front()->fileOffset(<u></u>);<br>
      size_t startAddr = _sections.front()-><u></u>virtualAddr();<br>
      for (auto ai : _sections) {<br>
@@ -260,16 +255,6 @@ public:<br>
<br>
    inline range<SliceIter> slices() { return _segmentSlices; }<br>
<br>
-  // These two accessors are still needed for a call to std::stable_sort.<br>
-  // Consider adding wrappers for two iterator algorithms.<br>
-  inline SliceIter slices_begin() {<br>
-    return _segmentSlices.begin();<br>
-  }<br>
-<br>
-  inline SliceIter slices_end() {<br>
-    return _segmentSlices.end();<br>
-  }<br>
-<br>
    Chunk<ELFT> *firstSection() { return _sections[0]; }<br>
<br>
  private:<br>
@@ -316,7 +301,7 @@ public:<br>
      // section points to the ELF header and the second chunk points to the<br>
      // actual program headers<br>
      this->setFileOffset(_sections.<u></u>back()->fileOffset());<br>
-    this->setVAddr(_sections.back(<u></u>)->virtualAddr());<br>
+    this->setVirtualAddr(_<u></u>sections.back()->virtualAddr()<u></u>);<br>
      this->_fsize = _sections.back()->fileSize();<br>
      this->_msize = _sections.back()->memSize();<br>
    }<br>
@@ -409,58 +394,132 @@ bool Segment<ELFT>::<u></u>compareSegments(Segm<br>
    return false;<br>
  }<br>
<br>
-template <class ELFT> void Segment<ELFT>::assignOffsets(<u></u>uint64_t<br>
startOffset) {<br>
+template <class ELFT><br>
+void Segment<ELFT>::<u></u>assignFileOffsets(uint64_t startOffset) {<br>
+  uint64_t fileOffset = startOffset;<br>
+  uint64_t curSliceFileOffset = fileOffset;<br>
+  bool isDataPageAlignedForNMagic = false;<br>
+<br>
+  this->setFileOffset(<u></u>startOffset);<br>
+  for (auto &slice : slices()) {<br>
+    // Align to the slice alignment<br>
+    fileOffset = llvm::RoundUpToAlignment(<u></u>fileOffset, slice->align2());<br>
+<br>
+    bool isFirstSection = true;<br>
+<br>
+    for (auto section : slice->sections()) {<br>
+      // If the linker outputmagic is set to OutputMagic::NMAGIC, align<br>
the Data<br>
+      // to a page boundary<br>
+      if (isFirstSection &&<br>
+          _outputMagic != ELFLinkingContext::<u></u>OutputMagic::NMAGIC &&<br>
+          _outputMagic != ELFLinkingContext::<u></u>OutputMagic::OMAGIC) {<br>
+        // Align to a page only if the output is not<br>
+        // OutputMagic::NMAGIC/<u></u>OutputMagic::OMAGIC<br>
+        fileOffset =<br>
+            llvm::RoundUpToAlignment(<u></u>fileOffset,<br>
this->_context.getPageSize());<br>
+      }<br>
+      if (!isDataPageAlignedForNMagic && needAlign(section)) {<br>
+        fileOffset =<br>
+            llvm::RoundUpToAlignment(<u></u>fileOffset,<br>
this->_context.getPageSize());<br>
+        isDataPageAlignedForNMagic = true;<br>
+      }<br>
+      // Align the section address<br>
+      fileOffset = llvm::RoundUpToAlignment(<u></u>fileOffset,<br>
section->align2());<br>
+<br>
+      if (isFirstSection) {<br>
+        slice->setFileOffset(<u></u>fileOffset);<br>
+        isFirstSection = false;<br>
+        curSliceFileOffset = fileOffset;<br>
+      }<br>
+      section->setFileOffset(<u></u>fileOffset);<br>
+      fileOffset += section->fileSize();<br>
+    }<br>
+    slice->setFileSize(fileOffset - curSliceFileOffset);<br>
+  }<br>
+  this->setFileSize(fileOffset - startOffset);<br>
+}<br>
+<br>
+/// \brief Assign virtual addresses to the slices<br>
+template <class ELFT> void Segment<ELFT>::<u></u>assignVirtualAddress(uint64_t<br>
addr) {<br>
    int startSection = 0;<br>
    int currSection = 0;<br>
-  SectionIter startSectionIter, endSectionIter;<br>
+  SectionIter startSectionIter;<br>
+<br>
    // slice align is set to the max alignment of the chunks that are<br>
    // contained in the slice<br>
    uint64_t sliceAlign = 0;<br>
    // Current slice size<br>
    uint64_t curSliceSize = 0;<br>
    // Current Slice File Offset<br>
-  uint64_t curSliceFileOffset = 0;<br>
+  uint64_t curSliceAddress = 0;<br>
<br>
    startSectionIter = _sections.begin();<br>
-  endSectionIter = _sections.end();<br>
    startSection = 0;<br>
    bool isFirstSection = true;<br>
    bool isDataPageAlignedForNMagic = false;<br>
+  uint64_t startAddr = addr;<br>
+  SegmentSlice<ELFT> *slice = nullptr;<br>
+  uint64_t tlsStartAddr = 0;<br>
+<br>
    for (auto si = _sections.begin(); si != _sections.end(); ++si) {<br>
+    // If this is first section in the segment, page align the section<br>
start<br>
+    // address. The linker needs to align the data section to a page<br>
boundary<br>
+    // only if NMAGIC is set.<br>
      if (isFirstSection) {<br>
-      // If the linker outputmagic is set to OutputMagic::NMAGIC, align<br>
the Data<br>
-      // to a page boundary<br>
-      if (!isDataPageAlignedForNMagic && needAlign(*si)) {<br>
-        startOffset =<br>
-            llvm::RoundUpToAlignment(<u></u>startOffset,<br>
this->_context.getPageSize());<br>
+      isFirstSection = false;<br>
+      if (_outputMagic != ELFLinkingContext::<u></u>OutputMagic::NMAGIC &&<br>
+          _outputMagic != ELFLinkingContext::<u></u>OutputMagic::OMAGIC)<br>
+        // Align to a page only if the output is not<br>
+        // OutputMagic::NMAGIC/<u></u>OutputMagic::OMAGIC<br>
+        startAddr =<br>
+            llvm::RoundUpToAlignment(<u></u>startAddr,<br>
this->_context.getPageSize());<br>
+      else if (!isDataPageAlignedForNMagic && needAlign(*si)) {<br>
+        // If the linker outputmagic is set to OutputMagic::NMAGIC, align<br>
the<br>
+        // Data to a page boundary.<br>
+        startAddr =<br>
+            llvm::RoundUpToAlignment(<u></u>startAddr,<br>
this->_context.getPageSize());<br>
          isDataPageAlignedForNMagic = true;<br>
        }<br>
        // align the startOffset to the section alignment<br>
-      uint64_t newOffset =<br>
-        llvm::RoundUpToAlignment(<u></u>startOffset, (*si)->align2());<br>
-      curSliceFileOffset = newOffset;<br>
+      uint64_t newAddr = llvm::RoundUpToAlignment(<u></u>startAddr,<br>
(*si)->align2());<br>
+      curSliceAddress = newAddr;<br>
        sliceAlign = (*si)->align2();<br>
-      this->setFileOffset(<u></u>startOffset);<br>
-      (*si)->setFileOffset(<u></u>newOffset);<br>
-      curSliceSize = (*si)->fileSize();<br>
-      isFirstSection = false;<br>
+      (*si)->setVirtualAddr(<u></u>curSliceAddress);<br>
+<br>
+      // Handle TLS.<br>
+      if (auto section = dyn_cast<AtomSection<ELFT>>(*<u></u>si)) {<br>
+        if (section->getSegmentType() == llvm::ELF::PT_TLS) {<br>
+          tlsStartAddr =<br>
+              llvm::RoundUpToAlignment(<u></u>tlsStartAddr, (*si)->align2());<br>
+          section->assignVirtualAddress(<u></u>tlsStartAddr);<br>
+          tlsStartAddr += (*si)->memSize();<br>
+        } else {<br>
+          section->assignVirtualAddress(<u></u>newAddr);<br>
+        }<br>
+      }<br>
+      // TBSS section is special in that it doesn't contribute to memory<br>
of any<br>
+      // segment. If we see a tbss section, don't add memory size to addr<br>
The<br>
+      // fileOffset is automatically taken care of since TBSS section<br>
does not<br>
+      // end up using file size<br>
+      if ((*si)->order() != DefaultLayout<ELFT>::ORDER_<u></u>TBSS)<br>
+        curSliceSize = (*si)->memSize();<br>
      } else {<br>
-      uint64_t curOffset = curSliceFileOffset + curSliceSize;<br>
-      // If the linker outputmagic is set to OutputMagic::NMAGIC, align<br>
the Data<br>
-      // to a page boundary<br>
+      uint64_t curAddr = curSliceAddress + curSliceSize;<br>
        if (!isDataPageAlignedForNMagic && needAlign(*si)) {<br>
-        curOffset =<br>
-            llvm::RoundUpToAlignment(<u></u>curOffset,<br>
this->_context.getPageSize());<br>
+        // If the linker outputmagic is set to OutputMagic::NMAGIC, align<br>
the<br>
+        // Data<br>
+        // to a page boundary<br>
+        curAddr =<br>
+            llvm::RoundUpToAlignment(<u></u>curAddr,<br>
this->_context.getPageSize());<br>
          isDataPageAlignedForNMagic = true;<br>
        }<br>
-      uint64_t newOffset = llvm::RoundUpToAlignment(<u></u>curOffset,<br>
(*si)->align2());<br>
-      SegmentSlice<ELFT> *slice = nullptr;<br>
-      // If the newOffset computed is more than a page away, let's create<br>
+      uint64_t newAddr = llvm::RoundUpToAlignment(<u></u>curAddr,<br>
(*si)->align2());<br>
+      // If the newAddress computed is more than a page away, let's create<br>
        // a separate segment, so that memory is not used up while running<br>
-      if (((newOffset - curOffset) > this->_context.getPageSize()) &&<br>
+      if (((newAddr - curAddr) > this->_context.getPageSize()) &&<br>
            (_outputMagic != ELFLinkingContext::<u></u>OutputMagic::NMAGIC &&<br>
             _outputMagic != ELFLinkingContext::<u></u>OutputMagic::OMAGIC)) {<br>
-<br>
+        slice = nullptr;<br>
          // TODO: use std::find here<br>
          for (auto s : slices()) {<br>
            if (s->startSection() == startSection) {<br>
@@ -473,30 +532,48 @@ template <class ELFT> void Segment<ELFT><br>
              SegmentSlice<ELFT>();<br>
            _segmentSlices.push_back(<u></u>slice);<br>
          }<br>
-        slice->set(curSliceFileOffset, startSection, currSection);<br>
-        slice->setSections(make_range(<u></u>startSectionIter, endSectionIter));<br>
-        slice->setSize(curSliceSize);<br>
+        slice->setStart(startSection);<br>
+        slice->setSections(make_range(<u></u>startSectionIter, si));<br>
+        slice->setMemSize(<u></u>curSliceSize);<br>
          slice->setAlign(sliceAlign);<br>
-        uint64_t newPageOffset =<br>
-            llvm::RoundUpToAlignment(<u></u>curOffset,<br>
this->_context.getPageSize());<br>
-        newOffset = llvm::RoundUpToAlignment(<u></u>newPageOffset,<br>
(*si)->align2());<br>
-        curSliceFileOffset = newOffset;<br>
-        startSectionIter = endSectionIter;<br>
+        slice->setVirtualAddr(<u></u>curSliceAddress);<br>
+        // Start new slice<br>
+        curSliceAddress = newAddr;<br>
+        (*si)->setVirtualAddr(<u></u>curSliceAddress);<br>
+        startSectionIter = si;<br>
          startSection = currSection;<br>
-        (*si)->setFileOffset(<u></u>curSliceFileOffset);<br>
-        curSliceSize = newOffset - curSliceFileOffset + (*si)->fileSize();<br>
+        if (auto section = dyn_cast<AtomSection<ELFT>>(*<u></u>si))<br>
+          section->assignVirtualAddress(<u></u>newAddr);<br>
+        curSliceSize = newAddr - curSliceAddress + (*si)->memSize();<br>
          sliceAlign = (*si)->align2();<br>
        } else {<br>
          if (sliceAlign < (*si)->align2())<br>
            sliceAlign = (*si)->align2();<br>
-        (*si)->setFileOffset(<u></u>newOffset);<br>
-        curSliceSize = newOffset - curSliceFileOffset + (*si)->fileSize();<br>
+        (*si)->setVirtualAddr(newAddr)<u></u>;<br>
+        // Handle TLS.<br>
+        if (auto section = dyn_cast<AtomSection<ELFT>>(*<u></u>si)) {<br>
+          if (section->getSegmentType() == llvm::ELF::PT_TLS) {<br>
+            tlsStartAddr =<br>
+                llvm::RoundUpToAlignment(<u></u>tlsStartAddr, (*si)->align2());<br>
+            section->assignVirtualAddress(<u></u>tlsStartAddr);<br>
+            tlsStartAddr += (*si)->memSize();<br>
+          } else {<br>
+            section->assignVirtualAddress(<u></u>newAddr);<br>
+          }<br>
+        }<br>
+        // TBSS section is special in that it doesn't contribute to<br>
memory of<br>
+        // any segment. If we see a tbss section, don't add memory size<br>
to addr<br>
+        // The fileOffset is automatically taken care of since TBSS<br>
section does<br>
+        // not end up using file size.<br>
+        if ((*si)->order() != DefaultLayout<ELFT>::ORDER_<u></u>TBSS)<br>
+          curSliceSize = newAddr - curSliceAddress + (*si)->memSize();<br>
+        else<br>
+          curSliceSize = newAddr - curSliceAddress;<br>
        }<br>
      }<br>
      currSection++;<br>
-    endSectionIter = si;<br>
    }<br>
-  SegmentSlice<ELFT> *slice = nullptr;<br>
+  slice = nullptr;<br>
    for (auto s : slices()) {<br>
      // TODO: add std::find<br>
      if (s->startSection() == startSection) {<br>
@@ -509,74 +586,17 @@ template <class ELFT> void Segment<ELFT><br>
        SegmentSlice<ELFT>();<br>
      _segmentSlices.push_back(<u></u>slice);<br>
    }<br>
-  slice->set(curSliceFileOffset, startSection, currSection);<br>
+  slice->setStart(startSection);<br>
+  slice->setVirtualAddr(<u></u>curSliceAddress);<br>
+  slice->setMemSize(<u></u>curSliceSize);<br>
    slice->setSections(make_range(<u></u>startSectionIter, _sections.end()));<br>
-  slice->setSize(curSliceSize);<br>
    slice->setAlign(sliceAlign);<br>
-  this->_fsize = curSliceFileOffset - startOffset + curSliceSize;<br>
-  std::stable_sort(slices_begin(<u></u>), slices_end(),<br>
-                   SegmentSlice<ELFT>::compare_<u></u>slices);<br>
-}<br>
<br>
-/// \brief Assign virtual addresses to the slices<br>
-template <class ELFT> void Segment<ELFT>::<u></u>assignVirtualAddress(uint64_t<br>
&addr) {<br>
-  bool isTLSSegment = false;<br>
-  bool isDataPageAlignedForNMagic = false;<br>
-  uint64_t tlsStartAddr = 0;<br>
-<br>
-  for (auto slice : slices()) {<br>
-    // Align to a page only if the output is not<br>
-    // OutputMagic::NMAGIC/<u></u>OutputMagic::OMAGIC<br>
-    if (_outputMagic != ELFLinkingContext::<u></u>OutputMagic::NMAGIC &&<br>
-        _outputMagic != ELFLinkingContext::<u></u>OutputMagic::OMAGIC)<br>
-      addr = llvm::RoundUpToAlignment(addr, this->_context.getPageSize());<br>
-<br>
-    // Align to the slice alignment<br>
-    addr = llvm::RoundUpToAlignment(addr, slice->align2());<br>
-<br>
-    bool virtualAddressSet = false;<br>
-    for (auto section : slice->sections()) {<br>
-      // If the linker outputmagic is set to OutputMagic::NMAGIC, align<br>
the Data<br>
-      // to a page boundary<br>
-      if (!isDataPageAlignedForNMagic && needAlign(section)) {<br>
-        addr = llvm::RoundUpToAlignment(addr,<br>
this->_context.getPageSize());<br>
-        isDataPageAlignedForNMagic = true;<br>
-      }<br>
-      // Align the section address<br>
-      addr = llvm::RoundUpToAlignment(addr, section->align2());<br>
-      // Check if the segment is of type TLS<br>
-      // The sections that belong to the TLS segment have their<br>
-      // virtual addresses that are relative To TP<br>
-      Section<ELFT> *currentSection = dyn_cast<Section<ELFT> >(section);<br>
-      if (currentSection)<br>
-        isTLSSegment = (currentSection-><u></u>getSegmentType() ==<br>
llvm::ELF::PT_TLS);<br>
-<br>
-      tlsStartAddr = (isTLSSegment)<br>
-                     ? llvm::RoundUpToAlignment(<u></u>tlsStartAddr,<br>
section->align2())<br>
-                     : 0;<br>
-      if (!virtualAddressSet) {<br>
-        slice->setVAddr(addr);<br>
-        virtualAddressSet = true;<br>
-      }<br>
-      section->setVAddr(addr);<br>
-      if (auto s = dyn_cast<Section<ELFT> >(section)) {<br>
-        if (isTLSSegment)<br>
-          s->assignVirtualAddress(<u></u>tlsStartAddr);<br>
-        else<br>
-          s->assignVirtualAddress(addr);<br>
-      }<br>
-      if (isTLSSegment)<br>
-        tlsStartAddr += section->memSize();<br>
-      section->setMemSize(addr + section->memSize() -<br>
section->virtualAddr());<br>
-      // TBSS section is special in that it doesn't contribute to memory<br>
of any<br>
-      // segment. If we see a tbss section, don't add memory size to addr<br>
-      // The fileOffset is automatically taken care of since TBSS section<br>
does<br>
-      // not end up using file size<br>
-      if (section->order() != DefaultLayout<ELFT>::ORDER_<u></u>TBSS)<br>
-        addr += section->memSize();<br>
-    }<br>
-    slice->setMemSize(addr - slice->virtualAddr());<br>
-  }<br>
+  // Set the segment memory size and the virtual address.<br>
+  this->setMemSize(<u></u>curSliceAddress - startAddr + curSliceSize);<br>
+  this->setVirtualAddr(<u></u>curSliceAddress);<br>
+  std::stable_sort(_<u></u>segmentSlices.begin(), _segmentSlices.end(),<br>
+                   SegmentSlice<ELFT>::compare_<u></u>slices);<br>
  }<br>
<br>
  // Write the Segment<br>
<br>
Modified: lld/trunk/test/elf/phdr.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf/" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/lld/trunk/test/elf/</a><br>
phdr.test?rev=221233&r1=<u></u>221232&r2=221233&view=diff<br>
==============================<u></u>==============================<br>
==================<br>
--- lld/trunk/test/elf/phdr.test (original)<br>
+++ lld/trunk/test/elf/phdr.test Mon Nov  3 21:57:04 2014<br>
@@ -50,7 +50,7 @@ I386-NEXT:     Offset: 0x1000<br>
  I386-NEXT:     VirtualAddress: 0x1000<br>
  I386-NEXT:     PhysicalAddress: 0x1000<br>
  I386-NEXT:     FileSize: 260<br>
-I386-NEXT:     MemSize: 4<br>
+I386-NEXT:     MemSize: 260<br>
  I386-NEXT:     Flags [ (0x6)<br>
  I386-NEXT:       PF_R (0x4)<br>
  I386-NEXT:       PF_W (0x2)<br>
@@ -63,7 +63,7 @@ I386-NEXT:     Offset: 0x4000<br>
  I386-NEXT:     VirtualAddress: 0x4000<br>
  I386-NEXT:     PhysicalAddress: 0x4000<br>
  I386-NEXT:     FileSize: 4<br>
-I386-NEXT:     MemSize: 16392<br>
+I386-NEXT:     MemSize: 8<br>
  I386-NEXT:     Flags [ (0x6)<br>
  I386-NEXT:       PF_R (0x4)<br>
  I386-NEXT:       PF_W (0x2)<br>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing listllvm-commits@cs.uiuc.<u></u>eduhttp://<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
<br>
<br>
<br>
______________________________<u></u>_________________<br></div></div>
llvm-commits mailing listllvm-commits@cs.uiuc.<u></u>eduhttp://<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><span class=""><br>
<br>
<br>
<br>
--<br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation<br>
<br>
<br>
</span></blockquote></blockquote><span class="im HOEnZb">
<br>
<br>
-- <br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation<br>
<br></span><div class="HOEnZb"><div class="h5">
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>