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

Mon Dec 3 05:31:15 PST 2018

jhenderson added inline comments.

================
Comment at: tools/llvm-objcopy/COFF/Object.cpp:275
+      memset(Ptr + S.Contents.size(), 0xcc,
+             S.Header.SizeOfRawData - S.Contents.size());
+    Ptr += S.Header.SizeOfRawData;
----------------
mstorsjo wrote:
> Open questions to reviewers: Here I pad executable sections with 0xcc, in the same way as LLD does.
> 
> It was requested to have tests that use yaml input instead of committing binaries, and the tests so far check that objcopy produces bytewise identical files to its input. As yaml2obj doesn't do the same 0xcc padding, we'd either need to make yaml2obj do the same padding as well, or stop doing it here. Which one do you prefer?
> 
> (Also, currently, LLD/COFF doesn't currently use different paddings for ARM/AArch64, which it probably should, and the same would go here as well.)
Not too sure, but it might be nice to find a way to specify the padding for any given file. For example, padding with 0xcc doesn't make much sense for x86 output in the data segment, but makes a lot of sense in the text segment.

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

https://reviews.llvm.org/D54939