[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

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

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

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

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

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

6

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

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

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
+# RUN: llvm-readobj --file-headers --sections %t | FileCheck %s
+
+CHECK:     SectionHeaderCount: 8
+
+CHECK:    Name: .text
+CHECK:    Name: .rodata.str1.1
+CHECK-NOT:    Name: .debug_str
+CHECK-NOT:    Name: .debug_abbrev
+CHECK-NOT:    Name: .debug_info
+CHECK-NOT:    Name: .debug_ranges
+CHECK-NOT:    Name: .debug_macinfo
+CHECK-NOT:    Name: .debug_addr
+CHECK-NOT:    Name: .debug_pubnames
+CHECK-NOT:    Name: .debug_pubtypes
+CHECK:    Name: .comment
+CHECK:    Name: .note.GNU-stack
+CHECK-NOT:    Name: .debug_frame
+CHECK-NOT:    Name: .debug_line
+CHECK:    Name: .symtab
+CHECK:    Name: .rela.text
+CHECK-NOT:    Name: .rela.debug_info
+CHECK-NOT:    Name: .rela.debug_addr
+CHECK-NOT:    Name: .rela.debug_pubnames
+CHECK-NOT:    Name: .rela.debug_pubtypes
+CHECK-NOT:    Name: .rela.debug_frame
+CHECK-NOT:    Name: .rela.debug_line
+CHECK:    Name: .strtab

tools/llvm-objcopy/CopyConfig.cpp

Expected<DriverConfig> parseStripOptions(ArrayRef<const char *> ArgsArr) {
 
   for (auto Arg : InputArgs.filtered(STRIP_keep_symbol))
     Config.SymbolsToKeep.emplace_back(Arg->getValue(), UseRegexp);
 
   if (!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) {