[PATCH] D53667: [llvm-objcopy] Handle -O <format> flag.

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht planned changes to this revision.
rupprecht added a comment.

In https://reviews.llvm.org/D53667#1276302, @jakehehrlich wrote:

> I spoke a bit offline with Jordan. I'm going to put a patch up by end of day that refactors things so that this can be done in the Writer. It's a bit hariy but I really don't want setArch or anything like it. The issue we were aware of before was that size was format dependent and we (me really) kluded around it. This actully points out that many other things are as well. The general shift that I've liked here is to slowy remove state from the internal representation and push those things toward constants in the writers. This change I have in mind will remove a lot of state from sections and additionally add that section headers be written on a section by section basis.


Yep, sorry I forgot to update this revision after that convo. Marking as needing further action. I'll update this when Jake's patch goes in.

Some other notes from when I was trying to work on this:

- Arch-dependent things are not just Size, but also EntrySize and AddressAlignment (possibly others, but that's what I've observed so far)
- The tricky thing (IMO) is that in some cases (e.g. data section) it _does_ make sense for a section to know what size it is, but in others (e.g. symbol tables) it's tied to the output arch (just 32/64 bit really)... but not entirely constant: e.g. for symbol tables EntrySize is either 16 or 24 depending on the arch (don't quote me on those constants :) ), but Size = EntrySize * <num of symbols>, so the writer can't just "tell" the section what its size is, but neither can the section just "tell" the writer what its size is. Writing an API to be flexible for that gets weird. Good luck :)

@jakehehrlich - if your patch is correct, it should require the change here for tools/llvm-objcopy/binary-input.test. The rest of it I can submit separately after your refactoring.


Repository:
  rL LLVM

https://reviews.llvm.org/D53667