[llvm-objcopy][NFC] Refactor symbol/section matching

[rupprecht]

Summary

The matchers for section/symbol related flags (e.g. --keep-symbol=Name or --regex --keep-symbol=foo.*) are currently just vectors that are matched linearlly. However, adding wildcard support would require negative matching too, e.g. a symbol should be removed if it matches a wildcard *but* doesn't match some other wildcard.

To make the next patch simpler, consolidate matching logic to a class defined in CopyConfig that takes care of matching.

[MaskRay]

Looks good. With a nit:

llvm/tools/llvm-objcopy/CopyConfig.h

108 ↗

(On Diff #215952)

AddMatcher and Matches should not be capitalized.

[jhenderson]

llvm/tools/llvm-objcopy/CopyConfig.h

101 ↗

(On Diff #215952)

Maybe worth a comment?

Also MatcherSet probably is a bad name, since it's more a vector than a set...

How about NameMatcher?

[rupprecht]

• Fix method casing
• Change class name to NameMatcher

[jhenderson]

LGTM.

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

addSection(Obj, ".gnu_debuglink", Contents,
                  IMAGE_SCN_MEM_DISCARDABLE);
 }
 
 static Error handleArgs(const CopyConfig &Config, Object &Obj) {
   // Perform the actual section removals.
   Obj.removeSections([&Config](const Section &Sec) {
     // Contrary to --only-keep-debug, --only-section fully removes sections that
     // aren't mentioned.
-    if (!Config.OnlySection.empty() &&
-        !is_contained(Config.OnlySection, Sec.Name))
+    if (!Config.OnlySection.empty() && !Config.OnlySection.matches(Sec.Name))
       return true;
 
     if (Config.StripDebug || Config.StripAll || Config.StripAllGNU ||
         Config.DiscardMode == DiscardType::All || Config.StripUnneeded) {
       if (isDebugSection(Sec) &&
           (Sec.Header.Characteristics & IMAGE_SCN_MEM_DISCARDABLE) != 0)
         return true;
     }
 
-    if (is_contained(Config.ToRemove, Sec.Name))
+    if (Config.ToRemove.matches(Sec.Name))
       return true;
 
     return false;
   });
 
   if (Config.OnlyKeepDebug) {
     // For --only-keep-debug, we keep all other sections, but remove their
     // content. The VirtualSize field in the section header is kept intact.
static Error handleArgs(const CopyConfig &Config, Object &Obj) {
 
   // Actually do removals of symbols.
   Obj.removeSymbols([&](const Symbol &Sym) {
     // For StripAll, all relocations have been stripped and we remove all
     // symbols.
     if (Config.StripAll || Config.StripAllGNU)
       return true;
 
-    if (is_contained(Config.SymbolsToRemove, Sym.Name)) {
+    if (Config.SymbolsToRemove.matches(Sym.Name)) {
       // Explicitly removing a referenced symbol is an error.
       if (Sym.Referenced)
         reportError(Config.OutputFilename,
                     createStringError(llvm::errc::invalid_argument,
                                       "not stripping symbol '%s' because it is "
                                       "named in a relocation",
                                       Sym.Name.str().c_str()));
       return true;
     }
 
     if (!Sym.Referenced) {
       // With --strip-unneeded, GNU objcopy removes all unreferenced local
       // symbols, and any unreferenced undefined external.
       // With --strip-unneeded-symbol we strip only specific unreferenced
       // local symbol instead of removing all of such.
       if (Sym.Sym.StorageClass == IMAGE_SYM_CLASS_STATIC ||
           Sym.Sym.SectionNumber == 0)
         if (Config.StripUnneeded ||
-            is_contained(Config.UnneededSymbolsToRemove, Sym.Name))
+            Config.UnneededSymbolsToRemove.matches(Sym.Name))
           return true;
 
       // GNU objcopy keeps referenced local symbols and external symbols
       // if --discard-all is set, similar to what --strip-unneeded does,
       // but undefined local symbols are kept when --discard-all is set.
       if (Config.DiscardMode == DiscardType::All &&
           Sym.Sym.StorageClass == IMAGE_SYM_CLASS_STATIC &&
           Sym.Sym.SectionNumber != 0)

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

class NameOrRegex {
   std::shared_ptr<Regex> R;
 
 public:
   NameOrRegex(StringRef Pattern, bool IsRegex);
   bool operator==(StringRef S) const { return R ? R->match(S) : Name == S; }
   bool operator!=(StringRef S) const { return !operator==(S); }
 };
 
+// Matcher that checks symbol or section names against the command line flags
+// provided for that option.
+class NameMatcher {
+  std::vector<NameOrRegex> Matchers;
+
+public:
+  void addMatcher(NameOrRegex Matcher) {
+    Matchers.push_back(std::move(Matcher));
+  }
+  bool matches(StringRef S) const { return is_contained(Matchers, S); }
+  bool empty() const { return Matchers.empty(); }
+};
+
 struct NewSymbolInfo {
   StringRef SymbolName;
   StringRef SectionName;
   uint64_t Value = 0;
   uint8_t Type = ELF::STT_NOTYPE;
   uint8_t Bind = ELF::STB_GLOBAL;
   uint8_t Visibility = ELF::STV_DEFAULT;
 };
struct CopyConfig {
   StringRef SymbolsPrefix;
   StringRef AllocSectionsPrefix;
   DiscardType DiscardMode = DiscardType::None;
 
   // Repeated options
   std::vector<StringRef> AddSection;
   std::vector<StringRef> DumpSection;
   std::vector<NewSymbolInfo> SymbolsToAdd;
-  std::vector<NameOrRegex> KeepSection;
-  std::vector<NameOrRegex> OnlySection;
-  std::vector<NameOrRegex> SymbolsToGlobalize;
-  std::vector<NameOrRegex> SymbolsToKeep;
-  std::vector<NameOrRegex> SymbolsToLocalize;
-  std::vector<NameOrRegex> SymbolsToRemove;
-  std::vector<NameOrRegex> UnneededSymbolsToRemove;
-  std::vector<NameOrRegex> SymbolsToWeaken;
-  std::vector<NameOrRegex> ToRemove;
-  std::vector<NameOrRegex> SymbolsToKeepGlobal;
+
+  // Section matchers
+  NameMatcher KeepSection;
+  NameMatcher OnlySection;
+  NameMatcher ToRemove;
+
+  // Symbol matchers
+  NameMatcher SymbolsToGlobalize;
+  NameMatcher SymbolsToKeep;
+  NameMatcher SymbolsToLocalize;
+  NameMatcher SymbolsToRemove;
+  NameMatcher UnneededSymbolsToRemove;
+  NameMatcher SymbolsToWeaken;
+  NameMatcher SymbolsToKeepGlobal;
 
   // Map options
   StringMap<SectionRename> SectionsToRename;
   StringMap<SectionFlagsUpdate> SetSectionFlags;
   StringMap<StringRef> SymbolsToRename;
 
   // ELF entry point address expression. The input parameter is an entry point
   // address in the input ELF file. The entry address in the output file is

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

getOutputTargetInfoByTargetName(StringRef TargetName) {
   else
     // This should never happen because `TargetName` is valid (it certainly
     // exists in the TargetMap).
     llvm_unreachable("unknown target prefix");
 
   return {TargetInfo{Format, MI}};
 }
 
-static Error addSymbolsFromFile(std::vector<NameOrRegex> &Symbols,
-                                BumpPtrAllocator &Alloc, StringRef Filename,
-                                bool UseRegex) {
+static Error addSymbolsFromFile(NameMatcher &Symbols, BumpPtrAllocator &Alloc,
+                                StringRef Filename, bool UseRegex) {
   StringSaver Saver(Alloc);
   SmallVector<StringRef, 16> Lines;
   auto BufOrErr = MemoryBuffer::getFile(Filename);
   if (!BufOrErr)
     return createFileError(Filename, BufOrErr.getError());
 
   BufOrErr.get()->getBuffer().split(Lines, '\n');
   for (StringRef Line : Lines) {
     // Ignore everything after '#', trim whitespace, and only add the symbol if
     // it's not empty.
     auto TrimmedLine = Line.split('#').first.trim();
     if (!TrimmedLine.empty())
-      Symbols.emplace_back(Saver.save(TrimmedLine), UseRegex);
+      Symbols.addMatcher({Saver.save(TrimmedLine), UseRegex});
   }
 
   return Error::success();
 }
 
 NameOrRegex::NameOrRegex(StringRef Pattern, bool IsRegex) {
   if (!IsRegex) {
     Name = Pattern;
if (Config.SetSectionFlags.count(SR.NewName))
       return createStringError(
           errc::invalid_argument,
           "--set-section-flags=%s conflicts with --rename-section=%s=%s",
           SR.NewName.str().c_str(), SR.OriginalName.str().c_str(),
           SR.NewName.str().c_str());
   }
 
   for (auto Arg : InputArgs.filtered(OBJCOPY_remove_section))
-    Config.ToRemove.emplace_back(Arg->getValue(), UseRegex);
+    Config.ToRemove.addMatcher({Arg->getValue(), UseRegex});
   for (auto Arg : InputArgs.filtered(OBJCOPY_keep_section))
-    Config.KeepSection.emplace_back(Arg->getValue(), UseRegex);
+    Config.KeepSection.addMatcher({Arg->getValue(), UseRegex});
   for (auto Arg : InputArgs.filtered(OBJCOPY_only_section))
-    Config.OnlySection.emplace_back(Arg->getValue(), UseRegex);
+    Config.OnlySection.addMatcher({Arg->getValue(), UseRegex});
   for (auto Arg : InputArgs.filtered(OBJCOPY_add_section)) {
     StringRef ArgValue(Arg->getValue());
     if (!ArgValue.contains('='))
       return createStringError(errc::invalid_argument,
                                "bad format for --add-section: missing '='");
     if (ArgValue.split("=").second.empty())
       return createStringError(
           errc::invalid_argument,
Config.DiscardMode =
             : DiscardType::Locals;
   Config.OnlyKeepDebug = InputArgs.hasArg(OBJCOPY_only_keep_debug);
   Config.KeepFileSymbols = InputArgs.hasArg(OBJCOPY_keep_file_symbols);
   Config.DecompressDebugSections =
       InputArgs.hasArg(OBJCOPY_decompress_debug_sections);
   if (Config.DiscardMode == DiscardType::All)
     Config.StripDebug = true;
   for (auto Arg : InputArgs.filtered(OBJCOPY_localize_symbol))
-    Config.SymbolsToLocalize.emplace_back(Arg->getValue(), UseRegex);
+    Config.SymbolsToLocalize.addMatcher({Arg->getValue(), UseRegex});
   for (auto Arg : InputArgs.filtered(OBJCOPY_localize_symbols))
     if (Error E = addSymbolsFromFile(Config.SymbolsToLocalize, DC.Alloc,
                                      Arg->getValue(), UseRegex))
       return std::move(E);
   for (auto Arg : InputArgs.filtered(OBJCOPY_keep_global_symbol))
-    Config.SymbolsToKeepGlobal.emplace_back(Arg->getValue(), UseRegex);
+    Config.SymbolsToKeepGlobal.addMatcher({Arg->getValue(), UseRegex});
   for (auto Arg : InputArgs.filtered(OBJCOPY_keep_global_symbols))
     if (Error E = addSymbolsFromFile(Config.SymbolsToKeepGlobal, DC.Alloc,
                                      Arg->getValue(), UseRegex))
       return std::move(E);
   for (auto Arg : InputArgs.filtered(OBJCOPY_globalize_symbol))
-    Config.SymbolsToGlobalize.emplace_back(Arg->getValue(), UseRegex);
+    Config.SymbolsToGlobalize.addMatcher({Arg->getValue(), UseRegex});
   for (auto Arg : InputArgs.filtered(OBJCOPY_globalize_symbols))
     if (Error E = addSymbolsFromFile(Config.SymbolsToGlobalize, DC.Alloc,
                                      Arg->getValue(), UseRegex))
       return std::move(E);
   for (auto Arg : InputArgs.filtered(OBJCOPY_weaken_symbol))
-    Config.SymbolsToWeaken.emplace_back(Arg->getValue(), UseRegex);
+    Config.SymbolsToWeaken.addMatcher({Arg->getValue(), UseRegex});
   for (auto Arg : InputArgs.filtered(OBJCOPY_weaken_symbols))
     if (Error E = addSymbolsFromFile(Config.SymbolsToWeaken, DC.Alloc,
                                      Arg->getValue(), UseRegex))
       return std::move(E);
   for (auto Arg : InputArgs.filtered(OBJCOPY_strip_symbol))
-    Config.SymbolsToRemove.emplace_back(Arg->getValue(), UseRegex);
+    Config.SymbolsToRemove.addMatcher({Arg->getValue(), UseRegex});
   for (auto Arg : InputArgs.filtered(OBJCOPY_strip_symbols))
     if (Error E = addSymbolsFromFile(Config.SymbolsToRemove, DC.Alloc,
                                      Arg->getValue(), UseRegex))
       return std::move(E);
   for (auto Arg : InputArgs.filtered(OBJCOPY_strip_unneeded_symbol))
-    Config.UnneededSymbolsToRemove.emplace_back(Arg->getValue(), UseRegex);
+    Config.UnneededSymbolsToRemove.addMatcher({Arg->getValue(), UseRegex});
   for (auto Arg : InputArgs.filtered(OBJCOPY_strip_unneeded_symbols))
     if (Error E = addSymbolsFromFile(Config.UnneededSymbolsToRemove, DC.Alloc,
                                      Arg->getValue(), UseRegex))
       return std::move(E);
   for (auto Arg : InputArgs.filtered(OBJCOPY_keep_symbol))
-    Config.SymbolsToKeep.emplace_back(Arg->getValue(), UseRegex);
+    Config.SymbolsToKeep.addMatcher({Arg->getValue(), UseRegex});
   for (auto Arg : InputArgs.filtered(OBJCOPY_keep_symbols))
     if (Error E = addSymbolsFromFile(Config.SymbolsToKeep, DC.Alloc,
                                      Arg->getValue(), UseRegex))
       return std::move(E);
   for (auto Arg : InputArgs.filtered(OBJCOPY_add_symbol)) {
     Expected<NewSymbolInfo> NSI = parseNewSymbolInfo(Arg->getValue());
     if (!NSI)
       return NSI.takeError();
parseStripOptions(ArrayRef<const char *> ArgsArr,
   Config.StripUnneeded = InputArgs.hasArg(STRIP_strip_unneeded);
   if (auto Arg = InputArgs.getLastArg(STRIP_strip_all, STRIP_no_strip_all))
     Config.StripAll = Arg->getOption().getID() == STRIP_strip_all;
   Config.StripAllGNU = InputArgs.hasArg(STRIP_strip_all_gnu);
   Config.OnlyKeepDebug = InputArgs.hasArg(STRIP_only_keep_debug);
   Config.KeepFileSymbols = InputArgs.hasArg(STRIP_keep_file_symbols);
 
   for (auto Arg : InputArgs.filtered(STRIP_keep_section))
-    Config.KeepSection.emplace_back(Arg->getValue(), UseRegexp);
+    Config.KeepSection.addMatcher({Arg->getValue(), UseRegexp});
 
   for (auto Arg : InputArgs.filtered(STRIP_remove_section))
-    Config.ToRemove.emplace_back(Arg->getValue(), UseRegexp);
+    Config.ToRemove.addMatcher({Arg->getValue(), UseRegexp});
 
   for (auto Arg : InputArgs.filtered(STRIP_strip_symbol))
-    Config.SymbolsToRemove.emplace_back(Arg->getValue(), UseRegexp);
+    Config.SymbolsToRemove.addMatcher({Arg->getValue(), UseRegexp});
 
   for (auto Arg : InputArgs.filtered(STRIP_keep_symbol))
-    Config.SymbolsToKeep.emplace_back(Arg->getValue(), UseRegexp);
+    Config.SymbolsToKeep.addMatcher({Arg->getValue(), UseRegexp});
 
   if (!InputArgs.hasArg(STRIP_no_strip_all) && !Config.StripDebug &&
       !Config.StripUnneeded && Config.DiscardMode == DiscardType::None &&
       !Config.StripAllGNU && Config.SymbolsToRemove.empty())
     Config.StripAll = true;
 
   if (Config.DiscardMode == DiscardType::All)
     Config.StripDebug = true;

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

if (!Obj.SymbolTable)
     return Error::success();
 
   Obj.SymbolTable->updateSymbols([&](Symbol &Sym) {
     // Common and undefined symbols don't make sense as local symbols, and can
     // even cause crashes if we localize those, so skip them.
     if (!Sym.isCommon() && Sym.getShndx() != SHN_UNDEF &&
         ((Config.LocalizeHidden &&
           (Sym.Visibility == STV_HIDDEN || Sym.Visibility == STV_INTERNAL)) ||
-         is_contained(Config.SymbolsToLocalize, Sym.Name)))
+         Config.SymbolsToLocalize.matches(Sym.Name)))
       Sym.Binding = STB_LOCAL;
 
     // Note: these two globalize flags have very similar names but different
     // meanings:
     //
     // --globalize-symbol: promote a symbol to global
     // --keep-global-symbol: all symbols except for these should be made local
     //
     // If --globalize-symbol is specified for a given symbol, it will be
     // global in the output file even if it is not included via
     // --keep-global-symbol. Because of that, make sure to check
     // --globalize-symbol second.
     if (!Config.SymbolsToKeepGlobal.empty() &&
-        !is_contained(Config.SymbolsToKeepGlobal, Sym.Name) &&
+        !Config.SymbolsToKeepGlobal.matches(Sym.Name) &&
         Sym.getShndx() != SHN_UNDEF)
       Sym.Binding = STB_LOCAL;
 
-    if (is_contained(Config.SymbolsToGlobalize, Sym.Name) &&
+    if (Config.SymbolsToGlobalize.matches(Sym.Name) &&
         Sym.getShndx() != SHN_UNDEF)
       Sym.Binding = STB_GLOBAL;
 
-    if (is_contained(Config.SymbolsToWeaken, Sym.Name) &&
-        Sym.Binding == STB_GLOBAL)
+    if (Config.SymbolsToWeaken.matches(Sym.Name) && Sym.Binding == STB_GLOBAL)
       Sym.Binding = STB_WEAK;
 
     if (Config.Weaken && Sym.Binding == STB_GLOBAL &&
         Sym.getShndx() != SHN_UNDEF)
       Sym.Binding = STB_WEAK;
 
     const auto I = Config.SymbolsToRename.find(Sym.Name);
     if (I != Config.SymbolsToRename.end())
       Sym.Name = I->getValue();
 
     if (!Config.SymbolsPrefix.empty() && Sym.Type != STT_SECTION)
       Sym.Name = (Config.SymbolsPrefix + Sym.Name).str();
   });
 
   // The purpose of this loop is to mark symbols referenced by sections
   // (like GroupSection or RelocationSection). This way, we know which
   // symbols are still 'needed' and which are not.
   if (Config.StripUnneeded || !Config.UnneededSymbolsToRemove.empty() ||
       !Config.OnlySection.empty()) {
     for (auto &Section : Obj.sections())
       Section.markSymbols();
   }
 
   auto RemoveSymbolsPred = [&](const Symbol &Sym) {
-    if (is_contained(Config.SymbolsToKeep, Sym.Name) ||
+    if (Config.SymbolsToKeep.matches(Sym.Name) ||
         (Config.KeepFileSymbols && Sym.Type == STT_FILE))
       return false;
 
     if ((Config.DiscardMode == DiscardType::All ||
          (Config.DiscardMode == DiscardType::Locals &&
           StringRef(Sym.Name).startswith(".L"))) &&
         Sym.Binding == STB_LOCAL && Sym.getShndx() != SHN_UNDEF &&
         Sym.Type != STT_FILE && Sym.Type != STT_SECTION)
       return true;
 
     if (Config.StripAll || Config.StripAllGNU)
       return true;
 
-    if (is_contained(Config.SymbolsToRemove, Sym.Name))
+    if (Config.SymbolsToRemove.matches(Sym.Name))
       return true;
 
     if ((Config.StripUnneeded ||
-         is_contained(Config.UnneededSymbolsToRemove, Sym.Name)) &&
+         Config.UnneededSymbolsToRemove.matches(Sym.Name)) &&
         (!Obj.isRelocatable() || isUnneededSymbol(Sym)))
       return true;
 
     // We want to remove undefined symbols if all references have been stripped.
     if (!Config.OnlySection.empty() && !Sym.Referenced &&
         Sym.getShndx() == SHN_UNDEF)
       return true;
 
     return false;
   };
 
   return Obj.removeSymbols(RemoveSymbolsPred);
 }
 
 static Error replaceAndRemoveSections(const CopyConfig &Config, Object &Obj) {
   SectionPred RemovePred = [](const SectionBase &) { return false; };
 
   // Removes:
   if (!Config.ToRemove.empty()) {
     RemovePred = [&Config](const SectionBase &Sec) {
-      return is_contained(Config.ToRemove, Sec.Name);
+      return Config.ToRemove.matches(Sec.Name);
     };
   }
 
   if (Config.StripDWO || !Config.SplitDWO.empty())
     RemovePred = [RemovePred](const SectionBase &Sec) {
       return isDWOSection(Sec) || RemovePred(Sec);
     };
 
RemovePred = [RemovePred](const SectionBase &Sec) {
       return (Sec.Flags & SHF_ALLOC) != 0 && !Sec.ParentSegment;
     };
   }
 
   // Explicit copies:
   if (!Config.OnlySection.empty()) {
     RemovePred = [&Config, RemovePred, &Obj](const SectionBase &Sec) {
       // Explicitly keep these sections regardless of previous removes.
-      if (is_contained(Config.OnlySection, Sec.Name))
+      if (Config.OnlySection.matches(Sec.Name))
         return false;
 
       // Allow all implicit removes.
       if (RemovePred(Sec))
         return true;
 
       // Keep special sections.
       if (Obj.SectionNames == &Sec)
         return false;
       if (Obj.SymbolTable == &Sec ||
           (Obj.SymbolTable && Obj.SymbolTable->getStrTab() == &Sec))
         return false;
 
       // Remove everything else.
       return true;
     };
   }
 
   if (!Config.KeepSection.empty()) {
     RemovePred = [&Config, RemovePred](const SectionBase &Sec) {
       // Explicitly keep these sections regardless of previous removes.
-      if (is_contained(Config.KeepSection, Sec.Name))
+      if (Config.KeepSection.matches(Sec.Name))
         return false;
       // Otherwise defer to RemovePred.
       return RemovePred(Sec);
     };
   }
 
   // This has to be the last predicate assignment.
   // If the option --keep-symbol has been specified