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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
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:
> echristo wrote:
> > MaskRay wrote:
> > > 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.
> > 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
> 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.
Nit: trailing `\n` should be unnecessary.


Repository:
  rL LLVM

https://reviews.llvm.org/D48554