[PATCH] D59488: [llvm-objcopy] - Calculate the string table section sizes correctly.

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, with a couple of comment nits.

I think you might have flushed out another bug with string table building, based on the size changes in the two archive tests. Could you double-check, and fix or file a bug, please? I'm guessing it's just that we're adding an extra null byte.



================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1612
 
+  // Now when all strings are added we want to finalize string table builders,
+  // because that affects section sizes and we need to do this before assigning
----------------
when -> that


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1613
+  // Now when all strings are added we want to finalize string table builders,
+  // because that affects section sizes and we need to do this before assigning
+  // the offsets.
----------------
I'd replace everything from the "and we need..." to the end of the comment with "which in turn affects section offsets."


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

https://reviews.llvm.org/D59488