Re: [RFA]: Fix crashing bug in set follow-fork-mode

[dan at cgsoftware dot com (Daniel Berlin+list.gdb-patches)]

Andrew Cagney <ac131313@cygnus.com> writes:

I have a better idea.

I just made it check the enum list, when the command is added, like
so:

while(1)
{
	if enumlist[i]==0
		break (this means it's properly null terminated)
	if (!isalnum(enumlist[i][0]))
		internal_error("<command name>'s enum list is not null
terminated, or contains non-alphanumeric characters.");
	i++;
}

This is better than the testsuite addition, because if you screw up,
gdb won't start, or it'll crash (it depends on where enumlist[i][0] is
located.)
Watch:
bash-2.03# ./gdb -nw
gdb-internal-error: follow-fork-mode has non-null terminated enum list, or non alpha-numeric character in enum list
bash-2.03#

Patch attached
Index: command.c
===================================================================
RCS file: /cvs/src/src/gdb/command.c,v
retrieving revision 1.5
diff -c -3 -p -r1.5 command.c
*** command.c	2000/03/23 23:43:19	1.5
--- command.c	2000/04/20 16:47:00
*************** add_set_enum_cmd (name, class, enumlist,
*** 320,327 ****
       char *doc;
       struct cmd_list_element **list;
  {
!   struct cmd_list_element *c
    = add_set_cmd (name, class, var_enum, var, doc, list);
    c->enums = enumlist;
  
    return c;
--- 320,337 ----
       char *doc;
       struct cmd_list_element **list;
  {
!  int i=0;
!  struct cmd_list_element *c
    = add_set_cmd (name, class, var_enum, var, doc, list);
+   /* Make sure enumlist is null terminated */
+   while (1)
+     {
+ 	if (enumlist[i]==0)
+ 	  break;
+ 	if (!isalnum(enumlist[i][0]))
+ 	  internal_error("%s has non-null terminated enum list, or non alpha-numeric character in enum list",name);
+  	i++;
+     }
    c->enums = enumlist;
  
    return c;


> Tim Mooney wrote:
> >
> > >Someone please commit this to the branch if it gets approved.
> > >I have no idea how to make a testcase for it.
> > >I knew something screwy was up when "set follow-fork-mode parent" said
> > >"ambiguous command: \"parent\"", and it clearly wasn't ambiguous at
> > >all.
> 
> > >So, i looked, and it had trashed our stack, and thought we had 6
> > >million matches, rather than one.
> > 
> > scheduler-lock seems to have the same problem (see the enum at line 765 of
> > infrun.c).  Conditions under which it happens are the exact same as in my
> > report for the follow-fork-mode problem.  I've left the patch to fix the
> > problem for you or someone else, since it's again very small.
> 
> Thanks!
> 
> I've applied the attatched to the 5.0 branch and trunk. Anyone come up
> with a testsuite addition to add to the trunk?
> 
> 	AndrewThu Apr 20 18:54:15 2000  Andrew Cagney  <cagney@b1.cygnus.com>
> 
> 	From Daniel Berlin <dan@cgsoftware.com> and Tim Mooney
>  	<mooney@dogbert.cc.ndsu.nodak.edu>:
> 	* infrun.c (follow_fork_mode_kind_names): NULL terminate
>  	array. Re-indent.
> 	(scheduler_enums): Ditto.
> 
> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.7
> diff -p -r1.7 infrun.c
> *** infrun.c	2000/04/13 10:22:22	1.7
> --- infrun.c	2000/04/20 10:32:34
> *************** static int follow_vfork_when_exec;
> *** 436,448 ****
> 
>   static char *follow_fork_mode_kind_names[] =
>   {
> ! /* ??rehrauer:  The "both" option is broken, by what may be a 10.20
> !    kernel problem.  It's also not terribly useful without a GUI to
> !    help the user drive two debuggers.  So for now, I'm disabling
> !    the "both" option.
> !    "parent", "child", "both", "ask" };
> !  */
> !   "parent", "child", "ask"};
> 
>   static char *follow_fork_mode_string = NULL;
>   
> --- 436,448 ----
> 
>   static char *follow_fork_mode_kind_names[] =
>   {
> !   /* ??rehrauer: The "both" option is broken, by what may be a 10.20
> !      kernel problem.  It's also not terribly useful without a GUI to
> !      help the user drive two debuggers.  So for now, I'm disabling the
> !      "both" option. */
> !   /* "parent", "child", "both", "ask" */
> !   "parent", "child", "ask", NULL
> ! };
> 
>   static char *follow_fork_mode_string = NULL;
>   
> *************** static char schedlock_on[] = "on";
> *** 762,768 ****
>   static char schedlock_step[] = "step";
>   static char *scheduler_mode = schedlock_off;
>   static char *scheduler_enums[] =
> ! {schedlock_off, schedlock_on, schedlock_step};
> 
>   static void
>   set_schedlock_func (char *args, int from_tty, struct cmd_list_element *c)
> --- 762,773 ----
>   static char schedlock_step[] = "step";
>   static char *scheduler_mode = schedlock_off;
>   static char *scheduler_enums[] =
> ! {
> !   schedlock_off,
> !   schedlock_on,
> !   schedlock_step,
> !   NULL
> ! };
> 
>   static void
>   set_schedlock_func (char *args, int from_tty, struct cmd_list_element *c)