Another proposal for frontends and queries.

Another proposal for frontends and queries.

[Marc Khouzam]

Hi,

I'm starting a new thread in an attempt to get this issue resolved before 7.0.

Three-line recap of the issue:

For some frontends queries are answered automatically by GDB.
Because of this, some actions that the frontends wants to trigger
are automatically cancelled by GDB.

More details, for those less familiar:

When using Eclipse, our connection through GDB is done so that the method
input_from_terminal_p() returns false.  Because of this, all queries are
answered automatically by GDB.  Queries with no default answer, as well
as queries with a default of 'y' are both automatically answered 'y';
but queries that default to 'n' are automatically answered 'n'.

The problem is that in some cases (specifically for PRecord), an
automatic answer of "no" will prevent an operation that the frontend
wants to do, from being performed.  To my knowledge, there is 
currently no way for a frontend to get around this limitation.

The issue if further complicated in the fact that a specific query
can be triggered by different commands.  Specifically, one of the
PRecord queries gets triggered when we attempt to change memory,
which can be from different MI or CLI commands.

Proposals:

One proposal was to add a way to turn off queries for PRecord.
http://sourceware.org/ml/gdb-patches/2009-09/msg00318.html
This was based on the fact that the only other nquery() was for
pending breakpoints, which has its own option to deal with that query.
This solves the current problem but is not future-proof.

Another proposal that was thrown out there was to disable queries
when having started GDB with the MI interpreter.  The idea is that
it would be frontends that would start GDB like that, and frontends
know what they are doing and don't need queries.
http://sourceware.org/ml/gdb/2009-07/msg00113.html
This seems too drastic to me.

Another solution was that when "set confirm" was "off" all queries
would be answered 'y' instead of using their respective default
answer.
http://sourceware.org/ml/gdb/2009-07/msg00150.html
This did not seem very popular.

My latest idea, based on the reactions to the previous suggestions
is to extend "set confirm" and add a new "force" option.
set confirm on/off would remain as before
set confirm force would automatically answer 'y' to all queries.

The patch below illustrates this latest suggestion.
I did the doc changes and NEWS, to help with making this clear.
I will add a Changelog if this solution is selected for inclusion.
(I'm not very comfortable with the declaration of new variables in
this patch, but the patch does illustrates things.)

Thanks for any help to get this resolved.

Marc

### Eclipse Workspace Patch 1.0
#P src
Index: gdb/top.h
===================================================================
RCS file: /cvs/src/src/gdb/top.h,v
retrieving revision 1.19
diff -u -r1.19 top.h
--- gdb/top.h	31 Aug 2009 20:18:45 -0000	1.19
+++ gdb/top.h	14 Sep 2009 02:27:12 -0000
@@ -27,7 +27,11 @@
 extern int linesize;
 extern FILE *instream;
 extern int in_user_command;
-extern int caution;
+extern const char *confirm_mode_string;
+extern const char confirm_on[];
+extern const char confirm_off[];
+extern const char confirm_force[];
+
 extern char gdb_dirbuf[1024];
 extern int inhibit_gdbinit;
 extern int epoch_interface;
Index: gdb/top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.171
diff -u -r1.171 top.c
--- gdb/top.c	10 Sep 2009 18:56:45 -0000	1.171
+++ gdb/top.c	14 Sep 2009 02:27:12 -0000
@@ -96,13 +96,25 @@
 
 /* Flag for whether we want all the "from_tty" gubbish printed.  */
 
-int caution = 1;		/* Default is yes, sigh. */
+const char confirm_on[] = "on";
+const char confirm_off[] = "off";
+const char confirm_force[] = "force";
+
+static const char *confirm_kind_names[] = {
+  confirm_on,
+  confirm_off,
+  confirm_force,
+  NULL 
+};
+
+const char *confirm_mode_string = confirm_on;  /* Default is on, sigh. */
+
 static void
-show_caution (struct ui_file *file, int from_tty,
+show_confirm (struct ui_file *file, int from_tty,
 	      struct cmd_list_element *c, const char *value)
 {
   fprintf_filtered (file, _("\
-Whether to confirm potentially dangerous operations is %s.\n"),
+Whether to confirm potentially dangerous operations is set to %s.\n"),
 		    value);
 }
 
@@ -293,8 +305,8 @@
 /* static */ int
 quit_cover (void *s)
 {
-  caution = 0;			/* Throw caution to the wind -- we're exiting.
-				   This prevents asking the user dumb questions.  */
+  confirm_mode_string = confirm_off; /* Throw caution to the wind -- we're exiting.
+					This prevents asking the user dumb questions.  */
   quit_command ((char *) 0, 0);
   return 0;
 }
@@ -444,13 +456,13 @@
       if (c->class == class_user)
 	execute_user_command (c, arg);
       else if (c->type == set_cmd || c->type == show_cmd)
-	do_setshow_command (arg, from_tty & caution, c);
+	do_setshow_command (arg, from_tty && (confirm_mode_string == confirm_on), c);
       else if (!cmd_func_p (c))
 	error (_("That is not a command, just a help topic."));
       else if (deprecated_call_command_hook)
-	deprecated_call_command_hook (c, arg, from_tty & caution);
+	deprecated_call_command_hook (c, arg, from_tty && (confirm_mode_string == confirm_on));
       else
-	cmd_func (c, arg, from_tty & caution);
+	cmd_func (c, arg, from_tty && (confirm_mode_string == confirm_on));
        
       /* If this command has been post-hooked, run the hook last. */
       execute_cmd_post_hook (c);
@@ -1656,12 +1668,19 @@
 			    show_history_filename,
 			    &sethistlist, &showhistlist);
 
-  add_setshow_boolean_cmd ("confirm", class_support, &caution, _("\
+  add_setshow_enum_cmd ("confirm", class_support,
+			confirm_kind_names,
+			&confirm_mode_string, _("\
 Set whether to confirm potentially dangerous operations."), _("\
-Show whether to confirm potentially dangerous operations."), NULL,
-			   NULL,
-			   show_caution,
-			   &setlist, &showlist);
+Show whether to confirm potentially dangerous operations."), _("\
+confirm can be one of:\n\
+  on    - confirm potentially dangerous operations with a query\n\
+  off   - automatically answer queries with the query's default value\n\
+  force - automatically answer queries in the affirmative, so as to perform the operation\n\
+By default, confirm is on."),
+			NULL,
+			show_confirm,
+			&setlist, &showlist);
 
   add_setshow_zinteger_cmd ("annotate", class_obscure, &annotation_level, _("\
 Set annotation_level."), _("\
Index: gdb/NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.330
diff -u -r1.330 NEWS
--- gdb/NEWS	10 Sep 2009 21:02:46 -0000	1.330
+++ gdb/NEWS	14 Sep 2009 02:27:11 -0000
@@ -416,6 +416,15 @@
   answer.  When set to auto (the default), GDB determines which
   mode to use based on the stdin settings.
 
+set confirm (on|off|force)
+show confirm
+  Controls whether GDB should confirm potentially dangerous operations.
+  The 'on' and 'off' settings remain as for the previous release; the
+  new 'force' option is added which will cause GDB to automatically
+  answer 'y' to all queries.  This differs from the 'off' setting in the
+  fact that in the 'off' setting, queries are answered as per their
+  default answer.
+
 * Removed commands
 
 info forks
Index: gdb/utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.219
diff -u -r1.219 utils.c
--- gdb/utils.c	18 Aug 2009 16:17:16 -0000	1.219
+++ gdb/utils.c	14 Sep 2009 02:27:13 -0000
@@ -932,7 +932,7 @@
       /* Default (yes/batch case) is to quit GDB.  When in batch mode
 	 this lessens the likelihood of GDB going into an infinite
 	 loop.  */
-      if (caution == 0)
+      if (confirm_mode_string != confirm_on)
         {
           /* Emit the message and quit.  */
           fputs_unfiltered (reason, gdb_stderr);
@@ -1440,9 +1440,13 @@
       n_string = "[n]";
     }
 
+  /* Automatically answer 'y' if the confirm option is set to 'force' */
+  if (confirm_mode_string == confirm_force)
+    return 1;
+
   /* Automatically answer the default value if the user did not want
      prompts or the command was issued with the server prefix.  */
-  if (! caution || server_command)
+  if ( (confirm_mode_string == confirm_off) || server_command)
     return def_value;
 
   /* If input isn't coming from the user directly, just say what
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.622
diff -u -r1.622 gdb.texinfo
--- gdb/doc/gdb.texinfo	10 Sep 2009 21:03:07 -0000	1.622
+++ gdb/doc/gdb.texinfo	14 Sep 2009 02:27:30 -0000
@@ -3805,7 +3805,8 @@
 Delete the breakpoints, watchpoints, or catchpoints of the breakpoint
 ranges specified as arguments.  If no argument is specified, delete all
 breakpoints (@value{GDBN} asks confirmation, unless you have @code{set
-confirm off}).  You can abbreviate this command as @code{d}.
+confirm off} or @code{set confirm force}).  You can abbreviate this 
+command as @code{d}.
 @end table
 
 @node Disabling
@@ -18062,11 +18063,14 @@
 @cindex confirmation
 @cindex stupid questions
 @item set confirm off
-Disables confirmation requests.
+Disables confirmation requests and answers with the default answer for that confirmation.
 
 @item set confirm on
 Enables confirmation requests (the default).
 
+@item set confirm force
+Disables confirmation requests and always answers with the affirmative.
+
 @kindex show confirm
 @item show confirm
 Displays state of confirmation requests.
  

Re: Another proposal for frontends and queries.

[Vladimir Prus]

Marc Khouzam wrote:

> Hi,
> 
> I'm starting a new thread in an attempt to get this issue resolved before 7.0.
> 
> Three-line recap of the issue:
> 
> For some frontends queries are answered automatically by GDB.
> Because of this, some actions that the frontends wants to trigger
> are automatically cancelled by GDB.
...
> My latest idea, based on the reactions to the previous suggestions
> is to extend "set confirm" and add a new "force" option.
> set confirm on/off would remain as before
> set confirm force would automatically answer 'y' to all queries.

So, the current problem is that some queries for which the frontend
wants implicit 'yes' are answered as 'no'. You propose an option to
make all queries answered as 'yes'. What if frontend wants some query
answered as 'no'? 

- Volodya

RE: Another proposal for frontends and queries.

[Marc Khouzam]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Vladimir Prus
> Sent: September-14-09 2:42 AM
> To: gdb-patches@sources.redhat.com
> Subject: Re: Another proposal for frontends and queries.
> 
> Marc Khouzam wrote:
> 
> > Hi,
> > 
> > I'm starting a new thread in an attempt to get this issue 
> resolved before 7.0.
> > 
> > Three-line recap of the issue:
> > 
> > For some frontends queries are answered automatically by GDB.
> > Because of this, some actions that the frontends wants to trigger
> > are automatically cancelled by GDB.
> ...
> > My latest idea, based on the reactions to the previous suggestions
> > is to extend "set confirm" and add a new "force" option.
> > set confirm on/off would remain as before
> > set confirm force would automatically answer 'y' to all queries.
> 
> So, the current problem is that some queries for which the frontend
> wants implicit 'yes' are answered as 'no'. You propose an option to
> make all queries answered as 'yes'. What if frontend wants some query
> answered as 'no'? 

Currently, all queries use 'y' to mean "perform action" and 'n' to mean
"cancel action".  Therefore, if a frontend does not want an action
performed, it should simply not send the command that will trigger
the action/query.  So, currently, there is no reason for a frontend
to want to force a 'n' to a query.
In the future, if this case does come up, we will need a specific
option for such a new query, kind of like "show/set breakpoint pending".

But I'm open to a new suggestion.

Thanks,
Marc

Re: Another proposal for frontends and queries.

[Joel Brobecker]

> Another proposal that was thrown out there was to disable queries
> when having started GDB with the MI interpreter.  The idea is that
> it would be frontends that would start GDB like that, and frontends
> know what they are doing and don't need queries.
> http://sourceware.org/ml/gdb/2009-07/msg00113.html
> This seems too drastic to me.

Isn't this is the approach that has always been taken in the past?
I think that we should bite the bullet and implement this approach.
The only issue, really, is how to detect that situation from the FE
so that it can itself ask the user for confirmation before it sends
the command... Or perhaps, another approach is to return an error
(like it probably does now) that the frontend detect, and then provide
a switch for that command to force the execution of that command despite
the reported consequences.

-- 
Joel

RE: Another proposal for frontends and queries.

[Marc Khouzam]

 

> -----Original Message-----
> From: Joel Brobecker [mailto:brobecker@adacore.com] 
> Sent: Monday, September 14, 2009 10:09 AM
> To: Marc Khouzam
> Cc: gdb-patches@sourceware.org
> Subject: Re: Another proposal for frontends and queries.
> 
> > Another proposal that was thrown out there was to disable queries
> > when having started GDB with the MI interpreter.  The idea is that
> > it would be frontends that would start GDB like that, and frontends
> > know what they are doing and don't need queries.
> > http://sourceware.org/ml/gdb/2009-07/msg00113.html
> > This seems too drastic to me.
> 
> Isn't this is the approach that has always been taken in the past?
> I think that we should bite the bullet and implement this approach.

As long as some solution is chosen, I'll be happy :-)

> The only issue, really, is how to detect that situation from the FE
> so that it can itself ask the user for confirmation before it sends
> the command... 

If a FE wants to ask the user directly, it can do that no matter
what GDB will do.  So I don't think there is a problem here.  But
maybe I misundertood your point?

> Or perhaps, another approach is to return an error
> (like it probably does now) that the frontend detect, and then provide
> a switch for that command to force the execution of that 
> command despite
> the reported consequences.

Having a switch on a command turns out to be insufficient because
there is not a 1-1 mapping between queries and commands.  Currently
PRecord will query before changing memory, but this will be triggered
by any of:
-var-assign a 8
p a=8
set var a=8
and others.

Marc

Re: Another proposal for frontends and queries.

[Pedro Alves]

On Monday 14 September 2009 15:27:08, Marc Khouzam wrote:
> Having a switch on a command turns out to be insufficient because
> there is not a 1-1 mapping between queries and commands.  Currently
> PRecord will query before changing memory, but this will be triggered
> by any of:
> -var-assign a 8
> p a=8
> set var a=8
> and others.

To me, this all sounds like we should should revisit why precord uses
nquery at all.  If we stopped using nquery, and used query (defaulting
to the action a frontend might want), what would the user lose?
Having to type n + enter, instead of just enter to cancel, doesn't seem like
a bad deal to me.  Plus, wouldn't we want these particular offending
precord queries to default to 'y' even when in script/batch mode?

I think we should scrap out nquery from record.c, turning those to
query's, 'error's or normal stops, appropriately.  What would be missed?

There's issue of how does a frontend handle queries in a cli console
with -interpreter-exec cli (how does a frontend simulate queries
done on a a real CLI attached to a terminal), but that seems like
a mostly independent problem.

- 
Pedro Alves

RE: Another proposal for frontends and queries.

[Marc Khouzam]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Pedro Alves
> Sent: Monday, September 14, 2009 10:49 AM
> To: gdb-patches@sourceware.org
> Cc: Marc Khouzam; 'Joel Brobecker'
> Subject: Re: Another proposal for frontends and queries.
> 
> On Monday 14 September 2009 15:27:08, Marc Khouzam wrote:
> > Having a switch on a command turns out to be insufficient because
> > there is not a 1-1 mapping between queries and commands.  Currently
> > PRecord will query before changing memory, but this will be 
> triggered
> > by any of:
> > -var-assign a 8
> > p a=8
> > set var a=8
> > and others.
> 
> To me, this all sounds like we should should revisit why precord uses
> nquery at all.  If we stopped using nquery, and used query (defaulting
> to the action a frontend might want), what would the user lose?
> Having to type n + enter, instead of just enter to cancel, 
> doesn't seem like
> a bad deal to me.  Plus, wouldn't we want these particular offending
> precord queries to default to 'y' even when in script/batch mode?
> 
> I think we should scrap out nquery from record.c, turning those to
> query's, 'error's or normal stops, appropriately.  What would 
> be missed?

You got my vote.
 
> There's issue of how does a frontend handle queries in a cli console
> with -interpreter-exec cli (how does a frontend simulate queries
> done on a a real CLI attached to a terminal), but that seems like
> a mostly independent problem.

For Eclipse, we pretty much turn off queries for our CLI console.
It is not the best solution, but since we don't want queries
for our UI using MI, we don't have a choice.
But you are right that is another issue.

Thanks,
Marc

Re: Another proposal for frontends and queries.

[Joel Brobecker]

> I think we should scrap out nquery from record.c, turning those to
> query's, 'error's or normal stops, appropriately.  What would be missed?

I tend to agree. I reviewed the few calls to nquery, and it would
be more consistent with the rest of GDB if the default was 'y'
rather than 'n'.

-- 
Joel

Re: Another proposal for frontends and queries.

[Joel Brobecker]

> > The only issue, really, is how to detect that situation from the FE
> > so that it can itself ask the user for confirmation before it sends
> > the command... 
> 
> If a FE wants to ask the user directly, it can do that no matter
> what GDB will do.  So I don't think there is a problem here.  But
> maybe I misundertood your point?

Another way to look at what I was trying to say is to compare the
situation between GDB/CLI and GDB/MI:

  1. In the first case, if you do:

       (gdb) print a = 2

     You should get a query from the debugger informing you that you
     are about to change some memory and therefore lose the record log.
     Are you sure?

  2. In the second case, assuming that the query gets auto-answered
     to 'y', you will not see the query, and you will lose your record
     log without having a chance to abort the operation. In this regard,
     the CLI interface is better (IMO).

     Usually, I would guess that FEs do sanity checks, asking users
     confirmation before sending an operation that could have unwanted
     consequences. For instance, before sending the "quit" command,
     they ask the user to confirm that they want to quit.

     In this particular case, however, it's not clear how to perform
     any sanity check at the FE level. Nor does it seem desirable, as
     it would duplicate in the FE some of the logic we already have
     in GDB.

     There is also what you said:

     > Having a switch on a command turns out to be insufficient because
     > there is not a 1-1 mapping between queries and commands.  Currently
     > PRecord will query before changing memory, but this will be triggered
     > by any of:
     > -var-assign a 8
     > p a=8
     > set var a=8

After having written all this, it seems to me that I am suggesting
a level of complexity that is unnecessary, at least for now. Changing
the nquery to a query seems like the right thing to do, and would
render all these considerations OBE.

-- 
Joel

RE: Another proposal for frontends and queries.

[Marc Khouzam]

 

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Joel Brobecker
> Sent: Monday, September 14, 2009 11:09 AM
> To: Pedro Alves
> Cc: gdb-patches@sourceware.org; Marc Khouzam
> Subject: Re: Another proposal for frontends and queries.
> 
> > I think we should scrap out nquery from record.c, turning those to
> > query's, 'error's or normal stops, appropriately.  What 
> would be missed?
> 
> I tend to agree. I reviewed the few calls to nquery, and it would
> be more consistent with the rest of GDB if the default was 'y'
> rather than 'n'.

Thinking back, I posted a patch for that solution
http://sourceware.org/ml/gdb/2009-07/msg00125.html
but Hui was worried about tui
http://sourceware.org/ml/gdb/2009-07/msg00132.html

Maybe that is not such a problem?

Re: Another proposal for frontends and queries.

[Joel Brobecker]

> Thinking back, I posted a patch for that solution
> http://sourceware.org/ml/gdb/2009-07/msg00125.html
> but Hui was worried about tui
> http://sourceware.org/ml/gdb/2009-07/msg00132.html
> 
> Maybe that is not such a problem?

I re-read Hui's answer, and it sounds like he thought that you were
suggesting we *remove* the query in in TUI mode, whereas you were just
suggesting to change the default to yes. I don't think that TUI mode
is going to be a problem (this is no different, as far as I can tell,
from all the other yes-defaulted queries that we already have).

-- 
Joel

Re: Another proposal for frontends and queries.

[Joel Brobecker]

Marc, Hui,

> > I think we should scrap out nquery from record.c, turning those to
> > query's, 'error's or normal stops, appropriately.  What would be missed?

Branch time is very soon, but such a change would be acceptable on
the branch too. I just want to make sure that we reach a solution before
the 7.0 release, and preferably before the first pre-release, which
should be a day or two after the branch.

Hui, would you agree with changing the nquery calls into calls to query?

-- 
Joel

Re: Another proposal for frontends and queries.

[Hui Zhu]

On Wed, Sep 16, 2009 at 00:26, Joel Brobecker <brobecker@adacore.com> wrote:
> Marc, Hui,
>
>> > I think we should scrap out nquery from record.c, turning those to
>> > query's, 'error's or normal stops, appropriately.  What would be missed?
>
> Branch time is very soon, but such a change would be acceptable on
> the branch too. I just want to make sure that we reach a solution before
> the 7.0 release, and preferably before the first pre-release, which
> should be a day or two after the branch.
>
> Hui, would you agree with changing the nquery calls into calls to query?

Sorry I didn't follow this thread very clear.

We really need to do it?  I think add a switch (Like Marc did) or fix
the nquery is better.

Thanks,
Hui

RE: Another proposal for frontends and queries.

[Marc Khouzam]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Hui Zhu
> Sent: Wednesday, September 16, 2009 2:25 AM
> To: Joel Brobecker
> Cc: gdb-patches@sourceware.org; Marc Khouzam; Michael Snyder
> Subject: Re: Another proposal for frontends and queries.
> 
> On Wed, Sep 16, 2009 at 00:26, Joel Brobecker 
> <brobecker@adacore.com> wrote:
> > Marc, Hui,
> >
> >> > I think we should scrap out nquery from record.c, 
> turning those to
> >> > query's, 'error's or normal stops, appropriately.  What 
> would be missed?
> >
> > Branch time is very soon, but such a change would be acceptable on
> > the branch too. I just want to make sure that we reach a 
> solution before
> > the 7.0 release, and preferably before the first pre-release, which
> > should be a day or two after the branch.
> >
> > Hui, would you agree with changing the nquery calls into 
> calls to query?
> 
> Sorry I didn't follow this thread very clear.
> 
> We really need to do it?  I think add a switch (Like Marc did) or fix
> the nquery is better.

I guess the question that has come up is: is it very important to use
nquery() in PReccord instead of query()?  If we simply use query() then
PRecord will work well with Eclipse (and maybe other frontends).
The user should be smart enough to understand what the query means and
should do just fine with ("y or n") instead of ("y or [n]")

Having that "[n]" seems of less value than getting PRecord to work with
Frontends, no?

Thanks

Marc

Re: Another proposal for frontends and queries.

[Hui Zhu]

>>
>> Sorry I didn't follow this thread very clear.
>>
>> We really need to do it?  I think add a switch (Like Marc did) or fix
>> the nquery is better.
>
> I guess the question that has come up is: is it very important to use
> nquery() in PReccord instead of query()?  If we simply use query() then
> PRecord will work well with Eclipse (and maybe other frontends).
> The user should be smart enough to understand what the query means and
> should do just fine with ("y or n") instead of ("y or [n]")
>
> Having that "[n]" seems of less value than getting PRecord to work with
> Frontends, no?

I cannot agree with it.

Thanks,
Hui

Re: Another proposal for frontends and queries.

[Pedro Alves]

On Wednesday 16 September 2009 14:06:00, Hui Zhu wrote:
> > Having that "[n]" seems of less value than getting PRecord to work with
> > Frontends, no?
> 
> I cannot agree with it.

Please be clear on _why_ and _what_ you cannot agree with.

The point we're making, is that from the user's perpective, this:

 foofooo? (y or n) <enter>
 Please answer y or n.
 foofooo? (y or n) n <enter>
 <action cancelled>

doesn't look any worse than:

 foofooo? (y or [n]) <enter>
 <action cancelled>

GDB uses 'query' (defaulting to yes), in other desctructive
situations, such as the old:

 (gdb) quit
 The program is running.  Quit anyway (and kill it)? (y or n)

or the old:

 (gdb) run
 The program being debugged has been started already.
 Start it from the beginning? (y or n) 


How is precord different?

We very seriously need a GDB HIG.

-- 
Pedro Alves

Re: Another proposal for frontends and queries.

[Hui Zhu]

>
>
> How is precord different?
>
> We very seriously need a GDB HIG.
>
> --

There is a patch to handle it.  I think this way is better. And I have
said :"If you really cannot handle query with Eclipse, I think this
patch is OK with me. "

BTW, I told Marc begin a new thread to discussion why Eclipse cannot
handle nquery.  What's happen?

Thanks,
Hui

RE: Another proposal for frontends and queries.

[Marc Khouzam]

 

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Hui Zhu
> Sent: Wednesday, September 16, 2009 9:46 AM
> To: Pedro Alves
> Cc: gdb-patches@sourceware.org; Marc Khouzam; Joel Brobecker; 
> Michael Snyder
> Subject: Re: Another proposal for frontends and queries.
> 
> >
> >
> > How is precord different?
> >
> > We very seriously need a GDB HIG.
> >
> > --
> 
> There is a patch to handle it.  I think this way is better. And I have
> said :"If you really cannot handle query with Eclipse, I think this
> patch is OK with me. "

Yes but some of the maintainers did not really like this solution.
 
> BTW, I told Marc begin a new thread to discussion why Eclipse cannot
> handle nquery.  What's happen?

This thread we're in now is this new thread you suggested.
I tried to summarize the issue and go over all the different proposals
(we are up to 4 different solutions).  My impression is that the maintainers
prefer to change PRecord (which is new) instead of changing the genereal 
GDB behaviour with respect to queries (which has been there for a long time).

Thanks

Marc

Re: Another proposal for frontends and queries.

[Hui Zhu]

>> >
>> >
>> > How is precord different?
>> >
>> > We very seriously need a GDB HIG.
>> >
>> > --
>>
>> There is a patch to handle it.  I think this way is better. And I have
>> said :"If you really cannot handle query with Eclipse, I think this
>> patch is OK with me. "
>
> Yes but some of the maintainers did not really like this solution.
>
>> BTW, I told Marc begin a new thread to discussion why Eclipse cannot
>> handle nquery.  What's happen?
>
> This thread we're in now is this new thread you suggested.
> I tried to summarize the issue and go over all the different proposals
> (we are up to 4 different solutions).  My impression is that the maintainers
> prefer to change PRecord (which is new) instead of changing the genereal
> GDB behaviour with respect to queries (which has been there for a long time).
>

Thanks for help me clear what happen.   Thanks a lot.

And about the patch I said can handle this issue is "set record query
<on|off>" patch.  I cannot find who don't like this patch.

On the other hand, about the nquery.
Let me talk my idea.  The nquery is a function of GDB right?  MI is a
function of GDB too.   Eclipse is a very important soft that support
GDB.
If you want change nquery to query, why not remove nquery directly?


Thanks,
Hui

Re: Another proposal for frontends and queries.

[Joel Brobecker]

> And about the patch I said can handle this issue is "set record query
> <on|off>" patch.  I cannot find who don't like this patch.

I am one of the maintainers would dislike this idea.

> On the other hand, about the nquery.
> Let me talk my idea.  The nquery is a function of GDB right?  MI is a
> function of GDB too.   Eclipse is a very important soft that support
> GDB.
> If you want change nquery to query, why not remove nquery directly?

I had the same thoughts, actually. But we have one more usage of that
function relative to pending breakpoints, and for some reason it was
decided that the default was 'n' rather than 'y'. Not sure why.

I'm also thinking that what we need is an enhancement of the MI protocol
to handle queries that the frontends can then pass to the user for
answering.  But this is just a wild idea at this point, as I don't know
the MI protocol much. Plus, it would probably be a fairly significant
update unsuitable for 7.0.

-- 
Joel

Re: Another proposal for frontends and queries.

[Hui Zhu]

On Wed, Sep 16, 2009 at 22:25, Joel Brobecker <brobecker@adacore.com> wrote:
>> And about the patch I said can handle this issue is "set record query
>> <on|off>" patch.  I cannot find who don't like this patch.
>
> I am one of the maintainers would dislike this idea.

Sorry.
But I cannot find you your comment to this patch.  I just find Eli and
Tom's mail.

>
>> On the other hand, about the nquery.
>> Let me talk my idea.  The nquery is a function of GDB right?  MI is a
>> function of GDB too.   Eclipse is a very important soft that support
>> GDB.
>> If you want change nquery to query, why not remove nquery directly?
>
> I had the same thoughts, actually. But we have one more usage of that
> function relative to pending breakpoints, and for some reason it was
> decided that the default was 'n' rather than 'y'. Not sure why.

Because it will delete the execution log follow this insn.

Thanks,
Hui

RE: Another proposal for frontends and queries.

[Marc Khouzam]

> -----Original Message-----
> From: Joel Brobecker [mailto:brobecker@adacore.com] 
> Sent: Wednesday, September 16, 2009 10:26 AM
> To: Hui Zhu
> Cc: Marc Khouzam; Pedro Alves; gdb-patches@sourceware.org; 
> Michael Snyder
> Subject: Re: Another proposal for frontends and queries.
> 
> > And about the patch I said can handle this issue is "set 
> record query
> > <on|off>" patch.  I cannot find who don't like this patch.
> 
> I am one of the maintainers would dislike this idea.
> 
> > On the other hand, about the nquery.
> > Let me talk my idea.  The nquery is a function of GDB 
> right?  MI is a
> > function of GDB too.   Eclipse is a very important soft that support
> > GDB.
> > If you want change nquery to query, why not remove nquery directly?
> 
> I had the same thoughts, actually. But we have one more usage of that
> function relative to pending breakpoints, and for some reason it was
> decided that the default was 'n' rather than 'y'. Not sure why.

The whole nquery/yquery was introduced because of pending breakpoints.
If I understand the emails properly (http://sourceware.org/ml/gdb/2004-02/msg00108.html)
the new pending breakpoint feature broke the test scripts because
breakpoints that failed to install would now be made pending.
The nquery was meant to have the test scripts avoid that change.

I wonder if the flag "set breakpoint pending" was added after that.
Using this flag probably would have been good enough to avoid the 
whole nquery.
Maybe that is a solution we can take?  Remove nquery/yquery and
use "set breakpoint pending" for the pending breakpoints in test scripts?


> 
> I'm also thinking that what we need is an enhancement of the 
> MI protocol
> to handle queries that the frontends can then pass to the user for
> answering.  But this is just a wild idea at this point, as I 
> don't know
> the MI protocol much. Plus, it would probably be a fairly significant
> update unsuitable for 7.0.
> 
> -- 
> Joel
> 

Re: Another proposal for frontends and queries.

[Pedro Alves]

On Wednesday 16 September 2009 15:33:36, Hui Zhu wrote:
> Because it will delete the execution log follow this insn.

The user in the CLI still has to type an explicit 'y'
for that to happen.  The default only applies when there's
no terminal.  The right question to be asking is: what should
be the default answer in batch scripts.

Keep in mind that this all relates to the fact that there
is no "record status" command, that scripts and frontends
could use to query the record status, and be able to decide
upfront what to do, instead of gdb doing a query after the
fact.  Last time I brought that up, it was agreed that it
would be a good thing to have.  Anyone working on that?

-- 
Pedro Alves

Re: Another proposal for frontends and queries.

[Eli Zaretskii]

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Wed, 16 Sep 2009 14:25:47 +0100
> Cc: Hui Zhu <teawater@gmail.com>,  Marc Khouzam <marc.khouzam@ericsson.com>,  Joel Brobecker <brobecker@adacore.com>,  Michael Snyder <msnyder@vmware.com>
> 
> GDB uses 'query' (defaulting to yes), in other desctructive
> situations, such as the old:
> 
>  (gdb) quit
>  The program is running.  Quit anyway (and kill it)? (y or n)
> 
> or the old:
> 
>  (gdb) run
>  The program being debugged has been started already.
>  Start it from the beginning? (y or n) 
> 
> 
> How is precord different?
> 
> We very seriously need a GDB HIG.

I don't consider my self a HIG, but I agree with Pedro's reasoning.

Re: Another proposal for frontends and queries.

[Tom Tromey]

>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:

>> And about the patch I said can handle this issue is "set record query
>> <on|off>" patch.  I cannot find who don't like this patch.

I don't like it, I think I said that in another thread.

The reason I don't like it is twofold.

First, queries are a general feature of gdb.  They are sprinkled about
rather liberally.  So, if one causes problems for a front end, then it
probably represents a general class of problems.  IMO, fixing it in this
ad hoc way is not a good way to attack a generic problem.

The thought experiment to perform here is to consider generalizing this
solution to all the queries in gdb.  I think that would result in
madness: hundreds of "disable this query" settings.

Second, this has the feeling of an "unbreak my gdb" option.  That is,
setting an option to avoid one particular query says to me that the
query is probably badly chosen in some way.

Tom

Re: Another proposal for frontends and queries.

[Tom Tromey]

>>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:

Marc> Having a switch on a command turns out to be insufficient because
Marc> there is not a 1-1 mapping between queries and commands.  Currently
Marc> PRecord will query before changing memory, but this will be triggered
Marc> by any of:
Marc> -var-assign a 8
Marc> p a=8
Marc> set var a=8
Marc> and others.

I am curious about something here.  If you know the answers offhand, I'd
appreciate it.  Otherwise I guess I'll look into it at some later point.

Does the record code also query when making an inferior function call
from an expression?  It seems like it ought to, as such a call might
modify the inferior's state.

Also, if an expression does multiple assignments, are there multiple
queries or just one?  It seems like there should just be one.

Tom

RE: Another proposal for frontends and queries.

[Marc Khouzam]

 

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com] 
> Sent: Wednesday, September 16, 2009 4:20 PM
> To: Marc Khouzam
> Cc: 'Joel Brobecker'; 'gdb-patches@sourceware.org'
> Subject: Re: Another proposal for frontends and queries.
> 
> >>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:
> 
> Marc> Having a switch on a command turns out to be 
> insufficient because
> Marc> there is not a 1-1 mapping between queries and 
> commands.  Currently
> Marc> PRecord will query before changing memory, but this 
> will be triggered
> Marc> by any of:
> Marc> -var-assign a 8
> Marc> p a=8
> Marc> set var a=8
> Marc> and others.
> 
> I am curious about something here.  If you know the answers 
> offhand, I'd
> appreciate it.  Otherwise I guess I'll look into it at some 
> later point.
> 
> Does the record code also query when making an inferior function call
> from an expression?  It seems like it ought to, as such a call might
> modify the inferior's state.

I gave it a go and I found that yes, when calling an inferior function
PRecord will prompt.

 
> Also, if an expression does multiple assignments, are there multiple
> queries or just one?  It seems like there should just be one.

I tried:
(gdb) p foo(3)+foo(4)
and got a single query.

Pretty nice.

Marc

Re: Another proposal for frontends and queries.

[Tom Tromey]

>>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:

Marc> Proposals:
[...]

One idea from the user 'heinz' (sorry, I don't know your real name!) was
to have a query in MI mode throw a specific error, then let the front
end reissue the command with the correct response.

This seems like it could be done without much difficulty.  We would need
a way to stuff answers into some queue that "query" reads.  Maybe we
would need a way to label queries (so that a new response will only be
given to a specific query) -- but this could be just some machine
generated thing sent along with the error.

This struck me as a rather good idea: it is simple to understand and
implement, and doesn't seem to have the issues associated with some
other possible approaches to supporting queries (e.g., reentrant event
loops).  I assume I must be missing something ;-)

Tom

Re: Another proposal for frontends and queries.

[Joel Brobecker]

> One idea from the user 'heinz' (sorry, I don't know your real name!) was
> to have a query in MI mode throw a specific error, then let the front
> end reissue the command with the correct response.

That's what I thought the general policy was before I started thinking
about it some more.  I know this was mentioned on IRC, but this assumes
that we don't perform any action if an error is thrown. Otherwise, the
user sends the command, receives the query, cancels the command, and
thinks nothing happen.

I've often let the best be the enemy of good, though.  It seems like
an acceptable restriction in the way we implement commands, particularly
since it allows us to avoid potentially complex improvements of
the command architecture.

That being said, for 7.0, we should just go with the easy and safe
route.  That way, existing frontend can work with gdb-7.0, rather
than having to update their code to handle the new approach.

-- 
Joel

Re: Another proposal for frontends and queries.

[Hui Zhu]

On Thu, Sep 17, 2009 at 02:02, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:
>
>>> And about the patch I said can handle this issue is "set record query
>>> <on|off>" patch.  I cannot find who don't like this patch.
>
> I don't like it, I think I said that in another thread.
>
> The reason I don't like it is twofold.
>
> First, queries are a general feature of gdb.  They are sprinkled about
> rather liberally.  So, if one causes problems for a front end, then it
> probably represents a general class of problems.  IMO, fixing it in this
> ad hoc way is not a good way to attack a generic problem.
>
> The thought experiment to perform here is to consider generalizing this
> solution to all the queries in gdb.  I think that would result in
> madness: hundreds of "disable this query" settings.
>
> Second, this has the feeling of an "unbreak my gdb" option.  That is,
> setting an option to avoid one particular query says to me that the
> query is probably badly chosen in some way.
>

Sorry I didn't get the mean of "It seems like a strange approach to
me." first time.  Now, I got it.

One day, I try to debug a big program with GDB prec. I don't know why
it get something wrong.  It get bug only after exec a long time.  And
prec will make inferior exec slow (I design dump and skip to make it
fast).  So I enter "record" and "c" command in GDB and go to bed.
In the next morning, I open the LCD and want see what happen.  I found
that GDB is broken by a query.  :(

That is why I like "set rec query off".  It will not be a only switch.
 It will include a lot of switches for each query.  For the advanced
user, he can set each switch and then set rec query off.  Then he can
let GDB unbreak record a long time, and come back to use replay debug
find what happen.

That is why I think "set prec query off" is not bad.

Thanks,
Hui

Re: Another proposal for frontends and queries.

[Joel Brobecker]

> One day, I try to debug a big program with GDB prec. I don't know why
> it get something wrong.  It get bug only after exec a long time.  And
> prec will make inferior exec slow (I design dump and skip to make it
> fast).  So I enter "record" and "c" command in GDB and go to bed.
> In the next morning, I open the LCD and want see what happen.  I found
> that GDB is broken by a query.  :(

s/broken/blocked/ right? I think that "set confirm off" is exactly
what you were looking for.

-- 
Joel

Re: Another proposal for frontends and queries.

[Michael Snyder]

Tom Tromey wrote:
>>>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:
> 
> Marc> Having a switch on a command turns out to be insufficient because
> Marc> there is not a 1-1 mapping between queries and commands.  Currently
> Marc> PRecord will query before changing memory, but this will be triggered
> Marc> by any of:
> Marc> -var-assign a 8
> Marc> p a=8
> Marc> set var a=8
> Marc> and others.
> 
> I am curious about something here.  If you know the answers offhand, I'd
> appreciate it.  Otherwise I guess I'll look into it at some later point.
> 
> Does the record code also query when making an inferior function call
> from an expression?  It seems like it ought to, as such a call might
> modify the inferior's state.

It will, because the set-up for the call will involve changing
the stack pointer register as well as the eip.  The register
changes will get a prompt, even if nothing gets written to the stack
(memory).


> Also, if an expression does multiple assignments, are there multiple
> queries or just one?  It seems like there should just be one.

At a guess, the first "no" will end it (because it will error out),
but you might have to say "yes" repeatedly.  I haven't tried it.

Re: Another proposal for frontends and queries.

[Michael Snyder]

Marc Khouzam wrote:
>  
>> Also, if an expression does multiple assignments, are there multiple
>> queries or just one?  It seems like there should just be one.
> 
> I tried:
> (gdb) p foo(3)+foo(4)
> and got a single query.
> 
> Pretty nice.

Marc, did you answer yes or no to the query?
And have you tried the opposite?

RE: Another proposal for frontends and queries.

[Marc Khouzam]

> >> Also, if an expression does multiple assignments, are 
> there multiple
> >> queries or just one?  It seems like there should just be one.
> > 
> > I tried:
> > (gdb) p foo(3)+foo(4)
> > and got a single query.
> > 
> > Pretty nice.
> 
> Marc, did you answer yes or no to the query?
> And have you tried the opposite?

I tried 'y' as well as 'n' and there was only a single query both times.
I also re-did the test with 
(gdb) p foo(3) + bar(4)
with each method changing a different memory location and I was able to see
that when pressing 'n' neither of the memory location was change, while when
pressing 'y' both memory locations were changed.

It's bullet-proof! :-)  
(except for nquery and frontends :-O)

Re: Another proposal for frontends and queries.

[Joel Brobecker]

> (gdb) p foo(3) + bar(4)
> with each method changing a different memory location and I was able to see
> that when pressing 'n' neither of the memory location was change, while when
> pressing 'y' both memory locations were changed.

Would you consider contributing a testcase to keep it that way? ;-)

-- 
Joel

RE: Another proposal for frontends and queries.

[Marc Khouzam]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Joel Brobecker
> Sent: September-16-09 9:04 PM
> To: Marc Khouzam
> Cc: Michael Snyder; 'tromey@redhat.com'; 'gdb-patches@sourceware.org'
> Subject: Re: Another proposal for frontends and queries.
> 
> > (gdb) p foo(3) + bar(4)
> > with each method changing a different memory location and I 
> was able to see
> > that when pressing 'n' neither of the memory location was 
> change, while when
> > pressing 'y' both memory locations were changed.
> 
> Would you consider contributing a testcase to keep it that way? ;-)


I'll do my best to fit that in somewhere in my schedule.

RE: Another proposal for frontends and queries.

[Marc Khouzam]

 

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Joel Brobecker
> Sent: Wednesday, September 16, 2009 5:41 PM
> To: Tom Tromey
> Cc: Marc Khouzam; gdb-patches@sourceware.org
> Subject: Re: Another proposal for frontends and queries.
> 
> > One idea from the user 'heinz' (sorry, I don't know your 
> real name!) was
> > to have a query in MI mode throw a specific error, then let 
> the front
> > end reissue the command with the correct response.

This is interesting.
Maybe there could even be a "^query" answer.
The tokenId could be used to label this new MI query.

One extra advantage is that an MI query will no longer block GDB.
Other MI commands could be accepted while leaving the queried
command "on hold".  With this, the frontend could choose to query
the user or not, without having GDB blocked during the operation.
I like this.

One worry I have is what will GDB do if the frontend does not answer
the query?  I guess it can leave the command in the "query queue" for
ever, probably won't hurt.

> this assumes
> that we don't perform any action if an error is thrown. Otherwise, the
> user sends the command, receives the query, cancels the command, and
> thinks nothing happen.

I believe this is what you meant Joel, but just to be sure:
for this to work, a query has to happen before any effect of the command
is applied.  If a command starts doing some work and then queries, then
GDB might be in an inconsistent state.  But this probably does not happen
with the existing queries... it sounds too dangerous.

> I've often let the best be the enemy of good, though.  It seems like
> an acceptable restriction in the way we implement commands, 
> particularly
> since it allows us to avoid potentially complex improvements of
> the command architecture.
> 
> That being said, for 7.0, we should just go with the easy and safe
> route.  That way, existing frontend can work with gdb-7.0, rather
> than having to update their code to handle the new approach.

Yes, in the case of Eclipse, we wouldn't be able to support this
until June 2010.

That would be nice addition to MI for a future release.  I'm looking
forward to making use of it.

Marc
 

Re: Another proposal for frontends and queries.

[Tom Tromey]

>>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:

Marc> This is interesting.
Marc> Maybe there could even be a "^query" answer.
Marc> The tokenId could be used to label this new MI query.

Marc> One extra advantage is that an MI query will no longer block GDB.
Marc> Other MI commands could be accepted while leaving the queried
Marc> command "on hold".  With this, the frontend could choose to query
Marc> the user or not, without having GDB blocked during the operation.
Marc> I like this.

Marc> One worry I have is what will GDB do if the frontend does not answer
Marc> the query?  I guess it can leave the command in the "query queue" for
Marc> ever, probably won't hurt.

I think we are talking about different approaches.

Having a query "on hold" implies, to me, either a change to use
continuation passing style, or that GDB will reenter the event loop from
query().

Either of these is certainly doable, but my experience with these
approaches (in GUI and CORBA contexts) has been fairly negative.

Perhaps I misunderstood what you are saying, though.


Instead this idea is to have query() issue a particular kind of error --
and abort the command.

Then the MI user could recognize this error, send a special command to
stuff some answers into a queue that query() would read, and then
restart the command from scratch.

There are two assumptions underlying this idea:

1. A command will always ask the same queries in the same order.
2. A command won't commit some change before a query.

#1 can be relaxed a little bit, if needed, by having each query report
its identity.  Then the query queue would actually be a map, associating
an identity with an answer.

#2 seems like a reasonable assumption.

With this approach, GDB would not keep any state when a query fails.
It would not need to.

Tom

Re: Another proposal for frontends and queries.

[Hui Zhu]

Hi guys,

I make a demo that add a switch can set the memory change query to
yquery or nquery.  Then Marc can set it in his part.

If you think this demo is OK.  I will post a patch for memory change
query and register change query.
And please help me with the words "memorychangedefault", "Set the
default query answer of memory change." and so on.  I need some clear
words.  :)

Thanks,
Hui

---
 record.c |   33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

--- a/record.c
+++ b/record.c
@@ -97,6 +97,8 @@ static int record_insn_num = 0;
 /* The target_ops of process record.  */
 static struct target_ops record_ops;

+static int record_memory_change_default = 0;
+
 /* The beneath function pointers.  */
 static struct target_ops *record_beneath_to_resume_ops;
 static void (*record_beneath_to_resume) (struct target_ops *, ptid_t, int,
@@ -1018,11 +1020,20 @@ record_xfer_partial (struct target_ops *
     {
       if (RECORD_IS_REPLAY)
 	{
+	  int n = 0;
+
 	  /* Let user choose if he wants to write memory or not.  */
-	  if (!nquery (_("Because GDB is in replay mode, writing to memory "
+          if (record_memory_change_default)
+            n = yquery (_("Because GDB is in replay mode, writing to memory "
 		         "will make the execution log unusable from this "
 		         "point onward.  Write memory at address %s?"),
-		       paddress (target_gdbarch, offset)))
+		       paddress (target_gdbarch, offset));
+	  else
+            n = nquery (_("Because GDB is in replay mode, writing to memory "
+		         "will make the execution log unusable from this "
+		         "point onward.  Write memory at address %s?"),
+		       paddress (target_gdbarch, offset));
+	  if (!n)
 	    error (_("Process record canceled the operation."));

 	  /* Destroy the record from here forward.  */
@@ -1240,6 +1251,15 @@ info_record_command (char *args, int fro
   cmd_show_list (info_record_cmdlist, from_tty, "");
 }

+static void
+show_record_memory_change_default (struct ui_file *file, int from_tty,
+                                   struct cmd_list_element *c,
+                                   const char *value)
+{
+  fprintf_filtered (file, _("The default query answer is %s.\n"),
+                    record_memory_change_default ? "yes" : "no");
+}
+
 void
 _initialize_record (void)
 {
@@ -1310,4 +1330,13 @@ record/replay buffer.  Zero means unlimi
   add_cmd ("insn-number", class_obscure, show_record_insn_number,
 	   _("Show the current number of instructions in the "
 	     "record/replay buffer."), &info_record_cmdlist);
+
+  add_setshow_boolean_cmd ("memorychangedefault", no_class,
+			   &record_memory_change_default, _("\
+Set the default query answer of memory change."), _("\
+Show the default query answer of memory change."), _("\
+\"yes\" or \"no\"."),
+			   NULL,
+			   show_record_memory_change_default,
+			   &set_record_cmdlist, &show_record_cmdlist);
 }

Re: Another proposal for frontends and queries.

[Michael Snyder]

Hui Zhu wrote:
> Hi guys,
> 
> I make a demo that add a switch can set the memory change query to
> yquery or nquery.  Then Marc can set it in his part.
> 
> If you think this demo is OK.  I will post a patch for memory change
> query and register change query.
> And please help me with the words "memorychangedefault", "Set the
> default query answer of memory change." and so on.  I need some clear
> words.  :)

How about "set precord-readonly".
One variable, to control both memory and register writes.
If true, you can't change any machine  state while debugging a 
recording.  No worries about queries.

> ---
>  record.c |   33 +++++++++++++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
> 
> --- a/record.c
> +++ b/record.c
> @@ -97,6 +97,8 @@ static int record_insn_num = 0;
>  /* The target_ops of process record.  */
>  static struct target_ops record_ops;
> 
> +static int record_memory_change_default = 0;
> +
>  /* The beneath function pointers.  */
>  static struct target_ops *record_beneath_to_resume_ops;
>  static void (*record_beneath_to_resume) (struct target_ops *, ptid_t, int,
> @@ -1018,11 +1020,20 @@ record_xfer_partial (struct target_ops *
>      {
>        if (RECORD_IS_REPLAY)
>  	{
> +	  int n = 0;
> +
>  	  /* Let user choose if he wants to write memory or not.  */
> -	  if (!nquery (_("Because GDB is in replay mode, writing to memory "
> +          if (record_memory_change_default)
> +            n = yquery (_("Because GDB is in replay mode, writing to memory "
>  		         "will make the execution log unusable from this "
>  		         "point onward.  Write memory at address %s?"),
> -		       paddress (target_gdbarch, offset)))
> +		       paddress (target_gdbarch, offset));
> +	  else
> +            n = nquery (_("Because GDB is in replay mode, writing to memory "
> +		         "will make the execution log unusable from this "
> +		         "point onward.  Write memory at address %s?"),
> +		       paddress (target_gdbarch, offset));
> +	  if (!n)
>  	    error (_("Process record canceled the operation."));
> 
>  	  /* Destroy the record from here forward.  */
> @@ -1240,6 +1251,15 @@ info_record_command (char *args, int fro
>    cmd_show_list (info_record_cmdlist, from_tty, "");
>  }
> 
> +static void
> +show_record_memory_change_default (struct ui_file *file, int from_tty,
> +                                   struct cmd_list_element *c,
> +                                   const char *value)
> +{
> +  fprintf_filtered (file, _("The default query answer is %s.\n"),
> +                    record_memory_change_default ? "yes" : "no");
> +}
> +
>  void
>  _initialize_record (void)
>  {
> @@ -1310,4 +1330,13 @@ record/replay buffer.  Zero means unlimi
>    add_cmd ("insn-number", class_obscure, show_record_insn_number,
>  	   _("Show the current number of instructions in the "
>  	     "record/replay buffer."), &info_record_cmdlist);
> +
> +  add_setshow_boolean_cmd ("memorychangedefault", no_class,
> +			   &record_memory_change_default, _("\
> +Set the default query answer of memory change."), _("\
> +Show the default query answer of memory change."), _("\
> +\"yes\" or \"no\"."),
> +			   NULL,
> +			   show_record_memory_change_default,
> +			   &set_record_cmdlist, &show_record_cmdlist);
>  }

Re: Another proposal for frontends and queries.

[Pedro Alves]

On Friday 18 September 2009 05:59:25, Hui Zhu wrote:
> +          if (record_memory_change_default)
> +            n = yquery (_("Because GDB is in replay mode, writing to memory "
>                          "will make the execution log unusable from this "
>                          "point onward.  Write memory at address %s?"),
> -                      paddress (target_gdbarch, offset)))
> +                      paddress (target_gdbarch, offset));
> +         else
> +            n = nquery (_("Because GDB is in replay mode, writing to memory "
> +                        "will make the execution log unusable from this "
> +                        "point onward.  Write memory at address %s?"),
> +                      paddress (target_gdbarch, offset));
> +         if (!n)

Having just "<enter>" behave differently depending on a setting, is
bad UI/usability, IMO.  If you switch to 'query', you force the user to
type 'y' or 'n' explicitly, which is a Good Thing.

-- 
Pedro Alves
\x16º&Öéj×!zÊÞ¶êçë¯|òX¬µªÜ†\a[¥«\…ë

Re: Another proposal for frontends and queries.

[Joel Brobecker]

> Having just "<enter>" behave differently depending on a setting, is
> bad UI/usability, IMO.  If you switch to 'query', you force the user to
> type 'y' or 'n' explicitly, which is a Good Thing.

I agree.

Hui:

You keep suggesting more of the same - basically, your suggestions
so far revolve around the same principle, which is adding an extra
setting that controls the query being made. But we have already
explained that this is something we would like to avoid, and we
explained why. In the meantime, the issue remains unresolved and
people like Marc continue being stuck with it.

There are two aspects to this issue:

  1. A specific short-term need: The default for queries in precord
     is 'n', and that causes auto-answers in Eclipse to be 'n' as
     well.  As a result, Eclipse uses cannot use precord fully.

     The answer to that problem is very simple: Do *NOT* provide
     a default answer. That's what "query" does.

  2. A longer-term issue: It should be possible for Front-Ends
     using GDB/MI to relay queries to the user, so that the user
     can enter confirmation before the operation is in fact
     executed.  That's longer term, as this needs some infrastructure
     work.

Right now, I'm only interested in (1). It's clear that you do not like
the idea of querying the user without providing a default. Can you
explain why?

-- 
Joel

RE: Another proposal for frontends and queries.

[Marc Khouzam]

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com] 
> Sent: Thursday, September 17, 2009 3:52 PM
> To: Marc Khouzam
> Cc: 'Joel Brobecker'; 'gdb-patches@sourceware.org'
> Subject: Re: Another proposal for frontends and queries.
> 
> >>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:
> 
> Marc> This is interesting.
> Marc> Maybe there could even be a "^query" answer.
> Marc> The tokenId could be used to label this new MI query.
> 
> Marc> One extra advantage is that an MI query will no longer 
> block GDB.
> Marc> Other MI commands could be accepted while leaving the queried
> Marc> command "on hold".  With this, the frontend could 
> choose to query
> Marc> the user or not, without having GDB blocked during the 
> operation.
> Marc> I like this.
> 
> Marc> One worry I have is what will GDB do if the frontend 
> does not answer
> Marc> the query?  I guess it can leave the command in the 
> "query queue" for
> Marc> ever, probably won't hurt.
> 
> I think we are talking about different approaches.
> 
> Having a query "on hold" implies, to me, either a change to use
> continuation passing style, or that GDB will reenter the 
> event loop from
> query().
> 
> Either of these is certainly doable, but my experience with these
> approaches (in GUI and CORBA contexts) has been fairly negative.
> 
> Perhaps I misunderstood what you are saying, though.

Nope, you got it right.  But now I see that was not the proposal.

> Instead this idea is to have query() issue a particular kind 
> of error --
> and abort the command.
> 
> Then the MI user could recognize this error, send a special command to
> stuff some answers into a queue that query() would read, and then
> restart the command from scratch.

Ok, now I see.  I didn't realize the command would be restarted from scratch.
Thanks for the explanation.  And that is a better solution.  Although I a bit
concerned about an infinite loop between the FE and GDB, if things are not
handled properly by the FE.

> There are two assumptions underlying this idea:
> 
> 1. A command will always ask the same queries in the same order.
> 2. A command won't commit some change before a query.
> 
> #1 can be relaxed a little bit, if needed, by having each query report
> its identity.  Then the query queue would actually be a map, 
> associating
> an identity with an answer.
> 
> #2 seems like a reasonable assumption.
> 
> With this approach, GDB would not keep any state when a query fails.
> It would not need to.

This makes sense.

The last (I think :-)) part that I don't understand is if a FE must
code in advance for a potential query?  This solution seems to go
towards a way that would allow a FE to be ready for any query
on any command.  A new query could be added to a command by a new GDB
version, and FEs would not need to change.  That would be nice.  
Is that how you see it?
I'm asking because, in some case a FE
will want to answer the query itself, without seeking input from the user.
For example, for PRecord, when the query asks if we should allow to change
memory, I won't ask the user, I just want to say 'y'.
I don't see how a FE could be smart enough to generically answer
queries like that? (unless we code for it in advance)

On the flip side, if a FE knows in advance the potential queries
to a specific command, then maybe all we need is some new MI general
parameter "-query <y | n>" that could be added by the frontend
to any command (some details would need to be worked out to handle
multiple queries in the same command though).

Does this make sense?

Thanks

Marc

Re: Another proposal for frontends and queries.

[Tom Tromey]

>>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:

Marc> The last (I think :-)) part that I don't understand is if a FE must
Marc> code in advance for a potential query?  This solution seems to go
Marc> towards a way that would allow a FE to be ready for any query
Marc> on any command.  A new query could be added to a command by a new GDB
Marc> version, and FEs would not need to change.  That would be nice.  
Marc> Is that how you see it?

Yeah -- a front end could choose what to do, even present the query to
the user.

Marc> I'm asking because, in some case a FE
Marc> will want to answer the query itself, without seeking input from the user.
Marc> For example, for PRecord, when the query asks if we should allow to change
Marc> memory, I won't ask the user, I just want to say 'y'.
Marc> I don't see how a FE could be smart enough to generically answer
Marc> queries like that? (unless we code for it in advance)

Yeah, good point.  This seems to imply that the query "error" should
include a token naming the query -- and I guess that such tokens should
remain stable over time.  (I was hoping to avoid that...)

Marc> On the flip side, if a FE knows in advance the potential queries
Marc> to a specific command, then maybe all we need is some new MI general
Marc> parameter "-query <y | n>" that could be added by the frontend
Marc> to any command (some details would need to be worked out to handle
Marc> multiple queries in the same command though).

Marc> Does this make sense?

Yes, except that the queries could also change over time.

I'm not sure this is a great idea after all.  Maybe the solution is
really just to have all queries default to "go ahead and do it" in MI.
I really don't know.

Tom

RE: Another proposal for frontends and queries.

[Marc Khouzam]

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com] 
> Sent: September-18-09 12:42 PM
> To: Marc Khouzam
> Cc: 'Joel Brobecker'; 'gdb-patches@sourceware.org'
> Subject: Re: Another proposal for frontends and queries.
> 
> >>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:
> 
> Marc> The last (I think :-)) part that I don't understand is 
> if a FE must
> Marc> code in advance for a potential query?  This solution 
> seems to go
> Marc> towards a way that would allow a FE to be ready for any query
> Marc> on any command.  A new query could be added to a 
> command by a new GDB
> Marc> version, and FEs would not need to change.  That would 
> be nice.  
> Marc> Is that how you see it?
> 
> Yeah -- a front end could choose what to do, even present the query to
> the user.
> 
> Marc> I'm asking because, in some case a FE
> Marc> will want to answer the query itself, without seeking 
> input from the user.
> Marc> For example, for PRecord, when the query asks if we 
> should allow to change
> Marc> memory, I won't ask the user, I just want to say 'y'.
> Marc> I don't see how a FE could be smart enough to generically answer
> Marc> queries like that? (unless we code for it in advance)
> 
> Yeah, good point.  This seems to imply that the query "error" should
> include a token naming the query -- and I guess that such 
> tokens should
> remain stable over time.  (I was hoping to avoid that...)
> 
> Marc> On the flip side, if a FE knows in advance the potential queries
> Marc> to a specific command, then maybe all we need is some 
> new MI general
> Marc> parameter "-query <y | n>" that could be added by the frontend
> Marc> to any command (some details would need to be worked 
> out to handle
> Marc> multiple queries in the same command though).
> 
> Marc> Does this make sense?
> 
> Yes, except that the queries could also change over time.
> 
> I'm not sure this is a great idea after all.  Maybe the solution is
> really just to have all queries default to "go ahead and do it" in MI.
> I really don't know.

Yeah, it's not simple.
In the short term, thanks to support from the maintainers, I'm hoping
we'll get to this default "go ahead and do it".
After that, maybe we can figure out some nice MI way to handle queries.

Marc

[RFA/commit] s/nquery/query/ in record.c (was "Re: Another proposal for frontends and queries.")

[Joel Brobecker]

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

It seems that the discussion regarding a short-term fix has died,
so I'm assuming that Hui is OK for now with changing nquery to query.

Here is a patch that I tested on x86_64-linux, no regression.
Will commit in the head and 7.0 branches in a couple of days pending
objections.

MarcK, did you write a patch like this one, a while ago. You should
get credit for it if you did.

2009-09-22  Joel Brobecker  <brobecker@adacore.com>

        * record.c (record_open, record_store_registers, record_xfer_partial):
        Replace calls to nquery by calls to query.

-- 
Joel

[-- Attachment #2: record.diff --]
[-- Type: text/x-diff, Size: 2448 bytes --]

commit c27c19686fafd2850ca2401280cf043ecbb60c48
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Tue Sep 22 15:03:52 2009 -0700

        * record.c (record_open, record_store_registers, record_xfer_partial):
        Replace calls to nquery by calls to query.

diff --git a/gdb/record.c b/gdb/record.c
index 8ad5bf8..c675e34 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -497,7 +497,7 @@ record_open (char *name, int from_tty)
   /* Check if record target is already running.  */
   if (current_target.to_stratum == record_stratum)
     {
-      if (!nquery
+      if (!query
 	  (_("Process record target already running, do you want to delete "
 	     "the old record log?")))
 	return;
@@ -1029,15 +1029,15 @@ record_store_registers (struct target_ops *ops, struct regcache *regcache,
 	  /* Let user choose if he wants to write register or not.  */
 	  if (regno < 0)
 	    n =
-	      nquery (_("Because GDB is in replay mode, changing the "
-			"value of a register will make the execution "
-			"log unusable from this point onward.  "
-			"Change all registers?"));
+	      query (_("Because GDB is in replay mode, changing the "
+		       "value of a register will make the execution "
+		       "log unusable from this point onward.  "
+		       "Change all registers?"));
 	  else
 	    n =
-	      nquery (_("Because GDB is in replay mode, changing the value "
-			"of a register will make the execution log unusable "
-			"from this point onward.  Change register %s?"),
+	      query (_("Because GDB is in replay mode, changing the value "
+		       "of a register will make the execution log unusable "
+		       "from this point onward.  Change register %s?"),
 		      gdbarch_register_name (get_regcache_arch (regcache),
 					       regno));
 
@@ -1085,9 +1085,9 @@ record_xfer_partial (struct target_ops *ops, enum target_object object,
       if (RECORD_IS_REPLAY)
 	{
 	  /* Let user choose if he wants to write memory or not.  */
-	  if (!nquery (_("Because GDB is in replay mode, writing to memory "
-		         "will make the execution log unusable from this "
-		         "point onward.  Write memory at address %s?"),
+	  if (!query (_("Because GDB is in replay mode, writing to memory "
+		        "will make the execution log unusable from this "
+		        "point onward.  Write memory at address %s?"),
 		       paddress (target_gdbarch, offset)))
 	    error (_("Process record canceled the operation."));
 

RE: [RFA/commit] s/nquery/query/ in record.c (was "Re: Another proposal for frontends and queries.")

[Marc Khouzam]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Joel Brobecker
> Sent: Tuesday, September 22, 2009 6:49 PM
> To: Marc Khouzam
> Cc: gdb-patches@sourceware.org
> Subject: [RFA/commit] s/nquery/query/ in record.c (was "Re: 
> Another proposal for frontends and queries.")
> 
> It seems that the discussion regarding a short-term fix has died,
> so I'm assuming that Hui is OK for now with changing nquery to query.
> 
> Here is a patch that I tested on x86_64-linux, no regression.
> Will commit in the head and 7.0 branches in a couple of days pending
> objections.
>
> MarcK, did you write a patch like this one, a while ago. You should
> get credit for it if you did.

Don't worry about it, as long as Eclipse can use PRecord fully, that's what
I was looking for.

Thanks a lot for following up on this!

Marc

Re: [RFA/commit] s/nquery/query/ in record.c (was "Re: Another proposal for frontends and queries.")

[Hui Zhu]

Thanks, Joel and Marc.  I think this patch is best for the current status.

Thanks,
Hui

On Wed, Sep 23, 2009 at 20:44, Marc Khouzam <marc.khouzam@ericsson.com> wrote:
>> -----Original Message-----
>> From: gdb-patches-owner@sourceware.org
>> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Joel Brobecker
>> Sent: Tuesday, September 22, 2009 6:49 PM
>> To: Marc Khouzam
>> Cc: gdb-patches@sourceware.org
>> Subject: [RFA/commit] s/nquery/query/ in record.c (was "Re:
>> Another proposal for frontends and queries.")
>>
>> It seems that the discussion regarding a short-term fix has died,
>> so I'm assuming that Hui is OK for now with changing nquery to query.
>>
>> Here is a patch that I tested on x86_64-linux, no regression.
>> Will commit in the head and 7.0 branches in a couple of days pending
>> objections.
>>
>> MarcK, did you write a patch like this one, a while ago. You should
>> get credit for it if you did.
>
> Don't worry about it, as long as Eclipse can use PRecord fully, that's what
> I was looking for.
>
> Thanks a lot for following up on this!
>
> Marc
>
>

Re: [RFA/commit] s/nquery/query/ in record.c (was "Re: Another proposal for frontends and queries.")

[Joel Brobecker]

> 2009-09-22  Joel Brobecker  <brobecker@adacore.com>
> 
>         * record.c (record_open, record_store_registers, record_xfer_partial):
>         Replace calls to nquery by calls to query.

Thanks, Hui, for confirming. Now checked in HEAD & the 7.0 branch.

-- 
Joel