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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-objdump.rst:206
+
+  Only works with --x86-asm-syntax=Intel and an executable binary.
+
----------------
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.


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-symbolize-operands.test:1
+// RUN: llvm-objdump -d --symbolize-operands --x86-asm-syntax=intel --no-show-raw-insn --no-leading-addr  %p/Inputs/disassemble-symbolize-operands.exe.elf-x86_64 | FileCheck %s
+
----------------
Please avoid a checked-in executable. Its content is difficult to inspect.
See `X86/elf-disassemble-symbol-references.yaml` for a yaml2obj example.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1576
 
+static void collectLocalBranchTargets(
+    ArrayRef<uint8_t> Bytes, const MCInstrAnalysis *MIA, MCDisassembler *DisAsm,
----------------
I wonder why you need this. For BOLT or similar post-link rewriting stuff?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84191/new/

https://reviews.llvm.org/D84191