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

[Thomas Köppe via Phabricator via llvm-commits]

tkoeppe marked 5 inline comments as done.
tkoeppe added a comment.

I cannot figure out why clang-format rejects `llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp`: when I run the formatter script (`clang/tools/clang-format/git-clang-format llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp`) it doesn't make any changes.



================
Comment at: llvm/test/tools/llvm-objcopy/ELF/set-section-flags-large-multiarch.test:30-32
+  - Name:   .foo
+    Type:   SHT_PROGBITS
+    Flags:  [ ]
----------------
jhenderson wrote:
> Is it worth having a version of this section (i.e. one without SHF_X86_64_LARGE) that isn't operated on by --set-section-flags, just to complete the test matrix? (Possibly not)
Yes, because there's a difference in behaviour: if you don't operate on the section, the flag is reinterpreserved when you change architectures, but if you do operate on it, then it might get dropped. I initially had a separate objcopy run for this, but maskray@ suggested just rolling that into a section that's not operated on.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/set-section-flags-large-multiarch.test:37
+    Type:          SHT_PROGBITS
+    Flags:         [ SHF_X86_64_LARGE ]
+
----------------
MaskRay wrote:
> 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.
@jhenderson (See this comment.)


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

https://reviews.llvm.org/D153262