[PATCH] D69188: [llvm-objcopy] Preserve .ARM.attributes section when stripping files

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-objcopy.rst:98-99
  For ELF objects, remove from the output all symbols and non-alloc sections not
- within segments, except for .gnu.warning sections and the section name table.
+ within segments, except for .gnu.warning, .ARM.attribute sections and the section
+ name table.
 
----------------
Nit: not strictly required, but when I worked on much of the documentation recently, I tried to keep the lines to 80 columns, like the source code, where possible, so please reflow this comment.


================
Comment at: llvm/docs/CommandGuide/llvm-strip.rst:80-81
  For ELF objects, remove from the output all symbols and non-alloc sections not
- within segments, except for .gnu.warning sections and the section name table.
+ within segments, except for .gnu.warning, .ARM.attribute sections and the section
+ name table.
 
----------------
Ditto.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/strip-preserve-arm-attributes.test:1
+## This test makes sure that --strip-all and --strip-all-gnu preserves
+## .ARM.attributes sections in ELF files. This is needed to maintain
----------------
preserves -> preserve


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/strip-preserve-arm-attributes.test:7
+# RUN: llvm-objcopy --strip-all %t %t2
+# RUN: llvm-readobj --file-headers --sections %t2 | FileCheck %s
+# RUN: llvm-objcopy --strip-all-gnu %t %t2
----------------
You don't need to do --file-headers in this or the other check two lines below.


================
Comment at: llvm/tools/llvm-objcopy/CommonOpts.td:43
                 HelpText<"Remove non-allocated sections outside segments. "
-                         ".gnu.warning* sections are not removed">;
+                         ".gnu.warning* and .ARM.attribute sections are not removed">;
 
----------------
This goes over the 80 character limit. There's a way to run clang-format on tablegen files, but I can't remember the specific invocation off the top of my head.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69188