[llvm] [BOLT] Correctly print preferred disassembly for annotated instructions (PR #120564)
Kristof Beyls via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 19 10:05:00 PST 2024
https://github.com/kbeyls updated https://github.com/llvm/llvm-project/pull/120564
>From 60dae85ef3d04f93868e40f4e2921d2167bcd434 Mon Sep 17 00:00:00 2001
From: Kristof Beyls <kristof.beyls at arm.com>
Date: Sat, 23 Sep 2023 20:12:53 +0200
Subject: [PATCH 1/2] Correctly print preferred disassembly for instructions
with an annotation.
This patch makes sure that BinaryContext::printInstruction prints the
preferred disassembly. Preferred disassembly only gets printed when
there are no annotations on the MCInst. Therefore, this patch
temporarily removes the annotations before printing it.
A few examples of before and after on AArch64 instructions are as
follows:
```
BEFORE AFTER
(preferred disassembly)
ret x30 ret
orr x30, xzr, x0 mov x30, x0
hint #29 autiasp
hint #12 autia1716
```
Clearly, the preferred disassembly is easier for developers to read,
and is the disassembly that tools should be printing.
This patch is motivated as part of future work on the
llvm-bolt-binary-analysis tool, making sure that the reports it prints
do use preferred disassembly.
This patch was cherry-picked from https://github.com/kbeyls/llvm-project/tree/bolt-gadget-scanner-prototype.
In this current patch, this only affects existing RISCV test cases.
This patch also does improve test cases in future patches that will
introduce a binary analysis for llvm-bolt-binary-analysis that checks
for correct application of pac-ret (pointer authentication on return
addresses).
---
bolt/lib/Core/BinaryContext.cpp | 10 +++++++++-
bolt/test/RISCV/call-annotations.s | 8 ++++----
bolt/test/RISCV/relax.s | 4 ++--
bolt/test/RISCV/reloc-branch.s | 2 +-
bolt/test/RISCV/reloc-jal.s | 2 +-
5 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index ac96b836ed5796..0fcbf7452cf880 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -1961,7 +1961,15 @@ void BinaryContext::printInstruction(raw_ostream &OS, const MCInst &Instruction,
OS << "\tjit\t" << MIB->getTargetSymbol(Instruction)->getName()
<< " # ID: " << DynamicID;
} else {
- InstPrinter->printInst(&Instruction, 0, "", *STI, OS);
+ // If there are annotations on the instruction, the MCInstPrinter will fail to
+ // print the preferred alias as it only does so when the number of operands is
+ // as expected. See
+ // https://github.com/llvm/llvm-project/blob/782f1a0d895646c364a53f9dcdd6d4ec1f3e5ea0/llvm/lib/MC/MCInstPrinter.cpp#L142
+ // Therefore, create a temporary copy of the Inst from which the annotations are
+ // removed, and print that Inst.
+ MCInst InstNoAnnot = Instruction;
+ MIB->stripAnnotations(InstNoAnnot);
+ InstPrinter->printInst(&InstNoAnnot, 0, "", *STI, OS);
}
if (MIB->isCall(Instruction)) {
if (MIB->isTailCall(Instruction))
diff --git a/bolt/test/RISCV/call-annotations.s b/bolt/test/RISCV/call-annotations.s
index ff99e0f1dfd848..f876544e214cab 100644
--- a/bolt/test/RISCV/call-annotations.s
+++ b/bolt/test/RISCV/call-annotations.s
@@ -16,13 +16,13 @@ f:
// CHECK-LABEL: Binary Function "_start" after building cfg {
// CHECK: auipc ra, f
-// CHECK-NEXT: jalr ra, -0x4(ra) # Offset: 4
-// CHECK-NEXT: jal ra, f # Offset: 8
-// CHECK-NEXT: jal zero, f # TAILCALL # Offset: 12
+// CHECK-NEXT: jalr -0x4(ra) # Offset: 4
+// CHECK-NEXT: jal f # Offset: 8
+// CHECK-NEXT: j f # TAILCALL # Offset: 12
// CHECK-LABEL: Binary Function "long_tail" after building cfg {
// CHECK: auipc t1, f
-// CHECK-NEXT: jalr zero, -0x18(t1) # TAILCALL # Offset: 8
+// CHECK-NEXT: jr -0x18(t1) # TAILCALL # Offset: 8
// CHECK-LABEL: Binary Function "compressed_tail" after building cfg {
// CHECK: jr a0 # TAILCALL # Offset: 0
diff --git a/bolt/test/RISCV/relax.s b/bolt/test/RISCV/relax.s
index ec390ea76b5c72..74f049b8f8dd94 100644
--- a/bolt/test/RISCV/relax.s
+++ b/bolt/test/RISCV/relax.s
@@ -5,9 +5,9 @@
// RUN: llvm-objdump -d %t.bolt | FileCheck --check-prefix=OBJDUMP %s
// CHECK: Binary Function "_start" after building cfg {
-// CHECK: jal ra, near_f
+// CHECK: jal near_f
// CHECK-NEXT: auipc ra, far_f
-// CHECK-NEXT: jalr ra, 0xc(ra)
+// CHECK-NEXT: jalr 0xc(ra)
// CHECK-NEXT: j near_f
// CHECK: Binary Function "_start" after fix-riscv-calls {
diff --git a/bolt/test/RISCV/reloc-branch.s b/bolt/test/RISCV/reloc-branch.s
index 6a8b5a28e19d92..780d978f36f447 100644
--- a/bolt/test/RISCV/reloc-branch.s
+++ b/bolt/test/RISCV/reloc-branch.s
@@ -7,7 +7,7 @@
.p2align 1
// CHECK: Binary Function "_start" after building cfg {
_start:
-// CHECK: beq zero, zero, .Ltmp0
+// CHECK: beqz zero, .Ltmp0
beq zero, zero, 1f
nop
// CHECK: .Ltmp0
diff --git a/bolt/test/RISCV/reloc-jal.s b/bolt/test/RISCV/reloc-jal.s
index ce54265fac05e9..62a2f1dbea03a7 100644
--- a/bolt/test/RISCV/reloc-jal.s
+++ b/bolt/test/RISCV/reloc-jal.s
@@ -14,7 +14,7 @@ f:
.globl _start
.p2align 1
_start:
-// CHECK: jal ra, f
+// CHECK: jal f
jal ra, f
ret
.size _start, .-_start
>From c1b2afa1b1bc128240f5405c803e1f6ccb83d308 Mon Sep 17 00:00:00 2001
From: Kristof Beyls <kristof.beyls at arm.com>
Date: Thu, 19 Dec 2024 18:03:35 +0000
Subject: [PATCH 2/2] clang-format
---
bolt/lib/Core/BinaryContext.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/bolt/lib/Core/BinaryContext.cpp b/bolt/lib/Core/BinaryContext.cpp
index 0fcbf7452cf880..f88e34b8e89623 100644
--- a/bolt/lib/Core/BinaryContext.cpp
+++ b/bolt/lib/Core/BinaryContext.cpp
@@ -1961,12 +1961,12 @@ void BinaryContext::printInstruction(raw_ostream &OS, const MCInst &Instruction,
OS << "\tjit\t" << MIB->getTargetSymbol(Instruction)->getName()
<< " # ID: " << DynamicID;
} else {
- // If there are annotations on the instruction, the MCInstPrinter will fail to
- // print the preferred alias as it only does so when the number of operands is
- // as expected. See
+ // If there are annotations on the instruction, the MCInstPrinter will fail
+ // to print the preferred alias as it only does so when the number of
+ // operands is as expected. See
// https://github.com/llvm/llvm-project/blob/782f1a0d895646c364a53f9dcdd6d4ec1f3e5ea0/llvm/lib/MC/MCInstPrinter.cpp#L142
- // Therefore, create a temporary copy of the Inst from which the annotations are
- // removed, and print that Inst.
+ // Therefore, create a temporary copy of the Inst from which the annotations
+ // are removed, and print that Inst.
MCInst InstNoAnnot = Instruction;
MIB->stripAnnotations(InstNoAnnot);
InstPrinter->printInst(&InstNoAnnot, 0, "", *STI, OS);
More information about the llvm-commits
mailing list