Re: [PATCH] breakpoint: Make sure location types match before swapping

[Simon Marchi <simark@simark.ca>]

On 2020-04-14 3:01 a.m., Keno Fischer wrote:
> Bump. It would be great to get this fixed.
> 
> On Tue, Mar 31, 2020 at 9:38 PM Keno Fischer <keno@juliacomputing.com> wrote:
>>
>> This patch fixes PR gdb/25741 "GDB tries to set breakpoint using Z0, but remove it using z1".
>> In particular, what occurs in that case is that a hardware breakpoint is hit,
>> after which GDB removes it and establishes a single step breakpoint at the
>> same location. Afterwards, rather than simply removing this breakpoint and
>> re-enabling the hardware breakpoint, GDB simply swaps the activation, without
>> informing the server, leading to an inconsistency in GDB's view of the world
>> and the server's view of the world. To remidy this situation, this
>> patch adds a check that ensures two breakpoint locations have the
>> same type before they are considered equal and thus eligible for silent
>> swapping.
>>
>> gdb/ChangeLog:
>>         * breakpoint.c (breakpoint_locations_match): Fix PR gdb/25741
>>
>> Signed-off-by: Keno Fischer <keno@juliacomputing.com>
>> ---
>>  gdb/breakpoint.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
>> index e49025461b..582dae1946 100644
>> --- a/gdb/breakpoint.c
>> +++ b/gdb/breakpoint.c
>> @@ -6838,7 +6838,7 @@ breakpoint_locations_match (struct bp_location *loc1,
>>      /* We compare bp_location.length in order to cover ranged breakpoints.  */
>>      return (breakpoint_address_match (loc1->pspace->aspace, loc1->address,
>>                                      loc2->pspace->aspace, loc2->address)
>> -           && loc1->length == loc2->length);
>> +           && loc1->length == loc2->length && loc1->loc_type == loc2->loc_type);
>>  }
>>
>>  static void
>> --
>> 2.24.0
>>

I think the change makes sense, but this is not an area I know well, and it's one that
is a bit sensitive.  I'll do a full test run and take a bit more time to look at it.

In the mean time, if anybody else wants to take a look, go for it.

Simon