[llvm-readelf] Don't suppress static symbol table with --dyn-symbols + --symbols

In r287786, a bug was introduced into llvm-readelf where it didn't print
the static symbol table if both --symbols and --dyn-symbols were
specified, even if there was no dynamic symbol table. This is obviously
incorrect.

This patch fixes this issue, by delegating the decision of which symbol
tables should be printed to the final dumper, rather than trying to
decide in the command-line option handling layer. The decision was made
to follow the approach taken in this patch because the LLVM style dumper
uses a different order to the original GNU style behaviour (and GNU
readelf) for ELF output. Other approaches resulted in behaviour changes
for other dumpers which felt wrong. In particular, I wanted to avoid
changing the order of the output for --symbols --dyn-symbols for LLVM
style, keep what is emitted by --symbols unchanged for all dumpers, and
avoid having different orders of .dynsym and .symtab dumping for GNU
"--symbols" and "--symbols --dyn-symbols".

Reviewed by: grimar, rupprecht

Differential Revision: https://reviews.llvm.org/D57016

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@351960 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/test/tools/llvm-readobj/demangle.test b/test/tools/llvm-readobj/demangle.test
index ffd0053e41475daddc4b64569aaec16f6f3f86f5..615cf77f1648ef7ba35121c5bbe480678823884b 100644 (file)
--- a/test/tools/llvm-readobj/demangle.test
+++ b/test/tools/llvm-readobj/demangle.test
@@ -67,24 +67,18 @@
 # LLVM-COMMON-NEXT:   ]
 
 ## Check GNU output style.
-## FIXME: The extra run for --symbols is because GNU mode only prints the dynamic symbols,
-## if --dyn-symbols is specified, even if --symbols is specified.
-# RUN: llvm-readelf --relocations --dyn-symbols --dyn-relocations \
+# RUN: llvm-readelf --symbols --relocations --dyn-symbols --dyn-relocations \
 # RUN:              --elf-section-groups --demangle %t.so > %t.gnu.long
-# RUN: llvm-readelf --symbols --demangle %t.so >> %t.gnu.long
-# RUN: llvm-readelf --relocations --dyn-symbols --dyn-relocations \
+# RUN: llvm-readelf --symbols --relocations --dyn-symbols --dyn-relocations \
 # RUN:              --elf-section-groups -C %t.so > %t.gnu.short
-# RUN: llvm-readelf --symbols -C %t.so >> %t.gnu.short
 # RUN: FileCheck %s --input-file %t.gnu.long --check-prefixes=GNU-COMMON,GNU-DEMANGLE
 # RUN: diff %t.gnu.long %t.gnu.short
 
 ## Check that default is no demangling.
-# RUN: llvm-readelf --relocations --dyn-symbols --dyn-relocations \
+# RUN: llvm-readelf --symbols --relocations --dyn-symbols --dyn-relocations \
 # RUN:              --elf-section-groups %t.so > %t.gnu.default
-# RUN: llvm-readelf --symbols %t.so >> %t.gnu.default
-# RUN: llvm-readelf --relocations --dyn-symbols --dyn-relocations \
+# RUN: llvm-readelf --symbols --relocations --dyn-symbols --dyn-relocations \
 # RUN:              --elf-section-groups --demangle=false %t.so > %t.gnu.nodemangle
-# RUN: llvm-readelf --symbols --demangle=false %t.so >> %t.gnu.nodemangle
 # RUN: FileCheck %s --input-file %t.gnu.default --check-prefixes=GNU-COMMON,GNU-MANGLED
 # RUN: diff %t.gnu.default %t.gnu.nodemangle
 
@@ -104,10 +98,6 @@
 # GNU-DEMANGLE-NEXT:       foo(int){{$}}
 # GNU-MANGLED-NEXT:        _Z3fooi{{$}}
 
-# GNU-COMMON:        COMDAT group section [{{.*}}] `.group'
-# GNU-DEMANGLE-SAME: [foo(char)]
-# GNU-MANGLED-SAME:  [_Z3fooc]
-
 # GNU-COMMON:        Symbol table '.symtab' contains 3 entries:
 # GNU-COMMON-NEXT:     Num: Value            Size Type   Bind  Vis     Ndx Name
 # GNU-COMMON-NEXT:       0: 0000000000000000    0 NOTYPE LOCAL DEFAULT UND
@@ -116,6 +106,10 @@
 # GNU-MANGLED-NEXT:        _Z3fooc{{$}}
 # GNU-MANGLED-NEXT:        _Z4blahf{{$}}
 
+# GNU-COMMON:        COMDAT group section [{{.*}}] `.group'
+# GNU-DEMANGLE-SAME: [foo(char)]
+# GNU-MANGLED-SAME:  [_Z3fooc]
+
 !ELF
 FileHeader:
   Class:           ELFCLASS64

diff --git a/test/tools/llvm-readobj/gnu-symbols.test b/test/tools/llvm-readobj/gnu-symbols.test
index a1b6f46cd52daf0469f8dcb52aebbe183f7d143e..18d43f6290d6ba6a8bddd77f284c4ccd9244f263 100644 (file)
--- a/test/tools/llvm-readobj/gnu-symbols.test
+++ b/test/tools/llvm-readobj/gnu-symbols.test
@@ -6,6 +6,8 @@ RUN: llvm-readobj --symbols %p/Inputs/gnuhash.so.elf-x86_64 --elf-output-style=G
 RUN:   | FileCheck %s --check-prefixes=SYMTAB,DYN
 RUN: llvm-readobj --dyn-symbols %p/Inputs/gnuhash.so.elf-x86_64 --elf-output-style=GNU \
 RUN:   | FileCheck %s --check-prefixes=NO-SYMTAB,DYN
+RUN: llvm-readobj --symbols --dyn-symbols %p/Inputs/gnuhash.so.elf-x86_64 --elf-output-style=GNU \
+RUN:   | FileCheck %s --check-prefixes=SYMTAB,DYN
 
 ELF32: Symbol table '.symtab' contains 5 entries:
 ELF32-NEXT:    Num:    Value  Size Type    Bind   Vis      Ndx Name
@@ -26,7 +28,7 @@ ELF64-NEXT:     5:  0000000000000000     0 TLS     GLOBAL DEFAULT  UND sym
 
 NO-SYMTAB-NOT: Symbol table '.symtab'
 
-DYN:Symbol table '.dynsym' contains 5 entries:
+DYN: Symbol table '.dynsym' contains 5 entries:
 DYN-NEXT:   Num:    Value          Size Type    Bind   Vis      Ndx Name
 DYN-NEXT:     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
 DYN-NEXT:     1: 00000000000001b8     0 NOTYPE  GLOBAL DEFAULT    4 foo
@@ -34,6 +36,7 @@ DYN-NEXT:     2: 0000000000200268     0 NOTYPE  GLOBAL DEFAULT    5 _edata
 DYN-NEXT:     3: 0000000000200268     0 NOTYPE  GLOBAL DEFAULT    5 _end
 DYN-NEXT:     4: 0000000000200268     0 NOTYPE  GLOBAL DEFAULT    5 __bss_start
 
+DYN-NOT: .dynsym
 NO-SYMTAB-NOT: Symbol table '.symtab'
 
 SYMTAB: Symbol table '.symtab' contains 12 entries:
@@ -50,3 +53,5 @@ SYMTAB-NEXT:     8: 0000000000200268     0 NOTYPE  GLOBAL DEFAULT    5 __bss_sta
 SYMTAB-NEXT:     9: 00000000000001b8     0 NOTYPE  GLOBAL DEFAULT    4 foo
 SYMTAB-NEXT:    10: 0000000000200268     0 NOTYPE  GLOBAL DEFAULT    5 _edata
 SYMTAB-NEXT:    11: 0000000000200268     0 NOTYPE  GLOBAL DEFAULT    5 _end
+
+DYN-NOT: .dynsym

diff --git a/tools/llvm-readobj/COFFDumper.cpp b/tools/llvm-readobj/COFFDumper.cpp
index 37d1c4e8c9a757954bb5e0fcaac35b3f86d8f19c..2af791ae17d271ca6da529a79632a99116071c30 100644 (file)
--- a/tools/llvm-readobj/COFFDumper.cpp
+++ b/tools/llvm-readobj/COFFDumper.cpp
@@ -80,8 +80,6 @@ public:
   void printFileHeaders() override;
   void printSectionHeaders() override;
   void printRelocations() override;
-  void printSymbols() override;
-  void printDynamicSymbols() override;
   void printUnwindInfo() override;
 
   void printNeededLibraries() override;
@@ -100,6 +98,8 @@ public:
   void printStackMap() const override;
   void printAddrsig() override;
 private:
+  void printSymbols() override;
+  void printDynamicSymbols() override;
   void printSymbol(const SymbolRef &Sym);
   void printRelocation(const SectionRef &Section, const RelocationRef &Reloc,
                        uint64_t Bias = 0);

diff --git a/tools/llvm-readobj/ELFDumper.cpp b/tools/llvm-readobj/ELFDumper.cpp
index b58128c44ae01bc44ac28f28e78bd2357c036398..d88828dbafcaa533c825f821cfe48c2ff16f9263 100644 (file)
--- a/tools/llvm-readobj/ELFDumper.cpp
+++ b/tools/llvm-readobj/ELFDumper.cpp
@@ -147,8 +147,7 @@ public:
   void printSectionHeaders() override;
   void printRelocations() override;
   void printDynamicRelocations() override;
-  void printSymbols() override;
-  void printDynamicSymbols() override;
+  void printSymbols(bool PrintSymbols, bool PrintDynamicSymbols) override;
   void printHashSymbols() override;
   void printUnwindInfo() override;
 
@@ -329,8 +328,8 @@ public:
   virtual void printGroupSections(const ELFFile<ELFT> *Obj) = 0;
   virtual void printRelocations(const ELFFile<ELFT> *Obj) = 0;
   virtual void printSectionHeaders(const ELFFile<ELFT> *Obj) = 0;
-  virtual void printSymbols(const ELFFile<ELFT> *Obj) = 0;
-  virtual void printDynamicSymbols(const ELFFile<ELFT> *Obj) = 0;
+  virtual void printSymbols(const ELFFile<ELFT> *Obj, bool PrintSymbols,
+                            bool PrintDynamicSymbols) = 0;
   virtual void printHashSymbols(const ELFFile<ELFT> *Obj) {}
   virtual void printDynamicRelocations(const ELFFile<ELFT> *Obj) = 0;
   virtual void printSymtabMessage(const ELFFile<ELFT> *Obj, StringRef Name,
@@ -365,8 +364,8 @@ public:
   void printGroupSections(const ELFFile<ELFT> *Obj) override;
   void printRelocations(const ELFO *Obj) override;
   void printSectionHeaders(const ELFO *Obj) override;
-  void printSymbols(const ELFO *Obj) override;
-  void printDynamicSymbols(const ELFO *Obj) override;
+  void printSymbols(const ELFO *Obj, bool PrintSymbols,
+                    bool PrintDynamicSymbols) override;
   void printHashSymbols(const ELFO *Obj) override;
   void printDynamicRelocations(const ELFO *Obj) override;
   void printSymtabMessage(const ELFO *Obj, StringRef Name,
@@ -459,8 +458,8 @@ public:
   void printRelocations(const ELFO *Obj) override;
   void printRelocations(const Elf_Shdr *Sec, const ELFO *Obj);
   void printSectionHeaders(const ELFO *Obj) override;
-  void printSymbols(const ELFO *Obj) override;
-  void printDynamicSymbols(const ELFO *Obj) override;
+  void printSymbols(const ELFO *Obj, bool PrintSymbols,
+                    bool PrintDynamicSymbols) override;
   void printDynamicRelocations(const ELFO *Obj) override;
   void printProgramHeaders(const ELFO *Obj) override;
   void printHashHistogram(const ELFFile<ELFT> *Obj) override;
@@ -474,6 +473,8 @@ public:
 private:
   void printRelocation(const ELFO *Obj, Elf_Rela Rel, const Elf_Shdr *SymTab);
   void printDynamicRelocation(const ELFO *Obj, Elf_Rela Rel);
+  void printSymbols(const ELFO *Obj);
+  void printDynamicSymbols(const ELFO *Obj);
   void printSymbol(const ELFO *Obj, const Elf_Sym *Symbol, const Elf_Sym *First,
                    StringRef StrTable, bool IsDynamic) override;
 
@@ -1622,14 +1623,11 @@ template <class ELFT> void ELFDumper<ELFT>::printDynamicRelocations() {
   ELFDumperStyle->printDynamicRelocations(ObjF->getELFFile());
 }
 
-template<class ELFT>
-void ELFDumper<ELFT>::printSymbols() {
-  ELFDumperStyle->printSymbols(ObjF->getELFFile());
-}
-
-template<class ELFT>
-void ELFDumper<ELFT>::printDynamicSymbols() {
-  ELFDumperStyle->printDynamicSymbols(ObjF->getELFFile());
+template <class ELFT>
+void ELFDumper<ELFT>::printSymbols(bool PrintSymbols,
+                                   bool PrintDynamicSymbols) {
+  ELFDumperStyle->printSymbols(ObjF->getELFFile(), PrintSymbols,
+                               PrintDynamicSymbols);
 }
 
 template<class ELFT>
@@ -3175,16 +3173,15 @@ void GNUStyle<ELFT>::printHashedSymbol(const ELFO *Obj, const Elf_Sym *FirstSym,
   OS << "\n";
 }
 
-template <class ELFT> void GNUStyle<ELFT>::printSymbols(const ELFO *Obj) {
-  if (opts::DynamicSymbols)
-    return;
-  this->dumper()->printSymbolsHelper(true);
-  this->dumper()->printSymbolsHelper(false);
-}
-
 template <class ELFT>
-void GNUStyle<ELFT>::printDynamicSymbols(const ELFO *Obj) {
+void GNUStyle<ELFT>::printSymbols(const ELFO *Obj, bool PrintSymbols,
+                                  bool PrintDynamicSymbols) {
+  if (!PrintSymbols && !PrintDynamicSymbols)
+    return;
+  // GNU readelf prints both the .dynsym and .symtab with --symbols.
   this->dumper()->printSymbolsHelper(true);
+  if (PrintSymbols)
+    this->dumper()->printSymbolsHelper(false);
 }
 
 template <class ELFT> void GNUStyle<ELFT>::printHashSymbols(const ELFO *Obj) {
@@ -4413,6 +4410,15 @@ void LLVMStyle<ELFT>::printSymbol(const ELFO *Obj, const Elf_Sym *Symbol,
   W.printHex("Section", SectionName, SectionIndex);
 }
 
+template <class ELFT>
+void LLVMStyle<ELFT>::printSymbols(const ELFO *Obj, bool PrintSymbols,
+                                   bool PrintDynamicSymbols) {
+  if (PrintSymbols)
+    printSymbols(Obj);
+  if (PrintDynamicSymbols)
+    printDynamicSymbols(Obj);
+}
+
 template <class ELFT> void LLVMStyle<ELFT>::printSymbols(const ELFO *Obj) {
   ListScope Group(W, "Symbols");
   this->dumper()->printSymbolsHelper(false);

diff --git a/tools/llvm-readobj/MachODumper.cpp b/tools/llvm-readobj/MachODumper.cpp
index ab35f15e3bd225f7a59069629026ebd84439eaf5..259f30de0bebaa3c4c176bb98213502a168fb697 100644 (file)
--- a/tools/llvm-readobj/MachODumper.cpp
+++ b/tools/llvm-readobj/MachODumper.cpp
@@ -33,8 +33,6 @@ public:
   void printFileHeaders() override;
   void printSectionHeaders() override;
   void printRelocations() override;
-  void printSymbols() override;
-  void printDynamicSymbols() override;
   void printUnwindInfo() override;
   void printStackMap() const override;
 
@@ -52,6 +50,8 @@ private:
   template<class MachHeader>
   void printFileHeaders(const MachHeader &Header);
 
+  void printSymbols() override;
+  void printDynamicSymbols() override;
   void printSymbol(const SymbolRef &Symbol);
 
   void printRelocation(const RelocationRef &Reloc);

diff --git a/tools/llvm-readobj/ObjDumper.h b/tools/llvm-readobj/ObjDumper.h
index fc50f7e277ddfc1404cfb36ca54570f8d012e4f9..17cc8bb552df656e15aeacd29781a909e5d1c71a 100644 (file)
--- a/tools/llvm-readobj/ObjDumper.h
+++ b/tools/llvm-readobj/ObjDumper.h
@@ -34,8 +34,12 @@ public:
   virtual void printFileHeaders() = 0;
   virtual void printSectionHeaders() = 0;
   virtual void printRelocations() = 0;
-  virtual void printSymbols() = 0;
-  virtual void printDynamicSymbols() = 0;
+  virtual void printSymbols(bool PrintSymbols, bool PrintDynamicSymbols) {
+    if (PrintSymbols)
+      printSymbols();
+    if (PrintDynamicSymbols)
+      printDynamicSymbols();
+  }
   virtual void printUnwindInfo() = 0;
 
   // Only implemented for ELF at this time.
@@ -93,6 +97,10 @@ public:
 
 protected:
   ScopedPrinter &W;
+
+private:
+  virtual void printSymbols() {};
+  virtual void printDynamicSymbols() {};
 };
 
 std::error_code createCOFFDumper(const object::ObjectFile *Obj,

diff --git a/tools/llvm-readobj/WasmDumper.cpp b/tools/llvm-readobj/WasmDumper.cpp
index 8cdf06aa14ac33ae601f058fd9e6e822550dd995..212232257cc9840242445d07550206715aaa33c1 100644 (file)
--- a/tools/llvm-readobj/WasmDumper.cpp
+++ b/tools/llvm-readobj/WasmDumper.cpp
@@ -48,8 +48,6 @@ public:
   void printFileHeaders() override;
   void printSectionHeaders() override;
   void printRelocations() override;
-  void printSymbols() override;
-  void printDynamicSymbols() override { llvm_unreachable("unimplemented"); }
   void printUnwindInfo() override { llvm_unreachable("unimplemented"); }
   void printStackMap() const override { llvm_unreachable("unimplemented"); }
 
@@ -58,6 +56,9 @@ protected:
   void printRelocation(const SectionRef &Section, const RelocationRef &Reloc);
 
 private:
+  void printSymbols() override;
+  void printDynamicSymbols() override { llvm_unreachable("unimplemented"); }
+
   const WasmObjectFile *Obj;
 };
 

diff --git a/tools/llvm-readobj/llvm-readobj.cpp b/tools/llvm-readobj/llvm-readobj.cpp
index 91526fe8380b68275e6825dd5362d133a748016b..206b67bbc785f7c25e9e331aa83ba5dd898cbb50 100644 (file)
--- a/tools/llvm-readobj/llvm-readobj.cpp
+++ b/tools/llvm-readobj/llvm-readobj.cpp
@@ -124,8 +124,10 @@ namespace opts {
 
   // -symbols
   // Also -s in llvm-readelf mode, or -t in llvm-readobj mode.
-  cl::opt<bool> Symbols("symbols",
-    cl::desc("Display the symbol table"));
+  cl::opt<bool>
+      Symbols("symbols",
+              cl::desc("Display the symbol table. Also display the dynamic "
+                       "symbol table when using GNU output style for ELF"));
   cl::alias SymbolsGNU("syms", cl::desc("Alias for --symbols"),
                        cl::aliasopt(Symbols));
 
@@ -462,10 +464,8 @@ static void dumpObject(const ObjectFile *Obj, ScopedPrinter &Writer) {
     Dumper->printRelocations();
   if (opts::DynRelocs)
     Dumper->printDynamicRelocations();
-  if (opts::Symbols)
-    Dumper->printSymbols();
-  if (opts::DynamicSymbols)
-    Dumper->printDynamicSymbols();
+  if (opts::Symbols || opts::DynamicSymbols)
+    Dumper->printSymbols(opts::Symbols, opts::DynamicSymbols);
   if (opts::HashSymbols)
     Dumper->printHashSymbols();
   if (opts::UnwindInfo)

diff --git a/tools/llvm-readobj/llvm-readobj.h b/tools/llvm-readobj/llvm-readobj.h
index f183855ce336e4e932f9d1b37e4c869051ac8a1c..500d7295e69468ae0e3fb3bc78d654da85560e5f 100644 (file)
--- a/tools/llvm-readobj/llvm-readobj.h
+++ b/tools/llvm-readobj/llvm-readobj.h
@@ -51,7 +51,6 @@ namespace opts {
   extern llvm::cl::opt<bool> SectionRelocations;
   extern llvm::cl::opt<bool> SectionSymbols;
   extern llvm::cl::opt<bool> SectionData;
-  extern llvm::cl::opt<bool> DynamicSymbols;
   extern llvm::cl::opt<bool> ExpandRelocs;
   extern llvm::cl::opt<bool> RawRelr;
   extern llvm::cl::opt<bool> CodeViewSubsectionBytes;