Re: [RFA/7.8] user breakpoint not inserted if software-single-step at same location

[Pedro Alves <palves@redhat.com>]

On 06/03/2014 02:35 PM, Joel Brobecker wrote:
> Hi Pedro,
> 
>>> Bah, I woke up realizing that the version I posted forgets to
>>> clone the shadow buffer!  Let me fix that and repost...
> 
> You are producing patches so fast, I am wondering if I will be able
> to keep up! :-)

:-)

>> @@ -15138,12 +15196,30 @@ deprecated_insert_raw_breakpoint (struct gdbarch *gdbarch,
>>  				  struct address_space *aspace, CORE_ADDR pc)
>>  {
>>    struct bp_target_info *bp_tgt;
>> +  struct bp_location *bl;
>>  
>>    bp_tgt = XCNEW (struct bp_target_info);
>>  
>>    bp_tgt->placed_address_space = aspace;
>>    bp_tgt->placed_address = pc;
>>  
>> +  /* If an unconditional non-raw breakpoint is already inserted at
>> +     that location, there's no need to insert another.  However, with
>> +     target-side evaluation of breakpoint conditions, if the
>> +     breakpoint that is currently inserted on the target is
>> +     conditional, we need to make it unconditional.  Note that a
>> +     breakpoint with target-side commands is not reported even if
>> +     unconditional, so we need to remove the commands from the target
>> +     as well.  */
>> +  bl = find_non_raw_software_breakpoint_inserted_here (aspace, pc);
>> +  if (bl != NULL
>> +      && VEC_empty (agent_expr_p, bl->target_info.conditions)
>> +      && VEC_empty (agent_expr_p, bl->target_info.tcommands))
>> +    {
>> +      bp_target_info_copy_insertion_state (bp_tgt, &bl->target_info);
>> +      return bp_tgt;
>> +    }
>> +
> 
> ISTM that you are assuming that there would only be one other breakpoint
> inserted at this location. What if there were more?

Yep, it's a valid assumption.  Only one of those can be the one that
is actually inserted in the target.  All others breakpoints are considered
duplicates, with bl->duplicate == 1 and bl->inserted == 0, and never reach
the target.  The duplicate location logic in the tail of update_global_location_list
takes care of it:

      /* This and the above ensure the invariant that the first location
	 is not duplicated, and is the inserted one.
	 All following are marked as duplicated, and are not inserted.  */
      if (loc->inserted)
	swap_insertion (loc, *loc_first_p);
      loc->duplicate = 1;

The one that is inserted will hold a merge of all the agent
expressions (in target_info.conditions and target_info.tcommands) of
the target-side conditions and commands of all breakpoints at that
address.  Those are computed just before that single breakpoint
is inserted (build_target_condition_list, build_target_command_list).

-- 
Pedro Alves