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

[sdmitriev]

Summary

This patch adds support for setting SHF_EXCLUDE flag for ELF sections.

[jhenderson]

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.

llvm/test/tools/llvm-objcopy/ELF/rename-section-flag.test

51–53

What's the reasoning for adding this extra part? Do you anticipate some interaction between exclude and readonly or contents that's relevant?

llvm/tools/llvm-objcopy/CopyConfig.h

73

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.

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

95–98

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?

[sdmitriev]

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

llvm/test/tools/llvm-objcopy/ELF/rename-section-flag.test

51–53

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.

llvm/test/tools/llvm-objcopy/ELF/set-section-flags.test

53–56

Should I remove this part as well?

llvm/tools/llvm-objcopy/CopyConfig.h

73

Sure, I will fix this.

[sdmitriev]

Addressed review comment.

[MaskRay]

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?

llvm/test/tools/llvm-objcopy/ELF/rename-section-flag.test

51–53

I also feel that this is redundant.

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

95–98

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.

[sdmitriev]

In D72128#1804073, @MaskRay wrote:

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?

No, there is currently no way to add it using llvm-objcopy, but that is what I want to do. That is the reason why I want to add this extension to llvm-objcopy.

llvm/test/tools/llvm-objcopy/ELF/rename-section-flag.test

51–53

Ok, I will remove it.

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

95–98

Breaking backward compatibility or compatibility with the GNU objcopy is probably not good, but having an explicit "noexclude" flag as @jhenderson suggested above would allow to preserve it, I guess. I will update the patch to add "noexclude" flag as well.

[MaskRay]

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

95–98

I still doubt whether a new flag in llvm-objcopy will be useful. Yes, it will diverge from GNU, but I suspect anyone other than clang-offload-wrapper has such as use case.

You can also send an email to [e-mail] asking whether they'd like add exclude, and whether SHF_EXCLUDE should be preserved. Ideally llvm-objcopy can avoid the flag noexclude.

[jhenderson]

llvm/test/tools/llvm-objcopy/ELF/set-section-flags.test

53–56

Yes please.

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

95–98

I second the suggestion to raise this with the GNU guys. They may have a specific interface they think should be used, and in that case, we've got one to follow too, without having to make hard decisions :)

[sdmitriev]

Rebased and addressed review comments.

[sdmitriev]

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

95–98

I have opened binutils bug report for this extension few days ago (please see https://sourceware.org/bugzilla/show_bug.cgi?id=25371), and GNU guys proposed to use +exclude/-exclude flags instead of exclude/noexclude. I will update this patch according to that proposal.

[MaskRay]

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

95–98

I replied there. I feel that inventing new syntax like +exclude and -exclude is really unnecessary. Let me send a patch to binutils.

[sdmitriev]

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

95–98

Are you going to use exclude/noexclude in your binutils patch? If so, I will update this patch to that syntax as well.

[MaskRay]

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

95–98

https://sourceware.org/ml/binutils/2020-01/msg00186.html

[sdmitriev]

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

95–98

Ok. I will update this patch to make it consistent with your binutils change.

[sdmitriev]

Addressed review comments.

[jhenderson]

Aside from the inline comment, this looks good to me. Might want to get @MaskRay to approve before committing though.

llvm/test/tools/llvm-objcopy/ELF/rename-section-flag-preserved.test

72–73 ↗

(On Diff #238742)

I can see this is unreferenced in this test, but removing it shouldn't be part of this commit. Please make a separate change just deleting this line and the blank line above it, without need for further review.

[sdmitriev]

llvm/test/tools/llvm-objcopy/ELF/rename-section-flag-preserved.test

72–73 ↗

(On Diff #238742)

I've just found that I accidentally missed this comment and because of that committed this change together with the other changes, sorry.

[jhenderson]

llvm/test/tools/llvm-objcopy/ELF/rename-section-flag-preserved.test

72–73 ↗

(On Diff #238742)

No worries. It happens.

[MaskRay]

Heads-up: GNU objcopy>=2.35 will support --set-section-flags=exclude. https://sourceware.org/ml/binutils/2020-02/msg00195.html

The current version does not remove 0x80000000 (SHF_EXCLUDE) when exclude is not specified, but it seems that Nick may want to fix this for architectures other than ARM/MIPS/PARISC.
The value 0x80000000 may be unfortunate. .debug_*.dwo uses the flag. From the discussions I don't think the llvm-objcopy vs GNU objcopy difference will cause any trouble.

llvm/docs/CommandGuide/llvm-objcopy.rst

  sections.
 
  Following is a list of supported flags and their effects:
 
  - `alloc` = add the `SHF_ALLOC` flag.
  - `load` = if the section has `SHT_NOBITS` type, mark it as a `SHT_PROGBITS`
    section.
  - `readonly` = if this flag is not specified, add the `SHF_WRITE` flag.
+ - `exclude` = add the `SHF_EXCLUDE` flag.
  - `code` = add the `SHF_EXECINSTR` flag.
  - `merge` = add the `SHF_MERGE` flag.
  - `strings` = add the `SHF_STRINGS` flag.
  - `contents` = if the section has `SHT_NOBITS` type, mark it as a `SHT_PROGBITS`
    section.
 
  The following flags are also accepted, but are ignored for GNU compatibility:
  `noload`, `debug`, `data`, `rom`, `share`.

llvm/test/tools/llvm-objcopy/ELF/rename-section-flag.test

 # RUN: yaml2obj %s > %t
 
 # Single flags on a section with no flags:
 # RUN: llvm-objcopy --rename-section=.foo=.bar,alloc  \
 # RUN:   --rename-section=.baz=.blah,alloc %t %t.alloc
 # RUN: llvm-readobj --sections %t.alloc | FileCheck %s --check-prefixes=CHECK,NOBITS,ALLOC,WRITE
 # RUN: llvm-objcopy --rename-section=.foo=.bar,load  \
 # RUN:   --rename-section=.baz=.blah,load %t %t.load
 # RUN: llvm-readobj --sections %t.load | FileCheck %s --check-prefixes=CHECK,PROGBITS,WRITE
 # RUN: llvm-objcopy --rename-section=.foo=.bar,noload  \
 # RUN:   --rename-section=.baz=.blah,noload %t %t.noload
 # RUN: llvm-readobj --sections %t.noload | FileCheck %s --check-prefixes=CHECK,PROGBITS,WRITE
 # RUN: llvm-objcopy --rename-section=.foo=.bar,readonly  \
 # RUN:   --rename-section=.baz=.blah,readonly %t %t.readonly
 # RUN: llvm-readobj --sections %t.readonly | FileCheck %s --check-prefixes=CHECK,PROGBITS
+# RUN: llvm-objcopy --rename-section=.foo=.bar,exclude  \
+# RUN:   --rename-section=.baz=.blah,exclude %t %t.exclude
+# RUN: llvm-readobj --sections %t.exclude | FileCheck %s --check-prefixes=CHECK,PROGBITS,EXCLUDE,WRITE
 # RUN: llvm-objcopy --rename-section=.foo=.bar,debug  \
 # RUN:   --rename-section=.baz=.blah,debug %t %t.debug
 # RUN: llvm-readobj --sections %t.debug | FileCheck %s --check-prefixes=CHECK,PROGBITS,WRITE
 # RUN: llvm-objcopy --rename-section=.foo=.bar,code  \
 # RUN:   --rename-section=.baz=.blah,code %t %t.code
 # RUN: llvm-readobj --sections %t.code | FileCheck %s --check-prefixes=CHECK,PROGBITS,EXEC,WRITE
 # RUN: llvm-objcopy --rename-section=.foo=.bar,data  \
 # RUN:   --rename-section=.baz=.blah,data %t %t.data

 
 # Multiple flags:
 # RUN: llvm-objcopy --rename-section=.foo=.bar,alloc,readonly,strings  \
 # RUN:   --rename-section=.baz=.blah,alloc,readonly,strings %t %t.alloc_ro_strings
 # RUN: llvm-readobj --sections %t.alloc_ro_strings | FileCheck %s --check-prefixes=CHECK,NOBITS,ALLOC,STRINGS
 # RUN: llvm-objcopy --rename-section=.foo=.bar,alloc,code  \
 # RUN:   --rename-section=.baz=.blah,alloc,code %t %t.alloc_code
 # RUN: llvm-readobj --sections %t.alloc_code | FileCheck %s --check-prefixes=CHECK,NOBITS,ALLOC,EXEC,WRITE
+# 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
 
 # Invalid flags:
 # RUN: not llvm-objcopy --rename-section=.foo=.bar,xyzzy %t %t.xyzzy 2>&1 | FileCheck %s --check-prefix=BAD-FLAG
 
 # Upper-case flags:
 # RUN: llvm-objcopy --rename-section=.foo=.bar,ALLOC,LOAD,NOLOAD,READONLY,DEBUG,CODE,DATA,ROM,CONTENTS,MERGE,STRINGS,SHARE \
 # RUN:   --rename-section=.baz=.blah,ALLOC,LOAD,NOLOAD,READONLY,DEBUG,CODE,DATA,ROM,CONTENTS,MERGE,STRINGS,SHARE %t %t.upper
 # RUN: llvm-readobj --sections %t.upper | FileCheck %s --check-prefixes=CHECK,PROGBITS,ALLOC,EXEC,MERGE,STRINGS

 
 # CHECK:         Name: .bar
 # CHECK-NEXT:    Type: SHT_PROGBITS
 # CHECK-NEXT:    Flags [
 # ALLOC-NEXT:      SHF_ALLOC (0x2)
 # EXEC-NEXT:       SHF_EXECINSTR (0x4)
 # MERGE-NEXT:      SHF_MERGE (0x10)
 # STRINGS-NEXT:    SHF_STRINGS (0x20)
+# EXCLUDE-NEXT:    SHF_EXCLUDE (0x80000000)
 # WRITE-NEXT:      SHF_WRITE (0x1)
 # CHECK-NEXT:    ]
 
 # CHECK:         Name: .blah
 # NOBITS-NEXT:   Type: SHT_NOBITS
 # PROGBITS-NEXT: Type: SHT_PROGBITS
 # CHECK-NEXT:    Flags [
 # ALLOC-NEXT:      SHF_ALLOC (0x2)
 # EXEC-NEXT:       SHF_EXECINSTR (0x4)
 # MERGE-NEXT:      SHF_MERGE (0x10)
 # STRINGS-NEXT:    SHF_STRINGS (0x20)
+# EXCLUDE-NEXT:    SHF_EXCLUDE (0x80000000)
 # WRITE-NEXT:      SHF_WRITE (0x1)
 # CHECK-NEXT:    ]
 
-# BAD-FLAG: unrecognized section flag 'xyzzy'. Flags supported for GNU compatibility: alloc, load, noload, readonly, debug, code, data, rom, share, contents, merge, strings
+# BAD-FLAG: unrecognized section flag 'xyzzy'. Flags supported for GNU compatibility: alloc, load, noload, readonly, exclude, debug, code, data, rom, share, contents, merge, strings

llvm/test/tools/llvm-objcopy/ELF/set-section-flags.test

 # RUN: yaml2obj %s > %t
 
 # Single flags on a section with no flags:
 # RUN: llvm-objcopy --set-section-flags=.foo=alloc \
 # RUN:   --set-section-flags=.baz=alloc --set-section-flags=.rela.baz=alloc %t %t.alloc
 # RUN: llvm-readobj --sections %t.alloc | FileCheck %s --check-prefixes=CHECK,NOBITS,ALLOC,WRITE
 # RUN: llvm-objcopy --set-section-flags=.foo=load \
 # RUN:   --set-section-flags=.baz=load --set-section-flags=.rela.baz=load %t %t.load
 # RUN: llvm-readobj --sections %t.load | FileCheck %s --check-prefixes=CHECK,PROGBITS,WRITE
 # RUN: llvm-objcopy --set-section-flags=.foo=noload \
 # RUN:   --set-section-flags=.baz=noload --set-section-flags=.rela.baz=noload %t %t.noload
 # RUN: llvm-readobj --sections %t.noload | FileCheck %s --check-prefixes=CHECK,PROGBITS,WRITE
 # RUN: llvm-objcopy --set-section-flags=.foo=readonly \
 # RUN:   --set-section-flags=.baz=readonly --set-section-flags=.rela.baz=readonly %t %t.readonly
 # RUN: llvm-readobj --sections %t.readonly | FileCheck %s --check-prefixes=CHECK,PROGBITS
+# RUN: llvm-objcopy --set-section-flags=.foo=exclude \
+# RUN:   --set-section-flags=.baz=exclude --set-section-flags=.rela.baz=exclude %t %t.exclude
+# RUN: llvm-readobj --sections %t.exclude | FileCheck %s --check-prefixes=CHECK,PROGBITS,EXCLUDE,WRITE
 # RUN: llvm-objcopy --set-section-flags=.foo=debug \
 # RUN:   --set-section-flags=.baz=debug --set-section-flags=.rela.baz=debug %t %t.debug
 # RUN: llvm-readobj --sections %t.debug | FileCheck %s --check-prefixes=CHECK,PROGBITS,WRITE
 # RUN: llvm-objcopy --set-section-flags=.foo=code \
 # RUN:   --set-section-flags=.baz=code --set-section-flags=.rela.baz=code %t %t.code
 # RUN: llvm-readobj --sections %t.code | FileCheck %s --check-prefixes=CHECK,PROGBITS,EXEC,WRITE
 # RUN: llvm-objcopy --set-section-flags=.foo=data \
 # RUN:   --set-section-flags=.baz=data --set-section-flags=.rela.baz=data %t %t.data

 # RUN: llvm-objcopy --set-section-flags=.foo=alloc,readonly,strings \
 # RUN:   --set-section-flags=.baz=alloc,readonly,strings \
 # RUN:   --set-section-flags=.rela.baz=alloc,readonly,strings %t %t.alloc_ro_strings
 # RUN: llvm-readobj --sections %t.alloc_ro_strings | FileCheck %s --check-prefixes=CHECK,NOBITS,ALLOC,STRINGS
 # RUN: llvm-objcopy --set-section-flags=.foo=alloc,code \
 # RUN:   --set-section-flags=.baz=alloc,code \
 # RUN:   --set-section-flags=.rela.baz=alloc,code %t %t.alloc_code
 # RUN: llvm-readobj --sections %t.alloc_code | FileCheck %s --check-prefixes=CHECK,NOBITS,ALLOC,EXEC,WRITE
+# 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
 
 # Invalid flags:
 # RUN: not llvm-objcopy --set-section-flags=.foo=xyzzy %t %t.xyzzy 2>&1 | FileCheck %s --check-prefix=BAD-FLAG
 
 # Bad flag format:
 # RUN: not llvm-objcopy --set-section-flags=.foo %t %t2 2>&1 | FileCheck %s --check-prefix=BAD-FORMAT
 
 # Setting flags for the same section multiple times:

 
 # CHECK:         Name: .foo
 # CHECK-NEXT:    Type: SHT_PROGBITS
 # CHECK-NEXT:    Flags [
 # ALLOC-NEXT:      SHF_ALLOC (0x2)
 # EXEC-NEXT:       SHF_EXECINSTR (0x4)
 # MERGE-NEXT:      SHF_MERGE (0x10)
 # STRINGS-NEXT:    SHF_STRINGS (0x20)
+# EXCLUDE-NEXT:    SHF_EXCLUDE (0x80000000)
 # WRITE-NEXT:      SHF_WRITE (0x1)
 # CHECK-NEXT:    ]
 
 # CHECK:         Name: .baz
 # NOBITS-NEXT:   Type: SHT_NOBITS
 # PROGBITS-NEXT: Type: SHT_PROGBITS
 # CHECK-NEXT:    Flags [
 # ALLOC-NEXT:      SHF_ALLOC (0x2)
 # EXEC-NEXT:       SHF_EXECINSTR (0x4)
 # MERGE-NEXT:      SHF_MERGE (0x10)
 # STRINGS-NEXT:    SHF_STRINGS (0x20)
+# EXCLUDE-NEXT:    SHF_EXCLUDE (0x80000000)
 # WRITE-NEXT:      SHF_WRITE (0x1)
 # CHECK-NEXT:    ]
 
 # CHECK:         Name: .rela.baz
 # CHECK-NEXT:    Type: SHT_RELA
 # CHECK-NEXT:    Flags [
 # ALLOC-NEXT:      SHF_ALLOC (0x2)
 # EXEC-NEXT:       SHF_EXECINSTR (0x4)
 # MERGE-NEXT:      SHF_MERGE (0x10)
 # STRINGS-NEXT:    SHF_STRINGS (0x20)
+# EXCLUDE-NEXT:    SHF_EXCLUDE (0x80000000)
 # WRITE-NEXT:      SHF_WRITE (0x1)
 # CHECK-NEXT:    ]
 
 # BAD-FORMAT: bad format for --set-section-flags: missing '='
 # MULTIPLE-SETS: --set-section-flags set multiple times for section '.foo'
 
-# BAD-FLAG: unrecognized section flag 'xyzzy'. Flags supported for GNU compatibility: alloc, load, noload, readonly, debug, code, data, rom, share, contents, merge, strings
+# BAD-FLAG: unrecognized section flag 'xyzzy'. Flags supported for GNU compatibility: alloc, load, noload, readonly, exclude, debug, code, data, rom, share, contents, merge, strings

llvm/tools/llvm-objcopy/CopyConfig.h

enum SectionFlag {
   SecDebug = 1 << 4,
   SecCode = 1 << 5,
   SecData = 1 << 6,
   SecRom = 1 << 7,
   SecMerge = 1 << 8,
   SecStrings = 1 << 9,
   SecContents = 1 << 10,
   SecShare = 1 << 11,
-  LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ SecShare)
+  SecExclude = 1 << 12,
+  LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ SecExclude)
 };
 
 struct SectionRename {
   StringRef OriginalName;
   StringRef NewName;
   Optional<SectionFlag> NewFlags;
 };
 

llvm/tools/llvm-objcopy/CopyConfig.cpp

return llvm::StringSwitch<SectionFlag>(SectionName)
       .CaseLower("debug", SectionFlag::SecDebug)
       .CaseLower("code", SectionFlag::SecCode)
       .CaseLower("data", SectionFlag::SecData)
       .CaseLower("rom", SectionFlag::SecRom)
       .CaseLower("merge", SectionFlag::SecMerge)
       .CaseLower("strings", SectionFlag::SecStrings)
       .CaseLower("contents", SectionFlag::SecContents)
       .CaseLower("share", SectionFlag::SecShare)
+      .CaseLower("exclude", SectionFlag::SecExclude)
       .Default(SectionFlag::SecNone);
 }
 
 static Expected<SectionFlag>
 parseSectionFlagSet(ArrayRef<StringRef> SectionFlags) {
   SectionFlag ParsedFlags = SectionFlag::SecNone;
   for (StringRef Flag : SectionFlags) {
     SectionFlag ParsedFlag = parseSectionRenameFlag(Flag);
     if (ParsedFlag == SectionFlag::SecNone)
       return createStringError(
           errc::invalid_argument,
           "unrecognized section flag '%s'. Flags supported for GNU "
-          "compatibility: alloc, load, noload, readonly, debug, code, data, "
-          "rom, share, contents, merge, strings",
+          "compatibility: alloc, load, noload, readonly, exclude, debug, code, "
+          "data, rom, share, contents, merge, strings",
           Flag.str().c_str());
     ParsedFlags |= ParsedFlag;
   }
 
   return ParsedFlags;
 }
 
 static Expected<SectionRename> parseRenameSectionValue(StringRef FlagValue) {

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

uint64_t getNewShfFlags(SectionFlag AllFlags) {
   if (!(AllFlags & SectionFlag::SecReadonly))
     NewFlags |= ELF::SHF_WRITE;
   if (AllFlags & SectionFlag::SecCode)
     NewFlags |= ELF::SHF_EXECINSTR;
   if (AllFlags & SectionFlag::SecMerge)
     NewFlags |= ELF::SHF_MERGE;
   if (AllFlags & SectionFlag::SecStrings)
     NewFlags |= ELF::SHF_STRINGS;
+  if (AllFlags & SectionFlag::SecExclude)
+    NewFlags |= ELF::SHF_EXCLUDE;
   return NewFlags;
 }
 
 static uint64_t getSectionFlagsPreserveMask(uint64_t OldFlags,
                                             uint64_t NewFlags) {
   // Preserve some flags which should not be dropped when setting flags.
   // Also, preserve anything OS/processor dependant.
-  const uint64_t PreserveMask = ELF::SHF_COMPRESSED | ELF::SHF_EXCLUDE |
-                                ELF::SHF_GROUP | ELF::SHF_LINK_ORDER |
-                                ELF::SHF_MASKOS | ELF::SHF_MASKPROC |
-                                ELF::SHF_TLS | ELF::SHF_INFO_LINK;
-  return (OldFlags & PreserveMask) | (NewFlags & ~PreserveMask);
+  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;
 }
 
 static void setSectionFlagsAndType(SectionBase &Sec, SectionFlag Flags) {
   Sec.Flags = getSectionFlagsPreserveMask(Sec.Flags, getNewShfFlags(Flags));
 
   // In GNU objcopy, certain flags promote SHT_NOBITS to SHT_PROGBITS. This rule
   // may promote more non-ALLOC sections than GNU objcopy, but it is fine as
   // non-ALLOC SHT_NOBITS sections do not make much sense.