Handle absolute symbols as branch targets in disassembly.

[saugustine]

Summary

Certain tests and embedded systems use sectionless symbols
as the target of branches or calls. Handle them in the disassembly.

[saugustine]

Add missing test file.

[saugustine]

Add missing newline.

[saugustine]

Restore missing files.

[saugustine]

Small update to fix segfault in llvm test.

[MaskRay]

test/tools/llvm-objdump/call-absolute-symbol-elf.test

2

Should this test be synthesized from a .s file using llvm-mc?

[MaskRay]

test/tools/llvm-objdump/call-absolute-symbol-elf.test

4

I find that on AArch64 PowerPC x86_64 ... all these call/branch instructions print the immediate value without taking account of PC (what they print are relative addresses, instead of absolute addresses as printed by objdump)

The PowerPC one at least appends .+ before the immediate to tell you it is a relative address.

https://github.com/llvm-mirror/llvm/tree/master/lib/Target/PowerPC/InstPrinter/PPCInstPrinter.cpp#L386
void PPCInstPrinter::printBranchOperand(const MCInst *MI, unsigned OpNo,

                                      raw_ostream &O) {
if (!MI->getOperand(OpNo).isImm())
  return printOperand(MI, OpNo, O);

// Branches can take an immediate operand.  This is used by the branch
// selection pass to print .+8, an eight byte displacement from the PC.
O << ".+";
printAbsBranchOperand(MI, OpNo, O);

}

tools/llvm-objdump/llvm-objdump.cpp

1725

Reasonable behavior. I have verified that objdump does something similar (foo + 4). It has one extra heuristic: if the target address is less than the absolute symbol of the smallest address, it can print something like foo - 4. But I think we don't need such heuristic.

[saugustine]

test/tools/llvm-objdump/call-absolute-symbol-elf.test

2

This is a good question, and I debated. But only about five of the ~100 files in llbm-objdump/Inputs are normal ASCII source of one kind or another. The rest are binaries. So I went that way. Totally willing to change it if you think it is important.

4

This patch doesn't change the printed value, it just adds the symbol it targets. This is sufficient for my needs, and I'm not sure what the dependencies of the current format are, or if it is a principled decision.

Before this patch:

201000: e8 fb f0 df ff callq -2100997

After:

201000: e8 fb f0 df ff callq -2100997 <foo>

[MaskRay]

test/tools/llvm-objdump/call-absolute-symbol-elf.test

2

Adding @enderby @ruiu who have touched test/Object/Inputs/* binaries.

There are also utilities obj2yaml and yaml2obj (introduced in 2012) to construct tests involving binary files. I wonder why the tests here are organized in such a way.

4

I should be clear that my comment was asking why pc-relative addresses are printed in the current way... This patch definitely improves the view so you can just ignore my comment..

[echristo]

test/tools/llvm-objdump/call-absolute-symbol-elf.test

2

I can elaborate here:

• Some of the tests predate obj2yaml/yaml2obj - especially since it didn't see widespread use until recently.

• For some tests you'll definitely want an existing object file because you've either got malformed output that you were trying to look at, or something that might change in the future.

• Otherwise a .s file is fine too.

My preference here, I think, is going to be:

1. yaml2obj if it's simple
2. .s file if possible
3. checked in object file

4

Almost assuredly no particular planning on the format of objdump output here.

[saugustine]

Switch from binary file as input to assembly in line in the test.

[saugustine]

test/tools/llvm-objdump/call-absolute-symbol-elf.test

2

It turns out that yaml2obj doesn't handle absolute symbols cleanly either. So I've just converted the very simple input to inline text in the test itself.

[echristo]

I'm ok here. Ray?

[MaskRay]

test/tools/llvm-objdump/call-absolute-symbol-elf.test

2

Nit: trailing \n should be unnecessary.

[saugustine]

This is committed as r335903, with a small update to fix targets that don't have ld.lld available in r335908.

[grimar]

test/tools/llvm-objdump/call-absolute-symbol-elf.test

1

btw, I think \n can be avoided here with the use of ;.
Here is how we do such things in LLD tests:

# RUN: echo '.section .init_array, "aw"; .quad 0' | llvm-mc -filetype=obj -triple=x86_64-unknown-linux - -o %t.o

It perhaps looks a bit cleaner.

I see that you switched to use the binary file in rL335908. I think it is a better way because does not seem
that adding a dependency to ld.lld was good for llvm-objdump tests.
That is also consistent with another test we have which is test\tools\llvm-objdump\X86\phdrs.test.

A good practice is to add the description of how to rebuild the binary object used.
Because in some rare cases it might be needed, but without such descriptions, it can be a painful task.
phdrs.test, for example, contains such description. May I request you to add one too, please?

test/tools/llvm-objdump/call-absolute-symbol-elf.test

+// RUN: echo -e ".text\ncallq foo\n" | llvm-mc -filetype=obj -triple=x86_64-pc-linux - -o %t1.o
+// RUN: ld.lld --defsym foo=0x100 %t1.o -o %t2
+// RUN: llvm-objdump --disassemble %t2 | FileCheck %s
+
+CHECK: 201000:  e8 fb f0 df ff  callq -2100997 <foo>

tools/llvm-objdump/llvm-objdump.cpp

for (const SectionRef &Section : ToolSectionFilter(*Obj)) {
     section_iterator Sec2 = Section.getRelocatedSection();
     if (Sec2 != Obj->section_end())
       SectionRelocMap[*Sec2].push_back(Section);
   }
 
   // Create a mapping from virtual address to symbol name.  This is used to
   // pretty print the symbols while disassembling.
   std::map<SectionRef, SectionSymbolsTy> AllSymbols;
+  SectionSymbolsTy AbsoluteSymbols;
   for (const SymbolRef &Symbol : Obj->symbols()) {
     Expected<uint64_t> AddressOrErr = Symbol.getAddress();
     if (!AddressOrErr)
       report_error(Obj->getFileName(), AddressOrErr.takeError());
     uint64_t Address = *AddressOrErr;
 
     Expected<StringRef> Name = Symbol.getName();
     if (!Name)
       report_error(Obj->getFileName(), Name.takeError());
     if (Name->empty())
       continue;
 
     Expected<section_iterator> SectionOrErr = Symbol.getSection();
     if (!SectionOrErr)
       report_error(Obj->getFileName(), SectionOrErr.takeError());
-    section_iterator SecI = *SectionOrErr;
-    if (SecI == Obj->section_end())
-      continue;
 
     uint8_t SymbolType = ELF::STT_NOTYPE;
     if (Obj->isELF())
       SymbolType = getElfSymbolType(Obj, Symbol);
 
+    section_iterator SecI = *SectionOrErr;
+    if (SecI != Obj->section_end())
       AllSymbols[*SecI].emplace_back(Address, *Name, SymbolType);
+    else
+      AbsoluteSymbols.emplace_back(Address, *Name, SymbolType);
+
 
   }
   if (AllSymbols.empty() && Obj->isELF())
     addDynamicElfSymbols(Obj, AllSymbols);
 
   // Create a mapping from virtual address to section.
   std::vector<std::pair<uint64_t, SectionRef>> SectionAddresses;
   for (SectionRef Sec : Obj->sections())
for (const auto &ExportEntry : COFFObj->export_directories()) {
           });
       if (Sec != SectionAddresses.begin())
         --Sec;
       else
         Sec = SectionAddresses.end();
 
       if (Sec != SectionAddresses.end())
         AllSymbols[Sec->second].emplace_back(VA, Name, ELF::STT_NOTYPE);
+      else
+        AbsoluteSymbols.emplace_back(VA, Name, ELF::STT_NOTYPE);
     }
   }
 
   // Sort all the symbols, this allows us to use a simple binary search to find
   // a symbol near an address.
   for (std::pair<const SectionRef, SectionSymbolsTy> &SecSyms : AllSymbols)
     array_pod_sort(SecSyms.second.begin(), SecSyms.second.end());
+  array_pod_sort(AbsoluteSymbols.begin(), AbsoluteSymbols.end());
 
   for (const SectionRef &Section : ToolSectionFilter(*Obj)) {
     if (!DisassembleAll && (!Section.isText() || Section.isVirtual()))
       continue;
 
     uint64_t SectionAddr = Section.getAddress();
     uint64_t SectSize = Section.getSize();
     if (!SectSize)
#endif
                   [](uint64_t LHS,
                       const std::pair<uint64_t, SectionRef> &RHS) {
                     return LHS < RHS.first;
                   });
               if (SectionAddress != SectionAddresses.begin()) {
                 --SectionAddress;
                 TargetSectionSymbols = &AllSymbols[SectionAddress->second];
               } else {
-                TargetSectionSymbols = nullptr;
+                TargetSectionSymbols = &AbsoluteSymbols;
               }
             }
 
             // Find the first symbol in the section whose offset is less than
-            // or equal to the target.
-            if (TargetSectionSymbols) {
-              auto TargetSym = std::upper_bound(
-                  TargetSectionSymbols->begin(), TargetSectionSymbols->end(),
-                  Target, [](uint64_t LHS,
-                             const std::tuple<uint64_t, StringRef, uint8_t> &RHS) {
-                    return LHS < std::get<0>(RHS);
-                  });
-              if (TargetSym != TargetSectionSymbols->begin()) {
-                --TargetSym;
-                uint64_t TargetAddress = std::get<0>(*TargetSym);
-                StringRef TargetName = std::get<1>(*TargetSym);
-                outs() << " <" << TargetName;
-                uint64_t Disp = Target - TargetAddress;
-                if (Disp)
-                  outs() << "+0x" << Twine::utohexstr(Disp);
-                outs() << '>';
-              }
-            }
-          }
-        }
+            // or equal to the target. If there isn't a section that contains
+            // the target, find the nearest preceding absolute symbol.
+            if (TargetSym == TargetSectionSymbols->begin()) {
+              TargetSectionSymbols = &AbsoluteSymbols;
+              TargetSym = std::upper_bound(
+                  AbsoluteSymbols.begin(), AbsoluteSymbols.end(),
+                  Target, [](uint64_t LHS,
+                             const std::tuple<uint64_t, StringRef, uint8_t> &RHS) {
+                            return LHS < std::get<0>(RHS);
+                          });
+            }
         outs() << "\n";
 
         // Hexagon does this in pretty printer
         if (Obj->getArch() != Triple::hexagon)
           // Print relocation for instruction.
           while (rel_cur != rel_end) {
             bool hidden = getHidden(*rel_cur);
             uint64_t addr = rel_cur->getOffset();