[PATCH] D87497: [llvm-objcopy][MachO] Fix --add-section

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/add-section-32.test:1
+## Show that llvm-objcopy adds a new section into the 32-bit object file if
+## --add-section is given.
----------------



================
Comment at: llvm/test/tools/llvm-objcopy/MachO/add-section-32.test:5
+# RUN: yaml2obj %s -o %t
+# RUN: echo -n abcdefg > %t.data
+
----------------
This is probably absolutely fine, but are you aware of the new `split-file` tool? It allows writing multiple inputs inline within the same file, for example:

```
## Split %s into separate files, located in new %t directory.
# RUN: split-file %s %t
<test logic>

## NB: the new line at the end of data.bin's contents will be included.
#--- data.bin
abcdefg
#--- input.yaml
--- !mach-o
...
```

Not sure there's necessarily any advantage to using it here, since the input is simple.


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/add-section-64.test:1
+## Show that llvm-objcopy adds a new section into the 64-bit object if
+## --add-section is given.
----------------



================
Comment at: llvm/test/tools/llvm-objcopy/MachO/add-section-64.test:7-15
+## Error case 1: Nonexistent input file is specified by --add-section.
+# RUN: not llvm-objcopy --add-section __TEXT,__text=%t.missing %t %t.nonexistent-file 2>&1 \
+# RUN:   | FileCheck %s -DINPUT=%t -DSECTION_DATA_FILE=%t.missing --check-prefix=NONEXSITENT-FILE
+# NONEXSITENT-FILE: error: '[[INPUT]]': '[[SECTION_DATA_FILE]]': {{[Nn]}}o such file or directory
+
+## Error case 2: Too long segment name.
+# RUN: not llvm-objcopy --add-section __TOOOOOOOOO_LONG,__text=%t.data %t %t.too-long-seg-name 2>&1 \
----------------
As these aren't specific to 64-bit objects, I might be inclined to split them into a separate test e.g. `add-section-error.test`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87497