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

[evgeny777]

Summary

The patch allows strip of entire symbol table in executable or DSO files with --strip-unneeded. This seems to be what GNU objcopy is doing.

[jhenderson]

Does GNU remove all the symbols or remove the symbol table entirely? The two are slightly different. If the latter, we should look at whether it's specific to the --strip-unneeded switch or more generally when all symbols end up getting discarded.

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

1 ↗

(On Diff #198612)

Perhaps worth renaming this test to something like strip-unneeded-all-symbols.test. This test could also do with a brief comment at the top stating its purpose.

9 ↗

(On Diff #198612)

Nit: reduce the unnecessary padding amount, here and below.

20 ↗

(On Diff #198612)

What's the point of having DynamicSymbols here?

29–32 ↗

(On Diff #198612)

I think you can simplify this test by removing most of these fields. The only relevant fields are the Binding, Section and Name fields, I believe. Also, what happens for undefined STB_GLOBALs in GNU objcopy? There should be a test for that case here.

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

426

What does GNU objcopy do for other ET_* types, e.g. ET_CORE or ET_* values in the OS and PROC-specific ranges? Could this be inverted to != ET_REL?

[jakehehrlich]

I'm confused as to what this is trying to accomplish. This really just marks every symbol as unneeded in a DSO and keeps the behavior for other cases. In that case why not strip the symbol table all togethor? This is a very round about way to accomplish that.

Can you document exactly what behavior you're seeing?

[evgeny777]

I'm confused as to what this is trying to accomplish.

I'm trying to strip symtab, because it's not needed in executables. We have .dynsym for dynamic linker, but it is not even readed by llvm-objcopy, so all symbols are coming from .symtab

This really just marks every symbol as unneeded in a DSO and keeps the behavior for other cases. In that case why not strip the symbol table all togethor? This is a very round about way to accomplish that.

This probably makes sense as well, but marking symbols unneeded seems more logical - one can figure out what is stripped looking at the single place in the code.

Can you document exactly what behavior you're seeing?

GNU objcopy strips all symbols from symtab in non-relocatable (ET_DYN or ET_EXEC) objects if those symbols are not referenced by relocations.
My understanding is that in DSOs and executables all static relocations should have been resolved by static linker, so we shouldn't bother about them.
For details please look at filter_symbols function in objcopy.c

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

426

May be. GNU objcopy considers everything which is not ET_DYN or ET_EXEC relocatable. See filter_symbols in objcopy.c

int relocatable = (abfd->flags & (EXEC_P | DYNAMIC)) == 0;

[jakehehrlich]

I'm still a bit unclear. Does it strip symbols that are duplicates of symbols in .dynsym? Does it just get rid of the the whole symbol table no matter what?

I think --strip-unneeded already strips debug sections as well so you can't "look in one place" to figure out what it does. You'll need extra stripping options regardless. If there is no documented behavior of GNU objcopy leaving .symtab in the ET_DYN and ET_EXEC cases we should just strip the symbol table all together.

Also what "strip unneeded" means is a bit unclear. There unfortunately isn't a spec anywhere nor is the use case implied by "needed" defined. We just have to match GNU objcopy. Arguments, which are quite reasonable otherwise, like ".symtab isn't needed to run the binary" don't target the right goal. If all you need is to run the binary you should use --strip-sections which gets rid of *everything* except what is needed to run/dynamically link the binary. The reality is that --strip-ubneeded is some hodge podge of behaviors we're just trying to match.

[evgeny777]

I'm still a bit unclear. Does it strip symbols that are duplicates of symbols in .dynsym? Does it just get rid of the the whole symbol table no matter what?

For ET_DYN/EXEC GNU objcopy strips all symbols from .symtab when they're not referenced by relocations. Given DYN/EXEC is a product of a static linker there're no such
symbols, so effectively all symbols from .symtab are stripped. As a result symtab section becomes empty and is not emitted to output binary.

I think --strip-unneeded already strips debug sections as well

llvm-objcopy --help
...
-strip-unneeded         Remove all symbols not needed by relocations
...

Also what "strip unneeded" means is a bit unclear.

I think it's pretty clear. You remove symbols not referenced by relocations compacting both .symtab and .strtab

There unfortunately isn't a spec anywhere nor is the use case implied by "needed" defined. We just have to match GNU objcopy.

I couldn't find any docs on the subject, so I can only suggest taking a look at GNU objcopy filter_symbols.

[jakehehrlich]

I'm still confused. If the behavior intended only has to do with the existence of relocations why are we making this change? A clear difference of behavior needs to be pointed out and that hasn't been made clear.

[evgeny777]

I'm still confused. If the behavior intended only has to do with the existence of relocations why are we making this change? A clear difference of behavior needs to be pointed out and that hasn't been made clear.

llvm-objcopy never strips STT_SECTION, STB_GLOBAL and SHN_UNDEF symbols, not matter if they referenced by relocations or not. This is right thing to do in ET_REL, but doesn't make sense in ET_DYN/EXEC.
See isUnneededSymbol.

[evgeny777]

Ping

[evgeny777]

Up

[rupprecht]

In D61672#1496975, @jakehehrlich wrote:

In that case why not strip the symbol table all togethor?

GNU strip does this. In a really simple test -- build a dummy "int main() { return 0; }" executable, then run strip/llvm-strip --strip-unneeded -- GNU strip removes .symtab and .strtab, whereas llvm-strip does not. The .symtab/.strtab sections are trivial (only containing the null symbol), so we should do the final step and remove it. (Both leave in .shstrtab though).

So if we're going for better GNU strip emulation, this patch doesn't even go far enough, although it is a step in the right direction.

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

426

Seconding what James said, I think we should refactor this out to a separate method. That way, any downstream users using llvm-objcopy with custom object file types that should be considered relocatable have a narrow/well defined place to put their local modifications and avoid merge conflicts any time this part is changed.

Ideally, this Object would be the one from libObject (ELFObjectFile<ELFT>::isRelocatableObject() in llvm/Object/ELFObjectFile.h), which I'm sure downstream users already have a local change for. Sadly, it's not, but downstream users can just add a second local patch.

I'm fine with either way, slightly preferring the way you have it here just because it probably emulates GNU objcopy better, but checking Type != ET_REL should also work.

[rupprecht]

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

426

In case I wasn't specific enough: by "refactor this" I mean just the (Obj.Type == ET_EXEC || Obj.Type == ET_DYN) check.

[evgeny777]

• Addressed comments
• Trivial .symtab and .strtab are now removed completely from non-relocatable objects

[jhenderson]

A number of my test comments haven't been addressed yet. Please do so.

tools/llvm-objcopy/ELF/Object.cpp

647–650

I'm not sure I see the point of this function. The symbol at index 0 is guaranteed to be the null symbol.

652

I'd be inclined to call this "isEmpty" or simply "empty". Whilst not strictly empty, the null symbol is not significant.

1793

Missing trailing full stop.
"remove trivial" -> "remove an empty".

1797

names, in such case -> names. In such a

[rupprecht]

tools/llvm-objcopy/ELF/Object.cpp

652

In fact SymbolTableSection::empty() already exists and is implemented like this (only checking size == 1, not verifying it's null)

1792–1794

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.

[evgeny777]

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

20 ↗

(On Diff #198612)

The point is to test that dynamic symbols are not touched by --strip-unneeded run on executable. Do you think it's not needed?

29–32 ↗

(On Diff #198612)

Also, what happens for undefined STB_GLOBALs in GNU objcopy?

Removal. .symtab is not used by dynamic loader, so it's not needed. Undefined STB_GLOBAL symbols are placed in .dynsym by static linker for runtime resolution.

tools/llvm-objcopy/ELF/Object.cpp

1792–1794

I don't think this part needs to be guarded by isRelocatable()

Relocation sections are typically linked to .symtab so you can't remove it even if it is empty. It is still possible to check object for relocation sections and then check those sections link field, but it seems like an overkill, IMO.

[evgeny777]

Addressed comments

[jhenderson]

The point is to test that dynamic symbols are not touched by --strip-unneeded run on executable. Do you think it's not needed?

It's not needed. We don't do anything in llvm-objcopy to the dynamic symbols ever, so I don't think we need to test this in this specific case.

test/tools/llvm-objcopy/ELF/no-symbol-relocation.test

9 ↗

(On Diff #204490)

Why are you changing this?

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

3

eliminate the static

4

by the dynamic loader.

18–21

As noted in an earlier diff, dynamic symbols aren't related to this change, so can be deleted (see out-of-line comment).

24

The type is irrelevant to the behaviour, so it can be deleted.

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

426

Why is this here? Is it tested?

tools/llvm-objcopy/ELF/Object.cpp

1792–1794

Probably worth a comment in the code to this effect. I don't feel strongly like we need to drop symbol tables in relocatable files.

1793

Ping here.

[evgeny777]

test/tools/llvm-objcopy/ELF/no-symbol-relocation.test

9 ↗

(On Diff #204490)

Original yaml generates executable with relocation section. This triggers an error when this patch is applied, because llvm-objcopy attempts to strip symtab and fails because of the linked relocation section.

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

426

I don't think I understand the question. Do you mean the check for !Obj.isRelocatable()? If so it's required to strip all symbols from static symbol table in executable. This in turn, will make static symbol empty and eventually it will be removed in removeUnneededSections

[evgeny777]

Addressed

[jakehehrlich]

tools/llvm-objcopy/ELF/Object.cpp

1792

This should not be a method on Object. The goal is to keep Object highly generic. Please see how to remove this from Object's interface.

1805–1807

I realize this is contrary to James's recommendation that this be a separate function but it would be nice if we used the same removeSections call to do this. This isn't critical however.

[evgeny777]

removeUnneededSections ripped from Object interface.

[jhenderson]

I don't think I have any more comments. Thanks!

[evgeny777]

@rupprecht @jakehehrlich Are you ok with this?

[evgeny777]

Up

[jakehehrlich]

Sorry looking now.

[jakehehrlich]

llvm/trunk/tools/llvm-objcopy/ELF/Object.cpp

1998 ↗

(On Diff #208155)

I think this method is overly specific. Is there a reason that the user can't invoke removeSections themselves?

[jakehehrlich]

(sorry I just realized this was already landed, I was out when it was bumped).

[rupprecht]

I'm seeing an error pulling this into our toolchain when trying to copy some files:

llvm-objcopy [file] --strip-unneeded /tmp/foo                                                                                                                        
llvm-objcopy: error: '[file]': not stripping symbol '' because it is named in a relocation

Going to figure out what the issue is and submit a fix

[rupprecht]

This is an issue for fully linked executables that still have relocation sections, e.g. because it was linked with the --emit-relocs flag:

$ echo 'int main() { return 0; }' | clang -Wl,--emit-relocs -x c - -o foo && llvm-objcopy --strip-unneeded foo                                                                                         
llvm-objcopy: error: 'foo': not stripping symbol '__gmon_start__' because it is named in a relocation

[rupprecht]

Temporarily reverted in r365712 due to the noted issue -- I don't think the fix should be too hard, but didn't seem trivial enough to just fix forward

[evgeny777]

Fixed issue with --emit-relocs. Existing test ELF/no-symbol-relocation.test checks this.

[evgeny777]

@rupprecht Do you have any comments on update? I'm going to re-commit it.

[rupprecht]

Sorry, I wanted to patch this in first and rerun tests to make sure the internal build step this broke doesn't have other issues -- I'll reply today

[rupprecht]

Everything looks good for the tests that were broken before, thanks for waiting!

test/tools/llvm-objcopy/ELF/preserve-segment-contents-ehdr-phdrs.test

 ## Show that llvm-objcopy correctly updates the elf header and program header
 ## table when they are within a segment.
 
 # RUN: yaml2obj %s -o %t.in
 ## Validate that the properties are different before the removal.
 # RUN: llvm-readobj --file-headers --program-headers %t.in | FileCheck %s --check-prefix=BEFORE
 # RUN: llvm-objcopy %t.in %t.out -R .remove_me
 # RUN: llvm-readobj --file-headers --program-headers %t.out | FileCheck %s --check-prefix=AFTER
 
 # BEFORE: SectionHeaderCount: 6
 # BEFORE:      Type: PT_LOAD
 # BEFORE-NEXT: Offset: 0x0
 # BEFORE:      Type: PT_LOAD
 # BEFORE-NEXT: Offset: 0x240
 
-# AFTER:      SectionHeaderCount: 5
+# AFTER:      SectionHeaderCount: 3
 # AFTER:      Type: PT_LOAD
 # AFTER-NEXT: Offset: 0x0
 # AFTER:      Type: PT_LOAD
 # AFTER-NEXT: Offset: 0xB0
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64

test/tools/llvm-objcopy/ELF/remove-multiple-sections.test

 # CHECK:     Size:
 # CHECK:     Link: 0
 # CHECK:     Info: 0
 # CHECK:     AddressAlignment: 0
 # CHECK:     EntrySize: 0
 # CHECK:   }
 # CHECK:   Section {
 # CHECK:     Index: 4
-# CHECK:     Name: .symtab
-# CHECK:     Type: SHT_SYMTAB (0x2)
-# CHECK:     Flags [ (0x0)
-# CHECK:     ]
-# CHECK:     Address: 0x0
-# CHECK:     Offset:
-# CHECK:     Size:
-# CHECK:     Link: 5
-# CHECK:     Info: 1
-# CHECK:     AddressAlignment: 8
-# CHECK:     EntrySize: 24
-# CHECK:   }
-# CHECK:   Section {
-# CHECK:     Index: 5
-# CHECK:     Name: .strtab
-# CHECK:     Type: SHT_STRTAB (0x3)
-# CHECK:     Flags [ (0x0)
-# CHECK:     ]
-# CHECK:     Address: 0x0
-# CHECK:     Offset:
-# CHECK:     Size:
-# CHECK:     Link: 0
-# CHECK:     Info: 0
-# CHECK:     AddressAlignment: 1
-# CHECK:     EntrySize: 0
-# CHECK:   }
-# CHECK:   Section {
-# CHECK:     Index: 6
 # CHECK:     Name: .shstrtab
 # CHECK:     Type: SHT_STRTAB (0x3)
 # CHECK:     Flags [ (0x0)
 # CHECK:     ]
 # CHECK:     Address: 0x0
 # CHECK:     Offset:
 # CHECK:     Size:
 # CHECK:     Link: 0
 # CHECK:     Info: 0
 # CHECK:     AddressAlignment: 1
 # CHECK:     EntrySize: 0
 # CHECK:   }
 # CHECK: ]

test/tools/llvm-objcopy/ELF/segment-shift-section-remove.test

Sections:
       - Section: .text
   - Type: PT_LOAD
     Flags: [ PF_X, PF_R ]
     VAddr: 0x3000
     PAddr: 0x3000
     Sections:
       - Section: .text3
 
-#CHECK: SectionHeaderCount: 6
+#CHECK: SectionHeaderCount: 4
 
 # CHECK:           Sections [
 # CHECK-NEXT:        Section {
 # CHECK-NEXT:          Index: 0
 # CHECK-NEXT:          Name:  (0)
 # CHECK-NEXT:          Type: SHT_NULL (0x0)
 # CHECK-NEXT:          Flags [ (0x0)
 # CHECK-NEXT:          ]

 # CHECK-NEXT:          Size: 4096
 # CHECK-NEXT:          Link: 0
 # CHECK-NEXT:          Info: 0
 # CHECK-NEXT:          AddressAlignment: 4096
 # CHECK-NEXT:          EntrySize: 0
 # CHECK-NEXT:        }
 # CHECK-NEXT:        Section {
 # CHECK-NEXT:          Index: 3
-# CHECK-NEXT:          Name: .symtab
-# CHECK-NEXT:          Type: SHT_SYMTAB (0x2)
-# CHECK-NEXT:          Flags [ (0x0)
-# CHECK-NEXT:          ]
-# CHECK-NEXT:          Address: 0x0
-# CHECK-NEXT:          Offset: 0x3000
-# CHECK-NEXT:          Size: 24
-# CHECK-NEXT:          Link: 4
-# CHECK-NEXT:          Info: 1
-# CHECK-NEXT:          AddressAlignment: 8
-# CHECK-NEXT:          EntrySize: 24
-# CHECK-NEXT:        }
-# CHECK-NEXT:        Section {
-# CHECK-NEXT:          Index: 4
-# CHECK-NEXT:          Name: .strtab
-# CHECK-NEXT:          Type: SHT_STRTAB (0x3)
-# CHECK-NEXT:          Flags [ (0x0)
-# CHECK-NEXT:          ]
-# CHECK-NEXT:          Address: 0x0
-# CHECK-NEXT:          Offset: 0x3018
-# CHECK-NEXT:          Size:
-# CHECK-NEXT:          Link: 0
-# CHECK-NEXT:          Info: 0
-# CHECK-NEXT:          AddressAlignment: 1
-# CHECK-NEXT:          EntrySize: 0
-# CHECK-NEXT:        }
-# CHECK-NEXT:        Section {
-# CHECK-NEXT:          Index: 5
 # CHECK-NEXT:          Name: .shstrtab
 # CHECK-NEXT:          Type: SHT_STRTAB (0x3)
 # CHECK-NEXT:          Flags [ (0x0)
 # CHECK-NEXT:          ]
 # CHECK-NEXT:          Address: 0x0
 # CHECK-NEXT:          Offset:
 # CHECK-NEXT:          Size:
 # CHECK-NEXT:          Link: 0

test/tools/llvm-objcopy/ELF/segment-test-remove-section.test

- Type: PT_LOAD
     Sections:
       - Section: .text
       - Section: .text2
       - Section: .text3
 
 # Make sure that when we remove a section we overwrite it with zeros
 # DATA: {{^[^[:blank:]]+}} 00 00 00 00
 
-#CHECK: SectionHeaderCount: 6
+#CHECK: SectionHeaderCount: 4
 
 # CHECK: Sections [
 # CHECK:   Section {
 # CHECK-NEXT:     Index: 0
 # CHECK-NEXT:     Name:  (0)
 # CHECK-NEXT:     Type: SHT_NULL (0x0)
 # CHECK-NEXT:     Flags [ (0x0)
 # CHECK-NEXT:     ]

 # CHECK-NEXT:     Size:
 # CHECK-NEXT:     Link: 0
 # CHECK-NEXT:     Info: 0
 # CHECK-NEXT:     AddressAlignment: 4096
 # CHECK-NEXT:     EntrySize: 0
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Section {
 # CHECK-NEXT:     Index: 3
-# CHECK-NEXT:     Name: .symtab
-# CHECK-NEXT:     Type: SHT_SYMTAB (0x2)
-# CHECK-NEXT:     Flags [ (0x0)
-# CHECK-NEXT:     ]
-# CHECK-NEXT:     Address: 0x0
-# CHECK-NEXT:     Offset:
-# CHECK-NEXT:     Size:
-# CHECK-NEXT:     Link: 4
-# CHECK-NEXT:     Info: 1
-# CHECK-NEXT:     AddressAlignment: 8
-# CHECK-NEXT:     EntrySize: 24
-# CHECK-NEXT:   }
-# CHECK-NEXT:   Section {
-# CHECK-NEXT:     Index: 4
-# CHECK-NEXT:     Name: .strtab
-# CHECK-NEXT:     Type: SHT_STRTAB (0x3)
-# CHECK-NEXT:     Flags [ (0x0)
-# CHECK-NEXT:     ]
-# CHECK-NEXT:     Address: 0x0
-# CHECK-NEXT:     Offset:
-# CHECK-NEXT:     Size:
-# CHECK-NEXT:     Link: 0
-# CHECK-NEXT:     Info: 0
-# CHECK-NEXT:     AddressAlignment: 1
-# CHECK-NEXT:     EntrySize: 0
-# CHECK-NEXT:   }
-# CHECK-NEXT:   Section {
-# CHECK-NEXT:     Index: 5
 # CHECK-NEXT:     Name: .shstrtab
 # CHECK-NEXT:     Type: SHT_STRTAB (0x3)
 # CHECK-NEXT:     Flags [ (0x0)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Address: 0x0
 # CHECK-NEXT:     Offset:
 # CHECK-NEXT:     Size:
 # CHECK-NEXT:     Link: 0

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

+## Stripping unneeded symbols from execuatble/DSO should
+## eliminate the static symbol table, because it's not used
+## by the dynamic loader. 
+
+# RUN: yaml2obj %s > %t
+# RUN: cp %t %t1
+# RUN: llvm-objcopy --strip-unneeded %t %t2
+# RUN: llvm-readobj --section-headers %t2 | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:    ELFCLASS64
+  Data:     ELFDATA2LSB
+  Type:     ET_EXEC
+  Machine:  EM_X86_64
+Symbols:
+  - Name:     bar
+  - Name:     foo
+    Binding:  STB_GLOBAL
+
+# CHECK-NOT: .symtab

tools/llvm-objcopy/ELF/ELFObjcopy.cpp

auto RemoveSymbolsPred = [&](const Symbol &Sym) {
     if (Config.StripAll || Config.StripAllGNU)
       return true;
 
     if (is_contained(Config.SymbolsToRemove, Sym.Name))
       return true;
 
     if ((Config.StripUnneeded ||
          is_contained(Config.UnneededSymbolsToRemove, Sym.Name)) &&
-        isUnneededSymbol(Sym))
+        (!Obj.isRelocatable() || isUnneededSymbol(Sym)))
       return true;
 
     // We want to remove undefined symbols if all references have been stripped.
     if (!Config.OnlySection.empty() && !Sym.Referenced &&
         Sym.getShndx() == SHN_UNDEF)
       return true;
 
     return false;

tools/llvm-objcopy/ELF/Object.h

public:
   uint64_t Entry;
   uint64_t SHOffset;
   uint32_t Type;
   uint32_t Machine;
   uint32_t Version;
   uint32_t Flags;
 
   bool HadShdrs = true;
+  bool MustBeRelocatable = false;
   StringTableSection *SectionNames = nullptr;
   SymbolTableSection *SymbolTable = nullptr;
   SectionIndexSection *SectionIndexTable = nullptr;
 
   void sortSections();
   SectionTableRef sections() { return SectionTableRef(Sections); }
   ConstRange<SectionBase> sections() const {
     return make_pointee_range(Sections);
public:
   ConstRange<Segment> segments() const { return make_pointee_range(Segments); }
 
   Error removeSections(bool AllowBrokenLinks,
                        std::function<bool(const SectionBase &)> ToRemove);
   Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove);
   template <class T, class... Ts> T &addSection(Ts &&... Args) {
     auto Sec = llvm::make_unique<T>(std::forward<Ts>(Args)...);
     auto Ptr = Sec.get();
+    MustBeRelocatable |= isa<RelocationSection>(*Ptr);
     Sections.emplace_back(std::move(Sec));
     Ptr->Index = Sections.size();
     return *Ptr;
   }
   Segment &addSegment(ArrayRef<uint8_t> Data) {
     Segments.emplace_back(llvm::make_unique<Segment>(Data));
     return *Segments.back();
   }
+  bool isRelocatable() const {
+    return (Type != ELF::ET_DYN && Type != ELF::ET_EXEC) || MustBeRelocatable;
+  }
 };
 
 } // end namespace elf
 } // end namespace objcopy
 } // end namespace llvm
 
 #endif // LLVM_TOOLS_OBJCOPY_OBJECT_H

tools/llvm-objcopy/ELF/Object.cpp

 bool Symbol::isCommon() const { return getShndx() == SHN_COMMON; }
 
 void SymbolTableSection::assignIndices() {
   uint32_t Index = 0;
   for (auto &Sym : Symbols)
     Sym->Index = Index++;
 }
 
 void SymbolTableSection::addSymbol(Twine Name, uint8_t Bind, uint8_t Type,
                                    SectionBase *DefinedIn, uint64_t Value,
                                    uint8_t Visibility, uint16_t Shndx,
                                    uint64_t SymbolSize) {
   Symbol Sym;
   Sym.Name = Name.str();
   Sym.Binding = Bind;
   Sym.Type = Type;
   Sym.DefinedIn = DefinedIn;
   if (DefinedIn != nullptr)
     DefinedIn->HasSymbol = true;
   if (DefinedIn == nullptr) {
     if (Shndx >= SHN_LORESERVE)
       Sym.ShndxType = static_cast<SymbolShndxType>(Shndx);
for (auto &Sec : Obj.removedSections()) {
     std::memset(Buf.getBufferStart() + Offset, 0, Sec.Size);
   }
 }
 
 template <class ELFT>
 ELFWriter<ELFT>::ELFWriter(Object &Obj, Buffer &Buf, bool WSH)
     : Writer(Obj, Buf), WriteSectionHeaders(WSH && Obj.HadShdrs) {}
 
 Error Object::removeSections(bool AllowBrokenLinks,
     std::function<bool(const SectionBase &)> ToRemove) {
 
   auto Iter = std::stable_partition(
       std::begin(Sections), std::end(Sections), [=](const SecPtr &Sec) {
         if (ToRemove(*Sec))
           return false;
         if (auto RelSec = dyn_cast<RelocationSectionBase>(Sec.get())) {
           if (auto ToRelSec = RelSec->getSection())
             return !ToRemove(*ToRelSec);
         }
         return true;
       });
   if (SymbolTable != nullptr && ToRemove(*SymbolTable))
     SymbolTable = nullptr;
   if (SectionNames != nullptr && ToRemove(*SectionNames))
     SectionNames = nullptr;
   if (SectionIndexTable != nullptr && ToRemove(*SectionIndexTable))
     SectionIndexTable = nullptr;
   // Now make sure there are no remaining references to the sections that will
   // be removed. Sometimes it is impossible to remove a reference so we emit
   // an error here instead.
   std::unordered_set<const SectionBase *> RemoveSections;
   RemoveSections.reserve(std::distance(Iter, std::end(Sections)));
template <class ELFT> Error ELFWriter<ELFT>::write() {
   writeEhdr();
   writePhdrs();
   writeSectionData();
   if (WriteSectionHeaders)
     writeShdrs();
   return Buf.commit();
 }
 
+static Error removeUnneededSections(Object &Obj) {
+  // We can remove an empty symbol table from non-relocatable objects.
+  // Relocatable objects typically have relocation sections whose
+  // sh_link field points to .symtab, so we can't remove .symtab
+  // even if it is empty.
+  if (Obj.isRelocatable() || Obj.SymbolTable == nullptr ||
+      !Obj.SymbolTable->empty())
+    return Error::success();
+
+  // .strtab can be used for section names. In such a case we shouldn't
+  // remove it.
+  auto *StrTab = Obj.SymbolTable->getStrTab() == Obj.SectionNames
+                     ? nullptr
+                     : Obj.SymbolTable->getStrTab();
+  return Obj.removeSections(false, [&](const SectionBase &Sec) {
+    return &Sec == Obj.SymbolTable || &Sec == StrTab;
+  });
+}
+
 template <class ELFT> Error ELFWriter<ELFT>::finalize() {
   // It could happen that SectionNames has been removed and yet the user wants
   // a section header table output. We need to throw an error if a user tries
   // to do that.
   if (Obj.SectionNames == nullptr && WriteSectionHeaders)
     return createStringError(llvm::errc::invalid_argument,
                              "cannot write section header table because "
                              "section header string table was removed");
 
+  if (Error E = removeUnneededSections(Obj))
+    return E;
   Obj.sortSections();
 
   // We need to assign indexes before we perform layout because we need to know
   // if we need large indexes or not. We can assign indexes first and check as
   // we go to see if we will actully need large indexes.
   bool NeedsLargeIndexes = false;
   if (Obj.sections().size() >= SHN_LORESERVE) {
     SectionTableRef Sections = Obj.sections();