[PATCH] D70970: [llvm-objcopy][WebAssembly] Add dump/add/remove-section support

Fri Jan 31 10:45:03 PST 2020

dschuff added inline comments.

================
Comment at: llvm/test/tools/llvm-objcopy/wasm/remove-section.test:9
+## Requests to remove nonexistent sections are silently ignored.
+# RUN: llvm-objcopy --remove-section=nonexistent=%t.sec %t
+
----------------
jhenderson wrote:
> > It's probably a good idea to check that there are no warnings produced as part of this operation.
> I think you missed this part. You can check for warnings by doing something like:
> `# RUN: llvm-objcopy --remove-section=nonexistent=%t.sec %t 2&>1 | count 0`
Ah yes I did miss this one.

================
Comment at: llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp:44
 static Error handleArgs(const CopyConfig &Config, Object &Obj) {
+  for (StringRef Flag : Config.AddSection) {
+    StringRef SecName, FileName;
----------------
jhenderson wrote:
> Check against GNU objcopy for ELF, but I suspect --add-section is applied after removal and dumping. This would allow replacing a section to be done by `--remove-section foo --add-section foo=foo.bin` or something along those lines.
Confirmed that GNU objdump does dump, remove, then add. Also added a test here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70970