[1/5] Support multiple breakpoint types per target in GDBServer.
Commit Message
Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>> PC can't be NULL after your patch #2. You can remove the second
>> sentence in this patch or patch #2.
>>
> I think you mean after patch #3 ?
>
> But it can still be NULL see in #3 :
>
> /* Default if no pc is set to arm breakpoint. */
> + if (pcptr == NULL)
> + {
> + *lenptr = arm_breakpoint_len;
> + return (unsigned char *) &arm_breakpoint;
> + }
I meant this change below in patch #2,
We only pass NULL to breakpoint_from_pc here, and we remove it from
patch #2. That is why I suggest that PCPTR can't be NULL.
[I am still reviewing this patch series. I get something I can't
explain after I enable thread event breakpoint in order to exercise
your patches. I'll send out my comments once I understand the them
fully.]
Comments
On 09/23/2015 10:46 AM, Yao Qi wrote:
> Antoine Tremblay <antoine.tremblay@ericsson.com> writes:
>
>>> PC can't be NULL after your patch #2. You can remove the second
>>> sentence in this patch or patch #2.
>>>
>> I think you mean after patch #3 ?
>>
>> But it can still be NULL see in #3 :
>>
>> /* Default if no pc is set to arm breakpoint. */
>> + if (pcptr == NULL)
>> + {
>> + *lenptr = arm_breakpoint_len;
>> + return (unsigned char *) &arm_breakpoint;
>> + }
>
> I meant this change below in patch #2,
>
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index bb08761..06387a0 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -7069,16 +7069,10 @@ void
> initialize_low (void)
> {
> struct sigaction sigchld_action;
> - int breakpoint_len = 0;
> - const unsigned char *breakpoint = NULL;
>
> memset (&sigchld_action, 0, sizeof (sigchld_action));
> set_target_ops (&linux_target_ops);
>
> - breakpoint = the_target->breakpoint_from_pc (NULL, &breakpoint_len);
> -
> - set_breakpoint_data (breakpoint,
> - breakpoint_len);
> linux_init_signals ();
> linux_ptrace_init_warnings ();
>
> We only pass NULL to breakpoint_from_pc here, and we remove it from
> patch #2. That is why I suggest that PCPTR can't be NULL.
>
Ok I see, indeed it is never called again with NULL, good point I'll
remove that and also remove the default NULL handling in the arm
implementation.
> [I am still reviewing this patch series. I get something I can't
> explain after I enable thread event breakpoint in order to exercise
> your patches. I'll send out my comments once I understand the them
> fully.]
>
Ok thank you, note I'm on IRC as hexa if you ever need some quick answer.
@@ -7069,16 +7069,10 @@ void
initialize_low (void)
{
struct sigaction sigchld_action;
- int breakpoint_len = 0;
- const unsigned char *breakpoint = NULL;
memset (&sigchld_action, 0, sizeof (sigchld_action));
set_target_ops (&linux_target_ops);
- breakpoint = the_target->breakpoint_from_pc (NULL, &breakpoint_len);
-
- set_breakpoint_data (breakpoint,
- breakpoint_len);
linux_init_signals ();
linux_ptrace_init_warnings ();