Fix another crash with gdb parameters in Python
authorTom Tromey <tromey@adacore.com>
Tue, 4 Jan 2022 15:52:40 +0000 (08:52 -0700)
committerTom Tromey <tromey@adacore.com>
Wed, 26 Jan 2022 13:49:51 +0000 (06:49 -0700)
While looking into the language-capturing issue, I found another way
to crash gdb using parameters from Python:

(gdb) python print(gdb.parameter('endian'))

(This is related to PR python/12188, though this patch isn't going to
fix what that bug is really about.)

The problem here is that the global variable that underlies the
"endian" parameter is initialized to NULL.  However, that's not a
valid value for an "enum" set/show parameter.

My understanding is that, in gdb, an "enum" parameter's underlying
variable must have a value that is "==" (not just strcmp-equal) to one
of the values coming from the enum array.  This invariant is relied on
in various places.

I started this patch by fixing the problem with "endian".  Then I
added some assertions to add_setshow_enum_cmd to try to catch other
problems of the same type.

This patch fixes all the problems that I found.  I also looked at all
the calls to add_setshow_enum_cmd to ensure that they were all
included in the gdb I tested.  I think they are: there are no calls in
nat-* files, or in remote-sim.c; and I was trying a build with all
targets, Python, and Guile enabled.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=12188

gdb/arch-utils.c
gdb/arm-tdep.c
gdb/charset.c
gdb/cli/cli-decode.c
gdb/guile/scm-param.c
gdb/language.c
gdb/osabi.c
gdb/python/py-param.c
gdb/testsuite/gdb.python/py-parameter.exp

index 38e3132668d6e4533fec1aa737aaea279712d179..3ee978a02498ef37a2c09c35ae483842afb1bad9 100644 (file)
@@ -359,7 +359,7 @@ static const char *const endian_enum[] =
   endian_auto,
   NULL,
 };
-static const char *set_endian_string;
+static const char *set_endian_string = endian_auto;
 
 enum bfd_endian
 selected_byte_order (void)
@@ -757,7 +757,8 @@ initialize_current_architecture (void)
      list of architectures.  */
   {
     /* Append ``auto''.  */
-    arches.push_back ("auto");
+    set_architecture_string = "auto";
+    arches.push_back (set_architecture_string);
     arches.push_back (nullptr);
     set_show_commands architecture_cmds
       = add_setshow_enum_cmd ("architecture", class_support,
index f6bd76a354d33bf9626eaeb60d8fcaff7eefa97d..f46913e8eaa2f525dc7e87008d0f395f97dc150a 100644 (file)
@@ -9802,6 +9802,8 @@ _initialize_arm_tdep ()
        size_t offset = strlen ("reg-names-");
        const char *style = disasm_options->name[i];
        valid_disassembly_styles[j++] = &style[offset];
+       if (strcmp (&style[offset], "std") == 0)
+         disassembly_style = &style[offset];
        length = snprintf (rdptr, rest, "%s - %s\n", &style[offset],
                           disasm_options->description[i]);
        rdptr += length;
index 9b1de08a93a4acbb12cdc4dbe7a4559210da5414..bf205ae087c82ee18c298e0465e422603304d138 100644 (file)
@@ -1029,6 +1029,9 @@ _initialize_charset ()
 #endif
 #endif
 
+  /* Recall that the first element is always "auto".  */
+  host_charset_name = charset_enum[0];
+  gdb_assert (strcmp (host_charset_name, "auto") == 0);
   add_setshow_enum_cmd ("charset", class_support,
                        charset_enum, &host_charset_name, _("\
 Set the host and target character sets."), _("\
@@ -1057,6 +1060,9 @@ To see a list of the character sets GDB supports, type `set host-charset <TAB>'.
                        show_host_charset_name,
                        &setlist, &showlist);
 
+  /* Recall that the first element is always "auto".  */
+  target_charset_name = charset_enum[0];
+  gdb_assert (strcmp (target_charset_name, "auto") == 0);
   add_setshow_enum_cmd ("target-charset", class_support,
                        charset_enum, &target_charset_name, _("\
 Set the target character set."), _("\
@@ -1069,6 +1075,9 @@ To see a list of the character sets GDB supports, type `set target-charset'<TAB>
                        show_target_charset_name,
                        &setlist, &showlist);
 
+  /* Recall that the first element is always "auto".  */
+  target_wide_charset_name = charset_enum[0];
+  gdb_assert (strcmp (target_wide_charset_name, "auto") == 0);
   add_setshow_enum_cmd ("target-wide-charset", class_support,
                        charset_enum, &target_wide_charset_name,
                        _("\
index c8cd95620a5c55791cfd69de9f4722f42529cf0e..5d9403068e7b94bcd2a7e7d658ae71ea91391f79 100644 (file)
@@ -613,6 +613,16 @@ add_setshow_enum_cmd (const char *name,
                      struct cmd_list_element **set_list,
                      struct cmd_list_element **show_list)
 {
+  /* We require *VAR to be initialized before this call, and
+     furthermore it must be == to one of the values in ENUMLIST.  */
+  gdb_assert (var != nullptr && *var != nullptr);
+  for (int i = 0; ; ++i)
+    {
+      gdb_assert (enumlist[i] != nullptr);
+      if (*var == enumlist[i])
+       break;
+    }
+
   set_show_commands commands
     =  add_setshow_cmd_full<const char *> (name, theclass, var_enum, var,
                                           set_doc, show_doc, help_doc,
index 2bbd46396fc739de5ae258447e79ae05f11bbb09..42727bbf406ff9dff3f6e2e43cabebad2d1c35d9 100644 (file)
@@ -458,12 +458,12 @@ add_setshow_generic (enum var_types param_type, enum command_class cmd_class,
       break;
 
     case var_enum:
+      /* Initialize the value, just in case.  */
+      make_setting (self).set<const char *> (self->enumeration[0]);
       commands = add_setshow_enum_cmd (cmd_name, cmd_class, self->enumeration,
                                       &self->value.cstringval, set_doc,
                                       show_doc, help_doc, set_func, show_func,
                                       set_list, show_list);
-      /* Initialize the value, just in case.  */
-      make_setting (self).set<const char *> (self->enumeration[0]);
       break;
 
     default:
index 0846b3e5eebebae9e4ebd4048d37c083bb9ab702..f000142fc2da99d65912668082c4832b096ef877 100644 (file)
@@ -476,7 +476,8 @@ add_set_language_command ()
   /* Display "auto", "local" and "unknown" first, and then the rest,
      alpha sorted.  */
   const char **language_names_p = language_names;
-  *language_names_p++ = language_def (language_auto)->name ();
+  language = language_def (language_auto)->name ();
+  *language_names_p++ = language;
   *language_names_p++ = "local";
   *language_names_p++ = language_def (language_unknown)->name ();
   const char **sort_begin = language_names_p;
@@ -1150,6 +1151,8 @@ _initialize_language ()
   add_alias_cmd ("c", setshow_check_cmds.show, no_class, 1, &showlist);
   add_alias_cmd ("ch", setshow_check_cmds.show, no_class, 1, &showlist);
 
+  range = type_or_range_names[3];
+  gdb_assert (strcmp (range, "auto") == 0);
   add_setshow_enum_cmd ("range", class_support, type_or_range_names,
                        &range,
                        _("Set range checking (on/warn/off/auto)."),
@@ -1158,6 +1161,8 @@ _initialize_language ()
                        show_range_command,
                        &setchecklist, &showchecklist);
 
+  case_sensitive = case_sensitive_names[2];
+  gdb_assert (strcmp (case_sensitive, "auto") == 0);
   add_setshow_enum_cmd ("case-sensitive", class_support, case_sensitive_names,
                        &case_sensitive, _("\
 Set case sensitivity in name search (on/off/auto)."), _("\
@@ -1174,10 +1179,6 @@ For Fortran the default is off; for other languages the default is on."),
 
   add_set_language_command ();
 
-  language = "auto";
-  range = "auto";
-  case_sensitive = "auto";
-
   /* Have the above take effect.  */
   set_language (language_auto);
 }
index 40c86e782e78cdf37ad001412bd0af30a0bfda0f..d4a98061dbd530e8a8d60b0c2e66ae95488f5cf2 100644 (file)
@@ -666,11 +666,13 @@ _initialize_gdb_osabi ()
                                  generic_elf_osabi_sniffer);
 
   /* Register the "set osabi" command.  */
+  user_osabi_state = osabi_auto;
+  set_osabi_string = gdb_osabi_available_names[0];
+  gdb_assert (strcmp (set_osabi_string, "auto") == 0);
   add_setshow_enum_cmd ("osabi", class_support, gdb_osabi_available_names,
                        &set_osabi_string,
                        _("Set OS ABI of target."),
                        _("Show OS ABI of target."),
                        NULL, set_osabi, show_osabi,
                        &setlist, &showlist);
-  user_osabi_state = osabi_auto;
 }
index 24678bc8a5e7027b88bbfedb823093c3691b1cd3..f5f0eca6d1a55d63bd54400dfb8b1f8f6e6131a8 100644 (file)
@@ -572,13 +572,13 @@ add_setshow_generic (int parmclass, enum command_class cmdclass,
       break;
 
     case var_enum:
+      /* Initialize the value, just in case.  */
+      self->value.cstringval = self->enumeration[0];
       commands = add_setshow_enum_cmd (cmd_name.get (), cmdclass,
                                       self->enumeration,
                                       &self->value.cstringval, set_doc,
                                       show_doc, help_doc, get_set_value,
                                       get_show_value, set_list, show_list);
-      /* Initialize the value, just in case.  */
-      self->value.cstringval = self->enumeration[0];
       break;
 
     default:
index b025c2562f0c8e82df15558b2743b792a5a0907a..2de148c1b27c25d85f25ec08429b7b5f9c3a190b 100644 (file)
@@ -364,3 +364,7 @@ test_really_undocumented_parameter
 test_deprecated_api_parameter
 test_integer_parameter
 test_throwing_parameter
+
+# This caused a gdb crash.
+gdb_test "python print(gdb.parameter('endian'))" "auto" \
+    "print endian parameter"