[PATCH] D33964: [LLVM][llvm-objcopy] Added basic plumbing to get things started

[Jake Ehrlich via Phabricator via llvm-commits]

jakehehrlich marked 7 inline comments as done.
jakehehrlich added inline comments.


================
Comment at: tools/llvm-objcopy/Object.cpp:197-198
+  // aligned properlly however so we should align it as needed. This only takes
+  // a little bit of tweaking to ensure that the sh_name is 4 byte aligned.
+  Offset = alignTo(Offset, sizeof(typename ELFT::Word));
+  SHOffset = Offset;
----------------
jhenderson wrote:
> jakehehrlich wrote:
> > jhenderson wrote:
> > > Why only the sh_name field? 4-byte alignment could cause other struct members to be misaligned (e.g. the sh_addr and sh_offset fields), because they are 8-bytes in size.
> > The next field (sh_type) is also a 4-byte word and thus anything after those two will be 8-byte aligned. So this is the minimal alignment I can choose which ensures that all the fields of the section header are aligned. I updated the comment to reflect this fact.
> Relative to the start of the section header, the fields will be aligned correctly. However, relative to the start of the file, they won't be: for example, in 64-bit ELF, with 8 byte address fields, the sh_name field was aligned to offset 4, the sh_type field will be at offset 8, and the 8-byte-sized sh_flags field will be at offset 12, which is misaligned.
Yep. You're right. I did my math wrong. Not sure what I was thinking there. Thanks for catching that! It should be fixed now.


Repository:
  rL LLVM

https://reviews.llvm.org/D33964