[llvm-objdump] Should print unknown d_tag in hex format

[Higuoxing]

Summary

Currently, llvm-objdump prints "unknown" instead of d_tag value in hex format. Because getDynamicTagAsString returns "unknown" rather than empty
string.

[grimar]

tools/llvm-objdump/ELFDump.cpp

179

Should getDynamicTagAsString just return nullptr on unknown flag maybe?
(It seems used only once in LLVM, for the place below, btw)

@jhenderson, what do you think?

[Higuoxing]

tools/llvm-objdump/ELFDump.cpp

179

There are several functions that return "unknown". I am not so confident to change those codes in /lib. In my opinion, I think those should return "".

[jhenderson]

tools/llvm-objdump/ELFDump.cpp

179

I'd say neither is the right thing to do. I actually think just printing the hex number is not good enough. I think it should be something like <unknown:>0x12345678, and I would highly recommend doing that inside getDynamicTagAsString(). It seems rather odd to be comparing against "unknown". The alternative would be to change the interface to return an Optional<> instead of a string, but the name of the function just makes me think I wouldn't need to do anything with the output at all.

[Higuoxing]

Addressed @jhenderson 's comment

• return unknown tag name as <unknown:>0x12345678

[jhenderson]

LGTM, assuming the case printing is correct, and the two other small comments are addressed.

lib/Object/ELF.cpp

473 ↗

(On Diff #188847)

This prints the hex digits as lower-case. Does that match what the rest of the llvm-objdump printing does?

test/tools/llvm-objdump/elf-dynamic-section.test

61

Maybe worth adding a hex digit to the tag value, to show upper/lower case behaviour.

tools/llvm-objdump/ELFDump.cpp

187

As we're messing about with different string types (const char *, std::string, StringRef etc), it's probably best to not use auto here.

[Higuoxing]

Addressed @jhenderson 's comment

• Change auto to std::string
• Update test case to make sure that hex values are printed in lower case. (Match GNU objdump's output)

test/tools/llvm-objdump/elf-dynamic-section.test

 # CHECK-NEXT:   FLAGS_1              0x0000000087654321
 # CHECK-NEXT:   VERSYM               0x0000000000001000
 # CHECK-NEXT:   VERDEF               0x0000000000001000
 # CHECK-NEXT:   VERDEFNUM            0x0000000000000000
 # CHECK-NEXT:   VERNEED              0x0000000000001000
 # CHECK-NEXT:   VERNEEDNUM           0x0000000000000000
 # CHECK-NEXT:   AUXILIARY            D
 # CHECK-NEXT:   FILTER               U
+# CHECK-NEXT:   0x12345678           0x0000000087654321
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_X86_64
 Sections:
- Name:    .dynamic
      - Tag:   DT_VERNEED
        Value: 0x1000
      - Tag:   DT_VERNEEDNUM
        Value: 0x0
      - Tag:   DT_AUXILIARY
        Value: 0x1
      - Tag:   DT_FILTER
        Value: 0x3
+     - Tag:   0x12345678
+       Value: 0x0000000087654321
      - Tag:   DT_NULL
        Value: 0x0
 ProgramHeaders:
   - Type: PT_LOAD
     VAddr: 0x1000
     Sections:
       - Section: .dynstr
       - Section: .dynamic
   - Type: PT_DYNAMIC
     VAddr: 0x1010
     Sections:
       - Section: .dynamic

tools/llvm-objdump/ELFDump.cpp

void printDynamicSection(const ELFFile<ELFT> *Elf, StringRef Filename) {
   if (!DynamicEntriesOrError)
     report_error(Filename, DynamicEntriesOrError.takeError());
 
   outs() << "Dynamic Section:\n";
   for (const auto &Dyn : *DynamicEntriesOrError) {
     if (Dyn.d_tag == ELF::DT_NULL)
       continue;
 
     StringRef Str = StringRef(Elf->getDynamicTagAsString(Dyn.d_tag));
 
-    if (Str.empty()) {
+    if (Str == "unknown") {
       std::string HexStr = utohexstr(static_cast<uint64_t>(Dyn.d_tag), true);
       outs() << format("  0x%-19s", HexStr.c_str());
     } else {
       // We use "-21" in order to match GNU objdump's output.
       outs() << format("  %-21s", Str.data());
     }
 
     const char *Fmt =
         ELFT::Is64Bits ? "0x%016" PRIx64 "\n" : "0x%08" PRIx64 "\n";
     if (Dyn.d_tag == ELF::DT_NEEDED || Dyn.d_tag == ELF::DT_RPATH ||
         Dyn.d_tag == ELF::DT_RUNPATH || Dyn.d_tag == ELF::DT_SONAME ||
         Dyn.d_tag == ELF::DT_AUXILIARY || Dyn.d_tag == ELF::DT_FILTER) {
       Expected<StringRef> StrTabOrErr = getDynamicStrTab(Elf);
       if (StrTabOrErr) {