python: Make gdb.execute("show commands") work (PR 23669)

Message ID 20180917012350.10194-1-simon.marchi@polymtl.ca
State New
Headers show
Series
  • python: Make gdb.execute("show commands") work (PR 23669)
Related show

Commit Message

Simon Marchi Sept. 17, 2018, 1:23 a.m.
Since commit

  56bcdbea2bed ("Let gdb.execute handle multi-line commands")

trying to use a command like gdb.execute("show commands") in Python
fails.  GDB ends up trying to run the "commands" command.

The reason is that GDB gets confused with the special "commands"
command.  In process_next_line, the lookup_cmd_1 function returns the
cmd_list_element representing the "commands" sub-command of "show".
Lower, we check the cmd_list_element to see if it matches various
control commands by name, including the "commands" command.  This is
where we wrongfully conclude that the executed command must be
"commands", when in reality it was "show commands".

The fix proposed in this patch removes the comparisons by name, instead
comparing the cmd_list_element object by pointer with the objects
created at initialization time.

Tested on the buildbot, though on a single builder (Fedora-x86_64-m64).

gdb/ChangeLog:

	PR python/23669
	* breakpoint.c (commands_cmd_element): New.
	(_initialize_breakpoint): Assign commands_cmd_element.
	* breakpoint.h (commands_cmd_element): New.
	* cli/cli-script.c (while_cmd_element, if_command,
	define_cmd_element): New.
	(command_name_equals): Remove.
	(process_next_line): Compare commands by pointer, not by name.
	(_initialize_cli_script): Assign the various cmd_list_element
	variables.
	* compile/compile.c (compile_cmd_element): New.
	(_initialize_compile): Assign compile_cmd_element.
	* compile/compile.h (compile_cmd_element): New.
	* guile/guile.c (guile_cmd_element): New.
	(install_gdb_commands): Assign guile_cmd_element.
	* guile/guile.h (guile_cmd_element): New.
	* python/python.c (python_cmd_element): New.
	(_initialize_python): Assign python_cmd_element.
	* python/python.h (python_cmd_element): New.
	* tracepoint.c (while_stepping_cmd_element): New.
	(_initialize_tracepoint): Assign while_stepping_cmd_element.
	* tracepoint.h (while_stepping_cmd_element): New.

gdb/testsuite/ChangeLog:

	PR python/23669
	* gdb.python/python.exp: Test gdb.execute("show commands").
---
 gdb/breakpoint.c                    |  7 +++-
 gdb/breakpoint.h                    |  3 ++
 gdb/cli/cli-script.c                | 56 ++++++++++++++---------------
 gdb/compile/compile.c               |  6 ++--
 gdb/compile/compile.h               |  3 ++
 gdb/guile/guile.c                   |  5 ++-
 gdb/guile/guile.h                   |  3 ++
 gdb/python/python.c                 |  5 ++-
 gdb/python/python.h                 |  3 ++
 gdb/testsuite/gdb.python/python.exp |  4 +++
 gdb/tracepoint.c                    |  6 +++-
 gdb/tracepoint.h                    |  3 ++
 12 files changed, 68 insertions(+), 36 deletions(-)

-- 
2.18.0

Comments

Tom Tromey Sept. 17, 2018, 6:37 a.m. | #1
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:


Simon> Since commit
Simon>   56bcdbea2bed ("Let gdb.execute handle multi-line commands")

Simon> trying to use a command like gdb.execute("show commands") in Python
Simon> fails.  GDB ends up trying to run the "commands" command.

Simon> The reason is that GDB gets confused with the special "commands"
Simon> command.  In process_next_line, the lookup_cmd_1 function returns the
Simon> cmd_list_element representing the "commands" sub-command of "show".
Simon> Lower, we check the cmd_list_element to see if it matches various
Simon> control commands by name, including the "commands" command.  This is
Simon> where we wrongfully conclude that the executed command must be
Simon> "commands", when in reality it was "show commands".

Simon> The fix proposed in this patch removes the comparisons by name, instead
Simon> comparing the cmd_list_element object by pointer with the objects
Simon> created at initialization time.

Thanks for doing this.  I think getting rid of command_name_equals is a
nice cleanup.  The patch looks good to me.

Tom
Simon Marchi Sept. 17, 2018, 12:28 p.m. | #2
On 2018-09-17 02:37, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

> 

> Simon> Since commit

> Simon>   56bcdbea2bed ("Let gdb.execute handle multi-line commands")

> 

> Simon> trying to use a command like gdb.execute("show commands") in 

> Python

> Simon> fails.  GDB ends up trying to run the "commands" command.

> 

> Simon> The reason is that GDB gets confused with the special "commands"

> Simon> command.  In process_next_line, the lookup_cmd_1 function 

> returns the

> Simon> cmd_list_element representing the "commands" sub-command of 

> "show".

> Simon> Lower, we check the cmd_list_element to see if it matches 

> various

> Simon> control commands by name, including the "commands" command.  

> This is

> Simon> where we wrongfully conclude that the executed command must be

> Simon> "commands", when in reality it was "show commands".

> 

> Simon> The fix proposed in this patch removes the comparisons by name, 

> instead

> Simon> comparing the cmd_list_element object by pointer with the 

> objects

> Simon> created at initialization time.

> 

> Thanks for doing this.  I think getting rid of command_name_equals is a

> nice cleanup.  The patch looks good to me.

> 

> Tom


Thanks I pushed it.  I forgot to mention, but I intend to push this to 
the 8.2 branch.  Any concerns about this?

Simon
Joel Brobecker Sept. 17, 2018, 5:02 p.m. | #3
> Thanks I pushed it.  I forgot to mention, but I intend to push this to the

> 8.2 branch.  Any concerns about this?


No problem on my end, the patch looks safe to me.

Thanks Simon,
-- 
Joel
Simon Marchi Sept. 17, 2018, 6:18 p.m. | #4
On 2018-09-17 13:02, Joel Brobecker wrote:
>> Thanks I pushed it.  I forgot to mention, but I intend to push this to 

>> the

>> 8.2 branch.  Any concerns about this?

> 

> No problem on my end, the patch looks safe to me.

> 

> Thanks Simon,


Thanks, I pushed it to the branch.

Simon

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 85cb7d7afcd5..7591648f5eb4 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -15506,6 +15506,10 @@  initialize_breakpoint_ops (void)
 
 static struct cmd_list_element *enablebreaklist = NULL;
 
+/* See breakpoint.h.  */
+
+cmd_list_element *commands_cmd_element = nullptr;
+
 void
 _initialize_breakpoint (void)
 {
@@ -15531,7 +15535,8 @@  _initialize_breakpoint (void)
 Set ignore-count of breakpoint number N to COUNT.\n\
 Usage is `ignore N COUNT'."));
 
-  add_com ("commands", class_breakpoint, commands_command, _("\
+  commands_cmd_element = add_com ("commands", class_breakpoint,
+				  commands_command, _("\
 Set commands to be executed when the given breakpoints are hit.\n\
 Give a space-separated breakpoint list as argument after \"commands\".\n\
 A list element can be a breakpoint number (e.g. `5') or a range of numbers\n\
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 63cb5370c16a..66375b68c27b 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1679,4 +1679,7 @@  extern void maybe_print_thread_hit_breakpoint (struct ui_out *uiout);
 /* Print the specified breakpoint.  */
 extern void print_breakpoint (breakpoint *bp);
 
+/* Command element for the 'commands' command.  */
+extern cmd_list_element *commands_cmd_element;
+
 #endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 8496fb85e6f5..e5b0f40cf40e 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -25,6 +25,7 @@ 
 #include "ui-out.h"
 #include "top.h"
 #include "breakpoint.h"
+#include "tracepoint.h"
 #include "cli/cli-cmds.h"
 #include "cli/cli-decode.h"
 #include "cli/cli-script.h"
@@ -33,6 +34,8 @@ 
 #include "interps.h"
 #include "compile/compile.h"
 #include "common/gdb_string_view.h"
+#include "python/python.h"
+#include "guile/guile.h"
 
 #include <vector>
 
@@ -58,6 +61,15 @@  static int command_nest_depth = 1;
 /* This is to prevent certain commands being printed twice.  */
 static int suppress_next_print_command_trace = 0;
 
+/* Command element for the 'while' command.  */
+static cmd_list_element *while_cmd_element = nullptr;
+
+/* Command element for the 'if' command.  */
+static cmd_list_element *if_cmd_element = nullptr;
+
+/* Command element for the 'define' command.  */
+static cmd_list_element *define_cmd_element = nullptr;
+
 /* Structure for arguments to user defined functions.  */
 
 class user_args
@@ -906,16 +918,6 @@  read_next_line (void)
   return command_line_input (prompt_ptr, "commands");
 }
 
-/* Return true if CMD's name is NAME.  */
-
-static bool
-command_name_equals (struct cmd_list_element *cmd, const char *name)
-{
-  return (cmd != NULL
-	  && cmd != CMD_LIST_AMBIGUOUS
-	  && strcmp (cmd->name, name) == 0);
-}
-
 /* Given an input line P, skip the command and return a pointer to the
    first argument.  */
 
@@ -990,7 +992,7 @@  process_next_line (const char *p, struct command_line **command,
 
       /* Check for while, if, break, continue, etc and build a new
 	 command line structure for them.  */
-      if (command_name_equals (cmd, "while-stepping"))
+      if (cmd == while_stepping_cmd_element)
 	{
 	  /* Because validate_actionline and encode_action lookup
 	     command's line as command, we need the line to
@@ -1005,34 +1007,28 @@  process_next_line (const char *p, struct command_line **command,
 	     not.  */
 	  *command = build_command_line (while_stepping_control, p);
 	}
-      else if (command_name_equals (cmd, "while"))
-	{
-	  *command = build_command_line (while_control, line_first_arg (p));
-	}
-      else if (command_name_equals (cmd, "if"))
-	{
-	  *command = build_command_line (if_control, line_first_arg (p));
-	}
-      else if (command_name_equals (cmd, "commands"))
-	{
-	  *command = build_command_line (commands_control, line_first_arg (p));
-	}
-      else if (command_name_equals (cmd, "define"))
+      else if (cmd == while_cmd_element)
+	*command = build_command_line (while_control, line_first_arg (p));
+      else if (cmd == if_cmd_element)
+	*command = build_command_line (if_control, line_first_arg (p));
+      else if (cmd == commands_cmd_element)
+	*command = build_command_line (commands_control, line_first_arg (p));
+      else if (cmd == define_cmd_element)
 	*command = build_command_line (define_control, line_first_arg (p));
-      else if (command_name_equals (cmd, "python") && !inline_cmd)
+      else if (cmd == python_cmd_element && !inline_cmd)
 	{
 	  /* Note that we ignore the inline "python command" form
 	     here.  */
 	  *command = build_command_line (python_control, "");
 	}
-      else if (command_name_equals (cmd, "compile") && !inline_cmd)
+      else if (cmd == compile_cmd_element && !inline_cmd)
 	{
 	  /* Note that we ignore the inline "compile command" form
 	     here.  */
 	  *command = build_command_line (compile_control, "");
 	  (*command)->control_u.compile.scope = COMPILE_I_INVALID_SCOPE;
 	}
-      else if (command_name_equals (cmd, "guile") && !inline_cmd)
+      else if (cmd == guile_cmd_element && !inline_cmd)
 	{
 	  /* Note that we ignore the inline "guile command" form here.  */
 	  *command = build_command_line (guile_control, "");
@@ -1597,7 +1593,7 @@  _initialize_cli_script (void)
 Document a user-defined command.\n\
 Give command name as argument.  Give documentation on following lines.\n\
 End with a line of just \"end\"."));
-  add_com ("define", class_support, define_command, _("\
+  define_cmd_element = add_com ("define", class_support, define_command, _("\
 Define a new command name.  Command name is argument.\n\
 Definition appears on following lines, one command per line.\n\
 End with a line of just \"end\".\n\
@@ -1606,13 +1602,13 @@  Commands defined in this way may accept an unlimited number of arguments\n\
 accessed via $arg0 .. $argN.  $argc tells how many arguments have\n\
 been passed."));
 
-  add_com ("while", class_support, while_command, _("\
+  while_cmd_element = add_com ("while", class_support, while_command, _("\
 Execute nested commands WHILE the conditional expression is non zero.\n\
 The conditional expression must follow the word `while' and must in turn be\n\
 followed by a new line.  The nested commands must be entered one per line,\n\
 and should be terminated by the word `end'."));
 
-  add_com ("if", class_support, if_command, _("\
+  if_cmd_element = add_com ("if", class_support, if_command, _("\
 Execute nested commands once IF the conditional expression is non zero.\n\
 The conditional expression must follow the word `if' and must in turn be\n\
 followed by a new line.  The nested commands must be entered one per line,\n\
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index f4d20913c3ec..02c162cf73c7 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -908,14 +908,16 @@  compile_instance::compile (const char *filename, int verbose_level)
 
 #undef FORWARD
 
+/* See compile.h.  */
+cmd_list_element *compile_cmd_element = nullptr;
 
 void
 _initialize_compile (void)
 {
   struct cmd_list_element *c = NULL;
 
-  add_prefix_cmd ("compile", class_obscure, compile_command,
-		  _("\
+  compile_cmd_element = add_prefix_cmd ("compile", class_obscure,
+					compile_command, _("\
 Command to compile source code and inject it into the inferior."),
 		  &compile_command_list, "compile ", 1, &cmdlist);
   add_com_alias ("expression", "compile", class_obscure, 0);
diff --git a/gdb/compile/compile.h b/gdb/compile/compile.h
index 89cc00fdd2d0..955f8836ca1b 100644
--- a/gdb/compile/compile.h
+++ b/gdb/compile/compile.h
@@ -103,4 +103,7 @@  extern void compile_dwarf_bounds_to_c (string_file *stream,
 
 extern void compile_print_value (struct value *val, void *data_voidp);
 
+/* Command element for the 'compile' command.  */
+extern cmd_list_element *compile_cmd_element;
+
 #endif /* GDB_COMPILE_H */
diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index 95b36febed04..9384e84bde0b 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -700,6 +700,9 @@  gdbscm_set_backtrace (int enable)
 
 #endif /* HAVE_GUILE */
 
+/* See guile.h.  */
+cmd_list_element *guile_cmd_element = nullptr;
+
 /* Install the various gdb commands used by Guile.  */
 
 static void
@@ -725,7 +728,7 @@  This command is only a placeholder.")
 
   /* Since "help guile" is easy to type, and intuitive, we add general help
      in using GDB+Guile to this command.  */
-  add_com ("guile", class_obscure, guile_command,
+  guile_cmd_element = add_com ("guile", class_obscure, guile_command,
 #ifdef HAVE_GUILE
 	   _("\
 Evaluate one or more Guile expressions.\n\
diff --git a/gdb/guile/guile.h b/gdb/guile/guile.h
index aaa005816e62..2faa23da814c 100644
--- a/gdb/guile/guile.h
+++ b/gdb/guile/guile.h
@@ -25,4 +25,7 @@ 
 /* This is all that guile exports to gdb.  */
 extern const struct extension_language_defn extension_language_guile;
 
+/* Command element for the 'guile' command.  */
+extern cmd_list_element *guile_cmd_element;
+
 #endif /* GDB_GUILE_H */
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 1a0562a22751..c16305b900fe 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1745,6 +1745,9 @@  do_start_initialization ()
 
 #endif /* HAVE_PYTHON */
 
+/* See python.h.  */
+cmd_list_element *python_cmd_element = nullptr;
+
 void
 _initialize_python (void)
 {
@@ -1774,7 +1777,7 @@  This command is only a placeholder.")
 	   );
   add_com_alias ("pi", "python-interactive", class_obscure, 1);
 
-  add_com ("python", class_obscure, python_command,
+  python_cmd_element = add_com ("python", class_obscure, python_command,
 #ifdef HAVE_PYTHON
 	   _("\
 Evaluate a Python command.\n\
diff --git a/gdb/python/python.h b/gdb/python/python.h
index de58fd81df83..4b36f8385313 100644
--- a/gdb/python/python.h
+++ b/gdb/python/python.h
@@ -25,4 +25,7 @@ 
 /* This is all that python exports to gdb.  */
 extern const struct extension_language_defn extension_language_python;
 
+/* Command element for the 'python' command.  */
+extern cmd_list_element *python_cmd_element;
+
 #endif /* GDB_PYTHON_H */
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index 1d7ea5b6b4b2..a37113c17c7b 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -497,3 +497,7 @@  gdb_test "python print(gdb.convenience_variable('cvar3'))" "None" \
     "print reset convenience variable from python"
 gdb_test "print \$cvar3" "= void" \
     "print reset convenience variable from CLI"
+
+# Test PR 23669, the following would invoke the "commands" command instead of
+# "show commands".
+gdb_test "python gdb.execute(\"show commands\")" "$decimal  print \\\$cvar3.*"
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index bb5a8507739f..6d964c154bf0 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -3977,6 +3977,9 @@  static const struct internalvar_funcs sdata_funcs =
   NULL
 };
 
+/* See tracepoint.h.  */
+cmd_list_element *while_stepping_cmd_element = nullptr;
+
 /* module initialization */
 void
 _initialize_tracepoint (void)
@@ -4088,7 +4091,8 @@  Entering \"end\" on a line by itself is the normal way to terminate\n\
 such a list.\n\n\
 Note: the \"end\" command cannot be used at the gdb prompt."));
 
-  add_com ("while-stepping", class_trace, while_stepping_pseudocommand, _("\
+  while_stepping_cmd_element = add_com ("while-stepping", class_trace,
+					while_stepping_pseudocommand, _("\
 Specify single-stepping behavior at a tracepoint.\n\
 Argument is number of instructions to trace in single-step mode\n\
 following the tracepoint.  This command is normally followed by\n\
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index c185672cc1b8..b43fbd760686 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -436,4 +436,7 @@  extern struct traceframe_info *get_traceframe_info (void);
 
 extern struct bp_location *get_traceframe_location (int *stepping_frame_p);
 
+/* Command element for the 'while-stepping' command.  */
+extern cmd_list_element *while_stepping_cmd_element;
+
 #endif	/* TRACEPOINT_H */