[llvm-objcopy] - Check dynamic relocation sections for broken references.

George Rimar · George Rimar · commit 67f590e2867a · 2019-04-30T11:02:09.000Z
This is a fix for https://bugs.llvm.org/show_bug.cgi?id=41371. Currently, it is possible to break the sh_link field of the dynamic relocation section by removing the section it refers to. The patch fixes an issue and adds 2 test cases. One of them shows that it does not seem possible to break the sh_info field. I added an assert to verify this. Differential revision: https://reviews.llvm.org/D60825 llvm-svn: 359552
diff --git a/llvm/test/tools/llvm-objcopy/ELF/dynrelocsec-remove-shinfo-reference.test b/llvm/test/tools/llvm-objcopy/ELF/dynrelocsec-remove-shinfo-reference.test
@@ -0,0 +1,30 @@
+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy -R .got.plt %t %t2
+
+## .rela.plt is a dynamic relocation section that has a connection
+## via sh_info field with its target section .got.plt.
+## Here we check that if the target section is removed then dynamic
+## relocation section is also removed and we do not end up with a broken
+## sh_info value, for example.
+
+# RUN: llvm-readelf --sections %t2 \
+# RUN:  | FileCheck %s --implicit-check-not=".got.plt" --implicit-check-not=".rela.plt"
+
+--- !ELF
+FileHeader:
+  Class:          ELFCLASS64
+  Data:           ELFDATA2LSB
+  Type:           ET_DYN
+  Machine:        EM_X86_64
+Sections:
+  - Name:         .rela.plt
+    Type:         SHT_RELA
+    Flags:        [ SHF_ALLOC ]
+    Link:         .dynsym
+    Info:         .got.plt
+  - Name:         .got.plt
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_WRITE, SHF_ALLOC ]
+DynamicSymbols:
+  - Name:          bar
+    Binding:       STB_GLOBAL
diff --git a/llvm/test/tools/llvm-objcopy/ELF/dynrelocsec-remove-shlink-reference.test b/llvm/test/tools/llvm-objcopy/ELF/dynrelocsec-remove-shlink-reference.test
@@ -0,0 +1,34 @@
+# RUN: yaml2obj %s > %t
+
+## Check we cannot remove the .dynsym symbol table because dynamic
+## relocation section .rela.dyn still references it via sh_link field.
+# RUN: not llvm-objcopy -R .dynsym %t %t2 2>&1 >/dev/null | FileCheck %s --check-prefix=ERR
+# ERR: error: Symbol table .dynsym cannot be removed because it is referenced by the relocation section .rela.dyn.
+
+## Check we can remove .dynsym after removing the reference.
+# RUN: llvm-objcopy -R .dynsym -R .rela.dyn %t %t2
+# RUN: llvm-readelf --sections %t2 | FileCheck %s --implicit-check-not=".dynsym"
+
+## Check we zero out sh_link field and allow producing output with the --allow-broken-links switch.
+# RUN: llvm-objcopy -R .dynsym --allow-broken-links %t %t2
+# RUN: llvm-readelf --sections %t2 | FileCheck %s --check-prefix=DROP-LINK
+# DROP-LINK:     [Nr] Name      Type Address          Off    Size   ES   Flg L
+# DROP-LINK:     [ 1] .rela.dyn RELA 0000000000000270 000040 000000 18   A   0
+# DROP-LINK-NOT: .dynsym
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_X86_64
+Sections:
+  - Name:          .rela.dyn
+    Type:          SHT_RELA
+    Flags:         [ SHF_ALLOC ]
+    Address:       0x0000000000000270
+    Link:          .dynsym
+    EntSize:       0x0000000000000018
+DynamicSymbols:
+  - Name:          bar
+    Binding:       STB_GLOBAL
diff --git a/llvm/tools/llvm-objcopy/ELF/Object.cpp b/llvm/tools/llvm-objcopy/ELF/Object.cpp
@@ -680,12 +680,33 @@ void DynamicRelocationSection::accept(SectionVisitor &Visitor) const {
 }
 
 void DynamicRelocationSection::accept(MutableSectionVisitor &Visitor) {
-  Visitor.visit(*this);
-}
-
-Error Section::removeSectionReferences(bool AllowBrokenDependency,
-    function_ref<bool(const SectionBase *)> ToRemove) {
-  if (ToRemove(LinkSection)) {
+  Visitor.visit(*this);
+}
+
+Error DynamicRelocationSection::removeSectionReferences(
+    bool AllowBrokenLinks, function_ref<bool(const SectionBase *)> ToRemove) {
+  if (ToRemove(Symbols)) {
+    if (!AllowBrokenLinks)
+      return createStringError(
+          llvm::errc::invalid_argument,
+          "Symbol table %s cannot be removed because it is "
+          "referenced by the relocation section %s.",
+          Symbols->Name.data(), this->Name.data());
+    Symbols = nullptr;
+  }
+
+  // SecToApplyRel contains a section referenced by sh_info field. It keeps
+  // a section to which the relocation section applies. When we remove any
+  // sections we also remove their relocation sections. Since we do that much
+  // earlier, this assert should never be triggered.
+  assert(!SecToApplyRel || !ToRemove(SecToApplyRel));
+
+  return Error::success();
+}
+
+Error Section::removeSectionReferences(bool AllowBrokenDependency,
+    function_ref<bool(const SectionBase *)> ToRemove) {
+  if (ToRemove(LinkSection)) {
     if (!AllowBrokenDependency)
       return createStringError(llvm::errc::invalid_argument,
                                "Section %s cannot be removed because it is "
diff --git a/llvm/tools/llvm-objcopy/ELF/Object.h b/llvm/tools/llvm-objcopy/ELF/Object.h
@@ -682,12 +682,15 @@ class DynamicRelocationSection
 
 public:
   explicit DynamicRelocationSection(ArrayRef<uint8_t> Data) : Contents(Data) {}
-
-  void accept(SectionVisitor &) const override;
-  void accept(MutableSectionVisitor &Visitor) override;
-
-  static bool classof(const SectionBase *S) {
-    if (!(S->Flags & ELF::SHF_ALLOC))
+
+  void accept(SectionVisitor &) const override;
+  void accept(MutableSectionVisitor &Visitor) override;
+  Error removeSectionReferences(
+      bool AllowBrokenLinks,
+      function_ref<bool(const SectionBase *)> ToRemove) override;
+
+  static bool classof(const SectionBase *S) {
+    if (!(S->Flags & ELF::SHF_ALLOC))
       return false;
     return S->Type == ELF::SHT_REL || S->Type == ELF::SHT_RELA;
   }
