[PATCH] D72128: [llvm-objcopy][ELF] Allow setting SHF_EXCLUDE flag for ELF sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

> As you probably know llvm-objcopy tool is used by another tool clang-offload-wrapper for creating fat object files (when OpenMP offloading is enabled). Fat object is really just a normal host object file with few extra sections (one section per each offload target) which contain target object as data. Clang-offload-bundler uses llvm-objcopy for adding target sections to the fat output.

Does clang-offload-wrapper add SHF_EXCLUDE sections with llvm-objcopy?



================
Comment at: llvm/test/tools/llvm-objcopy/ELF/rename-section-flag.test:51-53
+# RUN: llvm-objcopy --rename-section=.foo=.bar,contents,readonly,exclude  \
+# RUN:   --rename-section=.baz=.blah,contents,readonly,exclude %t %t.contents_ro_exclude
+# RUN: llvm-readobj --sections %t.contents_ro_exclude | FileCheck %s --check-prefixes=CHECK,PROGBITS,EXCLUDE
----------------
sdmitriev wrote:
> jhenderson wrote:
> > What's the reasoning for adding this extra part? Do you anticipate some interaction between exclude and readonly or contents that's relevant?
> No, nothing special, just wanted to add more test cases for the "exclude" flag:) I can remove this part if you believe it is redundant.
I also feel that this is redundant.


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:95-98
+  const uint64_t PreserveMask =
+      ELF::SHF_COMPRESSED | ELF::SHF_GROUP | ELF::SHF_LINK_ORDER |
+      ELF::SHF_MASKOS | ELF::SHF_MASKPROC | ELF::SHF_TLS | ELF::SHF_INFO_LINK;
+  return (OldFlags & PreserveMask) | NewFlags;
----------------
jhenderson wrote:
> I'm slightly concerned that SHF_EXCLUDE will no longer get preserved by default, which could break people's existing usage. I don't know if that's a big deal, but it could cause unexpected behaviour later in the build. Perhaps we should consider having exclude/noexclude options that add/remove the flag, but where the flag is otherwise maintained in its current state? What do others think?
`objcopy --set-section-flags=.text.foo=alloc c.o cc.o` GNU objcopy preserves `SHF_EXCLUDE`, while we won't after this patch.

The flags can be only be changed by --rename-section and --set-section-flags. Neither seems possible operations people may do to a `SHF_EXCLUDE` section. I am fine with the code.


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

https://reviews.llvm.org/D72128