[PATCH] D49678: [llvm-objcopy] Adding support for compressed DWARF debug sections.

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich accepted this revision.
jakehehrlich added a comment.
This revision is now accepted and ready to land.

I am satisfied that James' two remaining issues (as I understand them, more efficient looping in compressSections and minimal use of MCTargetOptions) have been solved and that mine have been solved.

Since this LGTM, James is going on leave, and Alex gave an LGTM, I think that means universal approval.



================
Comment at: tools/llvm-objcopy/Object.cpp:171
+                                     DebugCompressionType CompressionType)
+    : Data(std::begin(Sec.OriginalData), std::end(Sec.OriginalData)),
+      CompressionType(CompressionType) {
----------------
jakehehrlich wrote:
> plotfi wrote:
> > jhenderson wrote:
> > > Can't you just set this->OriginalData to Sec.OriginalData?
> > > 
> > > That would allow you to get rid of the Data member entirely.
> > No I dont think so. Unless removal of the section doesn't free up that data. 
> It doesn't free that memory usually but you should probably act is if you can't be sure since there is no invariant that says that OriginalData must come from the original binary. For instance it should in theory be perfectly valid to compress a compressed section and in that case this would *not* be allowed.
> 
> Either way, I don't think OriginalData should be the uncompressed data. It should be compressed data since that was the effective data at creation time. This solves the lifetime issue and is, IMO, more sensible anyhow.
LGTM


Repository:
  rL LLVM

https://reviews.llvm.org/D49678