[PATCH] D67689: [llvm-objcopy] Add support for --gap-fill and --pad-to options

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/gap-fill.test:1
+## This file contains regression tests for the --gap-fill option.
+## Strip the .space* sections out, now the object has spaces (that do not belong to any section) 
----------------
"regression tests" means different things to different people. Simply "tests" is sufficient here.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/gap-fill.test:20
+# RUN: yaml2obj %s > %t.init
+# RUN: llvm-objcopy %t.init %t --regex -R .space.*
+
----------------
You don't need to explicitly create a YAML with sections that get stripped to create the gaps in them. You can use the yaml2obj "Fill" pseudo-section type. It works the same way as regular sections, but no section header is created in the output for it, so the output looks like it has gaps. You just list it as a Section, with the required size in the Sections list, and `Type: Fill`. See llvm\test\tools\yaml2obj\ELF\custom-fill.yaml for example usage.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/gap-fill.test:41
+
+## Verify the preserve area at start of the first allocable seciton .nogap is not altered 
+# RUN: od -An -x -j 0x120 -N 0x2 %t > %t.space1
----------------



================
Comment at: llvm/test/tools/llvm-objcopy/ELF/gap-fill.test:180-184
+# EMPTY: no input file specified
+# BAD-FORMAT: bad number for --gap-fill:
+# BAD-INPUT: bad number for --gap-fill: x
+# BAD-INPUT2: bad number for --gap-fill: 0x1G
+# BAD-NUMBER: bad number for --gap-fill: 0x1122
----------------
It would be easier to follow the test if you put these checks up where they are used for the first time.


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:663
+
+    Segment* SecParent = Sec->ParentSegment;
+    assert(SecParent != nullptr);
----------------
Nit: don't forget to run clang-format on the code you modify.


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

https://reviews.llvm.org/D67689