[llvm-objcopy] [COFF] Implement --only-section

[mstorsjo]

Summary

This uses the same input data as for --only-keep-debug. As a contrast, --only-keep-debug truncates the other sections, while --only-section fully removes the other sections.

[jhenderson]

Just to confirm, the desired behaviour of combining certain switches (e.g. --strip-debug and --only-section) is for the explicitly mentioned --only-section section to be kept despite it potentially being removed by the other switch?

[mstorsjo]

In D56873#1363068, @jhenderson wrote:

Just to confirm, the desired behaviour of combining certain switches (e.g. --strip-debug and --only-section) is for the explicitly mentioned --only-section section to be kept despite it potentially being removed by the other switch?

Good point - that's the impression I got from reading the structure of handleArgs for ELF, and I hadn't doublechecked until now. However this does seem to be wrong - GNU objcopy returns the intersection of the sets of sections kept by the two options.

[mstorsjo]

Updated to test how --only-section combined with --strip-debug behaves - matching GNU objcopy's behaviour.

[rupprecht]

test/tools/llvm-objcopy/COFF/only-section.test

13

nit: these tests will be a little easier to read if they are indented something like this:

SECTIONS:           xSections:
SECTIONS-NEXT:      xIdx Name
SECTIONS-DEBUG-NEXT:x  .debug_discardable
SECTIONS-TEXT-NEXT: x  .text
SECTIONS-NEXT-EMPTY:x

(x's for clarity)

22

I'm not too familiar with using -EMPTY in filecheck, but is this supposed to be SYMBOLS-NEXT-EMPTY?

[mstorsjo]

test/tools/llvm-objcopy/COFF/only-section.test

13

Ok, I can align them.

22

-EMPTY essentially implies -NEXT, it requires the next line to either be empty, or there to be EOF here. I haven't intended to use them combined but apparently I did do that in the paragraph above this. I'll change that one for consistency with the rest (that's the only one I have with NEXT-EMPTY at all).

[jhenderson]

-EMPTY essentially implies -NEXT, it requires the next line to either be empty, or there to be EOF here. I haven't intended to use them combined but apparently I did do that in the paragraph above this. I'll change that one for consistency with the rest (that's the only one I have with NEXT-EMPTY at all).

For the record, you can't combine FileCheck suffixes (e.g. CHECK-NEXT-NOT is a NOT using a check prefix of CHECK-NEXT). There are some error checks to catch this, but I don't know how comprehensive they are.

By the way, @jakehehrlich may remember exactly what the intended behaviour is in llvm-objcopy for ELF and combining rules. We discussed it a while back, but I can't remember them exactly, but it was something like explicit section keeps trump implicit section strips, and two explicits are an error. You might want to double-check with him.

[mstorsjo]

In D56873#1365075, @jhenderson wrote:

By the way, @jakehehrlich may remember exactly what the intended behaviour is in llvm-objcopy for ELF and combining rules. We discussed it a while back, but I can't remember them exactly, but it was something like explicit section keeps trump implicit section strips, and two explicits are an error. You might want to double-check with him.

Ok. At least for combining --strip-debug with --only-section, the ELF version doesn't match what GNU objcopy does.

I don't have a concrete use/need for --only-section myself so I haven't invested that much thought into it, I only brought it along from the code from ELFObjcopy.cpp that I based the COFF version on.

test/tools/llvm-objcopy/COFF/only-section.test

+RUN: yaml2obj %p/Inputs/only-keep-sections.yaml > %t.in.exe
+
+RUN: llvm-objcopy --only-section .debug_discardable %t.in.exe %t.out.exe
+RUN: llvm-objdump --section-headers -t %t.out.exe | FileCheck %s --check-prefixes=SECTIONS,SYMBOLS
+
+SECTIONS: Sections:
+SECTIONS-NEXT: Idx Name
+SECTIONS-NEXT: .debug_discardable
+SECTIONS-NEXT-EMPTY:
+
+SYMBOLS: SYMBOL TABLE:
+SYMBOLS-NEXT: debug_discardable_sym
+SYMBOLS-EMPTY:

tools/llvm-objcopy/COFF/COFFObjcopy.cpp

 
 static bool isDebugSection(const Section &Sec) {
   return Sec.Name.startswith(".debug");
 }
 
 static Error handleArgs(const CopyConfig &Config, Object &Obj) {
   // Perform the actual section removals.
   Obj.removeSections([&Config](const Section &Sec) {
+    if (!Config.OnlySection.empty()) {
+      // Explicitly keep these sections.
+      if (is_contained(Config.OnlySection, Sec.Name))
+        return false;
+
+      // Remove everything else. Contrary to --only-keep-debug, --only-section
+      // fully removes everything else.
+      return true;
+    }
+
     if (Config.StripDebug || Config.StripAll || Config.StripAllGNU ||
         Config.DiscardAll || Config.StripUnneeded) {
       if (isDebugSection(Sec) &&
           (Sec.Header.Characteristics & IMAGE_SCN_MEM_DISCARDABLE) != 0)
         return true;
     }
 
     if (is_contained(Config.ToRemove, Sec.Name))