From 8e1746faa3574df8fc8a3d4d26c9bff6f7859d04 Mon Sep 17 00:00:00 2001 From: Paul Kirth Date: Fri, 17 Mar 2023 23:32:00 +0000 Subject: [PATCH] [llvm-readobj] Standardize JSON output for `Other` field Today, the LLVM output uses special handling when the Other field is 0. This output makes sense for a command line utility that a human will read, but JSON is a machine readable format, so being consistent is more important. Prior to this change, any consumer of the JSON output would need to handle the Other field specially, since the structure of the JSON would no longer be consistent. Changes to JSON output when Other flag == 0: ``` "Other": 0, -> "Other": { "RawFlags": 0, "Flags": [] }, ``` There are no changes to when Other flag != 0: ``` "Other": { -> "Other": { "RawFlags": 1, "RawFlags": 1, "Flags": [ "Flags": [ ... ... ] ] }, }, ``` This patch adds a overload for the JSONELFDumper's printSymbol() method, that uses consistent output formatting, regardless of the value of the Other field. Depends on D137092 Reviewed By: jhenderson Differential Revision: https://reviews.llvm.org/D137088 --- .../llvm-readobj/ELF/aarch64-symbols-stother.test | 49 ++++++++++++++++++++++ .../llvm-readobj/ELF/mips-symbols-stother.test | 34 +++++++++++++++ .../tools/llvm-readobj/ELF/symbol-visibility.test | 40 ++++++++++++++++++ llvm/tools/llvm-readobj/ELFDumper.cpp | 42 ++++++++++++++----- 4 files changed, 155 insertions(+), 10 deletions(-) diff --git a/llvm/test/tools/llvm-readobj/ELF/aarch64-symbols-stother.test b/llvm/test/tools/llvm-readobj/ELF/aarch64-symbols-stother.test index bc9d128..35205ae 100644 --- a/llvm/test/tools/llvm-readobj/ELF/aarch64-symbols-stother.test +++ b/llvm/test/tools/llvm-readobj/ELF/aarch64-symbols-stother.test @@ -2,6 +2,7 @@ # RUN: yaml2obj %s -o %t.o # RUN: llvm-readobj --symbols %t.o | FileCheck %s --check-prefix=LLVM +# RUN: llvm-readobj --symbols %t.o --elf-output-style=JSON --pretty-print | FileCheck %s --check-prefix=JSON # RUN: llvm-readelf --symbols %t.o | FileCheck %s --check-prefix=GNU # LLVM: Name: foo1 @@ -28,6 +29,54 @@ # GNU-NEXT: 3: 0000000000000000 0 NOTYPE LOCAL PROTECTED [VARIANT_PCS] UND foo3 # GNU-NEXT: 4: 0000000000000000 0 NOTYPE LOCAL PROTECTED UND foo4 +# JSON: "Value": "foo1", +# JSON: "Other": { +# JSON-NEXT: "RawFlags": 128, +# JSON-NEXT: "Flags": [ +# JSON-NEXT: { +# JSON-NEXT: "Name": "STO_AARCH64_VARIANT_PCS", +# JSON-NEXT: "Value": 128 +# JSON-NEXT: } +# JSON-NEXT: ] +# JSON-NEXT:}, + +# JSON: "Value": "foo2", +# JSON: "Other": { +# JSON-NEXT: "RawFlags": 192, +# JSON-NEXT: "Flags": [ +# JSON-NEXT: { +# JSON-NEXT: "Name": "STO_AARCH64_VARIANT_PCS", +# JSON-NEXT: "Value": 128 +# JSON-NEXT: } +# JSON-NEXT: ] +# JSON-NEXT:}, + +# JSON: "Value": "foo3", +# JSON: "Other": { +# JSON-NEXT: "RawFlags": 131, +# JSON-NEXT: "Flags": [ +# JSON-NEXT: { +# JSON-NEXT: "Name": "STO_AARCH64_VARIANT_PCS", +# JSON-NEXT: "Value": 128 +# JSON-NEXT: }, +# JSON-NEXT: { +# JSON-NEXT: "Name": "STV_PROTECTED", +# JSON-NEXT: "Value": 3 +# JSON-NEXT: } +# JSON-NEXT: ] +# JSON-NEXT:}, + +# JSON: "Value": "foo4", +# JSON: "Other": { +# JSON-NEXT: "RawFlags": 3, +# JSON-NEXT: "Flags": [ +# JSON-NEXT: { +# JSON-NEXT: "Name": "STV_PROTECTED", +# JSON-NEXT: "Value": 3 +# JSON-NEXT: } +# JSON-NEXT: ] +# JSON-NEXT:}, + --- !ELF FileHeader: Class: ELFCLASS64 diff --git a/llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test b/llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test index 37415fa..983aefb 100644 --- a/llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test +++ b/llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test @@ -2,6 +2,7 @@ # RUN: yaml2obj %s -o %t.o # RUN: llvm-readobj --symbols %t.o | FileCheck %s --strict-whitespace --check-prefix=MIPS-LLVM +# RUN: llvm-readobj --symbols %t.o --elf-output-style=JSON --pretty-print | FileCheck %s --check-prefix=MIPS-JSON # RUN: llvm-readelf --symbols %t.o | FileCheck %s --strict-whitespace --check-prefix=MIPS-GNU # MIPS-LLVM:Name: foo @@ -23,6 +24,39 @@ # MIPS-GNU-NEXT: 1: 00000000 0 NOTYPE LOCAL DEFAULT [] UND foo # MIPS-GNU-NEXT: 2: 00000000 0 NOTYPE LOCAL DEFAULT [] UND bar +# MIPS-JSON: "Value": "foo", +# MIPS-JSON: "Other": { +# MIPS-JSON-NEXT: "RawFlags": 172, +# MIPS-JSON-NEXT: "Flags": [ +# MIPS-JSON-NEXT: { +# MIPS-JSON-NEXT: "Name": "STO_MIPS_MICROMIPS", +# MIPS-JSON-NEXT: "Value": 128 +# MIPS-JSON-NEXT: }, +# MIPS-JSON-NEXT: { +# MIPS-JSON-NEXT: "Name": "STO_MIPS_OPTIONAL", +# MIPS-JSON-NEXT: "Value": 4 +# MIPS-JSON-NEXT: }, +# MIPS-JSON-NEXT: { +# MIPS-JSON-NEXT: "Name": "STO_MIPS_PIC", +# MIPS-JSON-NEXT: "Value": 32 +# MIPS-JSON-NEXT: }, +# MIPS-JSON-NEXT: { +# MIPS-JSON-NEXT: "Name": "STO_MIPS_PLT", +# MIPS-JSON-NEXT: "Value": 8 +# MIPS-JSON-NEXT: } +# MIPS-JSON-NEXT: ] +# MIPS-JSON-NEXT: }, +# MIPS-JSON: "Value": "bar", +# MIPS-JSON: "Other": { +# MIPS-JSON-NEXT: "RawFlags": 240, +# MIPS-JSON-NEXT: "Flags": [ +# MIPS-JSON-NEXT: { +# MIPS-JSON-NEXT: "Name": "STO_MIPS_MIPS16", +# MIPS-JSON-NEXT: "Value": 240 +# MIPS-JSON-NEXT: } +# MIPS-JSON-NEXT: ] +# MIPS-JSON-NEXT: }, + --- !ELF FileHeader: Class: ELFCLASS32 diff --git a/llvm/test/tools/llvm-readobj/ELF/symbol-visibility.test b/llvm/test/tools/llvm-readobj/ELF/symbol-visibility.test index 4c298b1..fa3a8c1 100644 --- a/llvm/test/tools/llvm-readobj/ELF/symbol-visibility.test +++ b/llvm/test/tools/llvm-readobj/ELF/symbol-visibility.test @@ -7,6 +7,7 @@ # RUN: yaml2obj --docnum=1 %s -o %t1.o # RUN: llvm-readobj --symbols %t1.o | FileCheck %s --check-prefix=LLVM # RUN: llvm-readelf --symbols %t1.o | FileCheck %s --strict-whitespace --check-prefix=GNU +# RUN: llvm-readobj --symbols --pretty-print --elf-output-style=JSON %t1.o | FileCheck %s --check-prefix=JSON # LLVM: Name: default # LLVM: Other: 0 @@ -30,6 +31,45 @@ # GNU-NEXT: HIDDEN UND hidden # GNU-NEXT: PROTECTED UND protected +# JSON: "Value": "default", +# JSON: "Other": { +# JSON-NEXT: "RawFlags": 0, +# JSON-NEXT: "Flags": [] +# JSON-NEXT: }, + +# JSON: "Value": "internal", +# JSON: "Other": { +# JSON-NEXT: "RawFlags": 1, +# JSON-NEXT: "Flags": [ +# JSON-NEXT: { +# JSON-NEXT: "Name": "STV_INTERNAL", +# JSON-NEXT: "Value": 1 +# JSON-NEXT: } +# JSON-NEXT: ] +# JSON-NEXT: }, + +# JSON: "Value": "hidden", +# JSON: "Other": { +# JSON-NEXT: "RawFlags": 2, +# JSON-NEXT: "Flags": [ +# JSON-NEXT: { +# JSON-NEXT: "Name": "STV_HIDDEN", +# JSON-NEXT: "Value": 2 +# JSON-NEXT: } +# JSON-NEXT: ] +# JSON-NEXT: }, + +# JSON: "Value": "protected", +# JSON: "Other": { +# JSON-NEXT: "RawFlags": 3, +# JSON-NEXT: "Flags": [ +# JSON-NEXT: { +# JSON-NEXT: "Name": "STV_PROTECTED", +# JSON-NEXT: "Value": 3 +# JSON-NEXT: } +# JSON-NEXT: ] +# JSON-NEXT: }, + --- !ELF FileHeader: Class: ELFCLASS32 diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp index 693b7e2..7e832ed 100644 --- a/llvm/tools/llvm-readobj/ELFDumper.cpp +++ b/llvm/tools/llvm-readobj/ELFDumper.cpp @@ -689,14 +689,14 @@ public: void printMemtag( const ArrayRef> DynamicEntries, const ArrayRef AndroidNoteDesc) override; + void printSymbolSection(const Elf_Sym &Symbol, unsigned SymIndex, + DataRegion ShndxTable) const; private: void printRelrReloc(const Elf_Relr &R) override; void printRelRelaReloc(const Relocation &R, const RelSymbol &RelSym) override; - void printSymbolSection(const Elf_Sym &Symbol, unsigned SymIndex, - DataRegion ShndxTable) const; void printSymbol(const Elf_Sym &Symbol, unsigned SymIndex, DataRegion ShndxTable, std::optional StrTable, bool IsDynamic, @@ -709,8 +709,10 @@ private: void printMipsGOT(const MipsGOTParser &Parser) override; void printMipsPLT(const MipsGOTParser &Parser) override; void printMipsABIFlags() override; + virtual void printZeroSymbolOtherField(const Elf_Sym &Symbol) const; protected: + void printSymbolOtherField(const Elf_Sym &Symbol) const; ScopedPrinter &W; }; @@ -726,6 +728,7 @@ public: void printFileSummary(StringRef FileStr, ObjectFile &Obj, ArrayRef InputFilenames, const Archive *A) override; + virtual void printZeroSymbolOtherField(const Elf_Sym &Symbol) const override; private: std::unique_ptr FileScope; @@ -6876,6 +6879,22 @@ void LLVMELFDumper::printSymbolSection( } template +void LLVMELFDumper::printSymbolOtherField(const Elf_Sym &Symbol) const { + std::vector> SymOtherFlags = + this->getOtherFlagsFromSymbol(this->Obj.getHeader(), Symbol); + W.printFlags("Other", Symbol.st_other, ArrayRef(SymOtherFlags), 0x3u); +} + +template +void LLVMELFDumper::printZeroSymbolOtherField( + const Elf_Sym &Symbol) const { + assert(Symbol.st_other == 0 && "non-zero Other Field"); + // Usually st_other flag is zero. Do not pollute the output + // by flags enumeration in that case. + W.printNumber("Other", 0); +} + +template void LLVMELFDumper::printSymbol(const Elf_Sym &Symbol, unsigned SymIndex, DataRegion ShndxTable, std::optional StrTable, @@ -6896,14 +6915,9 @@ void LLVMELFDumper::printSymbol(const Elf_Sym &Symbol, unsigned SymIndex, else W.printEnum("Type", SymbolType, ArrayRef(ElfSymbolTypes)); if (Symbol.st_other == 0) - // Usually st_other flag is zero. Do not pollute the output - // by flags enumeration in that case. - W.printNumber("Other", 0); - else { - std::vector> SymOtherFlags = - this->getOtherFlagsFromSymbol(this->Obj.getHeader(), Symbol); - W.printFlags("Other", Symbol.st_other, ArrayRef(SymOtherFlags), 0x3u); - } + printZeroSymbolOtherField(Symbol); + else + printSymbolOtherField(Symbol); printSymbolSection(Symbol, SymIndex, ShndxTable); } @@ -7660,3 +7674,11 @@ void JSONELFDumper::printFileSummary(StringRef FileStr, ObjectFile &Obj, std::string(formatv("{0}bit", 8 * Obj.getBytesInAddress()))); this->printLoadName(); } + +template +void JSONELFDumper::printZeroSymbolOtherField( + const Elf_Sym &Symbol) const { + // We want the JSON format to be uniform, since it is machine readable, so + // always print the `Other` field the same way. + this->printSymbolOtherField(Symbol); +} -- 2.7.4