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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay requested changes to this revision.
MaskRay added a comment.
This revision now requires changes to proceed.

I think the approach looks good. The rough corners (excess spaces) should be fixable.



================
Comment at: llvm/include/llvm/MC/MCInstPrinter.h:52
   const MCRegisterInfo &MRI;
+  const MCInstrAnalysis *MIA;
 
----------------
`= nullptr`

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


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1602
+      if (TargetKnown && !Labels.count(Target)) {
+        if (Target >= Start && Target < End)
+          Labels[Target] = "L" + Twine(LabelCount++).str();
----------------
`Target >= Start && Target < End` can be moved before `!Labels.count(Target)`


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1603
+        if (Target >= Start && Target < End)
+          Labels[Target] = "L" + Twine(LabelCount++).str();
+      }
----------------
`("L" + Twine(LabelCount++)).str()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84191