Re: [PATCH] [gdbserver] Disable conditional breakpoints on no-hardware-single-step targets

[Antoine Tremblay <antoine.tremblay@ericsson.com>]

On 05/08/2015 08:29 AM, Pedro Alves wrote:
> On 05/08/2015 01:12 PM, Antoine Tremblay wrote:
>
>> This looks very nice thanks! , but I do have one question , why is the
>> result a VEC ?
>>
>>   From the context and current code won't we have only one next instruction ?
>
> Nope.  Most frequent case is conditional branches where we don't know
> where the program will end up.  Might be the destination of the branch,
> if the instruction evals true, or after the branch, if the condition evals false.
> Even though the arm code manages to evaluate most conditions itself upfront,
> there are still some cases where it can't.  The way we handle it currently
> is that the get_next_pc functions call insert extra single-step breakpoints
> themselves, like e.g., in thumb_get_next_pc_raw:
>
> 	  else
> 	    {
> 	      int cond_negated;
>
> 	      /* There are conditional instructions after this one.
> 		 If this instruction modifies the flags, then we can
> 		 not predict what the next executed instruction will
> 		 be.  Fortunately, this instruction is architecturally
> 		 forbidden to branch; we know it will fall through.
> 		 Start by skipping past it.  */
> 	      pc += thumb_insn_size (inst1);
> 	      itstate = thumb_advance_itstate (itstate);
>
> 	      /* Set a breakpoint on the following instruction.  */
> 	      gdb_assert ((itstate & 0x0f) != 0);
> 	      arm_insert_single_step_breakpoint (gdbarch, aspace,
> 						 MAKE_THUMB_ADDR (pc));
> 	      cond_negated = (itstate >> 4) & 1;
>
>
> So you see how this is a misleading/surprising interface, naturally
> something that grew organically instead of being designed for
> multiple potential destinations.
>

Hooo , right I really though it could evaluate it upfront...seems like I 
had not read the function in enough detail yet :)

> Another case where the ARM code (and others like PPC) need more than
> one "next pc" is when dealing with atomic sequences.   See e.g.,
> arm_deal_with_atomic_sequence_raw. gdbserver needs all that
> atomic sequence code too.
>
Humm ok I will take a look into this too.

>>
>> Also, if you may,file structure wise, where would be a good place for
>> this abstration layer in your view ?
>
> Good question.  Maybe a new gdb/arch/ directory.  But I'd be fine
> with putting it in gdb/common/ for now.
>

Good, thanks a lot for your help, really saved me a quite a few hours!  :)

Regards,
Antoine