opcodes: PR 33384 invalid disassembler option message

Alan Modra amodra@gmail.com
Wed Sep 17 06:20:02 GMT 2025


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.
opcodes/
	* arc-dis.c (parse_option): Replace disassembler_options_cmp
	with strcmp.
	(parse_cpu_option): Likewise.
	(parse_disassembler_options): Don't use FOR_EACH_DISASSEMBLER_OPTION
	and temporarily overwrite commas in the option string with NUL.
	* csky-dis.c (parse_csky_dis_options): Don't xstrdup options.
	* ppc-dis.c (ppc_parse_cpu): Replace disassembler_options_cmp
	with strcmp.
	(powerpc_init_dialect): Don't use FOR_EACH_DISASSEMBLER_OPTION.
	Temporarily overwrite commas in the option string with NUL,
	and replace disassembler_options_cmp with strcmp.
	* arm-dis.c (parse_arm_disassembler_options): Likewise.
	* nfp-dis.c (parse_disassembler_options): Likewise.

diff --git a/include/dis-asm.h b/include/dis-asm.h
index c3e01996ad3..c10c4c85a38 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
diff --git a/opcodes/arc-dis.c b/opcodes/arc-dis.c
index 91efa969f17..5a1d4771158 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,12 +827,8 @@ 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);
@@ -845,20 +841,26 @@ static void
 parse_disassembler_options (struct disassemble_info *info)
 {
   struct arc_disassemble_info *arc_infop = info->private_data;
-  const char *option;
+  char *option = (char *) info->disassembler_options;
 
   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);
-    }
+  if (option != NULL)
+    while (1)
+      {
+	char *opt_end = strchr (option, ',');
+	if (opt_end != NULL)
+	  *opt_end = 0;
+	if (strncmp (option, "cpu=", 4) == 0)
+	  /* Strip leading `cpu=`.  */
+	  arc_infop->isa_mask = parse_cpu_option (option + 4);
+	else
+	  parse_option (arc_infop, option);
+	if (opt_end == NULL)
+	  break;
+	*opt_end = ',';
+	option = opt_end + 1;
+      }
 
   /* 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..fdc8ac8cb64 100644
--- a/opcodes/arm-dis.c
+++ b/opcodes/arm-dis.c
@@ -11909,16 +11909,19 @@ arm_symbol_is_valid (asymbol * sym,
 static void
 parse_arm_disassembler_options (const char *options)
 {
-  const char *opt;
+  char *opt = (char *) options;
 
   force_thumb = false;
-  FOR_EACH_DISASSEMBLER_OPTION (opt, options)
+  while (1)
     {
+      char *opt_end = strchr (opt, ',');
+      if (opt_end != NULL)
+	*opt_end = 0;
       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)
+	    if (strcmp (opt, regnames[i].name) == 0)
 	      {
 		regname_selected = i;
 		break;
@@ -11939,33 +11942,32 @@ parse_arm_disassembler_options (const char *options)
 	  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);
-	  else if (startswith (endptr, "cde")
-		   || startswith (endptr, "CDE"))
-	    cde_coprocs |= (1 << coproc_number);
+	    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
 	    {
-	      opcodes_error_handler (
-		  _("coprocN argument takes options \"generic\","
-		    " \"cde\", or \"CDE\": %s"), opt);
+	      endptr += 1;
+	      if (startswith (endptr, "generic"))
+		cde_coprocs &= ~(1 << coproc_number);
+	      else if (startswith (endptr, "cde")
+		       || startswith (endptr, "CDE"))
+		cde_coprocs |= (1 << coproc_number);
+	      else
+		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);
+      if (opt_end == NULL)
+	break;
+      *opt_end = ',';
+      opt = opt_end + 1;
     }
 
   return;
diff --git a/opcodes/csky-dis.c b/opcodes/csky-dis.c
index 01e21160752..3446b5adf35 100644
--- a/opcodes/csky-dis.c
+++ b/opcodes/csky-dis.c
@@ -265,23 +265,25 @@ csky_get_disassembler (bfd *abfd)
 
 /* Parse the string of disassembler options.  */
 static void
-parse_csky_dis_options (const char *opts_in)
+parse_csky_dis_options (const char *opts)
 {
-  char *opts = xstrdup (opts_in);
-  char *opt = opts;
-  char *opt_end = opts;
+  char *opt = (char *) opts;
 
-  for (; opt_end != NULL; opt = opt_end + 1)
+  while (1)
     {
-      if ((opt_end = strchr (opt, ',')) != NULL)
+      char *opt_end = strchr (opt, ',');
+      if (opt_end != NULL)
 	*opt_end = 0;
       if (strcmp (opt, "abi-names") == 0)
 	using_abi = 1;
       else
 	fprintf (stderr,
 		 "unrecognized disassembler option: %s", opt);
+      if (opt_end == NULL)
+	break;
+      *opt_end = ',';
+      opt = opt_end + 1;
     }
-  free (opts);
 }
 
 /* Get general register name.  */
diff --git a/opcodes/nfp-dis.c b/opcodes/nfp-dis.c
index 577ee638b9a..5d344f7603b 100644
--- a/opcodes/nfp-dis.c
+++ b/opcodes/nfp-dis.c
@@ -2734,30 +2734,39 @@ init_nfp6000_priv (nfp_priv_data * priv, struct disassemble_info *dinfo)
 static int
 parse_disassembler_options (nfp_opts * opts, struct disassemble_info *dinfo)
 {
-  const char *option;
+  char *option = (char *) dinfo->disassembler_options;
+  int ret = 0;
 
-  if (dinfo->disassembler_options == NULL)
+  if (option == 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;
-      }
-  }
+  while (1)
+    {
+      char *opt_end = strchr (option, ',');
+      if (opt_end != NULL)
+	*opt_end = 0;
+      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
+	{
+	  dinfo->fprintf_func (dinfo->stream, _("Invalid NFP option: %s"),
+			       option);
+	  ret = _NFP_ERR_STOP;
+	}
+      if (opt_end == NULL)
+	break;
+      *opt_end = ',';
+      option = opt_end + 1;
+    }
 
-  return 0;
+  return ret;
 }
 
 /* Called on first disassembly attempt so that dinfo->section is valid
diff --git a/opcodes/ppc-dis.c b/opcodes/ppc-dis.c
index 6e4a3b8d2b2..ef6f325d1b1 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)
 	  {
@@ -412,21 +412,29 @@ powerpc_init_dialect (struct disassemble_info *info)
       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);
-    }
+  char *opt = (char *) info->disassembler_options;
+  if (opt != NULL)
+    while (1)
+      {
+	ppc_cpu_t new_cpu = 0;
+	char *opt_end = strchr (opt, ',');
+	if (opt_end != NULL)
+	  *opt_end = 0;
+	if (strcmp (opt, "32") == 0)
+	  dialect &= ~(ppc_cpu_t) PPC_OPCODE_64;
+	else if (strcmp (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);
+	if (opt_end == NULL)
+	  break;
+	*opt_end = ',';
+	opt = opt_end + 1;
+      }
 
   info->private_data = priv;
   private_data (info)->dialect = dialect;

-- 
Alan Modra


More information about the Binutils mailing list