[PATCH] D51841: [llvm-objcopy] Dwarf decompression support.

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added a comment.

So I was thinking that we might try and detect compressed section at section construction time instead. The we can implement decompression in terms of CompressedSection...the only blemish seems to be that CompressedSection always allocates. I'm not super sure what the right solution to that is.

On a much more complicated note, I'm thinking I have a nicer and more generic way of handling compressed sections but it's not something you should worry about in this change (unless it sounds awesome to you and you're up for a more complex challenge). If we "decompress" (air quotes explained later) every compressed section at construction regardless of weather it is a debug section then we can even handle compressed symbol tables or relocations! To avoid too much allocation and copying we can lazily decompress (e.g. the airquotes) only when needed. I've had an idea floating around in the back of my head about lazily constructing sections anyway. I'm going to need to think more about a design here but that's what I'm thinking would eventually be a better design.



================
Comment at: tools/llvm-objcopy/Object.h:274
   uint64_t Type = ELF::SHT_NULL;
+  uint64_t DecompressedSize = 0;
+  uint64_t DecompressedAlign = 1;
----------------
Why do these need to be moved into SectionBase?


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:426
+static void
+replaceSections(const CopyConfig &Config, Object &Obj, SectionPred &RemovePred,
+                std::function<bool(const SectionBase &)> doReplacement,
----------------
So section replacement is very tricky to do right. It's one of the fundamental operations (like remove and add) that I want on Object. I have not implemented it yet because, well, it's complicated. It's easy to do when no relocations, symbols, or other parts of the system reference the given section but we're almost never so lucky. In the previous change we dealt with relocations via this method but using a different, less generic name. It might be worth doing that again. So either the name should change, or we should bite the bullet and implement a proper replace method in Object. Implementing a proper replace method will require a wide scale change to llvm-objcopy but would have a lot of benefits. My thinking was that replace could be implemented by adding a new method to SectionBase called "replaceSection" much like "removeSection" works now. Architecturally that's pretty simple. The problem is that *every* section needs to be extended with this method and every detail has to be implemented correctly. 

If you wanna do it, I'd be super happy to review it. If not, we can just rename this function to something less generic for now. Maybe something like "replaceDebugSections" since it really only handles relocations.


================
Comment at: tools/llvm-objcopy/llvm-objcopy.cpp:676
+                      return &Obj.addSection<OwnedDataSection>(
+                          S, true /* DoDecompression */);
+                    });
----------------
Instead of modifying OwnedDataSection can you just do the decompression here?


Repository:
  rL LLVM

https://reviews.llvm.org/D51841