[llvm-objcopy] Implement --set-section-flags.

[rupprecht]

Summary

--set-section-flags is used to change the section flags (e.g. SHF_ALLOC) for given sections. The flags allowed are the same from the existing --rename-section=.old=.new[,flags] feature.

Additionally, make sure that --set-section-flag cannot be used with --rename-section (either the source or destination), since --rename-section accepts flags. This avoids ambiguity for something like "--rename-section=.foo=.bar,alloc --set-section-flag=.bar,code".

[rupprecht]

• Add --set-section-flags to the list of unimplemented COFF methods

[jhenderson]

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

16 ↗

(On Diff #183416)

Nit: get rid of the content. It's not needed. Can you get rid of the Type and Flags fields too, or are they required?

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

16 ↗

(On Diff #183416)

Ditto, here and below.

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

54 ↗

(On Diff #183416)

Ditto.

llvm/tools/llvm-objcopy/CopyConfig.cpp

341–344 ↗

(On Diff #183416)

This should look almost the same as the --rename-section code above, i.e. where the error checking for bad format etc is done.

[rupprecht]

• Remove unnecessary portions from tests
• Make --set-section-flags arg parsing more similar to --rename-section

[rupprecht]

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

16 ↗

(On Diff #183416)

This flag checking is done during argument parsing, so no section is needed at all.

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

16 ↗

(On Diff #183416)

Removed content here, but I'd prefer to leave flags explicitly empty here (even though the test passes w/o it). Type is a required yaml field here.

[jakehehrlich]

Even though this is a large change I think I'm happy with it. Everything was either factoring out common code or handling error messages. I didn't see anything about how error messages were handled that was objectionable.

Please wait on someone else but this LGTM.

[jakehehrlich]

In D57198#1370341, @rupprecht wrote:

• Add --set-section-flags to the list of unimplemented COFF methods

Crap I guess we have to remember to do that for all new flags. I should have asked for this to be a white list instead of a black list. It doesn't make sense for ELF and eventually MachO to have to update flags when one or the other doesn't support this.

[jhenderson]

LGTM.

llvm/trunk/test/tools/llvm-objcopy/ELF/set-section-flags-and-rename.test

+# RUN: yaml2obj %s > %t
+
+# RUN: not llvm-objcopy --rename-section=.foo=.bar --set-section-flags=.foo=alloc %t %t.2 2>&1 | FileCheck %s --check-prefix=SET-FOO
+# RUN: not llvm-objcopy --rename-section=.foo=.bar --set-section-flags=.bar=alloc %t %t.2 2>&1 | FileCheck %s --check-prefix=SET-BAR
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+
+# SET-FOO: --set-section-flags=.foo conflicts with --rename-section=.foo=.bar.
+# SET-BAR: --set-section-flags=.bar conflicts with --rename-section=.foo=.bar.

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

+# RUN: yaml2obj %s > %t
+
+# RUN: llvm-objcopy --set-section-flags=.foo=alloc --set-section-flags=.bar=code %t %t.2
+# RUN: llvm-readobj --sections %t.2 | FileCheck %s --check-prefixes=CHECK,ALLOC,WRITE
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .foo
+    Type:            SHT_PROGBITS
+    Flags:           [ ]
+  - Name:            .bar
+    Type:            SHT_PROGBITS
+    Flags:           [ ]
+
+# CHECK:        Name: .foo
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_ALLOC (0x2)
+# CHECK-NEXT:     SHF_WRITE (0x1)
+# CHECK-NEXT:   ]
+
+# CHECK:        Name: .bar
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_EXECINSTR (0x4)
+# CHECK-NEXT:     SHF_WRITE (0x1)
+# CHECK-NEXT:   ]

llvm/trunk/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 %t %t.alloc
+# RUN: llvm-readobj --sections %t.alloc | FileCheck %s --check-prefixes=CHECK,ALLOC,WRITE
+# RUN: llvm-objcopy --set-section-flags=.foo=load %t %t.load
+# RUN: llvm-readobj --sections %t.load | FileCheck %s --check-prefixes=CHECK,WRITE
+# RUN: llvm-objcopy --set-section-flags=.foo=noload %t %t.noload
+# RUN: llvm-readobj --sections %t.noload | FileCheck %s --check-prefixes=CHECK,WRITE
+# RUN: llvm-objcopy --set-section-flags=.foo=readonly %t %t.readonly
+# RUN: llvm-readobj --sections %t.readonly | FileCheck %s --check-prefixes=CHECK
+# RUN: llvm-objcopy --set-section-flags=.foo=debug %t %t.debug
+# RUN: llvm-readobj --sections %t.debug | FileCheck %s --check-prefixes=CHECK,WRITE
+# RUN: llvm-objcopy --set-section-flags=.foo=code %t %t.code
+# RUN: llvm-readobj --sections %t.code | FileCheck %s --check-prefixes=CHECK,EXEC,WRITE
+# RUN: llvm-objcopy --set-section-flags=.foo=data %t %t.data
+# RUN: llvm-readobj --sections %t.data | FileCheck %s --check-prefixes=CHECK,WRITE
+# RUN: llvm-objcopy --set-section-flags=.foo=rom %t %t.rom
+# RUN: llvm-readobj --sections %t.rom | FileCheck %s --check-prefixes=CHECK,WRITE
+# RUN: llvm-objcopy --set-section-flags=.foo=contents %t %t.contents
+# RUN: llvm-readobj --sections %t.contents | FileCheck %s --check-prefixes=CHECK,WRITE
+# RUN: llvm-objcopy --set-section-flags=.foo=merge %t %t.merge
+# RUN: llvm-readobj --sections %t.merge | FileCheck %s --check-prefixes=CHECK,MERGE,WRITE
+# RUN: llvm-objcopy --set-section-flags=.foo=strings %t %t.strings
+# RUN: llvm-readobj --sections %t.strings | FileCheck %s --check-prefixes=CHECK,STRINGS,WRITE
+# RUN: llvm-objcopy --set-section-flags=.foo=share %t %t.share
+# RUN: llvm-readobj --sections %t.share | FileCheck %s --check-prefixes=CHECK,WRITE
+
+# Multiple flags:
+# RUN: llvm-objcopy --set-section-flags=.foo=alloc,readonly,strings %t %t.alloc_ro_strings
+# RUN: llvm-readobj --sections %t.alloc_ro_strings | FileCheck %s --check-prefixes=CHECK,ALLOC,STRINGS
+# RUN: llvm-objcopy --set-section-flags=.foo=alloc,code %t %t.alloc_code
+# RUN: llvm-readobj --sections %t.alloc_code | FileCheck %s --check-prefixes=CHECK,ALLOC,EXEC,WRITE
+
+# 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:
+# RUN: not llvm-objcopy --set-section-flags=.foo=alloc --set-section-flags=.foo=load %t %t2 2>&1 | FileCheck %s --check-prefix=MULTIPLE-SETS
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .foo
+    Type:            SHT_PROGBITS
+    Flags:           [ ]
+
+# 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)
+# 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.

llvm/trunk/tools/llvm-objcopy/COFF/COFFObjcopy.cpp

static Error handleArgs(const CopyConfig &Config, Object &Obj) {
 
   if (!Config.BuildIdLinkDir.empty() || Config.BuildIdLinkInput ||
       Config.BuildIdLinkOutput || !Config.SplitDWO.empty() ||
       !Config.SymbolsPrefix.empty() || !Config.AddSection.empty() ||
       !Config.DumpSection.empty() || !Config.KeepSection.empty() ||
       !Config.SymbolsToGlobalize.empty() || !Config.SymbolsToKeep.empty() ||
       !Config.SymbolsToLocalize.empty() || !Config.SymbolsToWeaken.empty() ||
       !Config.SymbolsToKeepGlobal.empty() || !Config.SectionsToRename.empty() ||
-      !Config.SymbolsToRename.empty() || Config.ExtractDWO ||
-      Config.KeepFileSymbols || Config.LocalizeHidden || Config.PreserveDates ||
-      Config.StripDWO || Config.StripNonAlloc || Config.StripSections ||
-      Config.Weaken || Config.DecompressDebugSections) {
+      !Config.SetSectionFlags.empty() || !Config.SymbolsToRename.empty() ||
+      Config.ExtractDWO || Config.KeepFileSymbols || Config.LocalizeHidden ||
+      Config.PreserveDates || Config.StripDWO || Config.StripNonAlloc ||
+      Config.StripSections || Config.Weaken || Config.DecompressDebugSections) {
     return createStringError(llvm::errc::invalid_argument,
                              "Option not supported by llvm-objcopy for COFF");
   }
 
   return Error::success();
 }
 
 void executeObjcopyOnBinary(const CopyConfig &Config,

llvm/trunk/tools/llvm-objcopy/CopyConfig.h

 };
 
 struct SectionRename {
   StringRef OriginalName;
   StringRef NewName;
   Optional<uint64_t> NewFlags;
 };
 
+struct SectionFlagsUpdate {
+  StringRef Name;
+  uint64_t NewFlags;
+};
+
 // Configuration for copying/stripping a single file.
 struct CopyConfig {
   // Main input/output options
   StringRef InputFilename;
   StringRef InputFormat;
   StringRef OutputFilename;
   StringRef OutputFormat;
 
struct CopyConfig {
   std::vector<StringRef> SymbolsToLocalize;
   std::vector<StringRef> SymbolsToRemove;
   std::vector<StringRef> SymbolsToWeaken;
   std::vector<StringRef> ToRemove;
   std::vector<std::string> SymbolsToKeepGlobal;
 
   // Map options
   StringMap<SectionRename> SectionsToRename;
+  StringMap<SectionFlagsUpdate> SetSectionFlags;
   StringMap<StringRef> SymbolsToRename;
 
   // Boolean options
   bool DeterministicArchives = true;
   bool DiscardAll = false;
   bool ExtractDWO = false;
   bool KeepFileSymbols = false;
   bool LocalizeHidden = false;

llvm/trunk/tools/llvm-objcopy/CopyConfig.cpp

return llvm::StringSwitch<SectionFlag>(SectionName)
       .Case("rom", SectionFlag::SecRom)
       .Case("merge", SectionFlag::SecMerge)
       .Case("strings", SectionFlag::SecStrings)
       .Case("contents", SectionFlag::SecContents)
       .Case("share", SectionFlag::SecShare)
       .Default(SectionFlag::SecNone);
 }
 
+static uint64_t parseSectionFlagSet(ArrayRef<StringRef> SectionFlags) {
+  SectionFlag ParsedFlags = SectionFlag::SecNone;
+  for (StringRef Flag : SectionFlags) {
+    SectionFlag ParsedFlag = parseSectionRenameFlag(Flag);
+    if (ParsedFlag == SectionFlag::SecNone)
+      error("Unrecognized section flag '" + Flag +
+            "'. Flags supported for GNU compatibility: alloc, load, noload, "
+            "readonly, debug, code, data, rom, share, contents, merge, "
+            "strings.");
+    ParsedFlags |= ParsedFlag;
+  }
+
+  uint64_t NewFlags = 0;
+  if (ParsedFlags & SectionFlag::SecAlloc)
+    NewFlags |= ELF::SHF_ALLOC;
+  if (!(ParsedFlags & SectionFlag::SecReadonly))
+    NewFlags |= ELF::SHF_WRITE;
+  if (ParsedFlags & SectionFlag::SecCode)
+    NewFlags |= ELF::SHF_EXECINSTR;
+  if (ParsedFlags & SectionFlag::SecMerge)
+    NewFlags |= ELF::SHF_MERGE;
+  if (ParsedFlags & SectionFlag::SecStrings)
+    NewFlags |= ELF::SHF_STRINGS;
+  return NewFlags;
+}
+
 static SectionRename parseRenameSectionValue(StringRef FlagValue) {
   if (!FlagValue.contains('='))
     error("Bad format for --rename-section: missing '='");
 
   // Initial split: ".foo" = ".bar,f1,f2,..."
   auto Old2New = FlagValue.split('=');
   SectionRename SR;
   SR.OriginalName = Old2New.first;
 
   // Flags split: ".bar" "f1" "f2" ...
   SmallVector<StringRef, 6> NameAndFlags;
   Old2New.second.split(NameAndFlags, ',');
   SR.NewName = NameAndFlags[0];
 
-  if (NameAndFlags.size() > 1) {
-    SectionFlag Flags = SectionFlag::SecNone;
-    for (size_t I = 1, Size = NameAndFlags.size(); I < Size; ++I) {
-      SectionFlag Flag = parseSectionRenameFlag(NameAndFlags[I]);
-      if (Flag == SectionFlag::SecNone)
-        error("Unrecognized section flag '" + NameAndFlags[I] +
-              "'. Flags supported for GNU compatibility: alloc, load, noload, "
-              "readonly, debug, code, data, rom, share, contents, merge, "
-              "strings.");
-      Flags |= Flag;
-    }
+  if (NameAndFlags.size() > 1)
+    SR.NewFlags = parseSectionFlagSet(makeArrayRef(NameAndFlags).drop_front());
 
-    SR.NewFlags = 0;
-    if (Flags & SectionFlag::SecAlloc)
-      *SR.NewFlags |= ELF::SHF_ALLOC;
-    if (!(Flags & SectionFlag::SecReadonly))
-      *SR.NewFlags |= ELF::SHF_WRITE;
-    if (Flags & SectionFlag::SecCode)
-      *SR.NewFlags |= ELF::SHF_EXECINSTR;
-    if (Flags & SectionFlag::SecMerge)
-      *SR.NewFlags |= ELF::SHF_MERGE;
-    if (Flags & SectionFlag::SecStrings)
-      *SR.NewFlags |= ELF::SHF_STRINGS;
-  }
+  return SR;
 
-  return SR;
+static SectionFlagsUpdate parseSetSectionFlagValue(StringRef FlagValue) {
+  if (!StringRef(FlagValue).contains('='))
+    error("Bad format for --set-section-flags: missing '='");
+
+  // Initial split: ".foo" = "f1,f2,..."
+  auto Section2Flags = StringRef(FlagValue).split('=');
+  SectionFlagsUpdate SFU;
+  SFU.Name = Section2Flags.first;
+
+  // Flags split: "f1" "f2" ...
+  SmallVector<StringRef, 6> SectionFlags;
+  Section2Flags.second.split(SectionFlags, ',');
+  SFU.NewFlags = parseSectionFlagSet(SectionFlags);
+
+  return SFU;
 }
 
 static const StringMap<MachineInfo> ArchMap{
     // Name, {EMachine, 64bit, LittleEndian}
     {"aarch64", {ELF::EM_AARCH64, true, true}},
     {"arm", {ELF::EM_ARM, false, true}},
     {"i386", {ELF::EM_386, false, true}},
     {"i386:x86-64", {ELF::EM_X86_64, true, true}},
if (!Config.SymbolsToRename.insert(Old2New).second)
       error("Multiple redefinition of symbol " + Old2New.first);
   }
 
   for (auto Arg : InputArgs.filtered(OBJCOPY_rename_section)) {
     SectionRename SR = parseRenameSectionValue(StringRef(Arg->getValue()));
     if (!Config.SectionsToRename.try_emplace(SR.OriginalName, SR).second)
       error("Multiple renames of section " + SR.OriginalName);
   }
+  for (auto Arg : InputArgs.filtered(OBJCOPY_set_section_flags)) {
+    SectionFlagsUpdate SFU = parseSetSectionFlagValue(Arg->getValue());
+    if (!Config.SetSectionFlags.try_emplace(SFU.Name, SFU).second)
+      error("--set-section-flags set multiple times for section " + SFU.Name);
+  }
+  // Prohibit combinations of --set-section-flags when the section name is used
+  // by --rename-section, either as a source or a destination.
+  for (const auto &E : Config.SectionsToRename) {
+    const SectionRename &SR = E.second;
+    if (Config.SetSectionFlags.count(SR.OriginalName))
+      error("--set-section-flags=" + SR.OriginalName +
+            " conflicts with --rename-section=" + SR.OriginalName + "=" +
+            SR.NewName);
+    if (Config.SetSectionFlags.count(SR.NewName))
+      error("--set-section-flags=" + SR.NewName +
+            " conflicts with --rename-section=" + SR.OriginalName + "=" +
+            SR.NewName);
+  }
 
   for (auto Arg : InputArgs.filtered(OBJCOPY_remove_section))
     Config.ToRemove.push_back(Arg->getValue());
   for (auto Arg : InputArgs.filtered(OBJCOPY_keep_section))
     Config.KeepSection.push_back(Arg->getValue());
   for (auto Arg : InputArgs.filtered(OBJCOPY_only_section))
     Config.OnlySection.push_back(Arg->getValue());
   for (auto Arg : InputArgs.filtered(OBJCOPY_add_section))

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

static bool onlyKeepDWOPred(const Object &Obj, const SectionBase &Sec) {
   // We can't remove the section header string table.
   if (&Sec == Obj.SectionNames)
     return false;
   // Short of keeping the string table we want to keep everything that is a DWO
   // section and remove everything else.
   return !isDWOSection(Sec);
 }
 
+static uint64_t setSectionFlagsPreserveMask(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);
+}
+
 static ElfType getOutputElfType(const Binary &Bin) {
   // Infer output ELF type from the input ELF object
   if (isa<ELFObjectFile<ELF32LE>>(Bin))
     return ELFT_ELF32LE;
   if (isa<ELFObjectFile<ELF64LE>>(Bin))
     return ELFT_ELF64LE;
   if (isa<ELFObjectFile<ELF32BE>>(Bin))
     return ELFT_ELF32BE;
static void handleArgs(const CopyConfig &Config, Object &Obj,
   Obj.removeSections(RemovePred);
 
   if (!Config.SectionsToRename.empty()) {
     for (auto &Sec : Obj.sections()) {
       const auto Iter = Config.SectionsToRename.find(Sec.Name);
       if (Iter != Config.SectionsToRename.end()) {
         const SectionRename &SR = Iter->second;
         Sec.Name = SR.NewName;
-        if (SR.NewFlags.hasValue()) {
-          // 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;
-          Sec.Flags = (Sec.Flags & PreserveMask) |
-                      (SR.NewFlags.getValue() & ~PreserveMask);
-        }
-      }
-    }
+        if (SR.NewFlags.hasValue())
+          Sec.Flags =
+              setSectionFlagsPreserveMask(Sec.Flags, SR.NewFlags.getValue());
+
+  if (!Config.SetSectionFlags.empty()) {
+    for (auto &Sec : Obj.sections()) {
+      const auto Iter = Config.SetSectionFlags.find(Sec.Name);
+      if (Iter != Config.SetSectionFlags.end()) {
+        const SectionFlagsUpdate &SFU = Iter->second;
+        Sec.Flags = setSectionFlagsPreserveMask(Sec.Flags, SFU.NewFlags);
+      }
+    }
   }
 
   if (!Config.AddSection.empty()) {
     for (const auto &Flag : Config.AddSection) {
       std::pair<StringRef, StringRef> SecPair = Flag.split("=");
       StringRef SecName = SecPair.first;
       StringRef File = SecPair.second;
       ErrorOr<std::unique_ptr<MemoryBuffer>> BufOrErr =

llvm/trunk/tools/llvm-objcopy/ObjcopyOpts.td

 defm only_section : Eq<"only-section", "Remove all but <section>">,
                     MetaVarName<"section">;
 def j : JoinedOrSeparate<["-"], "j">, Alias<only_section>;
 defm add_section
     : Eq<"add-section",
          "Make a section named <section> with the contents of <file>.">,
       MetaVarName<"section=file">;
 
+defm set_section_flags
+    : Eq<"set-section-flags",
+         "Set section flags for a given section. Flags supported for GNU "
+         "compatibility: alloc, load, noload, readonly, debug, code, data, "
+         "rom, share, contents, merge, strings.">,
+      MetaVarName<"section=flag1[,flag2,...]">;
+
 def strip_all
     : Flag<["-", "--"], "strip-all">,
       HelpText<
           "Remove non-allocated sections other than .gnu.warning* sections">;
 def S : Flag<["-"], "S">, Alias<strip_all>;
 def strip_all_gnu : Flag<["-", "--"], "strip-all-gnu">,
                     HelpText<"Compatible with GNU objcopy's --strip-all">;
 def strip_debug : Flag<["-", "--"], "strip-debug">,