Re: [PATCH v3 3/7] Catch gdb_exception_error instead of gdb_exception (in many places)

[Pedro Alves <pedro@palves.net>]

Hi Kevin,

I've started giving the series an initial skim, but haven't read it all yet.  Meanwhile,
here's one spot I noticed isn't correct.

On 2022-02-27 00:00, Kevin Buettner wrote:
> --- a/gdb/cli/cli-interp.c
> +++ b/gdb/cli/cli-interp.c
> @@ -368,7 +368,7 @@ safe_execute_command (struct ui_out *command_uiout, const char *command,
>      {
>        execute_command (command, from_tty);
>      }
> -  catch (gdb_exception &exception)
> +  catch (gdb_exception_error &exception)
>      {
>        e = std::move (exception);
>      }

This one here, as is, can leave things in a messed up state.  Note that the function returns
the exception.  And then the caller does this:

gdb_exception
cli_interp::exec (const char *command_str)
{
  struct cli_interp *cli = this;
  struct ui_file *old_stream;
  struct gdb_exception result;

  /* gdb_stdout could change between the time cli_uiout was
     initialized and now.  Since we're probably using a different
     interpreter which has a new ui_file for gdb_stdout, use that one
     instead of the default.

     It is important that it gets reset everytime, since the user
     could set gdb to use a different interpreter.  */
  old_stream = cli->cli_uiout->set_stream (gdb_stdout);
  result = safe_execute_command (cli->cli_uiout, command_str, 1);
  cli->cli_uiout->set_stream (old_stream);       <<<<<<<<<<<< 
  return result;
}

Note the "<<<<" line.  If a QUIT exception propagates, the line doesn't execute.

Looking at the caller higher up the in chain, we get to interpreter_exec_cmd,
which does:

  for (i = 1; i < nrules; i++)
    {
      struct gdb_exception e = interp_exec (interp_to_use, prules[i]);

      if (e.reason < 0)
	{
	  interp_set (old_interp, 0);
	  error (_("error in command: \"%s\"."), prules[i]);
	}
    }

That throws plain error, so it won't be sufficient to just revert your hunk, as that
would convert a quit to a plain error.  I don't know off hand why
we need to have safe_execute_command and interp_exec catch the exception and
return it via the normal return path.  That's normally needed when we need to
cross C code, like readline or ncurses, but that's not the case here.  Maybe
we can just eliminate safe_execute_command and let exceptions propagate normally.