[llvm-objcopy] Add --strip-unneeded-symbol(s)

[mstorsjo]

test/tools/llvm-objcopy/ELF/strip-unneeded.test

29

All the newly added tests produce results identical to some of the existing tests, removing all symbols. Wouldn't it be useful with a test that only removes some of the unneeded symbols but not all of them, to showcase the difference?

[evgeny777]

test/tools/llvm-objcopy/ELF/strip-unneeded.test

29

--strip-unneeded removes only bar, foobar and foobaz. File and section symbols are not removed, also foo is not removed because it's in comdat. This is checked by original test case (line 6). Additional tests check that

1. Unneeded symbols are removed, but not STT_FILE / STT_SECTON nor comdat (%t3)
2. Stripping all symbols by regex pattern .* is identical to ---strip-unneeded (%t4)
3. Stripping just bar, foobar and foobaz is identical to --strip-unneeded, because all others should be preserved (%t5)
4. The %t6 is identical to %t4, but using a file

What kind of extra test do you feel is needed?

[mstorsjo]

test/tools/llvm-objcopy/ELF/strip-unneeded.test

29

A test that removes only one of bar/foobar/foobaz but not all of them.

[evgeny777]

Addressed

[mstorsjo]

Thanks, looks good to me now. I'll leave someone else to do the overall approval though.

[rupprecht]

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

368

nit: these checks should be inverted, i.e. first check if the command line flag has been set, and then call isUnneededSymbol() to see if it applies.

test/tools/llvm-objcopy/COFF/strip-unneeded.test

 RUN: yaml2obj %p/Inputs/discard-locals.yaml > %t.in.o
 
 RUN: llvm-objdump -t %t.in.o | FileCheck %s --check-prefixes=SYMBOLS,SYMBOLS-PRE
 
 RUN: llvm-objcopy --strip-unneeded %t.in.o %t.out.o
 RUN: llvm-objdump -t %t.out.o | FileCheck %s --check-prefix=SYMBOLS
 
+RUN: llvm-objcopy --strip-unneeded-symbol=external_undefined_unreferenced \
+RUN:              --strip-unneeded-symbol=local_unreferenced \
+RUN:              --strip-unneeded-symbol=local_undefined_unreferenced \
+RUN:              --strip-unneeded-symbol='@feat.00' \
+RUN:               %t.in.o %t.out2.o
+RUN: cmp %t.out.o %t.out2.o
+
 SYMBOLS: SYMBOL TABLE:
 SYMBOLS-NEXT: external
 SYMBOLS-NEXT: external_undefined
 SYMBOLS-PRE-NEXT: external_undefined_unreferenced
 SYMBOLS-PRE-NEXT: local_unreferenced
 SYMBOLS-NEXT: local_referenced
 SYMBOLS-PRE-NEXT: local_undefined_unreferenced
 SYMBOLS-PRE-NEXT: @feat.00
 SYMBOLS-EMPTY:

test/tools/llvm-objcopy/ELF/strip-unneeded.test

 # RUN: yaml2obj %s > %t
 # RUN: cp %t %t1
 # RUN: llvm-objcopy --strip-unneeded %t %t2
 # Verify that llvm-objcopy has not modified the input.
 # RUN: cmp %t %t1
 # RUN: llvm-readobj --symbols %t2 | FileCheck %s
 
+# RUN: llvm-objcopy --strip-unneeded-symbol=bar \
+# RUN:              %t %t3
+# RUN: llvm-readobj --symbols %t3 | FileCheck %s --check-prefixes=STRIP-BAR,REMAIN
+
+# RUN: llvm-objcopy --regex --strip-unneeded-symbol='.*' %t %t4
+# RUN: cmp %t2 %t4
+
+# RUN: echo " bar # bar " > %t.list.txt
+# RUN: echo "foobar" >> %t.list.txt
+# RUN: echo "foobaz" >> %t.list.txt
+# RUN: echo " # comment " >> %t.list.txt
+# RUN: llvm-objcopy --strip-unneeded-symbols %t.list.txt %t %t5
+# RUN: cmp %t2 %t5
+
+# RUN: echo " .* # * - remove all " > %t.list2.txt
+# RUN: llvm-objcopy --regex --strip-unneeded-symbols %t.list2.txt %t %t6
+# RUN: cmp %t2 %t6
+
 # Verify that llvm-strip modifies the symbol table the same way.
 # RUN: llvm-strip --strip-unneeded %t
 # RUN: cmp %t %t2
 
 !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_REL
   Machine:         EM_X86_64
 Sections:
   - Name:            .text

 #CHECK-NEXT:    Value: 0x1018
 #CHECK-NEXT:    Size: 8
 #CHECK-NEXT:    Binding: Weak
 #CHECK-NEXT:    Type: Function
 #CHECK-NEXT:    Other: 0
 #CHECK-NEXT:    Section: .text
 #CHECK-NEXT:  }
 #CHECK-NEXT:]
+
+#STRIP-BAR-NOT: Name: bar ({{.*}})
+#REMAIN: Name: foobar
+#REMAIN: Name: foobaz

tools/llvm-objcopy/COFF/COFFObjcopy.cpp

if (is_contained(Config.SymbolsToRemove, Sym.Name)) {
                                       "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.
-      if (Config.StripUnneeded &&
-          (Sym.Sym.StorageClass == IMAGE_SYM_CLASS_STATIC ||
-           Sym.Sym.SectionNumber == 0))
+      // 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))
           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)
         return true;

tools/llvm-objcopy/CopyConfig.h

struct CopyConfig {
   std::vector<StringRef> AddSection;
   std::vector<StringRef> DumpSection;
   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;
 
   // Map options
   StringMap<SectionRename> SectionsToRename;
   StringMap<SectionFlagsUpdate> SetSectionFlags;
   StringMap<StringRef> SymbolsToRename;

tools/llvm-objcopy/CopyConfig.cpp

DriverConfig parseObjcopyOptions(ArrayRef<const char *> ArgsArr) {
   for (auto Arg : InputArgs.filtered(OBJCOPY_weaken_symbols))
     addSymbolsFromFile(Config.SymbolsToWeaken, DC.Alloc, Arg->getValue(),
                        UseRegex);
   for (auto Arg : InputArgs.filtered(OBJCOPY_strip_symbol))
     Config.SymbolsToRemove.emplace_back(Arg->getValue(), UseRegex);
   for (auto Arg : InputArgs.filtered(OBJCOPY_strip_symbols))
     addSymbolsFromFile(Config.SymbolsToRemove, DC.Alloc, Arg->getValue(),
                        UseRegex);
+  for (auto Arg : InputArgs.filtered(OBJCOPY_strip_unneeded_symbol))
+    Config.UnneededSymbolsToRemove.emplace_back(Arg->getValue(), UseRegex);
+  for (auto Arg : InputArgs.filtered(OBJCOPY_strip_unneeded_symbols))
+    addSymbolsFromFile(Config.UnneededSymbolsToRemove, DC.Alloc,
+                       Arg->getValue(), UseRegex);
   for (auto Arg : InputArgs.filtered(OBJCOPY_keep_symbol))
     Config.SymbolsToKeep.emplace_back(Arg->getValue(), UseRegex);
 
   Config.DeterministicArchives = InputArgs.hasFlag(
       OBJCOPY_enable_deterministic_archives,
       OBJCOPY_disable_deterministic_archives, /*default=*/true);
 
   Config.PreserveDates = InputArgs.hasArg(OBJCOPY_preserve_dates);

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

for (SectionBase *S : ToReplace) {
     }
   }
 
   RemovePred = [shouldReplace, RemovePred](const SectionBase &Sec) {
     return shouldReplace(Sec) || RemovePred(Sec);
   };
 }
 
+static bool isUnneededSymbol(const Symbol &Sym) {
+  return !Sym.Referenced &&
+         (Sym.Binding == STB_LOCAL || Sym.getShndx() == SHN_UNDEF) &&
+         Sym.Type != STT_FILE && Sym.Type != STT_SECTION;
+}
+
 // This function handles the high level operations of GNU objcopy including
 // handling command line options. It's important to outline certain properties
 // we expect to hold of the command line operations. Any operation that "keeps"
 // should keep regardless of a remove. Additionally any removal should respect
 // any previous removals. Lastly whether or not something is removed shouldn't
 // depend a) on the order the options occur in or b) on some opaque priority
 // system. The only priority is that keeps/copies overrule removes.
 static Error handleArgs(const CopyConfig &Config, Object &Obj,
Obj.SymbolTable->updateSymbols([&](Symbol &Sym) {
 
       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) {
+    if (Config.StripUnneeded || !Config.UnneededSymbolsToRemove.empty()) {
       for (auto &Section : Obj.sections())
         Section.markSymbols();
     }
 
     auto RemoveSymbolsPred = [&](const Symbol &Sym) {
       if (is_contained(Config.SymbolsToKeep, 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))
         return true;
 
-      if (Config.StripUnneeded && !Sym.Referenced &&
-          (Sym.Binding == STB_LOCAL || Sym.getShndx() == SHN_UNDEF) &&
-          Sym.Type != STT_FILE && Sym.Type != STT_SECTION)
+      if (isUnneededSymbol(Sym))
+        if (Config.StripUnneeded ||
+            is_contained(Config.UnneededSymbolsToRemove, Sym.Name))
           return true;
 
       return false;
     };
     if (Error E = Obj.removeSymbols(RemoveSymbolsPred))
       return E;
   }
 
   SectionPred RemovePred = [](const SectionBase &) { return false; };

tools/llvm-objcopy/ObjcopyOpts.td

 def strip_dwo : Flag<["-", "--"], "strip-dwo">,
                 HelpText<"Remove all DWARF .dwo sections from file">;
 def strip_sections : Flag<["-", "--"], "strip-sections">,
                      HelpText<"Remove all section headers">;
 def strip_non_alloc : Flag<["-", "--"], "strip-non-alloc">,
                       HelpText<"Remove all non-allocated sections">;
 def strip_unneeded : Flag<["-", "--"], "strip-unneeded">,
                      HelpText<"Remove all symbols not needed by relocations">;
+defm strip_unneeded_symbol 
+    : Eq<"strip-unneeded-symbol",
+         "Remove symbol <symbol> if it is not needed by relocations">,
+      MetaVarName<"symbol">;
+defm strip_unneeded_symbols
+    : Eq<"strip-unneeded-symbols",
+         "Reads a list of symbols from <filename> and removes them "
+         "if they are not needed by relocations">,
+      MetaVarName<"filename">;
 
 def extract_dwo
     : Flag<["-", "--"], "extract-dwo">,
       HelpText<
           "Remove all sections that are not DWARF .dwo sections from file">;
 
 def localize_hidden
     : Flag<["-", "--"], "localize-hidden">,