opcodes: PR 33384 invalid disassembler option message
Alan Modra
amodra@gmail.com
Sat Oct 4 00:21:20 GMT 2025
I changed my mind about this patch, and decided it was worth doing a
little more work to extract the common parts of breaking up a comma
separated option string to a for_each_disassembler_option function.
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))
--
Alan Modra
More information about the Binutils
mailing list