llvm-objcopy: Remove unused field. NFCI.

[jhenderson]

Could you hold off on doing this for a bit? As @jakehehrlich mentioned by email, this was being used to write segment data not owned by sections, I think, and I could do with that behaviour. I'm just investigating why it got removed at the moment, and may come forward with patch in the next few days to make use of it again. I may decide not to bother, in which case I'll give an LGTM.

[jhenderson]

Right, I did lots of digging and discovered that this was added originally for binary output support, in rL310127. The usage was removed when binary output support was changed to be driven by segments in r318324. It was never used for regular ELF output, but I think it should be.

I've discussed with a colleague and we definitely need the behaviour of preserving unlabelled space within program headers. As such, we will need the data. I or one of my colleagues will be looking at implementing this in the next few days, so please don't commit this.

[pcc]

An alternative to storing this field in Segment would be to store a reference to the file contents as a field in Object. That would allow you to access the segment contents using the OriginalOffset and Size fields, and also seems like it might allow you to remove the various Contents fields from the Section hierarchy. So maybe we should remove this field in case that approach turns out to work out better?

[jhenderson]

Yes, you're probably right. I've been thinking about this a bit since making my last comment, and just having the segment data doesn't necessarily seem all that useful, because we're actually only interested in the data not covered by sections. Go ahead and land this. I can always revert bits of it if the new implementation needs it.

[jakehehrlich]

So this change and the issue that James hit made me think about a few short comings in llvm-objcopy that I think should be addressed.

1. Consider the issue of data in a PT_LOAD that isn't covered by a section. We should maintain that IMO.
2. Consider the issue of added trap instructions that LLD adds. Right now we're not preserving that and thus stripping decreases the security of binaries produced by LLD
3. --strip-sections really should strip all sections, not just the allocated ones

I think soon we'll have to start copying from segments, when a section doesn't cover the region.

llvm/trunk/tools/llvm-objcopy/ELF/Object.h

public:
   uint64_t PAddr;
   uint64_t FileSize;
   uint64_t MemSize;
   uint64_t Align;
 
   uint32_t Index;
   uint64_t OriginalOffset;
   Segment *ParentSegment = nullptr;
-  ArrayRef<uint8_t> Contents;
-
-  explicit Segment(ArrayRef<uint8_t> Data) : Contents(Data) {}
-  Segment() {}
 
   const SectionBase *firstSection() const {
     if (!Sections.empty())
       return *Sections.begin();
     return nullptr;
   }
 
   void removeSection(const SectionBase *Sec) { Sections.erase(Sec); }
public:
   Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove);
   template <class T, class... Ts> T &addSection(Ts &&... Args) {
     auto Sec = llvm::make_unique<T>(std::forward<Ts>(Args)...);
     auto Ptr = Sec.get();
     Sections.emplace_back(std::move(Sec));
     Ptr->Index = Sections.size();
     return *Ptr;
   }
-  Segment &addSegment(ArrayRef<uint8_t> Data) {
-    Segments.emplace_back(llvm::make_unique<Segment>(Data));
+  Segment &addSegment() {
+    Segments.emplace_back(llvm::make_unique<Segment>());
     return *Segments.back();
   }
 };
 
 } // end namespace elf
 } // end namespace objcopy
 } // end namespace llvm
 
 #endif // LLVM_TOOLS_OBJCOPY_OBJECT_H

llvm/trunk/tools/llvm-objcopy/ELF/Object.cpp

if (&Child != &Parent && segmentOverlapsSegment(Child, Parent)) {
         }
     }
   }
 }
 
 template <class ELFT> void ELFBuilder<ELFT>::readProgramHeaders() {
   uint32_t Index = 0;
   for (const auto &Phdr : unwrapOrError(ElfFile.program_headers())) {
-    ArrayRef<uint8_t> Data{ElfFile.base() + Phdr.p_offset,
-                           (size_t)Phdr.p_filesz};
-    Segment &Seg = Obj.addSegment(Data);
+    Segment &Seg = Obj.addSegment();
     Seg.Type = Phdr.p_type;
     Seg.Flags = Phdr.p_flags;
     Seg.OriginalOffset = Phdr.p_offset;
     Seg.Offset = Phdr.p_offset;
     Seg.VAddr = Phdr.p_vaddr;
     Seg.PAddr = Phdr.p_paddr;
     Seg.FileSize = Phdr.p_filesz;
     Seg.MemSize = Phdr.p_memsz;