[PATCH] D62364: llvm-objcopy: Implement --extract-partition and --extract-main-partition.

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich added a comment.

Ok I spoke offline about some of my questions with Peter. Here are some useful facts:

All partitions share the same section header table. This means that if you don't use --extract-* parameters llvm-objcopy should still copy everything as needed. Not that this is intended to work but --strip-all (but not --strip-sections) would actually work as I would expect it to if we told it about these new section types (e.g. treat them like GNU warnings). Debug information is shared between them. I thought this was odd as they all had different address spaces. Peter did some tests and debuggers seem to handle when debug information references in DWARF to outside of the DSO just fine (Peter also pointed out that --gc-sections causes this sort of issue as well). I think --strip-sections would effectively extract the main partition.

There is no dynamic linker support planned for this so before you can run these sorts of DSOs or link them into things you have to extract them.

I'm happy with the general direction here. I don't think there are significant barriers to tweaking this sort of thing in the future or even to doing multi-partition extraction.



================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:734
                              object::ELFObjectFileBase &In, Buffer &Out) {
-  ELFReader Reader(&In);
+  ELFReader Reader(&In, Config);
   std::unique_ptr<Object> Obj = Reader.create();
----------------
pcc wrote:
> jakehehrlich wrote:
> > Adding the CopyConfig here is antithetical to the goal of making this code into a library at some point in the future. We should find a way to avoid this. This goes for propagation of Config elsewhere in the code as well. Propogation of CopyConfig is a hard no from me personally.
> In an earlier version of the code [1] I was just passing down the name of the partition to extract. Would that work better (maybe with the name passed as a ctor argument rather than as a field)? Alternatively, we could create something like a ReaderConfig to be passed down here that for the moment would just contain the partition name.
> 
> [1] https://github.com/pcc/llvm-project/blob/c9939239d7e829e2c779138aaedea061f4a095d0/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp#L597
Either ReaderConfig or just passing the partition name makes sense to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62364/new/

https://reviews.llvm.org/D62364