From: Alan Modra Date: Fri, 3 Oct 2025 23:15:07 +0000 (+0930) Subject: opcodes: PR 33384 invalid disassembler option message X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c572eb343a2696eec155c0b23519b22b7af8cfb2;p=thirdparty%2Fbinutils-gdb.git opcodes: PR 33384 invalid disassembler option message This is the binutils fix for PR 33384. Here we are assuming that no const char* comma-separated option strings are passed in to disassemble_info.disassembler_options. That is true for current usage in gdb and binutils. In fact, there is only one place that passes a string in read-only memory, gdb/tdep-i386.c:disassembly_flavor, and that one is a single option. include/ * dis-asm.h (struct disassemble_info): Comment. (disassembler_options_cmp, next_disassembler_option), (FOR_EACH_DISASSEMBLER_OPTION): Delete. (for_each_disassembler_option): Declare. opcodes/ * disassemble.c (disassembler_options_cmp): Delete. (for_each_disassembler_option): New function. * arc-dis.c (parse_option): Replace disassembler_options_cmp with strcmp. (parse_cpu_option): Likewise. (parse_disassembler_options): Replace FOR_EACH_DISASSEMBLER_OPTION with for_each_disassembler_option, and extract loop body to.. (arc_parse_option): ..this new function. * arm-dis.c (parse_arm_disassembler_options): Delete, extracting loop body to.. (arm_parse_option): ..this new function. (print_insn): Use for_each_disassembler_option. * csky-dis.c (parse_csky_dis_options): Delete, extracting loop body to.. (parse_csky_option): ..this new function. (print_insn_csky): Use for_each_disassembler_option. * nfp-dis.c (parse_disassembler_options): Replace FOR_EACH_DISASSEMBLER_OPTION with for_each_disassembler_option, and extract loop body to.. (nfp_parse_option): ..this new function. Use opcodes_error_handler here rather than info->fprintf_func to print error. * ppc-dis.c (ppc_parse_cpu): Replace disassembler_options_cmp with strcmp. (struct ppc_parse_data): New. (powerpc_init_dialect): Adjust to use new struct. Replace FOR_EACH_DISASSEMBLER_OPTION with for_each_disassembler_option, and extract loop body to.. (ppc_parse_option): ..this new function. --- diff --git a/include/dis-asm.h b/include/dis-asm.h index c3e01996ad3..d97ce0502f5 100644 --- a/include/dis-asm.h +++ b/include/dis-asm.h @@ -239,9 +239,9 @@ typedef struct disassemble_info size_t buffer_length; /* This variable may be set by the instruction decoder. It suggests - the number of bytes objdump should display on a single line. If - the instruction decoder sets this, it should always set it to - the same value in order to get reasonable looking output. */ + the number of bytes objdump should display on a single line. If + the instruction decoder sets this, it should always set it to + the same value in order to get reasonable looking output. */ int bytes_per_line; /* The next two variables control the way objdump displays the raw data. */ @@ -287,7 +287,13 @@ typedef struct disassemble_info zero if unknown. */ bfd_vma target2; /* Second target address for dref2 */ - /* Command line options specific to the target disassembler. */ + /* Command line options specific to the target disassembler. + Note that if this string contains multiple comma-separated + options, then it must not be in read-only memory. Commas may be + temporarily modified by the target disassembler when parsing + options. The string is const in the sense that on return from + the target disassembler the string will be exactly the same as + on entry. */ const char *disassembler_options; /* If non-zero then try not disassemble beyond this address, even if @@ -429,26 +435,10 @@ extern void disassembler_usage (FILE *); /* Remove whitespace and consecutive commas. */ extern char *remove_whitespace_and_extra_commas (char *); -/* Like STRCMP, but treat ',' the same as '\0' so that we match - strings like "foobar" against "foobar,xxyyzz,...". */ -extern int disassembler_options_cmp (const char *, const char *); - -/* A helper function for FOR_EACH_DISASSEMBLER_OPTION. */ -static inline const char * -next_disassembler_option (const char *options) -{ - const char *opt = strchr (options, ','); - if (opt != NULL) - opt++; - return opt; -} - -/* A macro for iterating over each comma separated option in OPTIONS. */ -#define FOR_EACH_DISASSEMBLER_OPTION(OPT, OPTIONS) \ - for ((OPT) = (OPTIONS); \ - (OPT) != NULL; \ - (OPT) = next_disassembler_option (OPT)) - +/* Iterate over each comma separated option in disassembler_options. */ +extern bool for_each_disassembler_option (struct disassemble_info *, + bool (*) (const char *, void *), + void *); /* This block of definitions is for particular callers who read instructions into a buffer before calling the instruction decoder. */ diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c index 91efa969f17..f162725d617 100644 --- a/opcodes/arc-dis.c +++ b/opcodes/arc-dis.c @@ -740,16 +740,16 @@ operand_iterator_next (struct arc_operand_iterator *iter, static void parse_option (struct arc_disassemble_info *arc_infop, const char *option) { - if (disassembler_options_cmp (option, "dsp") == 0) + if (strcmp (option, "dsp") == 0) add_to_decode (arc_infop, DSP, NONE); - else if (disassembler_options_cmp (option, "spfp") == 0) + else if (strcmp (option, "spfp") == 0) add_to_decode (arc_infop, FLOAT, SPX); - else if (disassembler_options_cmp (option, "dpfp") == 0) + else if (strcmp (option, "dpfp") == 0) add_to_decode (arc_infop, FLOAT, DPX); - else if (disassembler_options_cmp (option, "quarkse_em") == 0) + else if (strcmp (option, "quarkse_em") == 0) { add_to_decode (arc_infop, FLOAT, DPX); add_to_decode (arc_infop, FLOAT, SPX); @@ -757,10 +757,10 @@ parse_option (struct arc_disassemble_info *arc_infop, const char *option) add_to_decode (arc_infop, FLOAT, QUARKSE2); } - else if (disassembler_options_cmp (option, "fpuda") == 0) + else if (strcmp (option, "fpuda") == 0) add_to_decode (arc_infop, FLOAT, DPA); - else if (disassembler_options_cmp (option, "nps400") == 0) + else if (strcmp (option, "nps400") == 0) { add_to_decode (arc_infop, ACL, NPS400); add_to_decode (arc_infop, ARITH, NPS400); @@ -777,13 +777,13 @@ parse_option (struct arc_disassemble_info *arc_infop, const char *option) add_to_decode (arc_infop, ULTRAIP, NPS400); } - else if (disassembler_options_cmp (option, "fpus") == 0) + else if (strcmp (option, "fpus") == 0) { add_to_decode (arc_infop, FLOAT, SP); add_to_decode (arc_infop, FLOAT, CVT); } - else if (disassembler_options_cmp (option, "fpud") == 0) + else if (strcmp (option, "fpud") == 0) { add_to_decode (arc_infop, FLOAT, DP); add_to_decode (arc_infop, FLOAT, CVT); @@ -827,38 +827,37 @@ parse_cpu_option (const char *option) int i; for (i = 0; cpu_types[i].name; ++i) - { - if (!disassembler_options_cmp (cpu_types[i].name, option)) - { - return cpu_types[i].flags; - } - } + if (strcmp (cpu_types[i].name, option) == 0) + return cpu_types[i].flags; /* xgettext:c-format */ opcodes_error_handler (_("unrecognised disassembler CPU option: %s"), option); return ARC_OPCODE_NONE; } +static bool +arc_parse_option (const char *option, void *data) +{ + struct arc_disassemble_info *arc_infop = data; + + if (strncmp (option, "cpu=", 4) == 0) + /* Strip leading `cpu=`. */ + arc_infop->isa_mask = parse_cpu_option (option + 4); + else + parse_option (arc_infop, option); + return true; +} + /* Go over the options list and parse it. */ static void parse_disassembler_options (struct disassemble_info *info) { struct arc_disassemble_info *arc_infop = info->private_data; - const char *option; arc_infop->isa_mask = ARC_OPCODE_NONE; - FOR_EACH_DISASSEMBLER_OPTION (option, info->disassembler_options) - { - /* A CPU option? Cannot use STRING_COMMA_LEN because strncmp is also a - preprocessor macro. */ - if (strncmp (option, "cpu=", 4) == 0) - /* Strip leading `cpu=`. */ - arc_infop->isa_mask = parse_cpu_option (option + 4); - else - parse_option (arc_infop, option); - } + for_each_disassembler_option (info, arc_parse_option, arc_infop); /* Figure out CPU type, unless it was enforced via disassembler options. */ if (arc_infop->isa_mask == ARC_OPCODE_NONE) diff --git a/opcodes/arm-dis.c b/opcodes/arm-dis.c index 9d6a046b823..a548cdb3dcd 100644 --- a/opcodes/arm-dis.c +++ b/opcodes/arm-dis.c @@ -11904,52 +11904,43 @@ arm_symbol_is_valid (asymbol * sym, return (name && *name != '$' && strncmp (name, "__tagsym$$", 10)); } -/* Parse the string of disassembler options. */ +/* Parse a disassembler option. */ -static void -parse_arm_disassembler_options (const char *options) +static bool +arm_parse_option (const char *opt, void *data ATTRIBUTE_UNUSED) { - const char *opt; - - force_thumb = false; - FOR_EACH_DISASSEMBLER_OPTION (opt, options) + if (startswith (opt, "reg-names-")) { - if (startswith (opt, "reg-names-")) - { - unsigned int i; - for (i = 0; i < NUM_ARM_OPTIONS; i++) - if (disassembler_options_cmp (opt, regnames[i].name) == 0) - { - regname_selected = i; - break; - } + unsigned int i; + for (i = 0; i < NUM_ARM_OPTIONS; i++) + if (strcmp (opt, regnames[i].name) == 0) + { + regname_selected = i; + break; + } - if (i >= NUM_ARM_OPTIONS) - /* xgettext: c-format */ - opcodes_error_handler (_("unrecognised register name set: %s"), - opt); - } - else if (startswith (opt, "force-thumb")) - force_thumb = 1; - else if (startswith (opt, "no-force-thumb")) - force_thumb = 0; - else if (startswith (opt, "coproc")) + if (i >= NUM_ARM_OPTIONS) + /* xgettext: c-format */ + opcodes_error_handler (_("unrecognised register name set: %s"), + opt); + } + else if (startswith (opt, "force-thumb")) + force_thumb = 1; + else if (startswith (opt, "no-force-thumb")) + force_thumb = 0; + else if (startswith (opt, "coproc")) + { + const char *procptr = opt + sizeof ("coproc") - 1; + char *endptr; + uint8_t coproc_number = strtol (procptr, &endptr, 10); + if (endptr != procptr + 1 || coproc_number > 7) + opcodes_error_handler (_("cde coprocessor not between 0-7: %s"), + opt); + else if (*endptr != '=') + opcodes_error_handler (_("coproc must have an argument: %s"), + opt); + else { - const char *procptr = opt + sizeof ("coproc") - 1; - char *endptr; - uint8_t coproc_number = strtol (procptr, &endptr, 10); - if (endptr != procptr + 1 || coproc_number > 7) - { - opcodes_error_handler (_("cde coprocessor not between 0-7: %s"), - opt); - continue; - } - if (*endptr != '=') - { - opcodes_error_handler (_("coproc must have an argument: %s"), - opt); - continue; - } endptr += 1; if (startswith (endptr, "generic")) cde_coprocs &= ~(1 << coproc_number); @@ -11957,18 +11948,15 @@ parse_arm_disassembler_options (const char *options) || startswith (endptr, "CDE")) cde_coprocs |= (1 << coproc_number); else - { - opcodes_error_handler ( - _("coprocN argument takes options \"generic\"," - " \"cde\", or \"CDE\": %s"), opt); - } + opcodes_error_handler + (_("coprocN argument takes options \"generic\"," + " \"cde\", or \"CDE\": %s"), opt); } - else - /* xgettext: c-format */ - opcodes_error_handler (_("unrecognised disassembler option: %s"), opt); } - - return; + else + /* xgettext: c-format */ + opcodes_error_handler (_("unrecognised disassembler option: %s"), opt); + return true; } static bool @@ -12377,7 +12365,8 @@ print_insn (bfd_vma pc, struct disassemble_info *info, bool little) if (info->disassembler_options) { - parse_arm_disassembler_options (info->disassembler_options); + force_thumb = false; + for_each_disassembler_option (info, arm_parse_option, NULL); /* To avoid repeated parsing of these options, we remove them here. */ info->disassembler_options = NULL; diff --git a/opcodes/csky-dis.c b/opcodes/csky-dis.c index 01e21160752..afe54c444b5 100644 --- a/opcodes/csky-dis.c +++ b/opcodes/csky-dis.c @@ -263,25 +263,15 @@ csky_get_disassembler (bfd *abfd) return print_insn_csky; } -/* Parse the string of disassembler options. */ -static void -parse_csky_dis_options (const char *opts_in) +/* Parse a disassembler option. */ +static bool +parse_csky_option (const char *opt, void *data ATTRIBUTE_UNUSED) { - char *opts = xstrdup (opts_in); - char *opt = opts; - char *opt_end = opts; - - for (; opt_end != NULL; opt = opt_end + 1) - { - if ((opt_end = strchr (opt, ',')) != NULL) - *opt_end = 0; - if (strcmp (opt, "abi-names") == 0) - using_abi = 1; - else - fprintf (stderr, - "unrecognized disassembler option: %s", opt); - } - free (opts); + if (strcmp (opt, "abi-names") == 0) + using_abi = 1; + else + fprintf (stderr, "unrecognized disassembler option: %s", opt); + return true; } /* Get general register name. */ @@ -1059,7 +1049,7 @@ print_insn_csky (bfd_vma memaddr, struct disassemble_info *info) if (info->disassembler_options) { - parse_csky_dis_options (info->disassembler_options); + for_each_disassembler_option (info, parse_csky_option, NULL); info->disassembler_options = NULL; } diff --git a/opcodes/disassemble.c b/opcodes/disassemble.c index 53ee1ecf38b..0fb24ae4352 100644 --- a/opcodes/disassemble.c +++ b/opcodes/disassemble.c @@ -824,28 +824,30 @@ remove_whitespace_and_extra_commas (char *options) return (strlen (options) != 0) ? options : NULL; } -/* Like STRCMP, but treat ',' the same as '\0' so that we match - strings like "foobar" against "foobar,xxyyzz,...". */ +/* Call FUNC for each comma separated option in INFO->disassembler_options, + passing a zero terminated option and DATA. The iteration terminates + should FUNC return false. */ -int -disassembler_options_cmp (const char *s1, const char *s2) +bool +for_each_disassembler_option (struct disassemble_info *info, + bool (*func) (const char *, void *), + void *data) { - unsigned char c1, c2; - - do - { - c1 = (unsigned char) *s1++; - if (c1 == ',') - c1 = '\0'; - c2 = (unsigned char) *s2++; - if (c2 == ',') - c2 = '\0'; - if (c1 == '\0') - return c1 - c2; - } - while (c1 == c2); - - return c1 - c2; + char *opt = (char *) info->disassembler_options; + bool ok = true; + if (opt != NULL) + while (ok) + { + char *opt_end = strchr (opt, ','); + if (opt_end != NULL) + *opt_end = 0; + ok = func (opt, data); + if (opt_end == NULL) + break; + *opt_end = ','; + opt = opt_end + 1; + } + return ok; } void diff --git a/opcodes/nfp-dis.c b/opcodes/nfp-dis.c index 577ee638b9a..b5a7111a663 100644 --- a/opcodes/nfp-dis.c +++ b/opcodes/nfp-dis.c @@ -2731,31 +2731,33 @@ init_nfp6000_priv (nfp_priv_data * priv, struct disassemble_info *dinfo) return true; } +static bool +nfp_parse_option (const char *option, void *data) +{ + nfp_opts *opts = data; + + if (strcmp (option, "no-pc") == 0) + opts->show_pc = 0; + else if (strcmp (option, "ctx4") == 0) + { + if (!opts->ctx_mode) + opts->ctx_mode = 4; + } + else if (strcmp (option, "ctx8") == 0) + opts->ctx_mode = 8; + else + { + opcodes_error_handler (_("invalid NFP option: %s"), option); + return false; + } + return true; +} + static int parse_disassembler_options (nfp_opts * opts, struct disassemble_info *dinfo) { - const char *option; - - if (dinfo->disassembler_options == NULL) - return 0; - - FOR_EACH_DISASSEMBLER_OPTION (option, dinfo->disassembler_options) - { - if (disassembler_options_cmp (option, "no-pc") == 0) - opts->show_pc = 0; - else if (disassembler_options_cmp (option, "ctx4") == 0) - { - if (!opts->ctx_mode) - opts->ctx_mode = 4; - } - else if (disassembler_options_cmp (option, "ctx8") == 0) - opts->ctx_mode = 8; - else - { - dinfo->fprintf_func (dinfo->stream, _("Invalid NFP option: %s"), option); - return _NFP_ERR_STOP; - } - } + if (!for_each_disassembler_option (dinfo, nfp_parse_option, opts)) + return _NFP_ERR_STOP; return 0; } diff --git a/opcodes/ppc-dis.c b/opcodes/ppc-dis.c index 6e4a3b8d2b2..1ded17f71db 100644 --- a/opcodes/ppc-dis.c +++ b/opcodes/ppc-dis.c @@ -325,7 +325,7 @@ ppc_parse_cpu (ppc_cpu_t ppc_cpu, ppc_cpu_t *sticky, const char *arg) unsigned int i; for (i = 0; i < ARRAY_SIZE (ppc_opts); i++) - if (disassembler_options_cmp (ppc_opts[i].opt, arg) == 0) + if (strcmp (ppc_opts[i].opt, arg) == 0) { if (ppc_opts[i].sticky) { @@ -351,13 +351,36 @@ ppc_parse_cpu (ppc_cpu_t ppc_cpu, ppc_cpu_t *sticky, const char *arg) return ppc_cpu; } +struct ppc_parse_data +{ + ppc_cpu_t dialect; + ppc_cpu_t sticky; +}; + +static bool +ppc_parse_option (const char *opt, void *data) +{ + struct ppc_parse_data *res = data; + ppc_cpu_t new_cpu; + + if (strcmp (opt, "32") == 0) + res->dialect &= ~(ppc_cpu_t) PPC_OPCODE_64; + else if (strcmp (opt, "64") == 0) + res->dialect |= PPC_OPCODE_64; + else if ((new_cpu = ppc_parse_cpu (res->dialect, &res->sticky, opt)) != 0) + res->dialect = new_cpu; + else + /* xgettext: c-format */ + opcodes_error_handler (_("warning: ignoring unknown -M%s option"), opt); + return true; +} + /* Determine which set of machines to disassemble for. */ static void powerpc_init_dialect (struct disassemble_info *info) { - ppc_cpu_t dialect = 0; - ppc_cpu_t sticky = 0; + struct ppc_parse_data out = { 0, 0 }; struct dis_private *priv = calloc (1, sizeof (*priv)); if (priv == NULL) @@ -367,69 +390,57 @@ powerpc_init_dialect (struct disassemble_info *info) { case bfd_mach_ppc_403: case bfd_mach_ppc_403gc: - dialect = ppc_parse_cpu (dialect, &sticky, "403"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "403"); break; case bfd_mach_ppc_405: - dialect = ppc_parse_cpu (dialect, &sticky, "405"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "405"); break; case bfd_mach_ppc_601: - dialect = ppc_parse_cpu (dialect, &sticky, "601"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "601"); break; case bfd_mach_ppc_750: - dialect = ppc_parse_cpu (dialect, &sticky, "750cl"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "750cl"); break; case bfd_mach_ppc_a35: case bfd_mach_ppc_rs64ii: case bfd_mach_ppc_rs64iii: - dialect = ppc_parse_cpu (dialect, &sticky, "pwr2") | PPC_OPCODE_64; + out.dialect = (ppc_parse_cpu (out.dialect, &out.sticky, "pwr2") + | PPC_OPCODE_64); break; case bfd_mach_ppc_e500: - dialect = ppc_parse_cpu (dialect, &sticky, "e500"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "e500"); break; case bfd_mach_ppc_e500mc: - dialect = ppc_parse_cpu (dialect, &sticky, "e500mc"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "e500mc"); break; case bfd_mach_ppc_e500mc64: - dialect = ppc_parse_cpu (dialect, &sticky, "e500mc64"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "e500mc64"); break; case bfd_mach_ppc_e5500: - dialect = ppc_parse_cpu (dialect, &sticky, "e5500"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "e5500"); break; case bfd_mach_ppc_e6500: - dialect = ppc_parse_cpu (dialect, &sticky, "e6500"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "e6500"); break; case bfd_mach_ppc_titan: - dialect = ppc_parse_cpu (dialect, &sticky, "titan"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "titan"); break; case bfd_mach_ppc_vle: - dialect = ppc_parse_cpu (dialect, &sticky, "vle"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "vle"); break; default: if (info->arch == bfd_arch_powerpc) - dialect = ppc_parse_cpu (dialect, &sticky, "power11") | PPC_OPCODE_ANY; + out.dialect = (ppc_parse_cpu (out.dialect, &out.sticky, "power11") + | PPC_OPCODE_ANY); else - dialect = ppc_parse_cpu (dialect, &sticky, "pwr"); + out.dialect = ppc_parse_cpu (out.dialect, &out.sticky, "pwr"); break; } - const char *opt; - FOR_EACH_DISASSEMBLER_OPTION (opt, info->disassembler_options) - { - ppc_cpu_t new_cpu = 0; - - if (disassembler_options_cmp (opt, "32") == 0) - dialect &= ~(ppc_cpu_t) PPC_OPCODE_64; - else if (disassembler_options_cmp (opt, "64") == 0) - dialect |= PPC_OPCODE_64; - else if ((new_cpu = ppc_parse_cpu (dialect, &sticky, opt)) != 0) - dialect = new_cpu; - else - /* xgettext: c-format */ - opcodes_error_handler (_("warning: ignoring unknown -M%s option"), opt); - } + for_each_disassembler_option (info, ppc_parse_option, &out); info->private_data = priv; - private_data (info)->dialect = dialect; + private_data (info)->dialect = out.dialect; } #define PPC_OPCD_SEGS (1 + PPC_OP (-1))