Skip to content

Commit

Permalink
[BOLT] Correctly print preferred disassembly for annotated instructio…
Browse files Browse the repository at this point in the history
…ns (llvm#120564)

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  llvm#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).
  • Loading branch information
kbeyls authored Dec 20, 2024
1 parent 4096dd6 commit 4111841
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 9 deletions.
10 changes: 9 additions & 1 deletion bolt/lib/Core/BinaryContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
8 changes: 4 additions & 4 deletions bolt/test/RISCV/call-annotations.s
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions bolt/test/RISCV/relax.s
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion bolt/test/RISCV/reloc-branch.s
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion bolt/test/RISCV/reloc-jal.s
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ f:
.globl _start
.p2align 1
_start:
// CHECK: jal ra, f
// CHECK: jal f
jal ra, f
ret
.size _start, .-_start

0 comments on commit 4111841

Please sign in to comment.