Re: GDB 7.99.91 available for testing

Simon Marchi
2017-05-16T13:51:45Z

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