Re: [PATCH] [gdb/tdep] Fix gdb.base/watchpoint-running on {arm, ppc64le}-linux

[Luis Machado <luis.machado@arm.com>]

On 6/13/24 10:07, Tom de Vries wrote:
> On 6/7/24 14:05, Tom de Vries wrote:
>> On 6/7/24 12:18, Luis Machado wrote:
>>> On 6/7/24 07:35, Tom de Vries wrote:
>>>> When running test-case gdb.base/watchpoint-running on ppc64le-linux, we get:
>>>> ...
>>>> (gdb) watch global_var^M
>>>> warning: Error when detecting the debug register interface. \
>>>>    Debug registers will be unavailable.^M
>>>> Watchpoint 2: global_var^M
>>>> (gdb) FAIL: $exp: all-stop: hardware: watch global_var
>>>> FAIL: $exp: all-stop: hardware: watchpoint hit (timeout)
>>>> ...
>>>>
>>>> The problem is that ppc_linux_dreg_interface::detect fails to detect the
>>>> hardware watchpoint interface, because the calls to ptrace return with errno
>>>> set to ESRCH.
>>>>
>>>> This is a feature of ptrace: if a call is done while the tracee is not
>>>> ptrace-stopped, it returns ESRCH.
>>>>
>>>> Indeed, in the test-case "watch global_var" is executed while the inferior is
>>>> running, and that triggers the first call to ppc_linux_dreg_interface::detect.
>>>>
>>>> And because the detection failure is cached, subsequent attempts at setting
>>>> hardware watchpoints will also fail, even if the tracee is ptrace-stopped.
>>>>
>>>> Fix this by calling target_can_use_hardware_watchpoint from
>>>> linux_init_ptrace_procfs, which is called from both:
>>>> - linux_nat_target::post_attach, and
>>>> - linux_nat_target::post_startup_inferior.
>>>>
>>>> By fixing this here, we also fix the same problem for arm-linux.
>>>>
>>>> Tested on ppc64le-linux and arm-linux.
>>>>
>>>> PR tdep/31834
>>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31834
>>>> PR tdep/31705
>>>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31705
>>>> ---
>>>>   gdb/linux-nat.c | 12 ++++++++++++
>>>>   1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
>>>> index c95d420d416..d8b5a99269b 100644
>>>> --- a/gdb/linux-nat.c
>>>> +++ b/gdb/linux-nat.c
>>>> @@ -454,6 +454,18 @@ linux_init_ptrace_procfs (pid_t pid, int attached)
>>>>     linux_ptrace_init_warnings ();
>>>>     linux_proc_init_warnings ();
>>>>     proc_mem_file_is_writable ();
>>>> +
>>>> +  /* Some targets (for instance ppc and arm) may call ptrace to answer a
>>>> +     target_can_use_hardware_watchpoint query, and cache the result.  However,
>>>> +     the ptrace call will fail with errno ESRCH if the tracee is not
>>>> +     ptrace-stopped, making the query fail.  And if the caching mechanism does
>>>> +     not disregard an ESRCH result, all subsequent queries will also fail.
>>>> +     Call it now, where we known the tracee is ptrace-stopped.
>>>> +
>>>> +     Other targets (for instance aarch64) do the relevant ptrace call and
>>>> +     caching in their implementation of post_attach and post_startup_inferior,
>>>> +     in which case this call is expected to have no effect.  */
>>>> +  target_can_use_hardware_watchpoint (bp_hardware_watchpoint, 1, 0);
>>>>   }
>>>>   linux_nat_target::~linux_nat_target ()
>>>>
>>>> base-commit: f9478936896ada7786e8d68622f6e6ff78b97b0d
>>>
>>> Looks good from arm-linux's side. Thanks!
>>>
>>> Reviewed-By: Luis Machado <luis.machado@arm.com>
>>> Tested-By: Luis Machado <luis.machado@arm.com>
>>
>> Luis, thanks for the review and the testing.
>>
>> Pedro, since you reviewed a target-specific patch for the arm PR, any comments?
> 
> And just to mention this, I'd like to backport this fix to the gdb-15-branch.
> 
> Thanks,
> - Tom
> 

That's OK by me from arm's side.