[PATCH] D143725: [llvm-objdump][ARM] support --symbolize-operands for ARM/ELF

[Nam Cao via Phabricator via cfe-commits]

covanam added a comment.

In D143725#4127923 , @Esme wrote:

> Please include as much context as possible with your diff. 
> https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface

Thank you. Will read that and will add more context.




Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1145
+  // So far only supports ARM/Thumb, PowerPC and X86.
+  Triple triple = STI->getTargetTriple();
+  if (!triple.isPPC() && !triple.isX86() && !triple.isARM() &&

Esme wrote:
> Please capitalize the first letter of variable names.
Sorry I am newcomer. clang-format didn't say anything, so I thought this is 
okay.
Will be fixed. Same for the other one.



Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1169-1171
+  // On PowerPC and ARM/Thumb, if the address of a branch is the same as
+  // the target, it means that it's a function call. Do not mark the label
+  // for this case.

hoy wrote:
> Is this always true for linked binary?
Actually I don't know, from my practical experience, it seems to be true. 
However, I checked ARM's documentation on ELF files and they do not say 
anything about this.

I will think about this and will come back later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143725

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D143725: [llvm-objdump][ARM] support --symbolize-operands for ARM/ELF

[Nam Cao via Phabricator via cfe-commits]

covanam created this revision.
covanam added reviewers: shchenz, rlavaee, jhenderson, hoyFB, MaskRay, 
llvm-commits, cfe-commits.
Herald added subscribers: kristof.beyls, emaste.
Herald added a project: All.
covanam requested review of this revision.
Herald added a project: LLVM.

Support --symbolize-operands on ARM/Thumb architecture.
Only support ELF object files for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143725

Files:
  llvm/docs/CommandGuide/llvm-objdump.rst
  llvm/test/tools/llvm-objdump/ELF/ARM/symbolize-operand-arm.s
  llvm/test/tools/llvm-objdump/ELF/ARM/symbolize-operand-thumb.s
  llvm/tools/llvm-objdump/llvm-objdump.cpp

Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -1141,8 +1141,10 @@
 ArrayRef Bytes, const MCInstrAnalysis *MIA, MCDisassembler *DisAsm,
 MCInstPrinter *IP, const MCSubtargetInfo *STI, uint64_t SectionAddr,
 uint64_t Start, uint64_t End, std::unordered_map &Labels) {
-  // So far only supports PowerPC and X86.
-  if (!STI->getTargetTriple().isPPC() && !STI->getTargetTriple().isX86())
+  // So far only supports ARM/Thumb, PowerPC and X86.
+  Triple triple = STI->getTargetTriple();
+  if (!triple.isPPC() && !triple.isX86() && !triple.isARM() &&
+  !triple.isThumb())
 return;
 
   Labels.clear();
@@ -1164,11 +1166,14 @@
 if (Disassembled && MIA) {
   uint64_t Target;
   bool TargetKnown = MIA->evaluateBranch(Inst, Index, Size, Target);
-  // On PowerPC, if the address of a branch is the same as the target, it
-  // means that it's a function call. Do not mark the label for this case.
+  // On PowerPC and ARM/Thumb, if the address of a branch is the same as
+  // the target, it means that it's a function call. Do not mark the label
+  // for this case.
+  bool isFunctionCall =
+  Target == Index &&
+  (triple.isPPC() || triple.isARM() || triple.isThumb());
   if (TargetKnown && (Target >= Start && Target < End) &&
-  !Labels.count(Target) &&
-  !(STI->getTargetTriple().isPPC() && Target == Index))
+  !Labels.count(Target) && !isFunctionCall)
 Labels[Target] = ("L" + Twine(LabelCount++)).str();
 }
 Index += Size;
Index: llvm/test/tools/llvm-objdump/ELF/ARM/symbolize-operand-thumb.s
===
--- /dev/null
+++ llvm/test/tools/llvm-objdump/ELF/ARM/symbolize-operand-thumb.s
@@ -0,0 +1,37 @@
+@ RUN: llvm-mc %s -triple=thumbv7m -filetype=obj | \
+@ RUN:   llvm-objdump --symbolize-operands --no-show-raw-insn -d --triple=thumbv7m - | \
+@ RUN:   FileCheck %s
+
+.text
+foo:
+@ CHECK:   :
+
+@@ Check symbolize local branch
+  b .L0
+  beq.w .L1
+  b .L2
+.L0:
+  b .L2
+  bne .L1
+  b.w .L0
+.L2:
+  b.w .L1
+  b .L2
+.L1:
+  b.w .L0
+@ CHECK-NEXT:  0:   b	0x8  @ imm = #0x4
+@ CHECK-NEXT:  2:   beq.w 0x16 @ imm = #0x10
+@ CHECK-NEXT:  6:   b 0x10 @ imm = #0x6
+@ CHECK-NEXT: :
+@ CHECK-NEXT:  8:   b 0x10 @ imm = #0x4
+@ CHECK-NEXT:  a:   bne   0x16 @ imm = #0x8
+@ CHECK-NEXT:  c:   b.w0x8 @ imm = #-0x8
+@ CHECK-NEXT: :
+@ CHECK-NEXT: 10:   b.w   0x16 @ imm = #0x2
+@ CHECK-NEXT: 14:   b 0x10 @ imm = #-0x8
+@ CHECK-NEXT: :
+@ CHECK-NEXT: 16:   b.w0x8 @ imm = #-0x12
+
+@@ Check function calls are not symbolized
+  bl bar
+@ CHECK-NEXT: 1a:   bl 0x1a  @ imm = #-0x4
Index: llvm/test/tools/llvm-objdump/ELF/ARM/symbolize-operand-arm.s
===
--- /dev/null
+++ llvm/test/tools/llvm-objdump/ELF/ARM/symbolize-operand-arm.s
@@ -0,0 +1,38 @@
+@ RUN: llvm-mc %s -triple=armv8 -filetype=obj | \
+@ RUN:   llvm-objdump --symbolize-operands --no-show-raw-insn -d --triple=armv8 - | \
+@ RUN:   FileCheck %s
+
+.text
+foo:
+@ CHECK:   :
+
+@@ Check symbolize local branch
+  b .L0
+.L4:
+  beq .L1
+.L0:
+  b .L2
+  ble .L3
+.L2:
+  b .L3
+.L1:
+  b .L0
+.L3:
+  b .L4
+@ CHECK-NEXT:  0:   b   0x8 @ imm = #0x0
+@ CHECK-NEXT: :
+@ CHECK-NEXT:  4:   beq 0x14@ imm = #0x8
+@ CHECK-NEXT: :
+@ CHECK-NEXT:  8:   b	  0x10@ imm = #0x0
+@ CHECK-NEXT:  c:   ble	0x18@ imm = #0x4
+@ CHECK-NEXT: :
+@ CHECK-NEXT: 10:   b   0x18@ imm = #0x0
+@ CHECK-NEXT: :
+@ CHECK-NEXT: 14:   b   0x8 @ imm = #-0x14
+@ CHECK-NEXT: :
+@ CHECK-NEXT: 18:   b   0x4 @ imm = #-0x1c
+
+
+@@ Check function calls are not symbolized
+  bl bar
+@ CHECK-NEXT: 1c:   bl	0x1c  @ imm = #-0x8
Index: llvm/docs/CommandGuide/llvm-objdump.rst
===
--- llvm/docs/CommandGuide/llvm-objdump.rst
+++ llvm/docs/CommandGuide/llvm-objdump.rst
@@ -260,7 +260,7 @@
 
   When a bb-address-map section is present (i.e., the object file is built with ``-fbasic-block-s