[llvm-objdump] Dump inline relocations if the relocated section is specified with --section

[MaskRay]

Summary

This fixes PR41886: llvm-objdump -d -r -j .text doesn't show inline relocations of .text

[grimar]

This looks good (with my comment addressed :) But please wait unil anyone else also take a look.

tools/llvm-objdump/llvm-objdump.cpp

932 ↗

(On Diff #200728)

This part contains unneseccary variable renamings and also a comment removal.
I would keep the old style.

[jhenderson]

test/tools/llvm-objdump/X86/section-filter-relocs.test

15–22 ↗

(On Diff #200728)

With the behaviour change, I'm not sure you need all of this, because it's covered by the case on line 11. The purpose of this was mostly to show that -j limited which relocations were printed to the specified relocation sections. What do you think?

tools/llvm-objdump/llvm-objdump.cpp

932 ↗

(On Diff #200728)

I'm actually okay with the RelSec -> Relocated name as it is easier to follow, and don't mind it changing.

[MaskRay]

tools/llvm-objdump/llvm-objdump.cpp

932 ↗

(On Diff #200728)

My first impression of RelSec is something similar to .rel.sec, so I thought it relocated Section which obviously is not the case.. So I still prefer Relocated.

The comment is redundant.

Regarding Sec, I'm inclined to use Sec in case someone asks me to rename Relocated to RelocatedSec. RelocatedSection would be too verbose.

[grimar]

tools/llvm-objdump/llvm-objdump.cpp

932 ↗

(On Diff #200728)

Ok, at least I would restore the comment. It was an innocent victim in this attack I think.

[MaskRay]

Address review comments

[MaskRay]

Restore // Sort relocations by address.

[grimar]

LGTM

[MaskRay]

tools/llvm-objdump/llvm-objdump.cpp

932 ↗

(On Diff #200728)

OK, restores, I probably wouldn't attach that if I ran git blame first... BTW, I think llvm::stable_sort is probably better.

Because gABI says: "If multiple consecutive relocation records are applied to the same relocation location (r_offset), they are composed instead of being applied independently, as described above."

Though I can't find a case to make this break.

llvm/trunk/test/tools/llvm-objdump/X86/section-filter-relocs.test

 ## Test that --section works correctly for -d with -r.
-## FIXME: Inline relocations are only printed if the relocation section itself is
-##        specified with --section. This test just characterizes the existing behavior.
-##        See https://bugs.llvm.org/show_bug.cgi?id=41886
 # RUN: yaml2obj %s -o %t.o
 
 ## Show non-executable sections are not disassembled even if specified,
 ## and that only the specified executable sections are disassembled.
 ## Also show that no relocation sections are dumped because none are
 ## specified.
 ## FIXME: This is different behaviour to GNU objdump, which dumps the non-
 ##        executable sections if requested explicitly.
 ##        See https://bugs.llvm.org/show_bug.cgi?id=41897.
 # RUN: llvm-objdump -d -r %t.o --section=.text --section=.rodata \
-# RUN:   | FileCheck %s --check-prefix=DISASM --implicit-check-not=.text2 \
-# RUN:           --implicit-check-not=.rodata --implicit-check-not=R_X86_64
-
-## Show that only the specified relocation sections that patch the
-## disassembled sections are dumped.
-# RUN: llvm-objdump -d -r %t.o --section=.text \
-# RUN:                         --section=.rela.text --section=.rela.text2 \
 # RUN:   | FileCheck %s --check-prefixes=DISASM,RELOC --implicit-check-not=.text2 \
-# RUN:           --implicit-check-not=R_X86_64
+# RUN:           --implicit-check-not=.rodata
 
 # DISASM:       Disassembly of section .text:
 # DISASM-EMPTY:
 # DISASM-NEXT:  0000000000000400 .text:
 # DISASM-NEXT:  400: e8 00 00 00 00                callq   0 <.text+0x5>
 # RELOC-NEXT:                      00000401:  R_X86_64_PC32        foo+1
+# RELOC-NEXT:                      00000401:  R_X86_64_GOT32       foo
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:

llvm/trunk/tools/llvm-objdump/llvm-objdump.cpp

Wide("wide", cl::desc("Ignored for compatibility with GNU objdump"),
          cl::cat(ObjdumpCat));
 static cl::alias WideShort("w", cl::Grouping, cl::aliasopt(Wide));
 
 static StringSet<> DisasmFuncsSet;
 static StringRef ToolName;
 
 typedef std::vector<std::tuple<uint64_t, StringRef, uint8_t>> SectionSymbolsTy;
 
-SectionFilter ToolSectionFilter(object::ObjectFile const &O) {
-  return SectionFilter(
-      [](object::SectionRef const &S) {
-        if (FilterSections.empty())
-          return true;
-        StringRef String;
-        std::error_code error = S.getName(String);
-        if (error)
-          return false;
-        return is_contained(FilterSections, String);
-      },
-      O);
+static bool shouldKeep(object::SectionRef S) {
+}
+
+SectionFilter ToolSectionFilter(object::ObjectFile const &O) {
+  return SectionFilter([](object::SectionRef S) { return shouldKeep(S); }, O);
 }
 
 void error(std::error_code EC) {
   if (!EC)
     return;
   WithColor::error(errs(), ToolName)
       << "reading file: " << EC.message() << ".\n";
   errs().flush();
static size_t countSkippableZeroBytes(ArrayRef<uint8_t> Buf) {
   // instruction if it starts with a zero byte.
   return N & ~0x3;
 }
 
 // Returns a map from sections to their relocations.
 static std::map<SectionRef, std::vector<RelocationRef>>
 getRelocsMap(object::ObjectFile const &Obj) {
   std::map<SectionRef, std::vector<RelocationRef>> Ret;
-  for (const SectionRef &Section : ToolSectionFilter(Obj)) {
-    section_iterator RelSec = Section.getRelocatedSection();
-    if (RelSec == Obj.section_end())
+  for (SectionRef Sec : Obj.sections()) {
+    section_iterator Relocated = Sec.getRelocatedSection();
+    if (Relocated == Obj.section_end() || !shouldKeep(*Relocated))
       continue;
-    std::vector<RelocationRef> &V = Ret[*RelSec];
-    for (const RelocationRef &R : Section.relocations())
+    std::vector<RelocationRef> &V = Ret[*Relocated];
+    for (const RelocationRef &R : Sec.relocations())
       V.push_back(R);
     // Sort relocations by address.
-    llvm::sort(V, isRelocAddressLess);
+    llvm::stable_sort(V, isRelocAddressLess);
   }
   return Ret;
 }
 
 // Used for --adjust-vma to check if address should be adjusted by the
 // specified value for a given section.
 // For ELF we do not adjust non-allocatable sections like debug ones,
 // because they are not loadable.