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

[Seiya Nuta via Phabricator via llvm-commits]

seiya added a comment.

Commented regarding tests and nits. Could you update docs/CommandGuide/llvm-objcopy.rst as well?



================
Comment at: llvm/test/tools/llvm-objcopy/ELF/gap-fill.test:4
+#
+# clang-7 gapfillpadto.cpp -o gapfillpadto.elf
+# void swap(int *a, int *b) {
----------------
Is it possible to use yaml2obj instead of adding a binary file? I think we just need few sections to make sure that this patch works well. For example (not tested):

```
!ELF
FileHeader:
  Class:           ELFCLASS64
  Data:            ELFDATA2LSB
  Type:            ET_EXEC
  Machine:         EM_X86_64
Sections:
  - Name:            .text
    Type:            SHT_PROGBITS
    Content:         "AABBCCDD"
  - Name:            .foo
    Type:            SHT_PROGBITS
    Content:         "EEFF"
  - Name:            .bar
    Type:            SHT_NOBITS
```


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:632
+        continue;
+      auto GapBegin = (*I)->Addr + (*I)->Size;
+      auto GapEnd = (*(I + 1))->Addr;
----------------
nit: please avoid `auto` and use `uint64_t`.


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:633
+      auto GapBegin = (*I)->Addr + (*I)->Size;
+      auto GapEnd = (*(I + 1))->Addr;
+      if (GapEnd <= GapBegin)
----------------
ditto


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:642
+    auto LastSec = *(LoadableSections.end() - 1);
+    auto SecEndAddr = LastSec->Addr + LastSec->Size;
+    if (LastSec->Size && SecEndAddr < Config.PadTo.getValue())
----------------
ditto


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67689