[PATCH] D73107: [llvm-objcopy][COFF] Add support for --set-section-flags

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

The new `exclude` flag is pretty ELF-centric as things stand. I don't know if there's an equivalent COFF section characteristic that really maps to it. If not, we should either a) reject it, or b) accept it and do nothing with it except perhaps clear the flags. I tentatively prefer b) for simplicity, but regardless it should at least be tested. What do others think?

Documentation should be updated so that --set-section-flags is no longer listed as ELF-specific.



================
Comment at: llvm/test/tools/llvm-objcopy/COFF/set-section-flags.test:3
+
+# Single flag on a section with no flags:
+# RUN: llvm-objcopy --set-section-flags=.foo=alloc %t %t.alloc
----------------
Use '##' for comments to make them stand out better from lit/FileCheck directives. This is what we've tended to do in other new tests for readability.


================
Comment at: llvm/tools/llvm-objcopy/COFF/COFFObjcopy.cpp:220
+    for (Section &Sec : Obj.getMutableSections()) {
+      const auto It = Config.SetSectionFlags.find(Sec.Name);
+      if (It != Config.SetSectionFlags.end())
----------------
Any particular reason you're making this `const`?


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

https://reviews.llvm.org/D73107