[PATCH] x86: Pass "%s" to i386_dis_printf as the format string
H.J. Lu
hjl.tools@gmail.com
Sun May 24 00:09:47 GMT 2026
On Fri, May 15, 2026 at 9:08 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> 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.
> ---
> As long as we want to continue to use the ->print_address_func() hook,
> properly unifying %rip-relative comments with others won't be possible.
> That, however, is in line with direct addresses also getting printed
> specially, similarly by using ->print_address_func().
>
> Was it really intended for the original -Mannotate test to be run only for
> Linux targets? It certainly isn't intended here, yet I'd like the new
> tests to be in the same directory as the original one. x86-64.exp,
> however, bails right away for non-Linux (and remote hosts). I guess at
> least the *.d based testing loop should be moved up. Tests truly only
> working for Linux can easily be constrained in the *.d files themselves.
>
> --- /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
> --- /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
> --- /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
> --- 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
> .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
> .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
> {
> 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
> 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
> 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
> 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)
commit aeced13ee0cd570d78cb37ab1f9bba840958515a
Author: Jan Beulich <jbeulich@suse.com>
Date: Fri May 22 08:49:12 2026 +0200
x86/disasm: rework comment handling
caused:
libtool: compile: gcc -DHAVE_CONFIG_H -I. -I../../opcodes -I.
-I../../opcodes -I../bfd -I../../opcodes/../include
-I../../opcodes/../bfd -W -Wall -Wstrict-prototypes
-Wmissing-prototypes -Wshadow -Wstack-usage=262144 -O2 -flto=auto
-ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall
-Wno-complain-wrong-lang -Werror=format-security
-Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS
-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
-fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1
-m32 -march=i686 -mtune=generic -msse2 -mfpmath=sse -mstackrealign
-fasynchronous-unwind-tables -fstack-clash-protection
-mtls-dialect=gnu -flto=8 -std=gnu11 -fprofile-generate
-flto=jobserver -ffat-lto-objects -MT i386-dis.lo -MD -MP -MF
.deps/i386-dis.Tpo -c ../../opcodes/i386-dis.c -fPIC -DPIC -o
.libs/i386-dis.o
../../opcodes/i386-dis.c: In function ‘print_insn’:
../../opcodes/i386-dis.c:10378:9: error: format not a string literal
and no format arguments [-Werror=format-security]
10378 | i386_dis_printf (info, dis_style_comment_start, sep);
| ^~~~~~~~~~~~~~~
../../opcodes/i386-dis.c:10387:9: error: format not a string literal
and no format arguments [-Werror=format-security]
10387 | i386_dis_printf (info, dis_style_comment_start, sep);
| ^~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
make[4]: *** [Makefile:1076: i386-dis.lo] Error 1
Since i386_dis_printf is declared as
static void ATTRIBUTE_PRINTF_3 i386_dis_printf (const disassemble_info *,
enum disassembler_style,
const char *, ...);
the 3rd argument should be a string literal as the format string. Pass
"%s" to i386_dis_printf as the format string to fix the regression.
PR binutils/34168
* i386-dis.c (print_insn): Pass "%s" to i386_dis_printf as the
format string.
--
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-Pass-s-to-i386_dis_printf-as-the-format-string.patch
Type: text/x-patch
Size: 3466 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20260524/20da5033/attachment-0001.bin>
More information about the Binutils
mailing list