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

[Pedro Alves <pedro@palves.net>]

On 2024-06-17 19:22, Tom de Vries wrote:
> On 6/14/24 18:49, Pedro Alves wrote:

>> And, from another angle, why isn't aarch64 doing the same, why two mechanisms?
> 
> Well, the patch adds a fallback, that aarch64 doesn't need, but that powerpc and arm do need.  
> 

Aarch64 absolutely needs it, it's just that it already has the fix in place (by checking early in
post_startup_inferior/post_attach).  We're adding code to the other backends to handle it too, but using a
somewhat different solution.  If arm / ppc were being adjusted to use the same approach as aarch64 (like in a
previous patch in bugzilla), then I wouldn't have asked that question.  I see no good reason for multiple ways
of doing the same thing.

> There might be other targets that needs such a fallback, but that we don't know about.

We have a testcase that will show us if so.

> So, I don't mind your patch, it's certainly cleaner, but I don't mind a functional default implementation either.  
> So, I'd move the target_can_use_hardware_watchpoint call to the default implementation of low_init_process.

I'd rather not.  Let low level handle low level things using low level details.

> I ran into trouble building the patch due to type of pid parameter mismatches.
> 
> After fixing that, I ran into trouble on ppc64le, because low_prepare_to_resume is called before low_init_process.  

I guess it's while running through the shell, now that I think about it.

> I fixed that by sinking this code in the function a bit:
> ...
>   if (m_dreg_interface.unavailable_p ())
>     return;
> ...

Makes sense.

> 
> And after fixing this, I still ran into failures and identified at least two more locations that needed fixing due to the cleanup, at which point I decided that the cleanup part is out-of-scope for the patch fixing the PR.

OK.

> 
> So, this is what I have tested on x86_64-linux, aarch64-linux, arm-linux and ppc64le-linux.

OK, let's go with this, then.  Thank for testing!

Pedro Alves