[binutils-gdb] x86/disasm: rework comment handling

Jan Beulich via Binutils-cvs <[email protected]>
Newsgroups gmane.comp.gnu.binutils.cvs
Message-ID <[email protected]> 
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=aeced13ee0cd570d78cb37ab1f9bba840958515a

commit aeced13ee0cd570d78cb37ab1f9bba840958515a
Author: Jan Beulich <[email protected]>
Date:   Fri May 22 08:49:12 2026 +0200

    x86/disasm: rework comment handling
    
    Model this after operand handling, such that comments can be emitted in
    the same order as operands. %rip-relative address comments remain
    separate for now. While there correct style for the symbols associated
    with immediates: These aren't "comment starts", but symbol names.

Diff:
---
 .../testsuite/binutils-all/x86-64/comments-intel.d | 14 ++++++
 binutils/testsuite/binutils-all/x86-64/comments.d  | 14 ++++++
 binutils/testsuite/binutils-all/x86-64/comments.s  |  9 ++++
 opcodes/i386-dis.c                                 | 52 +++++++++++++++-------
 4 files changed, 74 insertions(+), 15 deletions(-)

diff --git a/binutils/testsuite/binutils-all/x86-64/comments-intel.d b/binutils/testsuite/binutils-all/x86-64/comments-intel.d
new file mode 100644
index 00000000000..3ebdafd6703
--- /dev/null
+++ b/binutils/testsuite/binutils-all/x86-64/comments-intel.d
@@ -0,0 +1,14 @@
+#name: disassembly comments (Intel)
+#source: comments.s
+#ld:
+#objdump: -dwMannotate,intel
+
+.*: +file format .*
+
+
+Disassembly of section .text:
+
+[0-9a-f]+ <_start>:
+[ 	]*[0-9a-f]+:	48 c7 05 [0-9a-f ]+	mov    QWORD PTR \[rip\+0x[0-9a-f]+\],0x[0-9a-f]+ +# [0-9a-f]+ <fptr>, \[_start\]
+[ 	]*[0-9a-f]+:	31 c0                	xor    eax,eax
+[ 	]*[0-9a-f]+:	c3                   	ret
diff --git a/binutils/testsuite/binutils-all/x86-64/comments.d b/binutils/testsuite/binutils-all/x86-64/comments.d
new file mode 100644
index 00000000000..15d35853411
--- /dev/null
+++ b/binutils/testsuite/binutils-all/x86-64/comments.d
@@ -0,0 +1,14 @@
+#name: disassembly comments (AT&T)
+#ld:
+#objdump: -dwMannotate,att
+
+.*: +file format .*
+
+
+Disassembly of section .text:
+
+[0-9a-f]+ <_start>:
+[ 	]*[0-9a-f]+:	48 c7 05 [0-9a-f ]+	movq   \$0x[0-9a-f]+,0x[0-9a-f]+\(%rip\) +# \[_start\], [0-9a-f]+ <fptr>
+[ 	]*[0-9a-f]+:	31 c0                	xor    %eax,%eax
+[ 	]*[0-9a-f]+:	c3                   	ret
+#pass
diff --git a/binutils/testsuite/binutils-all/x86-64/comments.s b/binutils/testsuite/binutils-all/x86-64/comments.s
new file mode 100644
index 00000000000..443dea7e3fa
--- /dev/null
+++ b/binutils/testsuite/binutils-all/x86-64/comments.s
@@ -0,0 +1,9 @@
+	.text
+	.global _start
+_start:
+	movq	$_start, fptr(%rip)
+	xor	%eax, %eax
+	ret
+
+	.data
+fptr:	.quad	-1
diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index ad0593cedd7..1faf1fa7785 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -179,8 +179,7 @@ struct instr_info
 
   char obuf[MAX_OPERAND_BUFFER_SIZE];
   char *obufp;
-  char cbuf[COMMENT_BUFFER_SIZE];
-  char * cbufp;
+  char *cbufp;
   char *mnemonicendp;
   const uint8_t *start_codep;
   uint8_t *codep;
@@ -258,6 +257,7 @@ struct instr_info
   signed char op_index[MAX_OPERANDS];
   bool op_riprel[MAX_OPERANDS];
   char *op_out[MAX_OPERANDS];
+  char *cm_out[MAX_OPERANDS];
   bfd_vma op_address[MAX_OPERANDS];
   bfd_vma start_pc;
 
@@ -9749,7 +9749,6 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
     .start_codep = priv.the_buffer,
     .codep = priv.the_buffer,
     .obufp = ins.obuf,
-    .cbufp = ins.cbuf,
     .last_lock_prefix = -1,
     .last_repz_prefix = -1,
     .last_repnz_prefix = -1,
@@ -9761,6 +9760,7 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
     .fwait_prefix = -1,
   };
   char op_out[MAX_OPERANDS][MAX_OPERAND_BUFFER_SIZE];
+  char cm_out[MAX_OPERANDS][COMMENT_BUFFER_SIZE];
 
   priv.orig_sizeflag = AFLAG | DFLAG;
   if ((info->mach & bfd_mach_i386_i386) != 0)
@@ -9874,6 +9874,8 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
     {
       op_out[i][0] = 0;
       ins.op_out[i] = op_out[i];
+      cm_out[i][0] = 0;
+      ins.cm_out[i] = cm_out[i];
     }
 
   sizeflag = priv.orig_sizeflag;
@@ -9992,6 +9994,7 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
 	  for (i = 0; i < MAX_OPERANDS; ++i)
 	    {
 	      ins.obufp = ins.op_out[i];
+	      ins.cbufp = ins.cm_out[i];
 	      ins.op_ad = MAX_OPERANDS - 1 - i;
 	      if (dp->op[i].rtn
 		  && !dp->op[i].rtn (&ins, dp->op[i].bytemode, sizeflag))
@@ -10317,6 +10320,10 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
 	  riprel = ins.op_riprel[i];
 	  ins.op_riprel[i] = ins.op_riprel[MAX_OPERANDS - 1 - i];
 	  ins.op_riprel[MAX_OPERANDS - 1 - i] = riprel;
+
+	  char *tmp = ins.cm_out[i];
+	  ins.cm_out[i] = ins.cm_out[MAX_OPERANDS - 1 - i];
+	  ins.cm_out[MAX_OPERANDS - 1 - i] = tmp;
 	}
     }
   else
@@ -10364,22 +10371,23 @@ print_insn (bfd_vma pc, disassemble_info *info, int intel_syntax)
 	needcomma = 1;
       }
 
+  const char *sep = "        # ";
   for (i = 0; i < MAX_OPERANDS; i++)
     if (ins.op_index[i] != -1 && ins.op_riprel[i])
       {
-	i386_dis_printf (info, dis_style_comment_start, "        # ");
+	i386_dis_printf (info, dis_style_comment_start, sep);
+	sep = ", ";
 	(*info->print_address_func)
 	  ((bfd_vma)(ins.start_pc + (ins.codep - ins.start_codep)
 		     + ins.op_address[ins.op_index[i]]),
-	  info);
-	break;
+	   info);
+      }
+    else if (*ins.cm_out[i])
+      {
+	i386_dis_printf (info, dis_style_comment_start, sep);
+	sep = ", ";
+	i386_dis_printf (info, dis_style_symbol, "%s", ins.cm_out[i]);
       }
-  if (ins.cbufp != ins.cbuf)
-    {
-      if (i == MAX_OPERANDS)
-	i386_dis_printf (info, dis_style_comment_start, "        # ");
-      i386_dis_printf (info, dis_style_comment_start, "%s", ins.cbuf);
-    }
   
   ret = ins.codep - priv.the_buffer;
  out:
@@ -10760,6 +10768,7 @@ dofloat (instr_info *ins, int sizeflag)
 
       putop (ins, float_mem[fp_indx], sizeflag);
       ins->obufp = ins->op_out[0];
+      ins->cbufp = ins->cm_out[0];
       ins->op_ad = 2;
       return OP_E (ins, float_mem_mode[fp_indx], sizeflag);
     }
@@ -10781,12 +10790,14 @@ dofloat (instr_info *ins, int sizeflag)
       putop (ins, dp->name, sizeflag);
 
       ins->obufp = ins->op_out[0];
+      ins->cbufp = ins->cm_out[0];
       ins->op_ad = 2;
       if (dp->op[0].rtn
 	  && !dp->op[0].rtn (ins, dp->op[0].bytemode, sizeflag))
 	return false;
 
       ins->obufp = ins->op_out[1];
+      ins->cbufp = ins->cm_out[1];
       ins->op_ad = 1;
       if (dp->op[1].rtn
 	  && !dp->op[1].rtn (ins, dp->op[1].bytemode, sizeflag))
@@ -11588,16 +11599,19 @@ static void
 cappend_with_style (instr_info *ins, const char *s,
 		    enum disassembler_style style)
 {
-  if (ins->cbufp + strlen (s) + 4 >= ins->cbuf + COMMENT_BUFFER_SIZE)
+  size_t len = strlen (ins->cbufp);
+
+  if (len + !!len + strlen (s) + 4 >= COMMENT_BUFFER_SIZE)
     return;
 
   unsigned num = (unsigned) style;
 
+  if (len)
+    *ins->cbufp++ = ' ';
   *ins->cbufp++ = STYLE_MARKER_CHAR;
   *ins->cbufp++ = (num < 10 ? ('0' + num)
 		   : ((num < 16) ? ('a' + (num - 10)) : '0'));
   *ins->cbufp++ = STYLE_MARKER_CHAR;
-  *ins->cbufp = '\0';
 
   ins->cbufp = stpcpy (ins->cbufp, s);
 }
@@ -11697,7 +11711,7 @@ oappend_immediate (instr_info *ins, bfd_vma imm)
 
   char * annotation = NULL;
 
-  if (asprintf (& annotation, " [%s]", sym->name) > 0)
+  if (asprintf (& annotation, "[%s]", sym->name) > 0)
     {
       /* Display the symbol associated with address 'imm'.  */
       cappend_with_style (ins, annotation, dis_style_symbol);
@@ -14255,6 +14269,10 @@ OP_VexW (instr_info *ins, int bytemode, int sizeflag)
 
       ins->op_out[2] = ins->op_out[1];
       ins->op_out[1] = tmp;
+
+      tmp = ins->cm_out[2];
+      ins->cm_out[2] = ins->cm_out[1];
+      ins->cm_out[1] = tmp;
     }
   return true;
 }
@@ -14288,6 +14306,10 @@ OP_REG_VexI4 (instr_info *ins, int bytemode, int sizeflag ATTRIBUTE_UNUSED)
 
       ins->op_out[3] = ins->op_out[2];
       ins->op_out[2] = tmp;
+
+      tmp = ins->cm_out[2];
+      ins->cm_out[3] = ins->cm_out[2];
+      ins->cm_out[2] = tmp;
     }
   return true;
 }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.