[llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

[hoyFB]

Summary

When diffing disassembly dump of two binaries, I see lots of noises from mismatched jump target addresses and global data references, which unnecessarily causes diffs on every function, making it impractical. I'm trying to symbolize the raw binary addresses to minimize the diff noise.
In this change, a local branch target is modeled as a label and the branch target operand will simply be printed as a label. Local labels are collected by a separate pre-decoding pass beforehand. A global data memory operand will be printed as a global symbol instead of the raw data address. Unfortunately, due to the way the disassembler is set up and to be less intrusive, a global symbol is always printed as the last operand of a memory access instruction. This is less than ideal but is probably acceptable from checking code quality point of view since on most targets an instruction can have at most one memory operand.

So far only the X86 disassemblers are supported.

Test Plan:

llvm-objdump -d --x86-asm-syntax=intel --no-show-raw-insn --no-leading-addr :

Disassembly of section .text:

<_start>:
               	push	rax
               	mov	dword ptr [rsp + 4], 0
               	mov	dword ptr [rsp], 0
               	mov	eax, dword ptr [rsp]
               	cmp	eax, dword ptr [rip + 4112]  # 202182 <g>
               	jge	0x20117e <_start+0x25>
               	call	0x201158 <foo>
               	inc	dword ptr [rsp]
               	jmp	0x201169 <_start+0x10>
               	xor	eax, eax
               	pop	rcx
               	ret

llvm-objdump -d --symbolize-operands --x86-asm-syntax=intel --no-show-raw-insn --no-leading-addr :

Disassembly of section .text:

<_start>:
               	push	rax
               	mov	dword ptr [rsp + 4], 0
               	mov	dword ptr [rsp], 0
<L1>:
               	mov	eax, dword ptr [rsp]
               	cmp	eax, dword ptr  <g>
               	jge	 <L0>
               	call	 <foo>
               	inc	dword ptr [rsp]
               	jmp	 <L1>
<L0>:
               	xor	eax, eax
               	pop	rcx
               	ret

Note that the jump instructions like jge 0x20117e <_start+0x25> without this work is printed as a real target address and an offset from the leading symbol. With a change in the optimizer that adds/deletes an instruction, the address and offset may shift for targets placed after the instruction. This will be a problem when diffing the disassembly from two optimizers where there are unnecessary false positives due to such branch target address changes. With --symbolize-operand, a label is printed for a branch target instead to reduce the false positives. Similarly, the disassemble of PC-relative global variable references is also prone to instruction insertion/deletion.

[hoyFB]

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

[hoyFB]

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

[MaskRay]

llvm/test/tools/llvm-objdump/X86/disassemble-symbolize-operands.s

3

lld is a subproject and depends on llvm. You cannot use ld.lld in llvm tests.

[jhenderson]

Could you post some examples of what the actual difference in output is, please? I don't quite follow from the tests/description what is intended to change as a result of this option.

Also, if you are adding a new option to llvm-objdump, please remember to document it in llvm/docs/CommandGuide/llvm-objdump.rst.

[hoyFB]

In D84191#2163668, @jhenderson wrote:

Could you post some examples of what the actual difference in output is, please? I don't quite follow from the tests/description what is intended to change as a result of this option.

Also, if you are adding a new option to llvm-objdump, please remember to document it in llvm/docs/CommandGuide/llvm-objdump.rst.

The main difference in output is to print branch target as labels instead of real addresses, also for PC-relative global references. I updated the summary. Please let me know if it is clear. Thanks.

llvm/test/tools/llvm-objdump/X86/disassemble-symbolize-operands.s

3

The original test seemed to pass. Did that end up using the pre-installed lld on the test machine?

I switched to using a prebuilt binary instead.

[hoyFB]

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

[hoyFB]

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

[MaskRay]

llvm/docs/CommandGuide/llvm-objdump.rst

206 ↗

(On Diff #279571)

You probably want to mean a linked image (not a relocatable object file).

Capitalized Intel isn't supported. att is much more common and --x86-asm-syntax=intel is rare. As a related fact, LLVM's Intel syntax assembler gets less scrutiny and tends to have some problems.

llvm/test/tools/llvm-objdump/X86/disassemble-symbolize-operands.test

1 ↗

(On Diff #279571)

Please avoid a checked-in executable. Its content is difficult to inspect.
See X86/elf-disassemble-symbol-references.yaml for a yaml2obj example.

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

1576

I wonder why you need this. For BOLT or similar post-link rewriting stuff?

[jhenderson]

Thanks for the example, they helped me understand much better what you're trying to achieve and +1 to the idea definitely. In a weird coincidence, one of our toolchain team only last week demo'ed to the team a script to make comparing diassembly much less noisy. This option may help with some of that.

llvm/docs/CommandGuide/llvm-objdump.rst

202 ↗

(On Diff #279571)

instead of real -> instead of a real

[jhenderson]

llvm/docs/CommandGuide/llvm-objdump.rst

202–204 ↗

(On Diff #279571)

It might be worth including a couple of simple examples of what gets printed with the option. I found the example you posted in the patch description a much clearer way of expressing this compared to just words (you obviously should still keep the words).

206 ↗

(On Diff #279571)

Is there anything fundamentally preventing this from working with the default assembly syntax or relocatable objects? Or is it just that you haven't implemented it yet?

208 ↗

(On Diff #279571)

Nit: delete second blank line.

llvm/include/llvm/MC/MCInstPrinter.h

68

"symbolize branch target and memory reference operands"

(no need to repeat operands twice this way)

llvm/test/tools/llvm-objdump/X86/disassemble-symbolize-operands.test

1 ↗

(On Diff #279571)

You probably want to use FileCheck's --match-full-lines to show that you are capturing all the content on the line and that there isn't any additional text before/after the thing you check for.

4–10 ↗

(On Diff #279571)

Consider using CHECK-NEXT, rather than CHECK, if possible.

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

1580–1583

I don't think X86 is exempt from potentially having data embedded in the text section. I think I've seen jump tables that are, for example.

It's not clear to me, why a target having potential data embedding is an issue?

1594

I'd be slightly concerned by this additional pass over the disassembly, due to performance reasons, but I guess it's behind a switch, so it's not that big a deal. Plus, I can't think how you'd identify where the labels should go otherwise, since they might appear before the reference.

[hoyFB]

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

[hoyFB]

llvm/docs/CommandGuide/llvm-objdump.rst

206 ↗

(On Diff #279571)

Yes, a linked image is more accurate.

Changed Intel to lower case.

The att syntax is now supported too.

206 ↗

(On Diff #279571)

We need a tool to compare executable image since very often the immediate object files are not available.

Regarding the assembly syntax, we are mostly comfortable reading the intel syntax. But since the att syntax is more common, I'll add support for that.

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

1576

Good question. This is needed when doing optimization work where we compare code quality by checking binary disassembly.

1580–1583

Yeah, on X86 a jump table could be placed inside a function as a .data instruction. So literally it is still an instruction. I noticed that on Arm there might be non-instruction data encoded in the .text section and special handling is needed when disassembling instructions. I removed the comment to reduce the confusion.

1594

Exactly, two-pass disassembling is needed to support backward jumps.

[hoyFB]

llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbololize-operands.yaml

9 ↗

(On Diff #279934)

This is less than ideal because of the current less intrusive implementation. It's probably fine from asm diffing standpoint. To fix this we may want to pass the symbol/label resolution result from the common code into the per-target inst printer and print the symbols right in place for a memory operand. But that may cause changes to the core MCInstPrinter interface and the callbacks to initialize target inst printers.

[hoyFB]

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

[jhenderson]

It looks like you might still have a binary file in the patch, that is no longer needed?

llvm/docs/CommandGuide/llvm-objdump.rst

206 ↗

(On Diff #279571)

I'm not sure you didn't misunderstand my comment earlier. Let's say a user does have a single intermediate object file they want to disassemble, and it has plenty of references within itself. These addresses will be present, thus comparing it against a different version of the same object might produce a noisy diff. It sounds like this option would be helpful there, so why doesn't it work?

209–212 ↗

(On Diff #279954)

I would actively include multi-line output snippets as your examples rather than using prose. Something like:
"

jge 0x20117e <_start+0x25>
dword ptr [rip + 4112]

might become

<g>:
  jge  <L0>
  dword ptr [rip + 4112]
<L0>:

"
(Please edit the above to reflect a theoretically real output. Also, please use the rst support for code examples. I think there will be other examples, e.g. in the llvm-symbolizer documentation.)

llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbololize-operands.yaml

2–3 ↗

(On Diff #279954)

These two lines are quite long. I'd recommend you split them over multiple lines like so:

# RUN: llvm-objdump %t -d --symbolize-operands --x86-asm-syntax=att --no-show-raw-insn --no-leading-addr | \
# RUN:   FileCheck %s --match-full-lines --check-prefix=ATT

9 ↗

(On Diff #279954)

What's with the spaces before the ',' here.

15–22 ↗

(On Diff #279954)

Here, and in the ATT example above, I'd add leading spaces, so that everything is lined up like it would be in the actual output. In other words, I'd expect something a bit like:

# INTEL:      <_start>:
# INTEL-NEXT:   push rax
# INTEL-NEXT: <L1>:
# INTEL-NEXT:   cmp  eax, dword ptr <g>
# INTEL-NEXT:   jge <L0>
# INTEL-NEXT:   jmp <L1>
# INTEL-NEXT: <L0>:
# INTEL-NEXT:   ret

44 ↗

(On Diff #279954)

g isn't obviously an arbitrary symbol name to me. I'd suggest using something more obvious, e.g. simply symbol.

9 ↗

(On Diff #279934)

It's not clear to me what is less than ideal here? What would you prefer in an ideal situation?

[hoyFB]

In D84191#2168738, @jhenderson wrote:

It looks like you might still have a binary file in the patch, that is no longer needed?

The binary file should have been removed.

llvm/docs/CommandGuide/llvm-objdump.rst

206 ↗

(On Diff #279571)

Thanks for the clarification. Yes, we have a fundamental issue with disassembling object files because the symbol resolution unit (in llvm-objdump.cpp) doesn't handle relocations. For local jump targets this is probably fine since the jump distance is computed from the start of the current function. For global jump targets like a function call target or a global variable access, the symbolization will not work without considering the relocations. This is true even without symbolization where a call target will be disassembled as an offset from the current function.

llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbololize-operands.yaml

9 ↗

(On Diff #279954)

It is for the alignment of the first operand which isn't printed with the symbolization.

9 ↗

(On Diff #279934)

Sorry, I should have commented on this line : # ATT-NEXT: cmpl , %eax <g> . Ideally I'd like something like cmpl <g>, %eax.

[hoyFB]

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

[jhenderson]

llvm/docs/CommandGuide/llvm-objdump.rst

206 ↗

(On Diff #280489)

You probably want to mention that this option currently only supports X86 output.

211 ↗

(On Diff #280489)

As this is associated with the option, it needs indenting with a couple of spaces, just like the text describing the option. Same goes below. If you take a look at the output, you'll see that adding a couple of spaces before should cause the code to be shifted slightly to the right.

llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbololize-operands.yaml

9 ↗

(On Diff #279954)

So I think we want to fix this, but I a) am not familiar enough with the MCInstPrinter etc code to have a good recommendation, and b) think it could be deferred to a follow-up patch. Perhaps @MaskRay has a suggestion?

17 ↗

(On Diff #280489)

Nit: add one more space (the '<' doesn't line up with those below currently).

[hoyFB]

llvm/docs/CommandGuide/llvm-objdump.rst

211 ↗

(On Diff #280489)

Done adding indentation.

BTW, how should I see the the out of the .rst file?

[hoyFB]

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

[jhenderson]

Looks good from my point of view, but please wait for @MaskRay.

llvm/docs/CommandGuide/llvm-objdump.rst

211 ↗

(On Diff #280489)

You need to set LLVM_ENABLE_SPHINX to on in your CMake configuration and then build the docs-llvm-html target. See https://llvm.org/docs/GettingStarted.html for details. Depending on your system configuration, you might need to install Sphinx and (I think) recommonmark too.

[hoyFB]

@MaskRay How does this change look to you? Please let me know your comments. Thanks!

[MaskRay]

llvm/docs/CommandGuide/llvm-objdump.rst

213 ↗

(On Diff #283033)

Delete the trailing space

217 ↗

(On Diff #283033)

Delete the trailing space

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

1576

I'm reading this function now.

1931

I'll generally refrain from DenseMap on integer types because -1 and -2 cannot be used as DenseMap keys. I'd use unordered_map to avoid gotchas (even if in this context it may work)

[MaskRay]

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

327

This needs to be ObjdumpCat

[MaskRay]

I have seen a similar feature in GNU objdump. FWIW CCed you on https://sourceware.org/pipermail/binutils/2020-August/112803.html

[MaskRay]

I think the approach looks good. The rough corners (excess spaces) should be fixable. This should be good to go once some remaining stuff are fixed. I CCed you on binutils but don't expect they to block our progress. Just hope we can take inspirations from them if any:) (And ideally we can find some common ground, though I don't expect strongly on it)

llvm/include/llvm/MC/MCInstPrinter.h

52

= nullptr

MCInstPrinter is used by non-objdump tools. This will be an uninitialized read.

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

1602

Target >= Start && Target < End can be moved before !Labels.count(Target)

1603

("L" + Twine(LabelCount++)).str()

[hoyFB]

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

1931

Good to know -1 and -2 are not allowed to be keys with DenseMap. Changed to unordered_map.

[hoyFB]

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

[hoyFB]

In D84191#2210764, @MaskRay wrote:

I have seen a similar feature in GNU objdump. FWIW CCed you on https://sourceware.org/pipermail/binutils/2020-August/112803.html

Thanks! Good to know there's similar need to the GNU tool.

[MaskRay]

The last update looks good. Let's wait a bit for binutils' comment.. I also have a question about whether we should name the option --symbolize-operands, or is there a better name. It drops the existing offset/address operand as well as adding a symbol.

llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbololize-operands.yaml

1 ↗

(On Diff #284881)

You can omit --docnum=1 since it is the only document

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

2083

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

"Use braces for the if block to keep it uniform with the else block."

[hoyFB]

In D84191#2212199, @MaskRay wrote:

The last update looks good. Let's wait a bit for binutils' comment.. I also have a question about whether we should name the option --symbolize-operands, or is there a better name. It drops the existing offset/address operand as well as adding a symbol.

Printing a symbol instead of an offset/address is sort of symbolization to me. That said, a more meaningful name is welcomed.

[hoyFB]

Updating D84191: [llvm-objdump] Symbolize binary addresses for low-noisy asm diff.

[MaskRay]

In the absence of binutils responses, let's do this. please consider pushing it at some point next week.

[dblaikie]

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

1931

(FWIW for signed integers, DenseMap's int tombstones are int max and int min - for unsigned integral types it's int max, and int max - 1 - but yeah, if you need the complete range of a 64 bit integer, DenseMap won't suit because of the need for empty/tombstone values)

[thakis]

llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbololize-operands.yaml

2 ↗

(On Diff #286176)

Does --x86-asm-syntax=intel do something different from -M Intel? The latter is the normal objdump name for this, so I'm wondering if the flag doesn't do what you need, or if you didn't use it for a different reason.

[hoy]

llvm/test/tools/llvm-objdump/X86/elf-disassemble-symbololize-operands.yaml

2 ↗

(On Diff #286176)

I didn't realize -M Intel worked with llvm-objdump. It worked with objdump though.

llvm-objdump -d -M Intel a.out
a.out:	file format elf64-x86-64
llvm-objdump: error: 'a.out': Unrecognized disassembler option: Intel

Am I missing anything?

llvm/include/llvm/MC/MCInstPrinter.h

 #include <cstdint>
 
 namespace llvm {
 
 class MCAsmInfo;
 class MCInst;
 class MCOperand;
 class MCInstrInfo;
+class MCInstrAnalysis;
 class MCRegisterInfo;
 class MCSubtargetInfo;
 class raw_ostream;
 class StringRef;
 
 /// Convert `Bytes' to a hex string and output to `OS'
 void dumpBytes(ArrayRef<uint8_t> Bytes, raw_ostream &OS);
 

 protected:
   /// A stream that comments can be emitted to if desired.  Each comment
   /// must end with a newline.  This will be null if verbose assembly emission
   /// is disabled.
   raw_ostream *CommentStream = nullptr;
   const MCAsmInfo &MAI;
   const MCInstrInfo &MII;
   const MCRegisterInfo &MRI;
+  const MCInstrAnalysis *MIA;
 
   /// True if we are printing marked up assembly.
   bool UseMarkup = false;
 
   /// True if we are printing immediates as hex.
   bool PrintImmHex = false;
 
   /// Which style to use for printing hexadecimal values.
   HexStyle::Style PrintHexStyle = HexStyle::C;
 
   /// If true, a branch immediate (e.g. bl 4) will be printed as a hexadecimal
   /// address (e.g. bl 0x20004). This is useful for a stream disassembler
   /// (llvm-objdump -d).
   bool PrintBranchImmAsAddress = false;
 
+  /// If true, symbolize branch target operands and memory reference operands.
+  bool SymbolizeOperands = false;
+
   /// Utility function for printing annotations.
   void printAnnotation(raw_ostream &OS, StringRef Annot);
 
   /// Helper for matching MCInsts to alias patterns when printing instructions.
   const char *matchAliasPatterns(const MCInst *MI, const MCSubtargetInfo *STI,
                                  const AliasMatchingData &M);
 
 public:
public:
   void setPrintImmHex(bool Value) { PrintImmHex = Value; }
 
   void setPrintHexStyle(HexStyle::Style Value) { PrintHexStyle = Value; }
 
   void setPrintBranchImmAsAddress(bool Value) {
     PrintBranchImmAsAddress = Value;
   }
 
+  void setSymbolizeOperands(bool Value) { SymbolizeOperands = Value; }
+  void setMCInstrAnalysis(const MCInstrAnalysis *Value) { MIA = Value; }
+
   /// Utility function to print immediates in decimal or hex.
   format_object<int64_t> formatImm(int64_t Value) const {
     return PrintImmHex ? formatHex(Value) : formatDec(Value);
   }
 
   /// Utility functions to print decimal/hexadecimal values.
   format_object<int64_t> formatDec(int64_t Value) const;
   format_object<int64_t> formatHex(int64_t Value) const;

llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp

 
 /// value (e.g. for jumps and calls). In Intel-style these print slightly
 /// differently than normal immediates. For example, a $ is not emitted.
 ///
 /// \p Address The address of the next instruction.
 /// \see MCInstPrinter::printInst
 void X86InstPrinterCommon::printPCRelImm(const MCInst *MI, uint64_t Address,
                                          unsigned OpNo, raw_ostream &O) {
+  // Do not print the numberic target address when symbolizing.
+  if (SymbolizeOperands)
+    return;
+
   const MCOperand &Op = MI->getOperand(OpNo);
   if (Op.isImm()) {
     if (PrintBranchImmAsAddress) {
       uint64_t Target = Address + Op.getImm();
       if (MAI.getCodePointerSize() == 4)
         Target &= 0xffffffff;
       O << formatHex(Target);
     } else

llvm/lib/Target/X86/MCTargetDesc/X86IntelInstPrinter.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "X86IntelInstPrinter.h"
 #include "X86BaseInfo.h"
 #include "X86InstComments.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCInstrAnalysis.h"
 #include "llvm/MC/MCInstrDesc.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <cassert>
 #include <cstdint>
 
if (Op.isReg()) {
     assert(Op.isExpr() && "unknown operand kind in printOperand");
     O << "offset ";
     Op.getExpr()->print(O, &MAI);
   }
 }
 
 void X86IntelInstPrinter::printMemReference(const MCInst *MI, unsigned Op,
                                             raw_ostream &O) {
+  // Do not print the exact form of the memory operand if it references a known
+  // binary object.
+  if (SymbolizeOperands && MIA) {
+    uint64_t Target;
+    if (MIA->evaluateBranch(*MI, 0, 0, Target))
+      return;
+    if (MIA->evaluateMemoryOperandAddress(*MI, 0, 0))
+      return;
+  }
   const MCOperand &BaseReg  = MI->getOperand(Op+X86::AddrBaseReg);
   unsigned ScaleVal         = MI->getOperand(Op+X86::AddrScaleAmt).getImm();
   const MCOperand &IndexReg = MI->getOperand(Op+X86::AddrIndexReg);
   const MCOperand &DispSpec = MI->getOperand(Op+X86::AddrDisp);
 
   // If this has a segment register, print it.
   printOptionalSegReg(MI, Op + X86::AddrSegmentReg, O);
 

llvm/test/tools/llvm-objdump/X86/disassemble-symbolize-operands.s

+# RUN: llvm-mc -filetype=obj %s -o %t
+# RUN: ld.lld %t -o %t2
+# RUN: llvm-objdump -d --symbolize-operands --x86-asm-syntax=intel --no-show-raw-insn --no-leading-addr %t2 | FileCheck %s
+
+  .text
+foo:
+  retq
+# CHECK: <_start>:
+_start:
+  pushq %rax
+  movl  $0, 4(%rsp)
+  movl  $0, (%rsp)
+# CHECK: <L1>:
+.LBB1_1:
+  movl  (%rsp), %eax
+# CHECK: cmp  eax, dword ptr <g>
+  cmpl  g(%rip), %eax
+# CHECK: jge <L0>
+  jge .LBB1_3
+# CHECK: call <foo>
+  callq foo
+  incl  (%rsp)
+# CHECK: jmp <L1>
+  jmp .LBB1_1
+# CHECK: <L0>:
+.LBB1_3:
+  xorl  %eax, %eax
+  popq  %rcx
+  retq
+  .bss
+g:
+  .long 0
+  .size g, 4

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

static cl::opt<uint64_t> StopAddress("stop-address",
                                      cl::init(UINT64_MAX), cl::cat(ObjdumpCat));
 
 cl::opt<bool> objdump::SymbolTable("syms", cl::desc("Display the symbol table"),
                                    cl::cat(ObjdumpCat));
 static cl::alias SymbolTableShort("t", cl::desc("Alias for --syms"),
                                   cl::NotHidden, cl::Grouping,
                                   cl::aliasopt(SymbolTable));
 
+static cl::opt<bool> SymbolizeOperands(
+    "symbolize-operands",
+    cl::desc("Symbolize instruction operands when disassembling"),
+    cl::cat(MachOCat));
+
 static cl::opt<bool> DynamicSymbolTable(
     "dynamic-syms",
     cl::desc("Display the contents of the dynamic symbol table"),
     cl::cat(ObjdumpCat));
 static cl::alias DynamicSymbolTableShort("T",
                                          cl::desc("Alias for --dynamic-syms"),
                                          cl::NotHidden, cl::Grouping,
                                          cl::aliasopt(DynamicSymbolTable));
static SymbolInfoTy createDummySymbolInfo(const ObjectFile *Obj,
                                           const uint64_t Addr, StringRef &Name,
                                           uint8_t Type) {
   if (Obj->isXCOFF() && SymbolDescription)
     return SymbolInfoTy(Addr, Name, None, None, false);
   else
     return SymbolInfoTy(Addr, Name, Type);
 }
 
+// facebook begin T69941224
+static void collectLocalBranchTargets(
+    ArrayRef<uint8_t> Bytes, const MCInstrAnalysis *MIA, MCDisassembler *DisAsm,
+    MCInstPrinter *IP, const MCSubtargetInfo *STI, uint64_t SectionAddr,
+    uint64_t Start, uint64_t End, DenseMap<uint64_t, std::string> &Labels) {
+  // So far only supports X86. Other target like Arm64 may have data embedded in
+  // the text section.
+  if (!STI->getTargetTriple().isX86())
+    return;
+
+  // Only support X86 Intel asm syntax
+  if (!DisAsm->getContext().getAsmInfo()->getAssemblerDialect())
+    return;
+
+  Labels.clear();
+  unsigned LabelCount = 0;
+  Start += SectionAddr;
+  End += SectionAddr;
+  uint64_t Index = Start;
+  while (Index < End) {
+    // Disassemble a real instruction and record function-local branch labels.
+    MCInst Inst;
+    uint64_t Size;
+    bool Disassembled = DisAsm->getInstruction(
+        Inst, Size, Bytes.slice(Index - SectionAddr), Index, nulls());
+    if (Size == 0)
+      Size = 1;
+
+    if (Disassembled && MIA) {
+      uint64_t Target;
+      bool TargetKnown = MIA->evaluateBranch(Inst, Index, Size, Target);
+      if (TargetKnown && !Labels.count(Target)) {
+        if (Target >= Start && Target < End)
+          Labels[Target] = "L" + Twine(LabelCount++).str();
+      }
+    }
+
+    Index += Size;
+  }
+}
+// facebook end T69941224
+
 static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,
                               MCContext &Ctx, MCDisassembler *PrimaryDisAsm,
                               MCDisassembler *SecondaryDisAsm,
                               const MCInstrAnalysis *MIA, MCInstPrinter *IP,
                               const MCSubtargetInfo *PrimarySTI,
                               const MCSubtargetInfo *SecondarySTI,
                               PrettyPrinter &PIP,
                               SourcePrinter &SP, bool InlineRelocs) {
for (unsigned SI = 0, SE = Symbols.size(); SI != SE; ++SI) {
         }
       }
 
       bool CheckARMELFData = hasMappingSymbols(Obj) &&
                              Symbols[SI].Type != ELF::STT_OBJECT &&
                              !DisassembleAll;
       bool DumpARMELFData = false;
       formatted_raw_ostream FOS(outs());
+
+      // facebook begin T69941224
+      DenseMap<uint64_t, std::string> AllLabels;
+      if (SymbolizeOperands)
+        collectLocalBranchTargets(Bytes, MIA, DisAsm, IP, PrimarySTI,
+                                  SectionAddr, Index, End, AllLabels);
+      // facebook end T69941224
+
       while (Index < End) {
         // ARM and AArch64 ELF binaries can interleave data and text in the
         // same section. We rely on the markers introduced to understand what
         // we need to dump. If the data marker is within a function, it is
         // denoted as a word/short etc.
         if (CheckARMELFData) {
           char Kind = getMappingSymbolKind(MappingSymbols, Index);
           DumpARMELFData = Kind == 'd';
for (unsigned SI = 0, SE = Symbols.size(); SI != SE; ++SI) {
             if (size_t N =
                     countSkippableZeroBytes(Bytes.slice(Index, MaxOffset))) {
               FOS << "\t\t..." << '\n';
               Index += N;
               continue;
             }
           }
 
+          // facebook begin T69941224
+          // Print local label if there's any.
+          auto Iter = AllLabels.find(SectionAddr + Index);
+          if (Iter != AllLabels.end())
+            FOS << "<" << Iter->second << ">:\n";
+          // facebook end T69941224
+
           // Disassemble a real instruction or a data when disassemble all is
           // provided
           MCInst Inst;
           bool Disassembled =
               DisAsm->getInstruction(Inst, Size, Bytes.slice(Index),
                                      SectionAddr + Index, CommentStream);
           if (Size == 0)
             Size = 1;
for (unsigned SI = 0, SE = Symbols.size(); SI != SE; ++SI) {
             bool PrintTarget =
                 MIA->evaluateBranch(Inst, SectionAddr + Index, Size, Target);
             if (!PrintTarget)
               if (Optional<uint64_t> MaybeTarget =
                       MIA->evaluateMemoryOperandAddress(
                           Inst, SectionAddr + Index, Size)) {
                 Target = *MaybeTarget;
                 PrintTarget = true;
+                // facebook begin T69941224
+                // Do not print real address when symbolizing.
+                if (!SymbolizeOperands)
                   FOS << "  # " << Twine::utohexstr(Target);
+                // facebook end T69941224
               }
             if (PrintTarget) {
               // In a relocatable object, the target's section must reside in
               // the same section as the call instruction or it is accessed
               // through a relocation.
               //
               // In a non-relocatable object, the target may be in any section.
               // In that case, locate the section(s) containing the target
for (unsigned SI = 0, SE = Symbols.size(); SI != SE; ++SI) {
                     *TargetSymbols,
                     [=](const SymbolInfoTy &O) { return O.Addr <= Target; });
                 if (It != TargetSymbols->begin()) {
                   TargetSym = &*(It - 1);
                   break;
                 }
               }
 
+              // facebook begin T69941224
+              // Print the labels corresponding to the target if there's any.
+              bool LabelAvailable = AllLabels.count(Target);
               if (TargetSym != nullptr) {
                 uint64_t TargetAddress = TargetSym->Addr;
+                uint64_t Disp = Target - TargetAddress;
                 std::string TargetName = TargetSym->Name.str();
                 if (Demangle)
                   TargetName = demangle(TargetName);
 
-                FOS << " <" << TargetName;
-                uint64_t Disp = Target - TargetAddress;
-                if (Disp)
-                  FOS << "+0x" << Twine::utohexstr(Disp);
-                FOS << '>';
+                FOS << " <";
+                if (!Disp)
+                  // Always Print the binary symbol precisely corresponding to
+                  // the target address.
+                  FOS << TargetName;
+                else if (!LabelAvailable) {
+                  // Always Print the binary symbol plus an offset if there's no
+                  // local label corresponding to the target address.
+                  FOS << TargetName << "+0x" << Twine::utohexstr(Disp);
+                } else
+                  FOS << AllLabels[Target];
+                FOS << ">";
+              } else if (LabelAvailable) {
+                FOS << " <" << AllLabels[Target] << ">";
               }
+              // facebook end T69941224
             }
           }
         }
 
         LVP.printAfterInst(FOS);
         FOS << "\n";
 
         // Hexagon does this in pretty printer
static void disassembleObject(const ObjectFile *Obj, bool InlineRelocs) {
   int AsmPrinterVariant = AsmInfo->getAssemblerDialect();
   std::unique_ptr<MCInstPrinter> IP(TheTarget->createMCInstPrinter(
       Triple(TripleName), AsmPrinterVariant, *AsmInfo, *MII, *MRI));
   if (!IP)
     reportError(Obj->getFileName(),
                 "no instruction printer for target " + TripleName);
   IP->setPrintImmHex(PrintImmHex);
   IP->setPrintBranchImmAsAddress(true);
+  // facebook begin T69941224
+  IP->setSymbolizeOperands(SymbolizeOperands);
+  IP->setMCInstrAnalysis(MIA.get());
+  // facebook end T69941224
 
   PrettyPrinter &PIP = selectPrettyPrinter(Triple(TripleName));
   SourcePrinter SP(Obj, TheTarget->getName());
 
   for (StringRef Opt : DisassemblerOptions)
     if (!IP->applyTargetSpecificCLOption(Opt))
       reportError(Obj->getFileName(),
                   "Unrecognized disassembler option: " + Opt);