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

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.

Reviewers: jhenderson, seiya, MaskRay, espindola, alexshap

Reviewed By: jhenderson, MaskRay

Subscribers: emaste, arichardson, jakehehrlich, abrachet, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D66432

llvm-svn: 369689

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

@@ -103,8 +103,7 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj) {
   Obj.removeSections([&Config](const Section &Sec) {
     // Contrary to --only-keep-debug, --only-section fully removes sections that
     // aren't mentioned.
-    if (!Config.OnlySection.empty() &&
+    if (!Config.OnlySection.empty() && !Config.OnlySection.matches(Sec.Name))
-        !is_contained(Config.OnlySection, Sec.Name))
       return true;
 
     if (Config.StripDebug || Config.StripAll || Config.StripAllGNU ||
@@ -114,7 +113,7 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj) {
         return true;
     }
 
-    if (is_contained(Config.ToRemove, Sec.Name))
+    if (Config.ToRemove.matches(Sec.Name))
       return true;
 
     return false;
@@ -148,7 +147,7 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj) {
     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,
@@ -167,7 +166,7 @@ static Error handleArgs(const CopyConfig &Config, Object &Obj) {
       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

llvm/tools/llvm-objcopy/CopyConfig.cpp

@@ -342,9 +342,8 @@ getOutputTargetInfoByTargetName(StringRef TargetName) {
   return {TargetInfo{Format, MI}};
 }
 
-static Error addSymbolsFromFile(std::vector<NameOrRegex> &Symbols,
+static Error addSymbolsFromFile(NameMatcher &Symbols, BumpPtrAllocator &Alloc,
-                                BumpPtrAllocator &Alloc, StringRef Filename,
+                                StringRef Filename, bool UseRegex) {
-                                bool UseRegex) {
   StringSaver Saver(Alloc);
   SmallVector<StringRef, 16> Lines;
   auto BufOrErr = MemoryBuffer::getFile(Filename);
@@ -357,7 +356,7 @@ static Error addSymbolsFromFile(std::vector<NameOrRegex> &Symbols,
     // 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();
@@ -613,11 +612,11 @@ Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
   }
 
   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('='))
@@ -655,43 +654,43 @@ Expected<DriverConfig> parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
   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))
@@ -820,16 +819,16 @@ parseStripOptions(ArrayRef<const char *> ArgsArr,
   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 &&

llvm/tools/llvm-objcopy/CopyConfig.h

@@ -98,6 +98,19 @@ public:
   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;
@@ -137,16 +150,20 @@ struct CopyConfig {
   std::vector<StringRef> AddSection;
   std::vector<StringRef> DumpSection;
   std::vector<NewSymbolInfo> SymbolsToAdd;
-  std::vector<NameOrRegex> KeepSection;
+
-  std::vector<NameOrRegex> OnlySection;
+  // Section matchers
-  std::vector<NameOrRegex> SymbolsToGlobalize;
+  NameMatcher KeepSection;
-  std::vector<NameOrRegex> SymbolsToKeep;
+  NameMatcher OnlySection;
-  std::vector<NameOrRegex> SymbolsToLocalize;
+  NameMatcher ToRemove;
-  std::vector<NameOrRegex> SymbolsToRemove;
+
-  std::vector<NameOrRegex> UnneededSymbolsToRemove;
+  // Symbol matchers
-  std::vector<NameOrRegex> SymbolsToWeaken;
+  NameMatcher SymbolsToGlobalize;
-  std::vector<NameOrRegex> ToRemove;
+  NameMatcher SymbolsToKeep;
-  std::vector<NameOrRegex> SymbolsToKeepGlobal;
+  NameMatcher SymbolsToLocalize;
+  NameMatcher SymbolsToRemove;
+  NameMatcher UnneededSymbolsToRemove;
+  NameMatcher SymbolsToWeaken;
+  NameMatcher SymbolsToKeepGlobal;
 
   // Map options
   StringMap<SectionRename> SectionsToRename;

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

@@ -356,7 +356,7 @@ static Error updateAndRemoveSymbols(const CopyConfig &Config, Object &Obj) {
     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
@@ -370,16 +370,15 @@ static Error updateAndRemoveSymbols(const CopyConfig &Config, Object &Obj) {
     // --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) &&
+    if (Config.SymbolsToWeaken.matches(Sym.Name) && Sym.Binding == STB_GLOBAL)
-        Sym.Binding == STB_GLOBAL)
       Sym.Binding = STB_WEAK;
 
     if (Config.Weaken && Sym.Binding == STB_GLOBAL &&
@@ -404,7 +403,7 @@ static Error updateAndRemoveSymbols(const CopyConfig &Config, Object &Obj) {
   }
 
   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;
 
@@ -418,11 +417,11 @@ static Error updateAndRemoveSymbols(const CopyConfig &Config, Object &Obj) {
     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;
 
@@ -443,7 +442,7 @@ static Error replaceAndRemoveSections(const CopyConfig &Config, Object &Obj) {
   // Removes:
   if (!Config.ToRemove.empty()) {
     RemovePred = [&Config](const SectionBase &Sec) {
-      return is_contained(Config.ToRemove, Sec.Name);
+      return Config.ToRemove.matches(Sec.Name);
     };
   }
 
@@ -523,7 +522,7 @@ static Error replaceAndRemoveSections(const CopyConfig &Config, Object &Obj) {
   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.
@@ -545,7 +544,7 @@ static Error replaceAndRemoveSections(const CopyConfig &Config, Object &Obj) {
   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);