From ac3fe48fd61a92d03c66152038df4fc184bf5fcd Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Mon, 12 Sep 2022 08:19:55 +0200 Subject: [PATCH] x86: avoid i386_dis_printf()'s staging area for a fair part of output While PR binutils/29483 has now been addressed differently, this originally proposed change still has its merits: Avoiding vsnprintf() for typically far more than half of the overall output results in a 2-3% performance gain in my testing (with debug builds of objdump, libbfd, and libopcodes). With that part of output no longer using staging_area[], the array also doesn't need to be quite as large anymore (the largest presently used size is 27, from "64-bit address is disabled"). While limiting the scope of "res" it became apparent that - no caller cares about the function's return value, - the comment about the return value was wrong, - a particular positive return value would have been meaningless to the caller. Therefore convert the function to return "void" at the same time. --- opcodes/i386-dis.c | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c index c1166a4446c..98d3ecd9f05 100644 --- a/opcodes/i386-dis.c +++ b/opcodes/i386-dis.c @@ -9267,31 +9267,40 @@ oappend_register (instr_info *ins, const char *s) STYLE is the default style to use in the fprintf_styled_func calls, however, FMT might include embedded style markers (see oappend_style), these embedded markers are not printed, but instead change the style - used in the next fprintf_styled_func call. + used in the next fprintf_styled_func call. */ - Return non-zero to indicate the print call was a success. */ - -static int ATTRIBUTE_PRINTF_3 +static void ATTRIBUTE_PRINTF_3 i386_dis_printf (instr_info *ins, enum disassembler_style style, const char *fmt, ...) { va_list ap; enum disassembler_style curr_style = style; - char *start, *curr; - char staging_area[MAX_OPERAND_BUFFER_SIZE]; - int res; + const char *start, *curr; + char staging_area[40]; va_start (ap, fmt); - res = vsnprintf (staging_area, sizeof (staging_area), fmt, ap); - va_end (ap); + /* In particular print_insn()'s processing of op_txt[] can hand rather long + strings here. Bypass vsnprintf() in such cases to avoid capacity issues + with the staging area. */ + if (strcmp (fmt, "%s")) + { + int res = vsnprintf (staging_area, sizeof (staging_area), fmt, ap); - if (res < 0) - return res; + va_end (ap); - if ((size_t) res >= sizeof (staging_area)) - abort (); + if (res < 0) + return; - start = curr = staging_area; + if ((size_t) res >= sizeof (staging_area)) + abort (); + + start = curr = staging_area; + } + else + { + start = curr = va_arg (ap, const char *); + va_end (ap); + } do { @@ -9306,10 +9315,7 @@ i386_dis_printf (instr_info *ins, enum disassembler_style style, curr_style, "%.*s", len, start); if (n < 0) - { - res = n; - break; - } + break; if (*curr == '\0') break; @@ -9343,8 +9349,6 @@ i386_dis_printf (instr_info *ins, enum disassembler_style style, ++curr; } while (true); - - return res; } static int -- 2.30.2