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

[Sergey Dmitriev via Phabricator via llvm-commits]

sdmitriev added a comment.

In D72128#1802526 <https://reviews.llvm.org/D72128#1802526>, @jhenderson wrote:

> I'm assuming that GNU objcopy doesn't have this flag? Also, what is your use-case for adding it?
>
> I think this is a perfectly reasonable extension to llvm-objcopy if there's a valid use-case, but in some cases it might just be more appropriate to remove the section.


Yes, as far as I know GNU objcopy does not support setting “exclude” (SHF_EXCLUDE) flag, though I believe having such ability would be useful. BTW, COFF has similar flag IMAGE_SCN_LNK_REMOVE, so this functionality can also be extended to COFF.

And you are right, I have a use case in mind which needs this functionality. 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. These sections ideally should be discarded by the linker when it links the host image since they do not participate in the host linking process at all, and the best way to achieve that would be adding SHF_EXCLUDE flag to those target object sections. That is the reason why I want to add this functionality to the llvm-objcopy tool.



================
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
----------------
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.


================
Comment at: llvm/test/tools/llvm-objcopy/ELF/set-section-flags.test:53-56
+# RUN: llvm-objcopy --set-section-flags=.foo=contents,readonly,exclude \
+# RUN:   --set-section-flags=.baz=contents,readonly,exclude \
+# RUN:   --set-section-flags=.rela.baz=contents,readonly,exclude %t %t.contents_ro_exclude
+# RUN: llvm-readobj --sections %t.contents_ro_exclude | FileCheck %s --check-prefixes=CHECK,PROGBITS,EXCLUDE
----------------
Should I remove this part as well?


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.h:73
+  SecExclude = 1 << 12,
+  LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ SecExclude)
 };
----------------
jhenderson wrote:
> This is an existing issue, but you might as well fix it as you change this line. The canonical way of adding a comment to name the parameter of a function is:
> `LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/SecExclude)`
> 
> Note the different spacing. Clang-format recognises this pattern and adjusts comments to match, but only if there is no space after the close of the comment.
Sure, I will fix this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72128