[PATCH] D56570: [llvm-objcopy] Use SHT_NOTE for added note sections.

Alexander Shaposhnikov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 10 22:56:16 PST 2019


alexshap added a comment.

I'd probably modify the test (to make it work on Windows (if it doesn't)) (or add a comment why it works if it does), other than that (and one minor inline comment) this diff looks like the right fix to me.



================
Comment at: test/tools/llvm-objcopy/ELF/add-section-special.test:2
+# RUN: yaml2obj %s > %t.o
+# RUN: llvm-objcopy --add-section=.note.foo=/dev/null %t.o %t-regular-note.o
+# RUN: llvm-objcopy --add-section=.note.GNU-stack=/dev/null %t.o %t-gnu-stack.o
----------------
just to double check - wouldn't this test have troubles on Windows ?


================
Comment at: tools/llvm-objcopy/ELF/ELFObjcopy.cpp:502
     for (const auto &Flag : Config.AddSection) {
       auto SecPair = Flag.split("=");
       auto SecName = SecPair.first;
----------------
side note: i think auto is overused here (lines 501 -508) (seems like the types here are not super-obvious)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56570





More information about the llvm-commits mailing list