[llvm-strip] Have --discard-all imply --strip-debug

[sidneym]

Summary

GNU strip removes debug sections when --discard-all is specified and this patch attempts to mimic that behavior.

[grimar]

test/tools/llvm-objcopy/ELF/discard-all-debug.test

2 ↗

(On Diff #196515)

When I call GNU strip, it works without --keep-symbol=.L.str for this input.
Do you know why whe have a difference here?

29 ↗

(On Diff #196515)

Seems instead of lines 3-29 all you really need is just this line:

# RUN: llvm-readobj --sections %t | FileCheck %s --implicit-check-not=.debug_

[sidneym]

test/tools/llvm-objcopy/ELF/discard-all-debug.test

2 ↗

(On Diff #196515)

GNU strip doesn't issue a message when a reference is still required, it just keeps it.
The fatal error llvm-strip produces can cause some issues when trying to use it as a drop-in replacement for gnu strip. The error is generated in removeSymbols and if there was a way to infer that strip was the executable perhaps that check could be avoided.

29 ↗

(On Diff #196515)

I didn't know about this option I will make the change, thanks.

[sidneym]

Update testcase.

[jakehehrlich]

This appears to be correct. StripDebug is a preety simple and minimal field so we can go ahead and add this. The keep symbol thing still looks a bit funny.

Can you add this behavior to llvm-objcopy as well? We don't want them to be inconsistent.

test/tools/llvm-objcopy/ELF/discard-all-debug.test

2 ↗

(On Diff #196515)

This doesn't explain grimar's question from my perspective and I have the same question.

[grimar]

I have no more comments, minor nit/suggestion is inline.

test/tools/llvm-objcopy/ELF/discard-all-debug.test

5 ↗

(On Diff #196661)

I am not sure it is useful to check the number of sections. All you need is to check that .debug_ sections are gone I think.
And that is done by --sections + --implicit-check-not=.debug_

[sidneym]

test/tools/llvm-objcopy/ELF/discard-all-debug.test

2 ↗

(On Diff #196515)

The binary used for the testcase has a symbol with a reference that cannot be stripped. GNU-strip will leave it in place with no message or warning, llvm errors out unless explicitly kept. The code doing it is: RelocationSection::removeSymbols in Object.cpp

[sidneym]

Make llvm-objcopy behave the same way strip does wrt --discard-all. Update testcase

llvm/trunk/test/tools/llvm-objcopy/ELF/discard-all-debug.test

+# RUN: cp %p/Inputs/dwarf.dwo %t
+# RUN: llvm-strip --keep-symbol=.L.str --discard-all %t -o %t1
+# RUN: llvm-readobj --file-headers --sections %t1 | FileCheck --implicit-check-not=.debug_ %s
+# RUN: llvm-objcopy --keep-symbol=.L.str --discard-all %t %t2
+# RUN: llvm-readobj --file-headers --sections %t2 | FileCheck --implicit-check-not=.debug_ %s

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

if (InputArgs.hasArg(OBJCOPY_discard_all, OBJCOPY_discard_locals))
     Config.DiscardMode =
         InputArgs.hasFlag(OBJCOPY_discard_all, OBJCOPY_discard_locals)
             ? DiscardType::All
             : 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);
   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);
Expected<DriverConfig> parseStripOptions(ArrayRef<const char *> ArgsArr) {
   for (auto Arg : InputArgs.filtered(STRIP_keep_symbol))
     Config.SymbolsToKeep.emplace_back(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;
+
   Config.DeterministicArchives =
       InputArgs.hasFlag(STRIP_enable_deterministic_archives,
                         STRIP_disable_deterministic_archives, /*default=*/true);
 
   Config.PreserveDates = InputArgs.hasArg(STRIP_preserve_dates);
 
   DriverConfig DC;
   if (Positional.size() == 1) {