[patch/rfc] Disable [eE] packets by default

[patch/rfc] Disable [eE] packets by default

[Andrew Cagney]

[-- Attachment #1: Type: text/plain, Size: 345 bytes --]

Hello,

This is a followup to the current discussion regarding the [Ee] packets. 
  It disables them by default.

While it is a brutal solution to a problem, it is also 6 days before 5.2 
branches.  It also clears the slate for another attempt at this.

Thoughts?
Andrew

PS: There is no doco patch.  This packet was never documented.  Sigh :-(

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 1251 bytes --]

2002-02-24  Andrew Cagney  <ac131313@redhat.com>

	* remote.c (_initialize_remote): By default, disable ``e'' and
	``E'' step out-of-range packets.

Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.76
diff -u -r1.76 remote.c
--- remote.c	2002/02/10 04:08:42	1.76
+++ remote.c	2002/02/25 00:19:23
@@ -6099,6 +6099,10 @@
 			 show_remote_protocol_e_packet_cmd,
 			 &remote_set_cmdlist, &remote_show_cmdlist,
 			 0);
+  /* Disable by default.  The ``e'' packet has nasty interactions with
+     the threading code - it relies on global state.  */
+  remote_protocol_e.detect = CMD_AUTO_BOOLEAN_FALSE;
+  update_packet_config (&remote_protocol_e);
 
   add_packet_config_cmd (&remote_protocol_E,
 			 "E", "step-over-range-w-signal",
@@ -6106,6 +6110,10 @@
 			 show_remote_protocol_E_packet_cmd,
 			 &remote_set_cmdlist, &remote_show_cmdlist,
 			 0);
+  /* Disable by default.  The ``e'' packet has nasty interactions with
+     the threading code - it relies on global state.  */
+  remote_protocol_E.detect = CMD_AUTO_BOOLEAN_FALSE;
+  update_packet_config (&remote_protocol_E);
 
   add_packet_config_cmd (&remote_protocol_P,
 			 "P", "set-register",

Re: [patch/rfc] Disable [eE] packets by default

[Michael Snyder]

Andrew Cagney wrote:
> 
> Hello,
> 
> This is a followup to the current discussion regarding the [Ee] packets.
>   It disables them by default.
> 
> While it is a brutal solution to a problem, it is also 6 days before 5.2
> branches.  It also clears the slate for another attempt at this.
> 
> Thoughts?

I think this is the right thing to do.  Previously we had
no way to test the 'e' packet.  Now that we do have, it is
seen to be buggy.  This is like an unreliable optimization.
You can give the user the opportunity to use it if he chooses, 
but you don't turn it on by default.

I would like to start discussing the right way to implement this.
For one thing, the current implementation has remote.c snatching
control away from infrun, and doing something completely 
different from what infrun asked it to do.  I'd like to have
the decision about whether to use step-over-range made in
infrun, not in the target layer.


> Andrew
> 
> PS: There is no doco patch.  This packet was never documented.  Sigh :-(
> 
>     ---------------------------------------------------------------
> 2002-02-24  Andrew Cagney  <ac131313@redhat.com>
> 
>         * remote.c (_initialize_remote): By default, disable ``e'' and
>         ``E'' step out-of-range packets.
> 
> Index: remote.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote.c,v
> retrieving revision 1.76
> diff -u -r1.76 remote.c
> --- remote.c    2002/02/10 04:08:42     1.76
> +++ remote.c    2002/02/25 00:19:23
> @@ -6099,6 +6099,10 @@
>                          show_remote_protocol_e_packet_cmd,
>                          &remote_set_cmdlist, &remote_show_cmdlist,
>                          0);
> +  /* Disable by default.  The ``e'' packet has nasty interactions with
> +     the threading code - it relies on global state.  */
> +  remote_protocol_e.detect = CMD_AUTO_BOOLEAN_FALSE;
> +  update_packet_config (&remote_protocol_e);
> 
>    add_packet_config_cmd (&remote_protocol_E,
>                          "E", "step-over-range-w-signal",
> @@ -6106,6 +6110,10 @@
>                          show_remote_protocol_E_packet_cmd,
>                          &remote_set_cmdlist, &remote_show_cmdlist,
>                          0);
> +  /* Disable by default.  The ``e'' packet has nasty interactions with
> +     the threading code - it relies on global state.  */
> +  remote_protocol_E.detect = CMD_AUTO_BOOLEAN_FALSE;
> +  update_packet_config (&remote_protocol_E);
> 
>    add_packet_config_cmd (&remote_protocol_P,
>                          "P", "set-register",

Re: [patch/rfc] Disable [eE] packets by default

[Frank Ch. Eigler]

msnyder wrote:

> [...]  I think this is the right thing to do.  [...]

OK.  Can you or Andrew explain the reference to "threading code" in
the patch comments?

> > +  /* Disable by default.  The ``e'' packet has nasty interactions with
> > +     the threading code - it relies on global state.  */
> > +  remote_protocol_E.detect = CMD_AUTO_BOOLEAN_FALSE;
> > +  update_packet_config (&remote_protocol_E);



> I would like to start discussing the right way to implement this.
> For one thing, the current implementation has remote.c snatching
> control away from infrun, and doing something completely 
> different from what infrun asked it to do.  

Right - it's interpreting the step=1 argument to remote_resume not
just as "stepi" but as "step if you can, stepi if you can't".


> I'd like to have the decision about whether to use step-over-range
> made in infrun, not in the target layer.

Yup.  One less-pessimistic approach would be to clear step_range_end
and step_range_start in proceed(), if the breakpoint_here_p()
condition is true.  This would mean that targets would have to do
stepi loops when stepping off of a breakpoint, but otherwise could use
step-over-range.


- FChE

Re: [patch/rfc] Disable [eE] packets by default

[Andrew Cagney]

> Andrew Cagney wrote:
> 
>> 
>> Hello,
>> 
>> This is a followup to the current discussion regarding the [Ee] packets.
>> It disables them by default.
>> 
>> While it is a brutal solution to a problem, it is also 6 days before 5.2
>> branches.  It also clears the slate for another attempt at this.
>> 
>> Thoughts?
> 
> 
> I think this is the right thing to do.  Previously we had
> no way to test the 'e' packet.  Now that we do have, it is
> seen to be buggy.  This is like an unreliable optimization.
> You can give the user the opportunity to use it if he chooses, 
> but you don't turn it on by default.


It is in.  We're now free to figure out how to really make this work :-)

> I would like to start discussing the right way to implement this.
> For one thing, the current implementation has remote.c snatching
> control away from infrun, and doing something completely 
> different from what infrun asked it to do.  I'd like to have
> the decision about whether to use step-over-range made in
> infrun, not in the target layer.

Yes, definitly.  I suspect step-out-of-range is a special case of step. 
  I think the decision to try to use both should be made by infrun.c.

As RichardE recently re-discovered, the way single-step is structured is 
similarly wierd.  Fortunatly the consequences are not so bad.

Andrew

Re: [patch/rfc] Disable [eE] packets by default

[Michael Snyder]

Andrew Cagney wrote:
> 
> > Andrew Cagney wrote:
> >
> >>
> >> Hello,
> >>
> >> This is a followup to the current discussion regarding the [Ee] packets.
> >> It disables them by default.
> >>
> >> While it is a brutal solution to a problem, it is also 6 days before 5.2
> >> branches.  It also clears the slate for another attempt at this.
> >>
> >> Thoughts?
> >
> >
> > I think this is the right thing to do.  Previously we had
> > no way to test the 'e' packet.  Now that we do have, it is
> > seen to be buggy.  This is like an unreliable optimization.
> > You can give the user the opportunity to use it if he chooses,
> > but you don't turn it on by default.
> 
> It is in.  We're now free to figure out how to really make this work :-)
> 
> > I would like to start discussing the right way to implement this.
> > For one thing, the current implementation has remote.c snatching
> > control away from infrun, and doing something completely
> > different from what infrun asked it to do.  I'd like to have
> > the decision about whether to use step-over-range made in
> > infrun, not in the target layer.
> 
> Yes, definitly.  I suspect step-out-of-range is a special case of step.
>   I think the decision to try to use both should be made by infrun.c.

I'm picturing a "to_step_over_range" target vector, and the decision
about whether or not to use it would be made in infrun/resume, 
right about the same place where it decides whether to use step or
continue.