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

[Simon Marchi <simark@simark.ca>]

On 12/16/22 09:07, Jan Vraný wrote:
> 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 ().

I ran the patch through my CI job, looks good.  You can add my

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon