[PATCH] D48554: Handle absolute symbols as branch targets in disassembly.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: test/tools/llvm-objdump/call-absolute-symbol-elf.test:1
+// RUN: llvm-objdump --disassemble %p/Inputs/call-absolute-symbol.elf-x86_64 | FileCheck %s
+
----------------
saugustine wrote:
> MaskRay wrote:
> > Should this test be synthesized from a `.s` file using llvm-mc?
> 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.
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.


================
Comment at: test/tools/llvm-objdump/call-absolute-symbol-elf.test:3
+
+CHECK: 201000:	e8 fb f0 df ff 	callq	-2100997 <foo>
----------------
saugustine wrote:
> MaskRay wrote:
> > 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);
> > }
> > 
> 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>
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..


Repository:
  rL LLVM

https://reviews.llvm.org/D48554