[PATCH] D81097: [llvm-objcopy] Reorder --dump-section before --remove-section for ELF. Fixes PR44283

[Sameer Arora via Phabricator via llvm-commits]

sameerarora101 updated this revision to Diff 268465.
sameerarora101 marked an inline comment as done.
sameerarora101 added a comment.

Updating source and test file according to the inline commments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81097

Files:
  llvm/test/tools/llvm-objcopy/ELF/dump-section-before-add-remove.test
  llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp


Index: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
===================================================================
--- llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
+++ llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
@@ -270,7 +270,7 @@
   auto OnlyKeepDWOPred = [&DWOFile](const SectionBase &Sec) {
     return onlyKeepDWOPred(*DWOFile, Sec);
   };
-  if (Error E = DWOFile->removeSections(Config.AllowBrokenLinks, 
+  if (Error E = DWOFile->removeSections(Config.AllowBrokenLinks,
                                         OnlyKeepDWOPred))
     return E;
   if (Config.OutputArch) {
@@ -578,11 +578,11 @@
   }
 
   if (Config.CompressionType != DebugCompressionType::None)
-    replaceDebugSections(Obj, RemovePred, isCompressable, 
+    replaceDebugSections(Obj, RemovePred, isCompressable,
                          [&Config, &Obj](const SectionBase *S) {
                            return &Obj.addSection<CompressedSection>(
                                 *S, Config.CompressionType);
-                        });
+                         });
   else if (Config.DecompressDebugSections)
     replaceDebugSections(
         Obj, RemovePred,
@@ -617,6 +617,16 @@
     Obj.OSABI = Config.OutputArch.getValue().OSABI;
   }
 
+  // Dump sections before add/remove for
+  // compatibility with GNU objcopy.
+  for (StringRef Flag : Config.DumpSection) {
+    StringRef SectionName;
+    StringRef FileName;
+    std::tie(SectionName, FileName) = Flag.split("=");
+    if (Error E = dumpSectionToFile(SectionName, FileName, Obj))
+      return E;
+  }
+
   // It is important to remove the sections first. For example, we want to
   // remove the relocation sections before removing the symbols. That allows
   // us to avoid reporting the inappropriate errors about removing symbols
@@ -725,14 +735,6 @@
       NewSection.Type = SHT_NOTE;
   }
 
-  for (const auto &Flag : Config.DumpSection) {
-    std::pair<StringRef, StringRef> SecPair = Flag.split("=");
-    StringRef SecName = SecPair.first;
-    StringRef File = SecPair.second;
-    if (Error E = dumpSectionToFile(SecName, File, Obj))
-      return E;
-  }
-
   if (!Config.AddGnuDebugLink.empty())
     Obj.addSection<GnuDebugLinkSection>(Config.AddGnuDebugLink,
                                         Config.GnuDebugLinkCRC32);
Index: llvm/test/tools/llvm-objcopy/ELF/dump-section-before-add-remove.test
===================================================================
--- /dev/null
+++ llvm/test/tools/llvm-objcopy/ELF/dump-section-before-add-remove.test
@@ -0,0 +1,36 @@
+# RUN: yaml2obj %s -o %t
+
+# CHECK: 0000000 de ad be ef
+# CHECK: 0000004
+
+
+## Verify that section is dumped before remove.
+# RUN: llvm-objcopy --dump-section .test2=%t1 -R .test2 %t %t2
+# RUN: od -t x1 %t1 | FileCheck %s --ignore-case --match-full-lines
+
+## Verify that the newly added section is not dumped.
+# RUN: echo CAFE > %t1.txt
+# RUN: not llvm-objcopy --dump-section .test3=%t3.txt --add-section .test3=%t1.txt %t %t4 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=NODUMP -DINPUT=%t
+
+# NODUMP: error: '[[INPUT]]': section '.test3' not found
+
+!ELF
+FileHeader:
+  Class:    ELFCLASS64
+  Data:     ELFDATA2LSB
+  Type:     ET_REL
+  Machine:  EM_X86_64
+Sections:
+  - Name:     .test1
+    Type:     SHT_PROGBITS
+    Flags:    [ ]
+  - Name:     .test2
+    Type:     SHT_PROGBITS
+    Flags:    [ ]
+    Content:  "DEADBEEF"
+  - Name:     .test4
+    Type:     SHT_PROGBITS
+    Flags:    [ ]
+    Content:  "BEEFDEAD"
+Symbols: []


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81097.268465.patch
Type: text/x-patch
Size: 3499 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200604/dbf899f1/attachment.bin>