[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