Make -exec-run and -exec-until into 'real' MI commands

Make -exec-run and -exec-until into 'real' MI commands

[Vladimir Prus]

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


At present, '-exec-run' and '-exec-until' are directly routed
to CLI, with very little MI intervention. I have tried to fix
that before, but there were backward compatibility concerns.

While working on multiexec MI patches, I really needed to have
-exec-run handled in MI, so I've made another attempt, which
is attached. Comments?

- Volodya


[-- Attachment #2: run-until.diff --]
[-- Type: text/x-patch, Size: 5573 bytes --]

commit 21beee1a1c8602552cabf74aa84ae20df8653b04
Author: Vladimir Prus <vladimir@codesourcery.com>
Date:   Fri Dec 25 16:04:03 2009 +0300

    Make -exec-run and -exec-until proper MI commands.
    
    	* mi/mi-cmds.h (mi_cmd_exec_run, mi_cmd_exec_until): Declare.
    	* mi/mi-cmds.c (mi_cmds): Adjust.
    	* mi/mi-main.c (recompose_args, mi_cmd_exec_run)
    	(mi_cmd_exec_until): New.

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 201d66b..729cc5f 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -65,10 +65,10 @@ struct mi_cmd mi_cmds[] =
   { "exec-next", { NULL, 0 }, mi_cmd_exec_next},
   { "exec-next-instruction", { NULL, 0 }, mi_cmd_exec_next_instruction},
   { "exec-return", { NULL, 0 }, mi_cmd_exec_return},
-  { "exec-run", { "run", 1 }, NULL},
+  { "exec-run", { NULL, 0}, mi_cmd_exec_run},
   { "exec-step", { NULL, 0 }, mi_cmd_exec_step},
   { "exec-step-instruction", { NULL, 0 }, mi_cmd_exec_step_instruction},
-  { "exec-until", { "until", 1 }, NULL},
+  { "exec-until", { NULL, 1 }, mi_cmd_exec_until},
   { "file-exec-and-symbols", { "file", 1 }, NULL },
   { "file-exec-file", { "exec-file", 1 }, NULL },
   { "file-list-exec-source-file", { NULL, 0 }, mi_cmd_file_list_exec_source_file},
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index f76e217..7e1c819 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -54,13 +54,15 @@ extern mi_cmd_argv_ftype mi_cmd_env_path;
 extern mi_cmd_argv_ftype mi_cmd_env_pwd;
 extern mi_cmd_argv_ftype mi_cmd_exec_continue;
 extern mi_cmd_argv_ftype mi_cmd_exec_finish;
+extern mi_cmd_argv_ftype mi_cmd_exec_interrupt;
 extern mi_cmd_argv_ftype mi_cmd_exec_jump;
 extern mi_cmd_argv_ftype mi_cmd_exec_next;
 extern mi_cmd_argv_ftype mi_cmd_exec_next_instruction;
 extern mi_cmd_argv_ftype mi_cmd_exec_return;
+extern mi_cmd_argv_ftype mi_cmd_exec_run;
 extern mi_cmd_argv_ftype mi_cmd_exec_step;
 extern mi_cmd_argv_ftype mi_cmd_exec_step_instruction;
-extern mi_cmd_argv_ftype mi_cmd_exec_interrupt;
+extern mi_cmd_argv_ftype mi_cmd_exec_until;
 extern mi_cmd_argv_ftype mi_cmd_file_list_exec_source_file;
 extern mi_cmd_argv_ftype mi_cmd_file_list_exec_source_files;
 extern mi_cmd_argv_ftype mi_cmd_gdb_exit;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index dde0062..c2c8ec6 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -275,6 +275,117 @@ mi_cmd_exec_interrupt (char *command, char **argv, int argc)
     error ("Usage: -exec-interrupt [--all|--thread-group id]");
 }
 
+/* Given MI command arguments, recompose then back into a single string
+   suitable for passing to a CLI command.  
+   The return value should be freed by the caller.  */
+
+static char *
+recompose_args (char **argv, int argc, int add_ampersand)
+{
+  int size = 0;
+  char *recomposed;
+  char *out;
+  int i;
+  char *p;
+  
+  for (i = 0; i < argc; ++i)
+    size += strlen (argv[i]) + 2 /* quotes, maybe */ + 1 /* space */;
+  out = recomposed = xmalloc (size + 1 /* & */ + 1 /* \0 */);
+  
+  for (i = 0; i < argc; ++i)
+    {
+      int has_whitespace = 0;
+      for (p = argv[i]; *p; ++p)
+	if (isspace (*p))
+	  {
+	    has_whitespace = 1;
+	    break;
+	  }
+      
+      if (i != 0)
+	out += sprintf (out, " ");
+      if (has_whitespace)
+	out += sprintf (out, "\"%s\"", argv[i]);
+      else
+	out += sprintf (out, "%s", argv[i]);
+    }
+  if (add_ampersand)
+    sprintf (out, "&");
+
+  return recomposed;
+}
+
+void
+mi_cmd_exec_until (char *command, char **argv, int argc)
+{
+  struct cleanup *back_to;
+  char *r = 0;
+
+  /* Previously, -exec-until was routed via CLI directly,
+     and quoting was processed by CLI.  Now, it's a full
+     blown MI command, and accepts a single argument.
+     
+     This means that if a frontend sends something like:
+
+         -exec-until C:/My Documents/a.c:10
+ 
+     we'll immediately error out and the frontend has to
+     quote it.  However, such usage would not have worked
+     before either -- maybe with more cryptic error.  So,
+     not backward compatibilty concerns here.  */
+
+  if (argc > 1)
+    error ("-exec-until must be invoked with a signle argument");
+
+  back_to = make_cleanup (null_cleanup, NULL);
+  if (argc == 1)
+    {
+      r = recompose_args (argv, argc, target_can_async_p ());
+      make_cleanup (xfree, r);
+    }
+  
+  mi_execute_cli_command ("until", argc == 1, r);
+
+  do_cleanups (back_to);
+}
+
+void
+mi_cmd_exec_run (char *command, char **argv, int argc)
+{
+  /* In GDB 7.0, '-exec-run' routed to CLI, and therefore any arguments were
+     passed to 'run' without processing.  Strictly speaking, -exec-run was
+     never documented to accept any argument, and no frontend that actually
+     passes an argument is known.  But in the case such frontend exist,
+     we reconstruct back the argument.
+
+     If either the input was overly quoted:
+
+	 "foo bar" "biz"
+
+     then the reconstructed string will not have quoting on second string.  Also,
+     if the input had two spaces between tokens, e.g.
+
+	 "foo bar"  "biz"
+
+     then the reconstructed string will have one.  It should not matter.  */
+
+  if (argc == 0)
+    {
+      mi_execute_cli_command ("run", target_can_async_p (), 
+			      target_can_async_p () ? "&" : NULL);
+    }
+  else
+    {
+      struct cleanup *back_to;
+      char *r = recompose_args (argv, argc, target_can_async_p ());
+      back_to = make_cleanup (xfree, r);
+      
+      mi_execute_cli_command ("run", 1, r);
+
+      do_cleanups (back_to);
+    }
+}
+
 static int
 find_thread_of_process (struct thread_info *ti, void *p)
 {

RE: Make -exec-run and -exec-until into 'real' MI commands

[Marc Khouzam]

 

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Vladimir Prus
> Sent: Wednesday, January 13, 2010 8:29 AM
> To: gdb-patches@sourceware.org
> Subject: Make -exec-run and -exec-until into 'real' MI commands
> 
> 
> At present, '-exec-run' and '-exec-until' are directly routed
> to CLI, with very little MI intervention. I have tried to fix
> that before, but there were backward compatibility concerns.
> 
> While working on multiexec MI patches, I really needed to have
> -exec-run handled in MI, so I've made another attempt, which
> is attached. Comments?
> 

I just skimmed to patch to see if there was FE impacts, which
I gather there is not.

I noticed a typo:

 error ("-exec-until must be invoked with a signle argument");
                                              ^^^

Marc

Re: Make -exec-run and -exec-until into 'real' MI commands

[Vladimir Prus]

On Wednesday 13 January 2010 16:44:06 Marc Khouzam wrote:

> 
> > -----Original Message-----
> > From: gdb-patches-owner@sourceware.org 
> > [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Vladimir Prus
> > Sent: Wednesday, January 13, 2010 8:29 AM
> > To: gdb-patches@sourceware.org
> > Subject: Make -exec-run and -exec-until into 'real' MI commands
> > 
> > 
> > At present, '-exec-run' and '-exec-until' are directly routed
> > to CLI, with very little MI intervention. I have tried to fix
> > that before, but there were backward compatibility concerns.
> > 
> > While working on multiexec MI patches, I really needed to have
> > -exec-run handled in MI, so I've made another attempt, which
> > is attached. Comments?
> > 
> 
> I just skimmed to patch to see if there was FE impacts, which
> I gather there is not.
> 
> I noticed a typo:
> 
>  error ("-exec-until must be invoked with a signle argument");
>                                               ^^^

Thanks, fixed that locally.

- Volodya

Re: Make -exec-run and -exec-until into 'real' MI commands

[Daniel Jacobowitz]

On Wed, Jan 13, 2010 at 04:29:25PM +0300, Vladimir Prus wrote:
> 
> At present, '-exec-run' and '-exec-until' are directly routed
> to CLI, with very little MI intervention. I have tried to fix
> that before, but there were backward compatibility concerns.
> 
> While working on multiexec MI patches, I really needed to have
> -exec-run handled in MI, so I've made another attempt, which
> is attached. Comments?

How much compatibility are you looking for?

It takes a long path through GDB, but the argument to -exec-run today
eventually gets passed to the shell with no additional quoting.  If
I type:

(gdb) run a b "a b" "\""

then GDB does:

execve("/bin/zsh", ["/bin/zsh", "-c", "exec /usr/bin/gdb a b \"a b\" \"\\\"\""], [/* 64 vars */])

That's the same as typing at a shell prompt:

exec /usr/bin/gdb a b "a b" "\""

Your patch doesn't undo the effect of mi_parse_argv, so this
won't be preserved.  Can we just save the unparsed argv somehow?

I don't remember the previous time this came up, but are there front
ends that pass arguments to -exec-run?  It's not documented as
accepting any.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Make -exec-run and -exec-until into 'real' MI commands

[Vladimir Prus]

On Wednesday 13 January 2010 23:09:58 Daniel Jacobowitz wrote:

> On Wed, Jan 13, 2010 at 04:29:25PM +0300, Vladimir Prus wrote:
> > 
> > At present, '-exec-run' and '-exec-until' are directly routed
> > to CLI, with very little MI intervention. I have tried to fix
> > that before, but there were backward compatibility concerns.
> > 
> > While working on multiexec MI patches, I really needed to have
> > -exec-run handled in MI, so I've made another attempt, which
> > is attached. Comments?
> 
> How much compatibility are you looking for?
> 
> It takes a long path through GDB, but the argument to -exec-run today
> eventually gets passed to the shell with no additional quoting.  If
> I type:
> 
> (gdb) run a b "a b" "\""
> 
> then GDB does:
> 
> execve("/bin/zsh", ["/bin/zsh", "-c", "exec /usr/bin/gdb a b \"a b\" \"\\\"\""], [/* 64 vars */])
> 
> That's the same as typing at a shell prompt:
> 
> exec /usr/bin/gdb a b "a b" "\""
> 
> Your patch doesn't undo the effect of mi_parse_argv, so this
> won't be preserved.  

I think that only "\"" won't be preserved. While "a b" will be present in reconstructed
string with quotes.

> Can we just save the unparsed argv somehow?

We can, but that would be ugly ...

 
> I don't remember the previous time this came up, but are there front
> ends that pass arguments to -exec-run?  It's not documented as
> accepting any.

... for the purpose of supporting frontends that use undocumented accidental
feature, and worse, we don't know any such frontends.

I believe the fact that no known frontends use args was raised the last time,
but you still wanted to assume they exist.

- Volodya


> 
> 


- Volodya

Re: Make -exec-run and -exec-until into 'real' MI commands

[Daniel Jacobowitz]

On Wed, Jan 13, 2010 at 11:13:24PM +0300, Vladimir Prus wrote:
> I think that only "\"" won't be preserved. While "a b" will be present in reconstructed
> string with quotes.

Won't we have problems with Windows paths, for example?

-exec-run 'c:\My Documents\test' "c:\My Documents\test"

% exec /usr/bin/gdb 'c:\My Documents\test' "c:\My Documents\test"

But the MI parser is going to turn that second \t into a tab
if I'm not mistaken.  This is a really hard transformation to
reverse.

> I believe the fact that no known frontends use args was raised the last time,
> but you still wanted to assume they exist.

Blech, I hate having to argue with myself.

I suggest asking for a second opinion then.  I don't think this
quoting is good enough to be a good idea.  I think we can get away
without it for -exec-run.  I don't know about -exec-until.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Make -exec-run and -exec-until into 'real' MI commands

[Joel Brobecker]

> I suggest asking for a second opinion then.  I don't think this
> quoting is good enough to be a good idea.  I think we can get away
> without it for -exec-run.  I don't know about -exec-until.

I agree that we should avoid a quoting mechanism if it leads to
surprising results. There is some code that does quoting in fork-child,
perhaps we could reuse that code?

-- 
Joel

Re: Make -exec-run and -exec-until into 'real' MI commands

[Vladimir Prus]

On Wednesday 13 January 2010 23:26:21 Daniel Jacobowitz wrote:

> On Wed, Jan 13, 2010 at 11:13:24PM +0300, Vladimir Prus wrote:
> > I think that only "\"" won't be preserved. While "a b" will be present in reconstructed
> > string with quotes.
> 
> Won't we have problems with Windows paths, for example?
> 
> -exec-run 'c:\My Documents\test' "c:\My Documents\test"
> 
> % exec /usr/bin/gdb 'c:\My Documents\test' "c:\My Documents\test"
> 
> But the MI parser is going to turn that second \t into a tab
> if I'm not mistaken.  This is a really hard transformation to
> reverse.
> 
> > I believe the fact that no known frontends use args was raised the last time,
> > but you still wanted to assume they exist.
> 
> Blech, I hate having to argue with myself.
> 
> I suggest asking for a second opinion then.  I don't think this
> quoting is good enough to be a good idea.  I think we can get away
> without it for -exec-run.  I don't know about -exec-until.

Apparently, nobody seem to have a second opinion :-(
So, unless I hear objections by tomorrow, I will commit a patch that:

a) make -exec-run refuse to accept any parameters, as no frontend
that cares is known.
b) does nothing with -exec-until, since your point about \t seems like
a killer. I'll fix -exec-until for MI3, whenever that comes around.


Thanks,

-- 
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722

Re: Make -exec-run and -exec-until into 'real' MI commands

[Vladimir Prus]

On Thursday 14 January 2010 07:46:03 Joel Brobecker wrote:

> > I suggest asking for a second opinion then.  I don't think this
> > quoting is good enough to be a good idea.  I think we can get away
> > without it for -exec-run.  I don't know about -exec-until.
> 
> I agree that we should avoid a quoting mechanism if it leads to
> surprising results. There is some code that does quoting in fork-child,
> perhaps we could reuse that code?

Will that help with \t issue identified by Dan? I don't think so.


Thanks,

-- 
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722

Re: Make -exec-run and -exec-until into 'real' MI commands

[Vladimir Prus]

[-- Attachment #1: Type: Text/Plain, Size: 957 bytes --]

On Monday 22 February 2010 17:23:48 Vladimir Prus wrote:

> > > I believe the fact that no known frontends use args was raised the last time,
> > > but you still wanted to assume they exist.
> > 
> > Blech, I hate having to argue with myself.
> > 
> > I suggest asking for a second opinion then.  I don't think this
> > quoting is good enough to be a good idea.  I think we can get away
> > without it for -exec-run.  I don't know about -exec-until.
> 
> Apparently, nobody seem to have a second opinion :-(
> So, unless I hear objections by tomorrow, I will commit a patch that:
> 
> a) make -exec-run refuse to accept any parameters, as no frontend
> that cares is known.
> b) does nothing with -exec-until, since your point about \t seems like
> a killer. I'll fix -exec-until for MI3, whenever that comes around.

Nobody has spoked up, so I've checked in the below.

Thanks,

-- 
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722

[-- Attachment #2: exec-run.diff --]
[-- Type: text/x-patch, Size: 2354 bytes --]

commit c5b28d25aa4e6126c642cfa72285041f7641aeae
Author: Vladimir Prus <vladimir@codesourcery.com>
Date:   Fri Dec 25 16:04:03 2009 +0300

    Make -exec-run a proper MI commands.
    
    	* mi/mi-cmds.h (mi_cmd_exec_run): Declare.
    	* mi/mi-cmds.c (mi_cmds): Adjust.
    	* mi/mi-main.c (mi_cmd_exec_run): New.

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 201d66b..1acd54c 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -65,7 +65,7 @@ struct mi_cmd mi_cmds[] =
   { "exec-next", { NULL, 0 }, mi_cmd_exec_next},
   { "exec-next-instruction", { NULL, 0 }, mi_cmd_exec_next_instruction},
   { "exec-return", { NULL, 0 }, mi_cmd_exec_return},
-  { "exec-run", { "run", 1 }, NULL},
+  { "exec-run", { NULL, 0}, mi_cmd_exec_run},
   { "exec-step", { NULL, 0 }, mi_cmd_exec_step},
   { "exec-step-instruction", { NULL, 0 }, mi_cmd_exec_step_instruction},
   { "exec-until", { "until", 1 }, NULL},
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index f76e217..7feb1c2 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -54,13 +54,14 @@ extern mi_cmd_argv_ftype mi_cmd_env_path;
 extern mi_cmd_argv_ftype mi_cmd_env_pwd;
 extern mi_cmd_argv_ftype mi_cmd_exec_continue;
 extern mi_cmd_argv_ftype mi_cmd_exec_finish;
+extern mi_cmd_argv_ftype mi_cmd_exec_interrupt;
 extern mi_cmd_argv_ftype mi_cmd_exec_jump;
 extern mi_cmd_argv_ftype mi_cmd_exec_next;
 extern mi_cmd_argv_ftype mi_cmd_exec_next_instruction;
 extern mi_cmd_argv_ftype mi_cmd_exec_return;
+extern mi_cmd_argv_ftype mi_cmd_exec_run;
 extern mi_cmd_argv_ftype mi_cmd_exec_step;
 extern mi_cmd_argv_ftype mi_cmd_exec_step_instruction;
-extern mi_cmd_argv_ftype mi_cmd_exec_interrupt;
 extern mi_cmd_argv_ftype mi_cmd_file_list_exec_source_file;
 extern mi_cmd_argv_ftype mi_cmd_file_list_exec_source_files;
 extern mi_cmd_argv_ftype mi_cmd_gdb_exit;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 9d9e3da..f1b745c 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -330,6 +330,13 @@ mi_cmd_exec_interrupt (char *command, char **argv, int argc)
     error ("Usage: -exec-interrupt [--all|--thread-group id]");
 }
 
+void
+mi_cmd_exec_run (char *command, char **argv, int argc)
+{
+  mi_execute_cli_command ("run", target_can_async_p (),
+			  target_can_async_p () ? "&" : NULL);
+}
+
 static int
 find_thread_of_process (struct thread_info *ti, void *p)
 {

Re: Make -exec-run and -exec-until into 'real' MI commands

[Joel Brobecker]

> Will that help with \t issue identified by Dan? I don't think so.

I'm a little late answering (was off network for a while), and I don't
have a particular opinion on the initial problem anyway, but it seemed
to me that it might. Can't be sure, though, I feel like something in
the discussion is escaping me...

-- 
Joel