[PATCH] D61672: [llvm-objcopy] Allow strip symtab in executables and DSOs

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:559
+
+bool SymbolTableSection::isTrivial() const {
+  assert(!Symbols.empty());
----------------
jhenderson wrote:
> I'd be inclined to call this "isEmpty" or simply "empty". Whilst not strictly empty, the null symbol is not significant.
In fact `SymbolTableSection::empty()` already exists and is implemented like this (only checking size == 1, not verifying it's null)


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1590-1592
+Error Object::removeUnneededSections() {
+  // We can remove trivial symbol table from non-relocatable objects
+  if (isRelocatable() || SymbolTable == nullptr || !SymbolTable->isTrivial())
----------------
I don't think this part needs to be guarded by `isRelocatable()`. Even GNU objcopy will drop the symbol table if it's empty (only contains the null symbol).

Demo:
```
$ cat /tmp/yaml.txt
--- !ELF
FileHeader:
  Class:           ELFCLASS64
  Data:            ELFDATA2LSB
  Type:            ET_REL
  Machine:         EM_X86_64
Sections:
  - Name:          .text
    Type:          SHT_PROGBITS
Symbols:
$ yaml2obj /tmp/yaml.txt > /tmp/null.o
$ objcopy /tmp/null.o /tmp/null-copy.o
$ readelf -SW /tmp/null.o /tmp/null-copy.o

File: /tmp/null.o
There are 5 section headers, starting at offset 0x40:

Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .text             PROGBITS        0000000000000000 000180 000000 00      0   0  0
  [ 2] .symtab           SYMTAB          0000000000000000 000180 000018 18      3   1  8
  [ 3] .strtab           STRTAB          0000000000000000 000198 000001 00      0   0  1
  [ 4] .shstrtab         STRTAB          0000000000000000 000199 000021 00      0   0  1
...
File: /tmp/null-copy.o
There are 3 section headers, starting at offset 0x58:

Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .text             PROGBITS        0000000000000000 000040 000000 00      0   0  1
  [ 2] .shstrtab         STRTAB          0000000000000000 000040 000011 00      0   0  1
...
```

If I remove `isRelocatable()` here, I can match that behavior.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61672/new/

https://reviews.llvm.org/D61672