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

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added inline comments.


================
Comment at: tools/llvm-objcopy/Object.cpp:188
+  Offset = Sec.Offset;
+  Type = ELF::SHT_PROGBITS;
+
----------------
plotfi wrote:
> jhenderson wrote:
> > This is extremely fragile: if we end up adding to the SectionBase list of members, this list will likely not be updated. Also, some of these fields may well be unset at the point of creation of this section (I'm thinking NameIndex specifically, but there may be others - I haven't bothered to check).
> > 
> > I think you might be able to get away with calling the base class's assignment method to set these fields, rather than manually copying each one.
> Base class has no assignment method. It'll try and add one. Also base class has no constructor because it's abstract I believe. 
@jhenderson There is a generic issue with the fragility of cloning the fields of other sections for the purposes of synthetic sections. So far we've kind of punted on this issue. (for instance, OwnedDataSection and DebugGnuLinkSection or whatever its called).

I think the defined operator is not the right way to go if C++ won't define that method for us. I think we should think about it more as "what fields need to be set right here and now". NameIndex for instance does not need to be set. I don't know...it's kind of unclear how this should work to me. Maybe a method that isn't '=' but is for copying the base fields that are always set that is inside the SectionBase class? Or perhaps the previous "fragile" solution is actually best with a few tweaks. Both seem equally fragile to me personally unless the compiler defines the method for us.


Repository:
  rL LLVM

https://reviews.llvm.org/D49678