[PATCH] D54939: [RFC] [llvm-objcopy] Initial COFF support

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added a comment.

In D54939#1317502 <https://reviews.llvm.org/D54939#1317502>, @jakehehrlich wrote:

> Also don't focus on byte for byte accuracy, test semantically so that we can make layout changes independent of content changes. It's clear to me now that we shouldn't even bother attempting exact binary matches even in the first patch.


Sure, in general it's not necessary to have byte for byte accuracy, but as we write a new file, I've tried to mimic the layout of MC and LLD as those should be sane and actually used in the wild.

The test itself doesn't need to check for such accuracy of course though. I guess I could check e.g. the input/output with e.g. `llvm-readobj -file-headers` or something like that, to check that the output generally looks sane.



================
Comment at: tools/llvm-objcopy/COFF/Object.h:83
+struct Section {
+  object::coff_section Header;
+  ArrayRef<uint8_t> Contents;
----------------
jakehehrlich wrote:
> It might be useful to declare this as the OriginalHeader and have a second Header. We discovered overtime that this would have been a good structure in the ELF case but currently we have a horrible hodgepodge of fields prefixed with "Original" and some without. Also, the name "Contents" doesn't necessarily need to change but it should be clear that such a thing is the original contents and nothing more.
So far, I don't keep a copy of the original header anywhere but I just patch things in this one copy (marginally when just doing a plain copy, patching more when actually changing things) before finally writing it to the output with one memcpy, so there's no distinction between original and current.

If we need to keep the original header separately later, couldn't we add the separate OriginalHeader field at that point?


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

https://reviews.llvm.org/D54939