[PATCH] D153262: [llvm-objcopy] --set-section-flags: allow "large" to add SHF_X86_64_LARGE

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/set-section-flags-large-multiarch.test:4
+## When targetting non-x86_64, the "large" flag is not allowed:
+# RUN: not llvm-objcopy --output-target=elf64-tradbigmips --set-section-flags=.foo=large %t %t.mips.large 2>&1 | FileCheck %s --check-prefixes=BAD-LARGE
+
----------------
The long option `--output-target` is fine, but the short `-O` is much more common and can make the RUN line easier to read.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/set-section-flags-large-multiarch.test:37
+    Type:          SHT_PROGBITS
+    Flags:         [ SHF_X86_64_LARGE ]
+
----------------
We may need two SHF_X86_64_LARGE sections, one operated on by `--set-section-flags`. This way we can demonstrate that the untouched section keeps having the SHF_X86_64_LARGE flag.


================
Comment at: llvm/tools/llvm-objcopy/ObjcopyOpts.td:88
          "compatibility: alloc, load, noload, readonly, exclude, debug, code, "
-         "data, rom, share, contents, merge, strings.">,
+         "data, rom, share, contents, merge, strings, large.">,
       MetaVarName<"section=flag1[,flag2,...]">;
----------------
While here, remove the trailing period. I think for TableGen help messages the convention is to omit the period. Some options may be inconsistent, just ignore them:)


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

https://reviews.llvm.org/D153262