Re: GDB 7.99.91 available for testing

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2017-05-15 17:11, Simon Marchi wrote:
> Hi Joel,
> 
> Somebody reported on IRC that the command "tty" (an alias for "set
> inferior-tty") didn't work anymore.  git bisect found one of my
> commits as the culprit:
> 
>   b593ecca856860a8b38deb808493bba4beef3aee
>   Makefiles: Flatten and sort file lists
> 
> This is likely due to the _initialize functions being called in a
> different order.  I think this should block the 8.0 release.  I'll
> have time to investigate this tonight or tomorrow.
> 
> Thanks,
> 
> Simon

Hi,

So indeed, the problem is due to the ordering of the _initialize 
functions that changed, _initialize_printcmd used to be called before 
_initialize_infcmd, now it's the reverse.

When it works, the timeline of events to be able to get the tty alias 
working is the following:

1. The "set" command is registered as a prefix command in printcmd.c, 
which adds it to the global command list (cmdlist).  setlist is the list 
of its subcommands.
2. The "set inferior-tty" command is added as a child of "set" (i.e. 
it's inserted into setlist) in infcmd.c.
3. The "tty" alias is created for "set inferior-tty" in infcmd.c.  This 
looks up "set" in cmdlist, then "inferior-tty" in the subcommands of 
"set".  It's found and everyone is happy.

With the _initialize functions called in a different order, steps are 
executed in the order 2-3-1.  When we try to create the alias, the "set" 
command has not been created and is therefore not part of cmdlist.  We 
can't find the "set inferior-tty" command, so the alias is not created.

I think the core issue is that it's currently possible to create 
subcommands for prefixes that have not been created yet.  We need some 
way of ensuring some kind of ordering.  Here are some ideas:


1. Force some kind of ordering through init.c

It's already done for at least one file, if you look at gdb/Makefile.in, 
you'll see that the gdbtypes file is put at the beginning, so that it's 
called first.  We could do the same with printcmd.  This is not a 
solution I would like in the long term, but maybe it's good for the 8.0 
release, as it would minimize the required changes.

2. Add dependencies between _initialize_functions

We could have a way for _initialize functions to call each other.  For 
example, since we know that _initialize_printcmd must be called before 
_initialize_infcmd, the latter could call the former.  A static flag in 
each function could ensure that each is called only once.

I don't like this one very much, since the dependencies between 
functions have to be manually deduced, they can become outdated and it's 
easy to forget some.

3. Create prefix commands on demand

Right now, if you want to create a prefix command under "set", you just 
refer to &setlist, regardless of whether the "set" command has been 
created or not yet.  Instead of referring directly to setlist, we could 
call something like set_prefix_list, which would create the "set" 
command if it hasn't been created yet, and return setlist.  Something 
like that:

   cmd_list_element **set_prefix_list ()
   {
     static cmd_list_element *setlist = NULL;
     bool initialized = false;

     if (!initialized)
       {
         // Create the set command
         add_prefix_cmd ("set", ..., &setlist, ...);
       }

     return &setlist;
   }

References to &setlist would be changed to set_prefix_list ().  This 
encodes the dependencies directly in the code: if want to add a 
subcommand to "set", you need to call set_prefix_list to obtain the list 
of "set"'s children, ensuring that "set" has been created.  So the deps 
can't get stale, or you can't miss one by mistake.

So far this is the approach I like the most for the master branch.  
There are many ways the code could be improved further (C++, better 
encapsulation), but this would be a manageable first step.  But for the 
immediate needs of the imminent 8.0 release, I think #1 would be better.

Any comments?

Simon