Re: [PATCH] gdb: fix command lookup in execute_command ()

["Jan Vraný" <Jan.Vrany@labware.com>]

On Tue, 2022-12-13 at 14:29 -0500, Simon Marchi wrote:
> On 12/13/22 13:48, Jan Vraný wrote:
> > On Tue, 2022-12-13 at 08:05 -0700, Tom Tromey wrote:
> > > > > > > > "Jan" == Jan Vrany via Gdb-patches <gdb-patches@sourceware.org> writes:
> > > 
> > > Jan> The below patch should fix this case - with it on my system
> > > Jan> gdb.base/define.exp passes.
> > > 
> > > Jan> Alternatively I may just revert the commit until better solution
> > > Jan> is found.
> > > 
> > > This patch looks ok to me, but could you say whether you ran all the
> > > tests or just define.exp?  Given the history I think a full regression
> > > test is warranted.
> > 
> > I did use try-build which succeeded (build #28) but it seems to me it
> > runs only very limited number of tests. 
> > 
> > I did run all of gdb.base and did not spot any new regression compared
> > to master with my (previous, broken) patch reverted. I did not run more
> > than gdb.base mainly because just gdb.base takes about an hour on my machine.
> > Also I get weird intermittent failures when running testsuite. 
> > 
> > Jan
> 
> Here are the unexpected failures I saw:
> 
> UNRESOLVED: gdb.base/bp-cmds-execution-x-script.exp: run to end
> UNRESOLVED: gdb.base/bp-cmds-run-with-ex.exp: execute bp commands
> UNRESOLVED: gdb.base/bp-cmds-sourced-script.exp: source the script
> ...
> UNRESOLVED: gdb.trace/save-trace.exp: absolute: read back saved tracepoints
> UNRESOLVED: gdb.trace/save-trace.exp: absolute: verify recovered tracepoints
> DUPLICATE: gdb.trace/save-trace.exp: absolute: verify recovered tracepoints
> UNRESOLVED: gdb.trace/save-trace.exp: absolute: verify default-collect
> UNRESOLVED: gdb.trace/save-trace.exp: verify help save tracepoints
> 
> 

I just rebased the below fix atop of Simon's buffer patch
(f8631e5e "gdb: remove static buffer in command_line_input")
and on my machine with ASan-enabled gdb:

  * all of gdb.base pass except 8 tests are shaky (they failed
    even before any of these changes)
  * gdb.python/py-cmd.exp passes

But given the history, I'm far from being sure...

Jan

-- >8 --
 Subject: [PATCH] gdb: fix command lookup in execute_command ()
 
 Commit b5661ff2 ("gdb: fix possible use-after-free when
 executing commands") used lookup_cmd_exact () to lookup
 command again after its execution to avoid possible
 use-after-free error.
 
 However this change broke test gdb.base/define.exp which
 defines a post-hook for subcommand ("target testsuite").
 In this case,  lookup_cmd_exact () returned NULL because
 there's no command 'testsuite' in top-level commands.
 
 This commit fixes this case by looking up the command again
 using the original command line via lookup_cmd ().
 ---
  gdb/top.c | 8 ++------
  1 file changed, 2 insertions(+), 6 deletions(-)
 
 diff --git a/gdb/top.c b/gdb/top.c
 index 742997808bd..caa08d98cec 100644
 --- a/gdb/top.c
 +++ b/gdb/top.c
 @@ -655,11 +655,6 @@ execute_command (const char *p, int from_tty)
  	    }
  	}
  
 -      /* Remember name of the command.  This is needed later when
 -	 executing command post-hooks to handle the case when command
 -	 is redefined or removed during it's execution.  See below.  */
 -      std::string c_name (c->name);
 -
        /* If this command has been pre-hooked, run the hook first.  */
        execute_cmd_pre_hook (c);
  
 @@ -702,7 +697,8 @@ execute_command (const char *p, int from_tty)
  	 We need to lookup the command again since during its execution,
  	 a command may redefine itself.  In this case, C pointer
  	 becomes invalid so we need to look it up again.  */
 -      c = lookup_cmd_exact (c_name.c_str (), cmdlist);
 +      const char *cmd2 = cmd_start;
 +      c = lookup_cmd (&cmd2, cmdlist, "", nullptr, 1, 1);
        if (c != nullptr)
  	execute_cmd_post_hook (c);