public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFC 0/2] Using pretty-printers for [] indexing
@ 2021-04-22 14:10 Andrew Burgess
  2021-04-22 14:10 ` [RFC 1/2] gdb: enable [] to find children using pretty-printers Andrew Burgess
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Andrew Burgess @ 2021-04-22 14:10 UTC (permalink / raw)
  To: gdb-patches

I've been playing with the idea of reusing the pretty-printers to
provide [] indexing.  I wonder what people thought of this?

All feedback welcome.

Thanks,
Andrew

---

Andrew Burgess (2):
  gdb: enable [] to find children using pretty-printers
  gdb: improve equal check used by the pretty-printer [] support

 gdb/ChangeLog                                 |  25 ++
 gdb/NEWS                                      |   7 +
 gdb/doc/ChangeLog                             |   5 +
 gdb/doc/python.texi                           |   9 +
 gdb/eval.c                                    |  92 ++++--
 gdb/extension-priv.h                          |   6 +
 gdb/extension.c                               |  22 ++
 gdb/extension.h                               |  18 ++
 gdb/guile/guile.c                             |   2 +
 gdb/python/py-prettyprint.c                   | 179 ++++++++++++
 gdb/python/python-internal.h                  |   4 +
 gdb/python/python.c                           |   2 +
 gdb/testsuite/ChangeLog                       |  23 ++
 .../gdb.python/py-prettyprint-subscript.c     | 124 ++++++++
 .../gdb.python/py-prettyprint-subscript.cc    | 187 ++++++++++++
 .../gdb.python/py-prettyprint-subscript.exp   | 194 +++++++++++++
 .../gdb.python/py-prettyprint-subscript.py    | 274 ++++++++++++++++++
 17 files changed, 1156 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.c
 create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.cc
 create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.exp
 create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.py

-- 
2.25.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC 1/2] gdb: enable [] to find children using pretty-printers
  2021-04-22 14:10 [RFC 0/2] Using pretty-printers for [] indexing Andrew Burgess
@ 2021-04-22 14:10 ` Andrew Burgess
  2021-04-22 14:18   ` Eli Zaretskii
  2021-05-13 16:28   ` Simon Marchi
  2021-04-22 14:10 ` [RFC 2/2] gdb: improve equal check used by the pretty-printer [] support Andrew Burgess
  2021-05-13 10:35 ` [RFC 0/2] Using pretty-printers for [] indexing Andrew Burgess
  2 siblings, 2 replies; 7+ messages in thread
From: Andrew Burgess @ 2021-04-22 14:10 UTC (permalink / raw)
  To: gdb-patches

This commit allows GDB to make use of an available pretty-printer to
provide [] indexing into a type.

Consider the following example before any pretty-printer is loaded:

  (gdb) p obj_int
  $1 = {
    m_name = 0x4052a0 "first int array",
    m_nitems = 4,
    m_items = 0x4052c0
  }
  (gdb) p obj_int[3]
  Structure has no component named operator[].

Now, after loading a pretty-printer:

  (gdb) p obj_int
  $2 = int array "first int array" with 4 elements = {3, 6, 4, 2}
  (gdb) p obj_int[3]
  $3 = 2

This []-indexing will only occur for objects whose pretty-printers
have a display_hint of 'array' or 'map'.  And the pretty-printer based
indexing is only used if compiled in methods (for C++) or xmethods are
not available.

The benefit I see in this commit is that this feels like an easy win
for the user; they write the pretty printer and they get this extra
functionality for free.

The downside is the extra complexity within GDB.  I'm still not
completely sure if the benefit is worth the cost, and I'd love to hear
people's thoughts on this.

gdb/ChangeLog:

	* NEWS: Mention use of pretty-printer for [] indexing.
	* eval.c (evaluate_subexp_standard): Use pretty printers to find
	children for BINOP_SUBSCRIPT.
	* extension-priv.h (struct extension_language_ops)
	<val_pretty_printer_find_child>: New member variable.
	* extension.c (ext_lang_pretty_printer_find_child): New function.
	* extension.h (ext_lang_pretty_printer_find_child): Declare new
	function.
	* guile/guile.c (guile_extension_ops): Add new field, initialised
	to NULL.
	* python/py-prettyprint.c (gdbpy_val_pretty_printer_find_child):
	New function.
	* python/python-internal.h (gdbpy_val_pretty_printer_find_child):
	Declare.
	* python/python.c (python_extension_ops): Add new field, initialised
	to gdbpy_val_pretty_printer_find_child.

gdb/doc/ChangeLog:

	* python.texi (Pretty Printing API): Mention the use of
	pretty-printers for [] indexing.

gdb/testsuite/ChangeLog:

	* gdb.python/py-prettyprint-subscript.c: New file.
	* gdb.python/py-prettyprint-subscript.cc: New file.
	* gdb.python/py-prettyprint-subscript.exp: New file.
	* gdb.python/py-prettyprint-subscript.py: New file.
---
 gdb/ChangeLog                                 |  19 ++
 gdb/NEWS                                      |   7 +
 gdb/doc/ChangeLog                             |   5 +
 gdb/doc/python.texi                           |   9 +
 gdb/eval.c                                    |  92 ++++++--
 gdb/extension-priv.h                          |   6 +
 gdb/extension.c                               |  22 ++
 gdb/extension.h                               |  18 ++
 gdb/guile/guile.c                             |   2 +
 gdb/python/py-prettyprint.c                   | 170 ++++++++++++++
 gdb/python/python-internal.h                  |   4 +
 gdb/python/python.c                           |   2 +
 gdb/testsuite/ChangeLog                       |   7 +
 .../gdb.python/py-prettyprint-subscript.c     | 124 ++++++++++
 .../gdb.python/py-prettyprint-subscript.cc    | 131 +++++++++++
 .../gdb.python/py-prettyprint-subscript.exp   | 182 ++++++++++++++
 .../gdb.python/py-prettyprint-subscript.py    | 222 ++++++++++++++++++
 17 files changed, 1005 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.c
 create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.cc
 create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.exp
 create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.py

diff --git a/gdb/NEWS b/gdb/NEWS
index faccf40dd41..a6a5758a480 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -60,6 +60,13 @@
   and "-eiex" that allow options (that would normally appear in a
   gdbearlyinit file) to be passed on the command line.
 
+* If a type has a pretty-printer defined, and the pretty-printer
+  display type is 'map' or 'array', then GDB will now use this pretty
+  printer to implement '[]' indexing into the type.  For example, if
+  object's pretty printer is of type array then object[3] will perform
+  array like indexing and the pretty printer will be used to find
+  element 3 in the array.
+
 * New commands
 
 set debug event-loop
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 9135d415dd1..3bf2ec89fd2 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -1494,6 +1494,15 @@
 printer exists, then this returns @code{None}.
 @end defun
 
+If a pretty printer is available for a value, and if the
+@code{display_hint} for that pretty printer is @samp{array} or
+@samp{map} then @value{GDBN} will make use of the pretty printer to
+provide @code{[]} indexing into the type, if no xmethods
+(@pxref{Xmethods In Python}) or compiled in @code{operator[]} can be
+found.  For example, if @code{object} has a pretty printer defined,
+and the @code{display_hint} is @samp{array}, then @code{object[3]}
+will return the value at index @code{3} in the array.
+
 @node Selecting Pretty-Printers
 @subsubsection Selecting Pretty-Printers
 @cindex selecting python pretty-printers
diff --git a/gdb/eval.c b/gdb/eval.c
index 1b3c974009a..7ce34c7c585 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1329,31 +1329,89 @@ eval_op_subscript (struct type *expect_type, struct expression *exp,
 		   enum noside noside, enum exp_opcode op,
 		   struct value *arg1, struct value *arg2)
 {
+  gdb::optional<gdb_exception> except;
+
+  value_ref_ptr arg1_ref = value_ref_ptr::new_reference (arg1);
+  value_ref_ptr arg2_ref = value_ref_ptr::new_reference (arg2);
+
   if (binop_user_defined_p (op, arg1, arg2))
-    return value_x_binop (arg1, arg2, op, OP_NULL, noside);
-  else
     {
-      /* If the user attempts to subscript something that is not an
-	 array or pointer type (like a plain int variable for example),
-	 then report this as an error.  */
-
-      arg1 = coerce_ref (arg1);
-      struct type *type = check_typedef (value_type (arg1));
-      if (type->code () != TYPE_CODE_ARRAY
-	  && type->code () != TYPE_CODE_PTR)
+      try
+	{
+	  return value_x_binop (arg1, arg2, op, OP_NULL, noside);
+	}
+      catch (const gdb_exception_error &e)
 	{
-	  if (type->name ())
-	    error (_("cannot subscript something of type `%s'"),
-		   type->name ());
+	  if (e.error == NOT_FOUND_ERROR)
+	    except.emplace (std::move (e));
 	  else
-	    error (_("cannot subscript requested type"));
+	    throw;
 	}
+    }
 
-      if (noside == EVAL_AVOID_SIDE_EFFECTS)
-	return value_zero (TYPE_TARGET_TYPE (type), VALUE_LVAL (arg1));
+  arg1 = coerce_ref (arg1);
+  arg2 = coerce_ref (arg2);
+
+  try
+    {
+      struct value *ret
+	= ext_lang_pretty_printer_find_child (arg1, arg2,
+					      exp->language_defn);
+      if (ret != nullptr)
+	return ret;
+    }
+  catch (const gdb_exception_error &e)
+    {
+      /* If the lookup via pretty-printer failed with a
+	 NOT_FOUND_ERROR, then this is the next best error to
+	 report.  */
+      if (e.error == NOT_FOUND_ERROR)
+	throw e;
+
+      /* Lookup via the pretty-printer failed.  If the lookup using
+	 value_x_binop also failed with NOT_FOUND_ERROR then report
+	 that error now.  */
+      if (except.has_value ()
+	  && except->error == NOT_FOUND_ERROR)
+	throw *except;
+
+      /* Otherwise, if value_x_binop gave some other error, lets
+	 report that one now.  */
+      if (except.has_value ())
+	throw *except;
+
+      /* Finally, fall-back to just reporting E.  */
+      throw e;
+    }
+
+  /* If EXCEPT holds a value at this point it means we tried using a
+     user defined function to perform the subscript, and this
+     failed.  We then had a go using the pretty-printer, and that
+     also failed, so now we report the error from the user defined
+     operation.  */
+  if (except.has_value ())
+    throw *except;
+
+  /* If the user attempts to subscript something that is not an
+     array or pointer type (like a plain int variable for example),
+     then report this as an error.  */
+
+  arg1 = coerce_ref (arg1);
+  struct type *type = check_typedef (value_type (arg1));
+  if (type->code () != TYPE_CODE_ARRAY
+      && type->code () != TYPE_CODE_PTR)
+    {
+      if (type->name ())
+	error (_("cannot subscript something of type `%s'"),
+	       type->name ());
       else
-	return value_subscript (arg1, value_as_long (arg2));
+	error (_("cannot subscript requested type"));
     }
+
+  if (noside == EVAL_AVOID_SIDE_EFFECTS)
+    return value_zero (TYPE_TARGET_TYPE (type), VALUE_LVAL (arg1));
+  else
+    return value_subscript (arg1, value_as_long (arg2));
 }
 
 /* A helper function for BINOP_EQUAL.  */
diff --git a/gdb/extension-priv.h b/gdb/extension-priv.h
index 1b6701ba6cc..f9dac680dbb 100644
--- a/gdb/extension-priv.h
+++ b/gdb/extension-priv.h
@@ -162,6 +162,12 @@ struct extension_language_ops
      const struct value_print_options *options,
      const struct language_defn *language);
 
+  /* See the documentation for ext_lang_pretty_printer_find_child in
+     extension.h for details.  */
+  struct value *(*val_pretty_printer_find_child)
+    (const struct extension_language_defn *extlang, struct value *object,
+     struct value *idx, const struct language_defn *language);
+
   /* GDB access to the "frame filter" feature.
      FRAME is the source frame to start frame-filter invocation.  FLAGS is an
      integer holding the flags for printing.  The following elements of
diff --git a/gdb/extension.c b/gdb/extension.c
index 0e0d42685fc..42a9292dc65 100644
--- a/gdb/extension.c
+++ b/gdb/extension.c
@@ -477,6 +477,28 @@ apply_ext_lang_val_pretty_printer (struct value *val,
   return 0;
 }
 
+/* See extension.h.  */
+
+value *
+ext_lang_pretty_printer_find_child (struct value *object,
+				    struct value *idx,
+				    const language_defn *language)
+{
+  for (const struct extension_language_defn *extlang : extension_languages)
+    {
+      if (extlang->ops == nullptr
+	  || extlang->ops->val_pretty_printer_find_child == NULL)
+	continue;
+      struct value *result
+	= extlang->ops->val_pretty_printer_find_child (extlang, object, idx,
+						       language);
+      if (result != nullptr)
+	return result;
+    }
+
+  return nullptr;
+}
+
 /* GDB access to the "frame filter" feature.
    FRAME is the source frame to start frame-filter invocation.  FLAGS is an
    integer holding the flags for printing.  The following elements of
diff --git a/gdb/extension.h b/gdb/extension.h
index a505c68d25e..cfb2913035f 100644
--- a/gdb/extension.h
+++ b/gdb/extension.h
@@ -302,6 +302,24 @@ extern const struct extension_language_defn *get_breakpoint_cond_ext_lang
 
 extern int breakpoint_ext_lang_cond_says_stop (struct breakpoint *);
 
+/* Use the pretty printers to lookup CHILD, a named child of OBJECT_VAL.
+   The lookup is done, either by calling the 'child' method within the
+   pretty printer, or by matching against the names returned by the
+   'children' method of the pretty printer.
+
+   If OBJECT doesn't have a pretty-printer, or the display_hint for OBJECT
+   is neither map or array then nullptr is silently returned.
+
+   If there's an error from the scripting language then an error will have
+   been shown to the user, and nullptr will be returned.
+
+   However, if OBJECT's display_hint was suitable, and GDB successfully
+   searched through all the children but was unable to find a matching
+   child, then a NOT_FOUND_ERROR will be thrown.  */
+
+extern value *ext_lang_pretty_printer_find_child
+  (struct value *object, struct value *idx, const language_defn *language);
+
 /* If a method with name METHOD_NAME is to be invoked on an object of type
    TYPE, then all extension languages are searched for implementations of
    methods with name METHOD_NAME.  All matches found are appended to the WORKERS
diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index 68c4532b390..18a90742bea 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -123,6 +123,8 @@ static const struct extension_language_ops guile_extension_ops =
 
   gdbscm_apply_val_pretty_printer,
 
+  NULL, /* gdbscm_val_pretty_printer_find_child, */
+
   NULL, /* gdbscm_apply_frame_filter, */
 
   gdbscm_preserve_values,
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index 7c0fdaa9e70..effbfcbfe28 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -684,3 +684,173 @@ gdbpy_default_visualizer (PyObject *self, PyObject *args)
 
   return find_pretty_printer (val_obj).release ();
 }
+
+/* See ext_lang_pretty_printer_find_child in extension.h for details.  */
+
+struct value *
+gdbpy_val_pretty_printer_find_child (const extension_language_defn *extlang,
+				     struct value *object,
+				     struct value *idx,
+				     const language_defn *language)
+{
+  /* A simple wrapper to make returning easier.  If we got an error then
+     print the Python stack, and return nullptr.  Otherwise, return the
+     value passed in.  */
+  auto print_errors_before_return = [] (struct value *ret) -> struct value *
+  {
+    if (PyErr_Occurred ())
+      {
+	if (!PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
+	  {
+	    ret = nullptr;
+	    gdbpy_print_stack ();
+	  }
+      }
+    return ret;
+  };
+
+  struct gdbarch *gdbarch = value_type (object)->arch ();
+
+  gdbpy_enter enter_py (gdbarch, language);
+
+  /* Prepare the value to be passed to the pretty printer.  */
+  gdbpy_ref<> val_obj (value_to_value_object_no_release (object));
+  if (val_obj == nullptr)
+    return print_errors_before_return (nullptr);
+
+  /* Find the pretty-printer constructor.  */
+  gdbpy_ref<> printer (find_pretty_printer (val_obj.get ()));
+  if (printer == nullptr || printer == Py_None)
+    return print_errors_before_return (nullptr);
+
+  /* If the pretty printer has no 'children' method then we're not going to
+     be able to lookup any children.  */
+  if (!PyObject_HasAttr (printer.get (), gdbpy_children_cst))
+    return print_errors_before_return (nullptr);
+
+  /* If we get here then the pretty printer has no 'child' method, but does
+     have the 'children' method.  We can manually walk the children and
+     find one that matches IDX.  First, find out how the children are
+     represented by the children method.  */
+  gdb::unique_xmalloc_ptr<char> hint (gdbpy_get_display_hint (printer.get ()));
+  bool is_map = hint && ! strcmp (hint.get (), "map");
+  bool is_array = hint && ! strcmp (hint.get (), "array");
+
+  /* There are 4 different display_hint values, map, array, string, or the
+     default (when display_hint returns None).  Of these it is only
+     feasible to find children for map and array.  For the other
+     display_hint modes just return nullptr so GDB will know it has to look
+     up the child on its own.  */
+  if (!is_map && !is_array)
+    return print_errors_before_return (nullptr);
+
+  /* Use the children method to create an iterator.  */
+  gdbpy_ref<> children (PyObject_CallMethodObjArgs (printer.get (),
+						    gdbpy_children_cst,
+						    NULL));
+  if (children == nullptr)
+    return print_errors_before_return (nullptr);
+
+  gdbpy_ref<> iter (PyObject_GetIter (children.get ()));
+  if (iter == nullptr)
+    return print_errors_before_return (nullptr);
+
+  /* This flag is only ever set if we're displaying a 'map'.  For maps
+     the children alternate between being names and values.  This is set
+     when we see a name child.  */
+
+  /* This flag is used when the display_hint is 'map'.  As we walk through
+     the children (for a map) alternative children are keys, then values.
+     When we see a matching key this flag is set true, the next child is
+     then returned as the result.  */
+  bool prev_matched_as_name = false;
+
+  /* This variable is used when the display_hint is 'array'.  We set this
+     variable here to be the array index we are searching for.  As we
+     iterate through the children when we reach this index, we return the
+     result.  */
+  LONGEST array_index_to_find = 0;
+  if (is_array)
+    {
+      /* Convert idx to an array index to look for.  */
+      if (check_typedef (value_type (idx))->code () != TYPE_CODE_INT)
+	return print_errors_before_return (nullptr);
+      array_index_to_find = value_as_long (idx);
+    }
+
+  /* Now iterate through the children.  */
+  gdbpy_ref<> item;
+  for (LONGEST i = 0; ; ++i)
+    {
+      PyObject *py_v;
+      const char *name;
+
+      /* Try to get the next item from the iterator.  When we get back
+	 nullptr then all children have been considered.  */
+      item.reset (PyIter_Next (iter.get ()));
+      if (item == nullptr)
+	break;
+
+      /* Ensure the data we got back was in the correct layout.  */
+      if (!PyTuple_Check (item.get ()) || PyTuple_Size (item.get ()) != 2)
+	{
+	  PyErr_SetString (PyExc_TypeError,
+			   _("Result of children iterator not a tuple"
+			     " of two elements."));
+	  return print_errors_before_return (nullptr);
+	}
+      if (!PyArg_ParseTuple (item.get (), "sO", &name, &py_v))
+	{
+	  /* The user won't necessarily get a stack trace here, so provide
+	     more context.  */
+	  if (gdbpy_print_python_errors_p ())
+	    fprintf_unfiltered (gdb_stderr,
+				_("Bad result from children iterator.\n"));
+	  return print_errors_before_return (nullptr);
+	}
+
+      if (is_map)
+	{
+	  if (i % 2 == 0)
+	    {
+	      /* This child represents the name of the next child we will
+		 pull from the iterator.  Check to see if this is the name
+		 we're looking for.  */
+	      struct value *tmp = convert_value_from_python (py_v);
+	      prev_matched_as_name = value_equal (idx, tmp);
+	    }
+	  else if (prev_matched_as_name)
+	    {
+	      gdb_assert (i % 2 == 1);
+	      /* The previous child (which represented a name) matched the
+		 name we are looking for, this then is the value we want.  */
+	      struct value *tmp = convert_value_from_python (py_v);
+	      return print_errors_before_return (tmp);
+	    }
+	}
+      else if (is_array)
+	{
+	  /* This assumes that all languages index their arrays from 0,
+	     which we know is not the case.  It might be nice to handle
+	     these languages slightly differently here.  */
+	  if (i == array_index_to_find)
+	    {
+	      struct value *tmp = convert_value_from_python (py_v);
+	      return print_errors_before_return (tmp);
+	    }
+	}
+      else
+	gdb_assert_not_reached ("unexpected display hint mode");
+    }
+
+  if (PyErr_Occurred ())
+    return print_errors_before_return (nullptr);
+
+  if (is_array)
+    throw_error (NOT_FOUND_ERROR, _("Array subscript %s out of bounds."),
+		 plongest (array_index_to_find));
+  else if (is_map)
+    throw_error (NOT_FOUND_ERROR, _("Unknown key value."));
+
+  return nullptr;
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 56702cad53a..7b8b5291d7c 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -392,6 +392,10 @@ extern enum ext_lang_rc gdbpy_apply_val_pretty_printer
    struct ui_file *stream, int recurse,
    const struct value_print_options *options,
    const struct language_defn *language);
+extern struct value *gdbpy_val_pretty_printer_find_child
+  (const extension_language_defn *extlang,
+   struct value *object, struct value *index,
+   const language_defn *language);
 extern enum ext_lang_bt_status gdbpy_apply_frame_filter
   (const struct extension_language_defn *,
    struct frame_info *frame, frame_filter_flags flags,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 9eed258c181..3326d35220c 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -173,6 +173,8 @@ const struct extension_language_ops python_extension_ops =
 
   gdbpy_apply_val_pretty_printer,
 
+  gdbpy_val_pretty_printer_find_child,
+
   gdbpy_apply_frame_filter,
 
   gdbpy_preserve_values,
diff --git a/gdb/testsuite/gdb.python/py-prettyprint-subscript.c b/gdb/testsuite/gdb.python/py-prettyprint-subscript.c
new file mode 100644
index 00000000000..358479017f9
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prettyprint-subscript.c
@@ -0,0 +1,124 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <string.h>
+#include <stdlib.h>
+
+#define CONCAT2(a,b) a##b
+
+#define MAKE_ARRAY_TYPE(TYPENAME,TYPE)				\
+  typedef struct TYPENAME					\
+  {								\
+    const char *m_name;						\
+    int m_nitems;						\
+    TYPE *m_items;						\
+  } TYPENAME;							\
+								\
+  void								\
+  CONCAT2(TYPENAME,_init) (TYPENAME *object, const char *name)	\
+  {								\
+    object->m_name = strdup (name);				\
+    object->m_nitems = 0;					\
+    object->m_items = NULL;					\
+  }								\
+								\
+  void								\
+  CONCAT2(TYPENAME,_push_back) (TYPENAME *object, TYPE item)	\
+  {								\
+    object->m_nitems++;						\
+    object->m_items						\
+      = realloc (object->m_items, (sizeof (TYPE)		\
+				   * object->m_nitems));	\
+    object->m_items[object->m_nitems - 1] = item;		\
+  }
+
+#define MAKE_MAP_TYPE(TYPENAME,K_TYPE,V_TYPE)			\
+  struct CONCAT2(TYPENAME,_entry)				\
+  {								\
+    K_TYPE key;							\
+    V_TYPE value;						\
+  };								\
+								\
+  typedef struct TYPENAME					\
+  {								\
+    const char *m_name;						\
+    int m_nitems;						\
+    struct CONCAT2(TYPENAME,_entry) *m_items;			\
+  } TYPENAME;							\
+								\
+  void								\
+  CONCAT2(TYPENAME,_init) (TYPENAME *object, const char *name)	\
+  {								\
+    object->m_name = strdup (name);				\
+    object->m_nitems = 0;						\
+    object->m_items = NULL;					\
+  }								\
+								\
+  void								\
+  CONCAT2(TYPENAME,_insert) (TYPENAME *object, K_TYPE key, V_TYPE value) \
+  {									\
+    int i;								\
+									\
+    for (i = 0; i < object->m_nitems; ++i)				\
+      {									\
+	if (object->m_items[i].key == key)				\
+	  {								\
+	    object->m_items[i].value = value;				\
+	    return;							\
+	  }								\
+      }									\
+    									\
+    object->m_nitems++;							\
+    object->m_items							\
+      = realloc (object->m_items, (sizeof (struct CONCAT2(TYPENAME,_entry)) \
+				   * object->m_nitems));		\
+    object->m_items[object->m_nitems - 1].key = key;			\
+    object->m_items[object->m_nitems - 1].value = value;		\
+  }
+
+MAKE_ARRAY_TYPE (int_array,int)
+MAKE_ARRAY_TYPE (float_array,float)
+
+MAKE_MAP_TYPE (int_int_map,int,int)
+
+int
+main ()
+{
+  int_array obj_int;
+  float_array obj_float;
+  int_int_map obj_int_int;
+
+  int_array_init (&obj_int, "first int array");
+  int_array_push_back (&obj_int, 3);
+  int_array_push_back (&obj_int, 6);
+  int_array_push_back (&obj_int, 4);
+  int_array_push_back (&obj_int, 2);
+
+  float_array_init (&obj_float, "first float array");
+  float_array_push_back (&obj_float, 3.1);
+  float_array_push_back (&obj_float, 6.2);
+  float_array_push_back (&obj_float, 4.3);
+  float_array_push_back (&obj_float, 2.4);
+
+  int_int_map_init (&obj_int_int, "int to int map");
+  int_int_map_insert (&obj_int_int, 3, 99);
+  int_int_map_insert (&obj_int_int, 5, 21);
+  int_int_map_insert (&obj_int_int, 8, 16);
+  int_int_map_insert (&obj_int_int, 9, 42);
+
+  return 0;	/* Breakpoint 1.  */
+}
diff --git a/gdb/testsuite/gdb.python/py-prettyprint-subscript.cc b/gdb/testsuite/gdb.python/py-prettyprint-subscript.cc
new file mode 100644
index 00000000000..c446fc82d67
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prettyprint-subscript.cc
@@ -0,0 +1,131 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <cstring>
+#include <cstdlib>
+#include <stdexcept>
+
+template<typename TYPE>
+class generic_array
+{
+public:
+  generic_array (const char *name)
+    : m_nitems (0),
+      m_items (nullptr)
+  {
+    m_name = strdup (name);
+  }
+
+  void
+  push_back (TYPE item)
+  {
+    m_nitems++;
+    m_items = (TYPE *) realloc (m_items, (sizeof (TYPE) * m_nitems));
+    m_items[m_nitems - 1] = item;
+  }
+
+#ifdef DEFINE_SUBSCRIPT_OPERATOR
+  TYPE &
+  operator[] (int idx)
+  {
+    if (idx < 0 || idx >= m_nitems)
+      throw std::out_of_range ("array index out of bounds");
+    return m_items[idx];
+  }
+#endif
+
+private:
+  const char *m_name;
+  int m_nitems;
+  TYPE *m_items;
+};
+
+template<typename K_TYPE, typename V_TYPE>
+class generic_map
+{
+public:
+  generic_map (const char *name)
+    : m_nitems (0),
+      m_items (nullptr)
+  {
+    m_name = strdup (name);
+  }
+
+  void
+  insert (K_TYPE key, V_TYPE value)
+  {
+    for (int i = 0; i < m_nitems; ++i)
+      {
+	if (m_items[i].key == key)
+	  {
+	    m_items[i].value = value;
+	    return;
+	  }
+      }
+
+    m_nitems++;
+    m_items
+      = (map_entry *) realloc (m_items, (sizeof (map_entry)) * m_nitems);
+    m_items[m_nitems - 1].key = key;
+    m_items[m_nitems - 1].value = value;
+}
+
+private:
+
+  struct map_entry
+  {
+    K_TYPE key;
+    V_TYPE value;
+  };
+
+  const char *m_name;
+  int m_nitems;
+  map_entry *m_items;
+};
+
+volatile int dump_int;
+volatile float dump_float;
+
+int
+main ()
+{
+  generic_array<int> obj_int ("first int array");
+  generic_array<float> obj_float ("first float array");
+  generic_map<int,int> obj_int_int ("int to int map");
+
+  obj_int.push_back (3);
+  obj_int.push_back (6);
+  obj_int.push_back (4);
+  obj_int.push_back (2);
+
+  obj_float.push_back (3.1);
+  obj_float.push_back (6.2);
+  obj_float.push_back (4.3);
+  obj_float.push_back (2.4);
+
+  obj_int_int.insert (3, 99);
+  obj_int_int.insert (5, 21);
+  obj_int_int.insert (8, 16);
+  obj_int_int.insert (9, 42);
+
+#ifdef DEFINE_SUBSCRIPT_OPERATOR
+  dump_int = obj_int[0];
+  dump_float = obj_float[0];
+#endif
+
+  return 0;	/* Breakpoint 1.  */
+}
diff --git a/gdb/testsuite/gdb.python/py-prettyprint-subscript.exp b/gdb/testsuite/gdb.python/py-prettyprint-subscript.exp
new file mode 100644
index 00000000000..d53aadc2643
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prettyprint-subscript.exp
@@ -0,0 +1,182 @@
+# Copyright (C) 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Use the pretty-printer to provide subscripted access to arrays and
+# maps.
+
+load_lib gdb-python.exp
+
+# Skip all tests if Python scripting is not enabled.  We need to start
+# GDB in order to check if Python is supported.
+gdb_exit
+gdb_start
+if { [skip_python_tests] } { continue }
+
+# Send the pretty-printer script to the remote.
+set remote_python_file [gdb_remote_download host \
+			    ${srcdir}/${subdir}/${gdb_test_file_name}.py]
+
+# Return a string that is a regular expression for the error we expect
+# when the user attempts to access an out of bounds array index.
+#
+# IDX is the integer for the index being accessed.
+# USE_SUBSCRIPT_OPERATORS is a boolean, have the operator[] methods
+#    been compiled in?
+# USE_XMETHODS is a boolean, have the Python xmethods been registered?
+proc build_array_bounds_error { idx use_subscript_operators use_xmethods } {
+    if { $use_xmethods } {
+	set msg [multi_line \
+		     "Python Exception <class 'gdb.GdbError'>: array index ${idx} out of bounds" \
+		     "Error while executing Python code."]
+    } elseif { $use_subscript_operators } {
+	return [multi_line \
+		    "The program being debugged entered a std::terminate call, most likely" \
+		    "caused by an unhandled C\\+\\+ exception\\..*" \
+		    "Evaluation of the expression containing the function \\(.*::operator\\\[\\\]\\(int\\)\\)" \
+		    "will be abandoned\\."]
+    } else {
+	set msg "Array subscript ${idx} out of bounds."
+    }
+
+    return [string_to_regexp $msg]
+}
+
+# Return a string that is a regular expression for the error we expect
+# when the user attempts to access an invalid key in a map.
+#
+# IDX is the integer for the index being accessed.
+# USE_XMETHODS is a boolean, have the Python xmethods been registered?
+proc build_map_key_error { key use_xmethods } {
+    if { $use_xmethods } {
+	set msg [multi_line \
+		     "Python Exception <class 'gdb.GdbError'>: invalid map key ${key}" \
+		     "Error while executing Python code."]
+    } else {
+	set msg "Unknown key value."
+    }
+
+    return [string_to_regexp $msg]
+}
+
+# Run the tests using the source file for LANG (either 'c' or 'c++').
+# USE_SUBSCRIPT_OPERATORS is a boolean, should the operator[] methods
+# be compiled in?  USE_XMETHODS is a boolean, should the Python
+# xmethods be loaded?
+proc run_tests {lang use_subscript_operators use_xmethods} {
+    global testfile srcfile srcdir subdir remote_python_file
+    global decimal hex
+
+    set options [list debug]
+    if { $lang == "c++" } {
+	standard_testfile .cc
+	set suffix "-cpp"
+	lappend options {c++}
+    } else {
+	standard_testfile .c
+	set suffix ""
+    }
+
+    set maybe_ref ""
+    if { $use_subscript_operators } {
+	lappend options {additional_flags=-DDEFINE_SUBSCRIPT_OPERATOR}
+	set suffix "${suffix}-operators"
+	if { !$use_xmethods } {
+	    set maybe_ref "\\(\\S+ &\\) @$hex: "
+	}
+    }
+
+    set maybe_xm ""
+    if { $use_xmethods } {
+	set suffix "${suffix}-xm"
+	set maybe_xm "xmethod found it\r\n"
+    }
+
+    if { [prepare_for_testing "failed to prepare" "${testfile}${suffix}" \
+	      $srcfile ${options}] } {
+	return
+    }
+
+    if ![runto_main] {
+	return -1
+    }
+
+    gdb_test_no_output "source ${remote_python_file}" "load python file"
+
+    if { $use_xmethods } {
+	gdb_test_no_output "python register_xmethods ()"
+    }
+
+    gdb_breakpoint [gdb_get_line_number "Breakpoint 1"]
+    gdb_continue_to_breakpoint "Breakpoint 1"
+
+    gdb_test "p obj_int" \
+	" = int array \"first int array\" with 4 elements = \\{3, 6, 4, 2\\}"
+    gdb_test "p obj_int\[0\]" "${maybe_xm}\\\$${decimal} = ${maybe_ref}3"
+    gdb_test "p obj_int\[1\]" "${maybe_xm}\\\$${decimal} = ${maybe_ref}6"
+    gdb_test "p obj_int\[2\]" "${maybe_xm}\\\$${decimal} = ${maybe_ref}4"
+    gdb_test "p obj_int\[3\]" "${maybe_xm}\\\$${decimal} = ${maybe_ref}2"
+
+    set err_msg [build_array_bounds_error -1 $use_subscript_operators $use_xmethods]
+    verbose -log "Expecting '${err_msg}'"
+    gdb_test "p obj_int\[-1\]" ${err_msg}
+    set err_msg [build_array_bounds_error 4 $use_subscript_operators $use_xmethods]
+    gdb_test "p obj_int\[4\]" ${err_msg}
+
+    gdb_test "p obj_float" \
+	" = float array \"first float array\" with 4 elements = \\{3\\.$decimal, 6\\.$decimal, 4\\.$decimal, 2\\.$decimal\\}"
+    gdb_test "p obj_float\[0\]" "${maybe_xm}\\\$${decimal} = ${maybe_ref}3\\.$decimal"
+    gdb_test "p obj_float\[1\]" "${maybe_xm}\\\$${decimal} = ${maybe_ref}6\\.$decimal"
+    gdb_test "p obj_float\[2\]" "${maybe_xm}\\\$${decimal} = ${maybe_ref}4\\.$decimal"
+    gdb_test "p obj_float\[3\]" "${maybe_xm}\\\$${decimal} = ${maybe_ref}2\\.$decimal"
+    set err_msg [build_array_bounds_error -1 $use_subscript_operators $use_xmethods]
+    gdb_test "p obj_float\[-1\]" ${err_msg}
+    set err_msg [build_array_bounds_error 4 $use_subscript_operators $use_xmethods]
+    gdb_test "p obj_float\[4\]" ${err_msg}
+
+    gdb_test "p obj_int_int" \
+	" = int -> int map \"int to int map\" with 4 elements = \\{\\\[3\\\] = 99, \\\[5\\\] = 21, \\\[8\\\] = 16, \\\[9\\\] = 42\\}"
+    gdb_test "p obj_int_int\[3\]" "${maybe_xm}\\\$${decimal} = 99"
+    gdb_test "p obj_int_int\[5\]" "${maybe_xm}\\\$${decimal} = 21"
+    gdb_test "p obj_int_int\[8\]" "${maybe_xm}\\\$${decimal} = 16"
+    gdb_test "p obj_int_int\[9\]" "${maybe_xm}\\\$${decimal} = 42"
+    set err_msg [build_map_key_error 2 $use_xmethods]
+    gdb_test "p obj_int_int\[2\]" ${err_msg}
+    set err_msg [build_map_key_error 10 $use_xmethods]
+    gdb_test "p obj_int_int\[10\]" ${err_msg}
+}
+
+# Each test-spec is {
+#  LANGUAGE - Either 'c' or 'c++'
+#  USE_SUBSCRIPT_OPERATORS - A bool.
+#  USE_XMETHODS - A bool.
+# }
+foreach test_spec { {c false false} \
+		    {c++ false false} \
+		    {c++ false true} \
+		    {c++ true false} \
+		    {c++ true true} \
+		  } {
+    set lang [lindex $test_spec 0]
+    set use_subscript_operators [lindex $test_spec 1]
+    set use_xmethods [lindex $test_spec 2]
+
+    set prefix "lang=${lang}"
+    set prefix "${prefix}, subscript_ops=${use_subscript_operators}"
+    set prefix "${prefix}, xmethods=${use_xmethods}"
+
+    with_test_prefix $prefix {
+	run_tests ${lang} ${use_subscript_operators} ${use_xmethods}
+    }
+}
diff --git a/gdb/testsuite/gdb.python/py-prettyprint-subscript.py b/gdb/testsuite/gdb.python/py-prettyprint-subscript.py
new file mode 100644
index 00000000000..79eae646240
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-prettyprint-subscript.py
@@ -0,0 +1,222 @@
+# Copyright (C) 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import re
+import gdb
+import gdb.xmethod
+
+def _iterator (pointer, len):
+    start = pointer
+    end = pointer + len
+    while pointer != end:
+        yield ('[%d]' % int (pointer - start), pointer.dereference())
+        pointer += 1
+
+class pp_generic_array:
+    def __init__ (self, type, val):
+        self.val = val
+        self.type = type
+
+    def to_string (self):
+        name = self.val['m_name'].string ()
+        return ('%s array \"%s\" with %d elements'
+                % (self.type, name, self.val['m_nitems']))
+
+    def children(self):
+        return _iterator(self.val['m_items'], self.val['m_nitems'])
+
+    def display_hint (self):
+        return 'array'
+
+class pp_generic_map:
+    def __init__ (self, key_type, value_type, val):
+        self.val = val
+        self.key_type = key_type
+        self.value_type = value_type
+
+    def to_string (self):
+        name = self.val['m_name'].string ()
+        return ('%s -> %s map \"%s\" with %d elements'
+                % (self.key_type, self.value_type, name,
+                   self.val['m_nitems']))
+
+    def children(self):
+        start = self.val['m_items']
+        end = start + self.val['m_nitems']
+        pointer = start
+        while pointer != end:
+            obj = pointer.dereference ()
+            yield ('[%d]' % int (pointer - start), obj['key'])
+            yield ('[%d]' % int (pointer - start), obj['value'])
+            pointer += 1
+
+    def display_hint (self):
+        return 'map'
+
+class pretty_printer_finder:
+    def __init__ (self):
+        self.dict = {}
+        self.dict[re.compile ('^int_array$')] \
+            = lambda v : pp_generic_array ('int', v)
+        self.dict[re.compile ('^float_array$')] \
+            = lambda v : pp_generic_array ('float', v)
+        self.dict[re.compile ('^int_int_map$')] \
+            = lambda v : pp_generic_map ('int', 'int', v)
+        self.dict[re.compile ('^generic_array<int>$')] \
+            = lambda v : pp_generic_array ('int', v)
+        self.dict[re.compile ('^generic_array<float>$')] \
+            = lambda v : pp_generic_array ('float', v)
+        self.dict[re.compile ('^generic_map<int, int>$')] \
+            = lambda v : pp_generic_map ('int', 'int', v)
+
+    def lookup (self, value):
+        type = value.type
+
+        # If it points to a reference, get the reference.
+        if type.code == gdb.TYPE_CODE_REF:
+            type = type.target ()
+
+        # Get the unqualified type, stripped of typedefs.
+        type = type.unqualified ().strip_typedefs ()
+
+        # Get the type name.
+        typename = type.tag
+
+        if typename == None:
+            return None
+
+        # Iterate over local dictionary of types to determine
+        # if a printer is registered for that type.  Return an
+        # instantiation of the printer if found.
+        for function in self.dict:
+            if function.match (typename):
+                return self.dict[function] (value)
+
+        # Cannot find a pretty printer.  Return None.
+        print ("No PP for %s" % typename)
+        return None
+
+pp_finder = pretty_printer_finder ()
+gdb.pretty_printers.append (lambda v : pp_finder.lookup (v))
+
+class generic_array_subscript (gdb.xmethod.XMethod):
+
+    class worker (gdb.xmethod.XMethodWorker):
+        def get_arg_types (self):
+            return [gdb.lookup_type ('int')]
+
+        def get_result_type(self, obj):
+            return gdb.lookup_type(self.type)
+
+        def __call__(self, obj, idx):
+            idx = int (idx)
+            if idx < 0 or idx >= obj['m_nitems']:
+                raise gdb.GdbError ("array index %d out of bounds" % idx)
+            ptr = obj['m_items'] + idx
+            print ("xmethod found it")
+            return ptr.dereference ()
+
+        def __init__ (self, type):
+            gdb.xmethod.XMethodWorker.__init__ (self)
+            self.type = type
+
+    def __init__ (self, type):
+        gdb.xmethod.XMethod.__init__ (self, "generic_array<%s>::operator[]"
+                                      % type)
+        self.type = type
+
+    def get_worker (self, method_name):
+        if method_name == 'operator[]':
+            return self.worker (self.type)
+
+    def class_tag (self):
+        return "generic_array<%s>" % self.type
+
+class generic_array_matcher (gdb.xmethod.XMethodMatcher):
+    def __init__ (self):
+        gdb.xmethod.XMethodMatcher.__init__ (self, 'generic_array_matcher')
+        self.methods = [generic_array_subscript ('int'),
+                        generic_array_subscript ('float')]
+
+    def match (self, class_type, method_name):
+        if (class_type.tag == 'generic_array<int>'
+            or class_type.tag == 'generic_array<float>'):
+            workers = []
+            for method in self.methods:
+                if method.enabled and class_type.tag == method.class_tag ():
+                    worker = method.get_worker (method_name)
+                    if worker:
+                        workers.append (worker)
+            return workers
+        return None
+
+class generic_map_subscript (gdb.xmethod.XMethod):
+
+    class worker (gdb.xmethod.XMethodWorker):
+        def get_arg_types (self):
+            return [gdb.lookup_type ('int')]
+
+        def get_result_type(self, obj):
+            return gdb.lookup_type(self.type)
+
+        def __call__(self, obj, idx):
+            if str (idx.type) != self.k_type:
+                raise gdb.GdbError ("invalid key type")
+            start = obj['m_items']
+            end = start + obj['m_nitems']
+            pointer = start
+            while pointer != end:
+                child = pointer.dereference ()
+                if (child['key'] == idx):
+                    print ("xmethod found it")
+                    return child['value']
+                pointer += 1
+            raise gdb.GdbError ("invalid map key %s" % idx)
+
+        def __init__ (self, k_type, v_type):
+            gdb.xmethod.XMethodWorker.__init__ (self)
+            self.k_type = k_type
+            self.v_type = v_type
+
+    def __init__ (self, k_type, v_type):
+        gdb.xmethod.XMethod.__init__ (self, "generic_map<%s, %s>::operator[]"
+                                      % (k_type, v_type))
+        self.k_type = k_type
+        self.v_type = v_type
+
+    def get_worker (self, method_name):
+        if method_name == 'operator[]':
+            return self.worker (self.k_type, self.v_type)
+
+    def class_tag (self):
+        return "generic_map<%s, %s>" % (self.k_type, self.v_type)
+
+class generic_map_matcher (gdb.xmethod.XMethodMatcher):
+    def __init__ (self):
+        gdb.xmethod.XMethodMatcher.__init__ (self, 'generic_map_matcher')
+        self.methods = [generic_map_subscript ('int', 'int')]
+
+    def match (self, class_type, method_name):
+        workers = []
+        for method in self.methods:
+            if method.enabled and class_type.tag == method.class_tag ():
+                worker = method.get_worker (method_name)
+                if worker:
+                    workers.append (worker)
+        return workers
+
+def register_xmethods ():
+    gdb.xmethod.register_xmethod_matcher (None, generic_array_matcher ())
+    gdb.xmethod.register_xmethod_matcher (None, generic_map_matcher ())
-- 
2.25.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC 2/2] gdb: improve equal check used by the pretty-printer [] support
  2021-04-22 14:10 [RFC 0/2] Using pretty-printers for [] indexing Andrew Burgess
  2021-04-22 14:10 ` [RFC 1/2] gdb: enable [] to find children using pretty-printers Andrew Burgess
@ 2021-04-22 14:10 ` Andrew Burgess
  2021-05-13 10:35 ` [RFC 0/2] Using pretty-printers for [] indexing Andrew Burgess
  2 siblings, 0 replies; 7+ messages in thread
From: Andrew Burgess @ 2021-04-22 14:10 UTC (permalink / raw)
  To: gdb-patches

When performing a BINOP_SUBSCRIPT via the pretty printers, if the
pretty printer represents a map, then the index field could be any
type.

This commit extends GDB so that we can use any compiled in equality
operators, or any available xmethods when comparing the index as
returned by the pretty-printer to the user supplied index, this allows
for better handling of C++ string based maps.

gdb/ChangeLog:

	* python/py-prettyprint.c: Add 'expop.h' include.
	(gdbpy_val_pretty_printer_find_child): Call eval_op_equal instead
	of value_equal.

gdb/testsuite/ChangeLog:

	* gdb.python/py-prettyprint-subscript.cc (class string_type): New
	class.
	(dump_bool): New global.
	(obj_str_int): New global.
	(main): Fill new global.
	* gdb.python/py-prettyprint-subscript.exp (run_tests): Add new
	tests.
	* gdb.python/py-prettyprint-subscript.py (class pp_string_type):
	New class.
	(class pretty_printer_finder): Add lookup for string to int type.
	(generic_array_subscript::worker::__call__): Add comment.
	(generic_map_subscript::worker::__call__): Add comment.
	(class string_type_xmethod): New class.
---
 gdb/ChangeLog                                 |  6 ++
 gdb/python/py-prettyprint.c                   | 11 +++-
 gdb/testsuite/ChangeLog                       | 16 ++++++
 .../gdb.python/py-prettyprint-subscript.cc    | 56 +++++++++++++++++++
 .../gdb.python/py-prettyprint-subscript.exp   | 12 ++++
 .../gdb.python/py-prettyprint-subscript.py    | 52 +++++++++++++++++
 6 files changed, 152 insertions(+), 1 deletion(-)

diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index effbfcbfe28..2e083375f28 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -26,6 +26,7 @@
 #include "python.h"
 #include "python-internal.h"
 #include "cli/cli-style.h"
+#include "expop.h"
 
 /* Return type of print_string_repr.  */
 
@@ -817,7 +818,15 @@ gdbpy_val_pretty_printer_find_child (const extension_language_defn *extlang,
 		 pull from the iterator.  Check to see if this is the name
 		 we're looking for.  */
 	      struct value *tmp = convert_value_from_python (py_v);
-	      prev_matched_as_name = value_equal (idx, tmp);
+	      struct expression exp (language, gdbarch);
+
+	      /* Call through eval_op_equal here so that we get the chance
+		 to use the compiled in operators, or any xmethods that are
+		 available.  This call will fall back to value_equal if
+		 there's no special handling available.  */
+	      struct value *res = eval_op_equal (nullptr, &exp, EVAL_NORMAL,
+						 BINOP_EQUAL, tmp, idx);
+	      prev_matched_as_name = (bool) value_as_long (res);
 	    }
 	  else if (prev_matched_as_name)
 	    {
diff --git a/gdb/testsuite/gdb.python/py-prettyprint-subscript.cc b/gdb/testsuite/gdb.python/py-prettyprint-subscript.cc
index c446fc82d67..cfda4dc127f 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint-subscript.cc
+++ b/gdb/testsuite/gdb.python/py-prettyprint-subscript.cc
@@ -19,6 +19,52 @@
 #include <cstdlib>
 #include <stdexcept>
 
+/* A very minimal string like class.  This is used instead of std::string
+   so that we can provide known pretty-printers and xmethods for this
+   class.  */
+
+class string_type
+{
+public:
+  string_type ()
+  {
+    m_s = strdup ("");
+  }
+
+  string_type (const char *s)
+  {
+    m_s = strdup (s);
+  }
+
+  string_type (const string_type &other)
+  {
+    m_s = strdup (other.m_s);
+  }
+
+  string_type &
+  operator= (const string_type &other)
+  {
+    free (m_s);
+    m_s = strdup (other.m_s);
+    return *this;
+  }
+
+  bool
+  operator== (const string_type &other)
+  {
+    return strcmp (m_s, other.m_s) == 0;
+  }
+
+  bool
+  operator== (const char *other)
+  {
+    return other != nullptr && strcmp (m_s, other) == 0;
+  }
+
+private:
+  char *m_s = nullptr;
+};
+
 template<typename TYPE>
 class generic_array
 {
@@ -99,6 +145,7 @@ private:
 
 volatile int dump_int;
 volatile float dump_float;
+volatile bool dump_bool;
 
 int
 main ()
@@ -106,6 +153,7 @@ main ()
   generic_array<int> obj_int ("first int array");
   generic_array<float> obj_float ("first float array");
   generic_map<int,int> obj_int_int ("int to int map");
+  generic_map<string_type,int> obj_str_int ("string to int map");
 
   obj_int.push_back (3);
   obj_int.push_back (6);
@@ -122,6 +170,14 @@ main ()
   obj_int_int.insert (8, 16);
   obj_int_int.insert (9, 42);
 
+  string_type arg1 = "abc";
+  string_type arg2 = "def";
+  string_type arg3 = "ghi";
+
+  obj_str_int.insert ("abc", 1);
+  obj_str_int.insert ("def", 2);
+  obj_str_int.insert ("ghi", 3);
+
 #ifdef DEFINE_SUBSCRIPT_OPERATOR
   dump_int = obj_int[0];
   dump_float = obj_float[0];
diff --git a/gdb/testsuite/gdb.python/py-prettyprint-subscript.exp b/gdb/testsuite/gdb.python/py-prettyprint-subscript.exp
index d53aadc2643..89436f97d5a 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint-subscript.exp
+++ b/gdb/testsuite/gdb.python/py-prettyprint-subscript.exp
@@ -155,6 +155,18 @@ proc run_tests {lang use_subscript_operators use_xmethods} {
     gdb_test "p obj_int_int\[2\]" ${err_msg}
     set err_msg [build_map_key_error 10 $use_xmethods]
     gdb_test "p obj_int_int\[10\]" ${err_msg}
+
+    # The following tests are only available for C++.
+    if { $lang == "c++" } {
+	gdb_test "p obj_str_int" \
+	    " = string_type -> int map \"string to int map\" with 3 elements = \\{\\\[\"abc\"\\\] = 1, \\\[\"def\"\\\] = 2, \\\[\"ghi\"\\\] = 3\\}"
+	gdb_test "p obj_str_int\[arg1\]" " = 1"
+	gdb_test "p obj_str_int\[arg2\]" " = 2"
+	gdb_test "p obj_str_int\[arg3\]" " = 3"
+	gdb_test "p obj_str_int\[\"abc\"\]" " = 1"
+	gdb_test "p obj_str_int\[\"def\"\]" " = 2"
+	gdb_test "p obj_str_int\[\"ghi\"\]" " = 3"
+    }
 }
 
 # Each test-spec is {
diff --git a/gdb/testsuite/gdb.python/py-prettyprint-subscript.py b/gdb/testsuite/gdb.python/py-prettyprint-subscript.py
index 79eae646240..33c19efb6f0 100644
--- a/gdb/testsuite/gdb.python/py-prettyprint-subscript.py
+++ b/gdb/testsuite/gdb.python/py-prettyprint-subscript.py
@@ -65,6 +65,16 @@ class pp_generic_map:
     def display_hint (self):
         return 'map'
 
+class pp_string_type:
+    def __init__ (self, val):
+        self.val = val
+
+    def to_string (self):
+        return self.val['m_s'].string ()
+
+    def display_hint (self):
+        return 'string'
+
 class pretty_printer_finder:
     def __init__ (self):
         self.dict = {}
@@ -80,6 +90,9 @@ class pretty_printer_finder:
             = lambda v : pp_generic_array ('float', v)
         self.dict[re.compile ('^generic_map<int, int>$')] \
             = lambda v : pp_generic_map ('int', 'int', v)
+        self.dict[re.compile ('^generic_map<string_type, int>$')] \
+            = lambda v : pp_generic_map ('string_type', 'int', v)
+        self.dict[re.compile ('^string_type$')] = pp_string_type
 
     def lookup (self, value):
         type = value.type
@@ -125,6 +138,7 @@ class generic_array_subscript (gdb.xmethod.XMethod):
             if idx < 0 or idx >= obj['m_nitems']:
                 raise gdb.GdbError ("array index %d out of bounds" % idx)
             ptr = obj['m_items'] + idx
+            # This output is checked for by the test.
             print ("xmethod found it")
             return ptr.dereference ()
 
@@ -180,6 +194,7 @@ class generic_map_subscript (gdb.xmethod.XMethod):
             while pointer != end:
                 child = pointer.dereference ()
                 if (child['key'] == idx):
+                    # This output is checked for by the test.
                     print ("xmethod found it")
                     return child['value']
                 pointer += 1
@@ -217,6 +232,43 @@ class generic_map_matcher (gdb.xmethod.XMethodMatcher):
                     workers.append (worker)
         return workers
 
+class string_type_xmethod (gdb.xmethod.XMethod):
+
+    class worker (gdb.xmethod.XMethodWorker):
+        def get_arg_types (self):
+            return [gdb.lookup_type ('char').pointer ()]
+
+        def get_result_type(self, obj):
+            return gdb.lookup_type('bool')
+
+        def __call__(self, obj, other):
+            return gdb.Value (obj['m_s'].string () == other.string ())
+
+    def get_worker (self, method_name):
+        # Method name is always 'operator=='.
+        return self.worker ()
+
+    def __init__ (self):
+        gdb.xmethod.XMethod.__init__ (self, "string_type::operator==")
+
+class string_type_matcher (gdb.xmethod.XMethodMatcher):
+    def __init__ (self):
+        gdb.xmethod.XMethodMatcher.__init__ (self, 'string_type_matcher')
+        self.methods = [string_type_xmethod ()]
+
+    def match (self, class_type, method_name):
+        workers = []
+        for method in self.methods:
+            if (method.enabled and class_type.tag == "string_type"
+                and method_name == "operator=="):
+                worker = method.get_worker (method_name)
+                if worker:
+                    workers.append (worker)
+        return workers
+
 def register_xmethods ():
     gdb.xmethod.register_xmethod_matcher (None, generic_array_matcher ())
     gdb.xmethod.register_xmethod_matcher (None, generic_map_matcher ())
+
+# Always register the string_type xmethod support.
+gdb.xmethod.register_xmethod_matcher (None, string_type_matcher ())
-- 
2.25.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 1/2] gdb: enable [] to find children using pretty-printers
  2021-04-22 14:10 ` [RFC 1/2] gdb: enable [] to find children using pretty-printers Andrew Burgess
@ 2021-04-22 14:18   ` Eli Zaretskii
  2021-05-13 16:28   ` Simon Marchi
  1 sibling, 0 replies; 7+ messages in thread
From: Eli Zaretskii @ 2021-04-22 14:18 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: gdb-patches

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Thu, 22 Apr 2021 15:10:43 +0100
> 
> gdb/ChangeLog:
> 
> 	* NEWS: Mention use of pretty-printer for [] indexing.
> 	* eval.c (evaluate_subexp_standard): Use pretty printers to find
> 	children for BINOP_SUBSCRIPT.
> 	* extension-priv.h (struct extension_language_ops)
> 	<val_pretty_printer_find_child>: New member variable.
> 	* extension.c (ext_lang_pretty_printer_find_child): New function.
> 	* extension.h (ext_lang_pretty_printer_find_child): Declare new
> 	function.
> 	* guile/guile.c (guile_extension_ops): Add new field, initialised
> 	to NULL.
> 	* python/py-prettyprint.c (gdbpy_val_pretty_printer_find_child):
> 	New function.
> 	* python/python-internal.h (gdbpy_val_pretty_printer_find_child):
> 	Declare.
> 	* python/python.c (python_extension_ops): Add new field, initialised
> 	to gdbpy_val_pretty_printer_find_child.
> 
> gdb/doc/ChangeLog:
> 
> 	* python.texi (Pretty Printing API): Mention the use of
> 	pretty-printers for [] indexing.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.python/py-prettyprint-subscript.c: New file.
> 	* gdb.python/py-prettyprint-subscript.cc: New file.
> 	* gdb.python/py-prettyprint-subscript.exp: New file.
> 	* gdb.python/py-prettyprint-subscript.py: New file.

Thanks, the documentation parts are OK.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 0/2] Using pretty-printers for [] indexing
  2021-04-22 14:10 [RFC 0/2] Using pretty-printers for [] indexing Andrew Burgess
  2021-04-22 14:10 ` [RFC 1/2] gdb: enable [] to find children using pretty-printers Andrew Burgess
  2021-04-22 14:10 ` [RFC 2/2] gdb: improve equal check used by the pretty-printer [] support Andrew Burgess
@ 2021-05-13 10:35 ` Andrew Burgess
  2021-06-17 18:37   ` Simon Marchi
  2 siblings, 1 reply; 7+ messages in thread
From: Andrew Burgess @ 2021-05-13 10:35 UTC (permalink / raw)
  To: gdb-patches

Ping!

Any thoughts?

thanks,
Andrew

* Andrew Burgess <andrew.burgess@embecosm.com> [2021-04-22 15:10:42 +0100]:

> I've been playing with the idea of reusing the pretty-printers to
> provide [] indexing.  I wonder what people thought of this?
> 
> All feedback welcome.
> 
> Thanks,
> Andrew
> 
> ---
> 
> Andrew Burgess (2):
>   gdb: enable [] to find children using pretty-printers
>   gdb: improve equal check used by the pretty-printer [] support
> 
>  gdb/ChangeLog                                 |  25 ++
>  gdb/NEWS                                      |   7 +
>  gdb/doc/ChangeLog                             |   5 +
>  gdb/doc/python.texi                           |   9 +
>  gdb/eval.c                                    |  92 ++++--
>  gdb/extension-priv.h                          |   6 +
>  gdb/extension.c                               |  22 ++
>  gdb/extension.h                               |  18 ++
>  gdb/guile/guile.c                             |   2 +
>  gdb/python/py-prettyprint.c                   | 179 ++++++++++++
>  gdb/python/python-internal.h                  |   4 +
>  gdb/python/python.c                           |   2 +
>  gdb/testsuite/ChangeLog                       |  23 ++
>  .../gdb.python/py-prettyprint-subscript.c     | 124 ++++++++
>  .../gdb.python/py-prettyprint-subscript.cc    | 187 ++++++++++++
>  .../gdb.python/py-prettyprint-subscript.exp   | 194 +++++++++++++
>  .../gdb.python/py-prettyprint-subscript.py    | 274 ++++++++++++++++++
>  17 files changed, 1156 insertions(+), 17 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.c
>  create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.cc
>  create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.exp
>  create mode 100644 gdb/testsuite/gdb.python/py-prettyprint-subscript.py
> 
> -- 
> 2.25.4
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 1/2] gdb: enable [] to find children using pretty-printers
  2021-04-22 14:10 ` [RFC 1/2] gdb: enable [] to find children using pretty-printers Andrew Burgess
  2021-04-22 14:18   ` Eli Zaretskii
@ 2021-05-13 16:28   ` Simon Marchi
  1 sibling, 0 replies; 7+ messages in thread
From: Simon Marchi @ 2021-05-13 16:28 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 2021-04-22 10:10 a.m., Andrew Burgess wrote:
> This commit allows GDB to make use of an available pretty-printer to
> provide [] indexing into a type.
> 
> Consider the following example before any pretty-printer is loaded:
> 
>   (gdb) p obj_int
>   $1 = {
>     m_name = 0x4052a0 "first int array",
>     m_nitems = 4,
>     m_items = 0x4052c0
>   }
>   (gdb) p obj_int[3]
>   Structure has no component named operator[].
> 
> Now, after loading a pretty-printer:
> 
>   (gdb) p obj_int
>   $2 = int array "first int array" with 4 elements = {3, 6, 4, 2}
>   (gdb) p obj_int[3]
>   $3 = 2

When reading this, I thought: isn't what pretty printers already do?
But apparently not.

It sounds at first like there's an overlap with xmethods, but maybe not
completely.  From what I understand, you would only write an xmethod to
replicate a method that exists in the program, but which may not be
available to call.  If writing a pretty printer for an array-like
structure in C, then there's no concept of method at all, so you
couldn't write an xmethod I suppose.  So this is where I can see your
feature becoming handy.

> diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
> index 7c0fdaa9e70..effbfcbfe28 100644
> --- a/gdb/python/py-prettyprint.c
> +++ b/gdb/python/py-prettyprint.c
> @@ -684,3 +684,173 @@ gdbpy_default_visualizer (PyObject *self, PyObject *args)
>  
>    return find_pretty_printer (val_obj).release ();
>  }
> +
> +/* See ext_lang_pretty_printer_find_child in extension.h for details.  */
> +
> +struct value *
> +gdbpy_val_pretty_printer_find_child (const extension_language_defn *extlang,
> +				     struct value *object,
> +				     struct value *idx,
> +				     const language_defn *language)
> +{
> +  /* A simple wrapper to make returning easier.  If we got an error then
> +     print the Python stack, and return nullptr.  Otherwise, return the
> +     value passed in.  */
> +  auto print_errors_before_return = [] (struct value *ret) -> struct value *
> +  {
> +    if (PyErr_Occurred ())
> +      {
> +	if (!PyErr_ExceptionMatches (gdbpy_gdb_memory_error))
> +	  {
> +	    ret = nullptr;
> +	    gdbpy_print_stack ();
> +	  }
> +      }
> +    return ret;
> +  };
> +
> +  struct gdbarch *gdbarch = value_type (object)->arch ();
> +
> +  gdbpy_enter enter_py (gdbarch, language);
> +
> +  /* Prepare the value to be passed to the pretty printer.  */
> +  gdbpy_ref<> val_obj (value_to_value_object_no_release (object));
> +  if (val_obj == nullptr)
> +    return print_errors_before_return (nullptr);
> +
> +  /* Find the pretty-printer constructor.  */
> +  gdbpy_ref<> printer (find_pretty_printer (val_obj.get ()));
> +  if (printer == nullptr || printer == Py_None)
> +    return print_errors_before_return (nullptr);
> +
> +  /* If the pretty printer has no 'children' method then we're not going to
> +     be able to lookup any children.  */
> +  if (!PyObject_HasAttr (printer.get (), gdbpy_children_cst))
> +    return print_errors_before_return (nullptr);
> +
> +  /* If we get here then the pretty printer has no 'child' method, but does
> +     have the 'children' method.  We can manually walk the children and
> +     find one that matches IDX.  First, find out how the children are
> +     represented by the children method.  */

Here, you talk about the child method, but I couldn't find any doc /
code about that.

While reading the commit message, I had that thought, maybe you had the
same.  If you do:

  (gdb) print myobject[50000]

That kind sucks that we have to call the "children" method and consume
/ create 49999 values that we are not going to use.  It would be much
better if we could ask the pretty printer, give me the item at index
50000.  Reading the documented API [1], that doesn't seem possible.

[1] https://sourceware.org/gdb/onlinedocs/gdb/Pretty-Printing-API.html#Pretty-Printing-API

Simon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC 0/2] Using pretty-printers for [] indexing
  2021-05-13 10:35 ` [RFC 0/2] Using pretty-printers for [] indexing Andrew Burgess
@ 2021-06-17 18:37   ` Simon Marchi
  0 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi @ 2021-06-17 18:37 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches

On 2021-05-13 6:35 a.m., Andrew Burgess wrote:
> Ping!
> 
> Any thoughts?
> 
> thanks,
> Andrew

Hi Andrew,

I am writing a pretty-printer for a C++ intrusive list type that I plan
on contributing to GDB in an upcoming series (initially written by
Pedro).  I think that this patch of yours would be very helpful, let me
explain why.

To use the list, you put an intrusive_list member in the type you want
to link:

    struct item
    {
      item (int x) : x (x) {}

      int x;
      intrusive_list_node<item> node;
    };

That intrusive_list_node simply contains next and prev pointers:

    template<typename T>
    struct intrusive_list_node
    {
      intrusive_list_node<T> *next = nullptr;
      intrusive_list_node<T> *prev = nullptr;
    };

The problem is that inspecting this list in GDB, you can easily get
pointers to intrusive_list_node objects, but not to the enclosing object
(the struct item in my example):

    (gdb) p list.m_head.next 
    $7 = (intrusive_list_node<item> *) 0x7fffffffdd48
    (gdb) p list.m_head.next.next
    $8 = (intrusive_list_node<item> *) 0x7fffffffdd30
    (gdb) p list.m_head.next.next.next
    $9 = (intrusive_list_node<item> *) 0x7fffffffdd18

I wrote a pretty printer for the list type:

    (gdb) p list
    $11 = intrusive list of item, linked through &item::node = {{
        x = 1,
        node = {
          next = 0x7fffffffdd30,
          prev = 0x7fffffffdd58
        }
      }, {
        x = 2,
        node = {
          next = 0x7fffffffdd18,
          prev = 0x7fffffffdd48
        }
      }, {
        x = 3,
        node = {
          next = 0x7fffffffdd58,
          prev = 0x7fffffffdd30
        }
      }}

Your patch makes it possible to get list items by indexing, which yields
a struct item directly:

    (gdb) p list[1]
    $12 = {
      x = 2,
      node = {
        next = 0x7fffffffdd18,
        prev = 0x7fffffffdd48
      }
    }
    (gdb) p list[1].x
    $13 = 2

Sooo yeah, I'd like to see this patch merged eventually.

The only problem that I don't think I'll be able to solve (but that's
not related to your patch) is if you just have a pointer to a struct
item and would like to find the next item:

    (gdb) p $14
    $16 = (item *) 0x7fffffffdd28
    (gdb) p $14->node.next
    $17 = (intrusive_list_node<item> *) 0x7fffffffdd18

I don't think there's an easy way to get back to the enclosing item, as
the type intrusive_list_node<item> by itself doesn't have a clue of
what it is embedded into.

Simon

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-06-17 18:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 14:10 [RFC 0/2] Using pretty-printers for [] indexing Andrew Burgess
2021-04-22 14:10 ` [RFC 1/2] gdb: enable [] to find children using pretty-printers Andrew Burgess
2021-04-22 14:18   ` Eli Zaretskii
2021-05-13 16:28   ` Simon Marchi
2021-04-22 14:10 ` [RFC 2/2] gdb: improve equal check used by the pretty-printer [] support Andrew Burgess
2021-05-13 10:35 ` [RFC 0/2] Using pretty-printers for [] indexing Andrew Burgess
2021-06-17 18:37   ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).