[PATCH] opcodes/avr: Fill in insn info for branching instructions

[PATCH] opcodes/avr: Fill in insn info for branching instructions

[Marcus Nilsson]

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

The --visualize-jumps option for objdump is a great feature and I've used
it a couple of times to get an idea of the control flow, so I was a bit
surprised when it didn't work when using avr-objdump.

The attached patch adds support for jump visualization by filling in the
required fields for disassemble_info in avr_operand(), going by the
constraint letters defined in include/opcode/avr-dis.h

First time submitting a patch here so please let me know if I've got the
procedure wrong, and any feedback.

Cheers,
Marcus Nilsson

[-- Attachment #2: 0001-Avr-opcodes-Fill-in-insn-info-for-branching-instruct.patch --]
[-- Type: text/x-patch, Size: 3255 bytes --]

From 242058ee934d003f2a382d56a1d7eddb0b32bb90 Mon Sep 17 00:00:00 2001
From: Marcus Nilsson <brainbomb@gmail.com>
Date: Sat, 6 Nov 2021 23:44:12 +0100
Subject: [PATCH] Avr/opcodes: Fill in insn info for branching instructions

Change signature of avr_operand() to include disassemble_info and fill
in insn_type and target on branching instructions. This enables use of
--visualize-jumps option on Avr target.

* opcodes/avr-dis.c (avr_operand); Pass in disassemble_info and fill
in insn_type on branching instructions.
---
 opcodes/avr-dis.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/opcodes/avr-dis.c b/opcodes/avr-dis.c
index 06f8a0c663b..ccd223ea56a 100644
--- a/opcodes/avr-dis.c
+++ b/opcodes/avr-dis.c
@@ -50,7 +50,7 @@ static const char * comment_start = "0x";
 
 static int
 avr_operand (unsigned int insn, unsigned int insn2, unsigned int pc, int constraint,
-             char *opcode_str, char *buf, char *comment, int regs, int *sym, bfd_vma *sym_addr)
+             char *opcode_str, char *buf, char *comment, int regs, int *sym, bfd_vma *sym_addr, disassemble_info *info)
 {
   int ok = 1;
   *sym = 0;
@@ -161,6 +161,9 @@ avr_operand (unsigned int insn, unsigned int insn2, unsigned int pc, int constra
 	 objdump_print_address() which would affect many targets.  */
       sprintf (buf, "%#lx", (unsigned long) *sym_addr);
       strcpy (comment, comment_start);
+        info->insn_info_valid = 1;
+        info->insn_type = dis_jsr;
+        info->target = *sym_addr;
       break;
 
     case 'L':
@@ -170,6 +173,9 @@ avr_operand (unsigned int insn, unsigned int insn2, unsigned int pc, int constra
         *sym = 1;
         *sym_addr = pc + 2 + rel_addr;
 	strcpy (comment, comment_start);
+        info->insn_info_valid = 1;
+        info->insn_type = dis_branch;
+        info->target = *sym_addr;
       }
       break;
 
@@ -181,6 +187,9 @@ avr_operand (unsigned int insn, unsigned int insn2, unsigned int pc, int constra
         *sym = 1;
         *sym_addr = pc + 2 + rel_addr;
 	strcpy (comment, comment_start);
+        info->insn_info_valid = 1;
+        info->insn_type = dis_condbranch;
+        info->target = *sym_addr;
       }
       break;
 
@@ -314,6 +323,13 @@ print_insn_avr (bfd_vma addr, disassemble_info *info)
   int sym_op1 = 0, sym_op2 = 0;
   bfd_vma sym_addr1, sym_addr2;
 
+  /* Clear instruction information field.  */
+  info->insn_info_valid = 0;
+  info->branch_delay_insns = 0;
+  info->data_size = 0;
+  info->insn_type = dis_noninsn;
+  info->target = 0;
+  info->target2 = 0;
 
   if (!initialized)
     {
@@ -395,11 +411,11 @@ print_insn_avr (bfd_vma addr, disassemble_info *info)
 	{
 	  int regs = REGISTER_P (*constraints);
 
-	  ok = avr_operand (insn, insn2, addr, *constraints, opcode_str, op1, comment1, 0, &sym_op1, &sym_addr1);
+	  ok = avr_operand (insn, insn2, addr, *constraints, opcode_str, op1, comment1, 0, &sym_op1, &sym_addr1, info);
 
 	  if (ok && *(++constraints) == ',')
 	    ok = avr_operand (insn, insn2, addr, *(++constraints), opcode_str, op2,
-			      *comment1 ? comment2 : comment1, regs, &sym_op2, &sym_addr2);
+			      *comment1 ? comment2 : comment1, regs, &sym_op2, &sym_addr2, info);
 	}
     }
 
-- 
2.33.0

Re: [PATCH] opcodes/avr: Fill in insn info for branching instructions

[Marcus Nilsson]

A light ping checking if there is any interest in reviewing this patch?

Thank you,
Marcus

On Thu, Nov 11, 2021 at 5:31 PM Marcus Nilsson <brainbomb@gmail.com> wrote:

> The --visualize-jumps option for objdump is a great feature and I've used
> it a couple of times to get an idea of the control flow, so I was a bit
> surprised when it didn't work when using avr-objdump.
>
> The attached patch adds support for jump visualization by filling in the
> required fields for disassemble_info in avr_operand(), going by the
> constraint letters defined in include/opcode/avr-dis.h
>
> First time submitting a patch here so please let me know if I've got the
> procedure wrong, and any feedback.
>
> Cheers,
> Marcus Nilsson
>
>

-- 
Marcus Nilsson
-
tel: +46(0)704911844
mail: marcus@landmarknilsson.se
github: github.com/metmo
linkedin: linkedin.com/in/landmarknilsson

Re: [PATCH] opcodes/avr: Fill in insn info for branching instructions

[Nick Clifton]

Hi Marcus,

> A light ping checking if there is any interest in reviewing this patch?

Oops - sorry.  Patch approved and applied.

Cheers
   Nick