[RFA,01/10] Add a selftest that detects a 'corrupted' command tree structure in GDB.

Message ID 20200510205530.21923-2-philippe.waroquiers@skynet.be
State Superseded
Headers show
Series
  • fix/improve cmd structure, class_alias, help, apropos
Related show

Commit Message

Hannes Domani via Gdb-patches May 10, 2020, 8:55 p.m.
The GDB data structure that records the GDB commands is made of
'struct cmd_list_element' defined in cli-decode.h.

A cmd_list_element has various pointers to other cmd_list_element structures,
All these pointers are together building a graph of commands.

However, when following the 'next' and '*prefixlist' pointers of
cmd_list_element, the structure must better be a tree.

If such pointers do not form a tree, then some other elements of
cmd_list_element cannot get a correct semantic.  In particular, the prefixname
has no correct meaning if the same prefix command can be reached via 2 different
paths.

This commit introduces a selftest that detects (at least some cases of) errors
leading to 'next' and '*prefixlist' not giving a tree structure.

The new 'command_structure_invariants' selftest detects one single case where
the command structure is not a tree:

  (gdb) maintenance selftest command_structure_invariants
  Running selftest command_structure_invariants.
  list 0x56362e204b98 duplicated, reachable via prefix 'show ' and 'info set '.  Duplicated list first command is 'ada'
  Self test failed: self-test failed at ../../classfix/gdb/unittests/command-def-selftests.c:160
  Ran 1 unit tests, 1 failed
  (gdb)

YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* unittests/help-doc-selftests.c: Rename to
	unittests/command-def-selftests.c
	* unittests/command-def-selftests.c (help_doc_tests): Update some
	comments.
	(command_structure_tests, traverse_command_structure): New namespace
	and function.
	(command_structure_invariants_tests): New function.
	(_initialize_command_def_selftests) Renamed from
	_initialize_help_doc_selftests, register command_structure_invariants
	selftest.
---
 gdb/Makefile.in                               |  2 +-
 ...oc-selftests.c => command-def-selftests.c} | 81 ++++++++++++++++++-
 2 files changed, 79 insertions(+), 4 deletions(-)
 rename gdb/unittests/{help-doc-selftests.c => command-def-selftests.c} (62%)

-- 
2.20.1

Comments

Tom Tromey May 14, 2020, 3:51 p.m. | #1
>>>>> "Philippe" == Philippe Waroquiers via Gdb-patches <gdb-patches@sourceware.org> writes:


Philippe> This commit introduces a selftest that detects (at least some cases of) errors
Philippe> leading to 'next' and '*prefixlist' not giving a tree structure.

Thank you.

Philippe> +/* Verify that a list of commands is present in the tree only once.  */
Philippe> +
Philippe> +static void
Philippe> +command_structure_invariants_tests ()
Philippe> +{
Philippe> +
Philippe> +  traverse_command_structure (&cmdlist, "");

Spurious blank line after the "{".

Philippe> +
Philippe> +  /* Release memory, be ready to be re-run.  */
Philippe> +  lists.clear ();
Philippe> +
Philippe> +  SELF_CHECK (nr_duplicates == 0);

This should also reset nr_duplicates.

Tom

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index e3ce6a285f..32d0eee7c6 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -427,13 +427,13 @@  SELFTESTS_SRCS = \
 	unittests/array-view-selftests.c \
 	unittests/child-path-selftests.c \
 	unittests/cli-utils-selftests.c \
+	unittests/command-def-selftests.c \
 	unittests/common-utils-selftests.c \
 	unittests/copy_bitwise-selftests.c \
 	unittests/environ-selftests.c \
 	unittests/filtered_iterator-selftests.c \
 	unittests/format_pieces-selftests.c \
 	unittests/function-view-selftests.c \
-	unittests/help-doc-selftests.c \
 	unittests/lookup_name_info-selftests.c \
 	unittests/memory-map-selftests.c \
 	unittests/memrange-selftests.c \
diff --git a/gdb/unittests/help-doc-selftests.c b/gdb/unittests/command-def-selftests.c
similarity index 62%
rename from gdb/unittests/help-doc-selftests.c
rename to gdb/unittests/command-def-selftests.c
index 16ffc4f990..55e1278fc3 100644
--- a/gdb/unittests/help-doc-selftests.c
+++ b/gdb/unittests/command-def-selftests.c
@@ -1,4 +1,4 @@ 
-/* Self tests for help doc for GDB, the GNU debugger.
+/* Self tests for GDB command definitions for GDB, the GNU debugger.
 
    Copyright (C) 2019-2020 Free Software Foundation, Inc.
 
@@ -22,7 +22,12 @@ 
 #include "cli/cli-decode.h"
 #include "gdbsupport/selftest.h"
 
+#include <map>
+
 namespace selftests {
+
+/* Verify some invariants of GDB commands documentation.  */
+
 namespace help_doc_tests {
 
 static unsigned int nr_failed_invariants;
@@ -96,13 +101,83 @@  help_doc_invariants_tests ()
 }
 
 } /* namespace help_doc_tests */
+
+/* Verify some invariants of GDB command structure.  */
+
+namespace command_structure_tests {
+
+unsigned int nr_duplicates = 0;
+
+/* A map associating a list with the prefix leading to it.  */
+
+std::map<cmd_list_element **, const char *> lists;
+
+/* Store each command list in lists, associated with the prefix to reach it.  A
+   list must only be found once.  */
+
+static void
+traverse_command_structure (struct cmd_list_element **list,
+			    const char *prefix)
+{
+  struct cmd_list_element *c;
+
+  auto dupl = lists.find (list);
+  if (dupl != lists.end ())
+    {
+      fprintf_filtered (gdb_stdout,
+			"list %p duplicated,"
+			" reachable via prefix '%s' and '%s'."
+			"  Duplicated list first command is '%s'\n",
+			list,
+			prefix, dupl->second,
+			(*list)->name);
+      nr_duplicates++;
+      return;
+    }
+
+  lists.insert ({list, prefix});
+
+  /* Walk through the commands.  */
+  for (c = *list; c; c = c->next)
+    {
+      /* If this command has subcommands and is not an alias,
+	 traverse the subcommands.  */
+      if (c->prefixlist != NULL && c->cmd_pointer == nullptr)
+	{
+	  /* Recursively call ourselves on the subcommand list,
+	     passing the right prefix in.  */
+	  traverse_command_structure (c->prefixlist, c->prefixname);
+	}
+    }
+}
+
+/* Verify that a list of commands is present in the tree only once.  */
+
+static void
+command_structure_invariants_tests ()
+{
+
+  traverse_command_structure (&cmdlist, "");
+
+  /* Release memory, be ready to be re-run.  */
+  lists.clear ();
+
+  SELF_CHECK (nr_duplicates == 0);
+}
+
+}
+
 } /* namespace selftests */
 
-void _initialize_help_doc_selftests ();
+void _initialize_command_def_selftests ();
 void
-_initialize_help_doc_selftests ()
+_initialize_command_def_selftests ()
 {
   selftests::register_test
     ("help_doc_invariants",
      selftests::help_doc_tests::help_doc_invariants_tests);
+
+  selftests::register_test
+    ("command_structure_invariants",
+     selftests::command_structure_tests::command_structure_invariants_tests);
 }