This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch v9] Debug methods in GDB Python


Siva Chandra writes:
 > Hi,
 > 
 > Attached is the v9 of the patch which adds debug methods feature to
 > the Python API. I have added doc changes in this version (barring a
 > NEWS entry). I request a full review this time.
 > 
 > [...]
 >

Hi.  Continuing where I left off.

 > diff --git a/gdb/python/py-debugmethods.c b/gdb/python/py-debugmethods.c
 > new file mode 100644
 > index 0000000..e267564
 > --- /dev/null
 > +++ b/gdb/python/py-debugmethods.c
 > @@ -0,0 +1,695 @@
 > +/* Support for debug methods in Python.
 > +
 > +   Copyright (C) 2013-2014 Free Software Foundation, Inc.
 > +
 > +   This file is part of GDB.
 > +
 > +   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 "defs.h"
 > +#include "arch-utils.h"
 > +#include "extension.h"
 > +#include "extension-priv.h"
 > +#include "objfiles.h"
 > +#include "value.h"
 > +#include "language.h"
 > +
 > +#include "python.h"
 > +
 > +#ifdef HAVE_PYTHON
 > +#include "python-internal.h"
 > +
 > +static const char enabled_field_name[] = "enabled";
 > +static const char match_method_name[] = "match";
 > +static const char get_argtypes_method_name[] = "get_argtypes";
 > +static const char invoke_method_name[] = "invoke";
 > +static const char matchers_attr_str[] = "debugmethod_matchers";
 > +
 > +static PyObject *py_match_method_name = NULL;
 > +static PyObject *py_get_argtypes_method_name = NULL;
 > +static PyObject *py_invoke_method_name = NULL;
 > +
 > +struct gdbpy_worker_data
 > +{
 > +  PyObject *worker;
 > +  PyObject *this_type;
 > +};
 > +
 > +static struct debug_method_worker *new_python_debug_method_worker (
 > +   PyObject *item, PyObject *py_obj_type);
 
I'm not sure what the correct indentation level for a line continued
after a trailing (, but I think(!) it's not 3.
Let's go with 2 for now.
[The important thing is that the line is distinguishabled from the
next line - in this case the open { is in column 0 so we're good.]

> +
 > +/* Implementation of free_debug_method_worker_data for Python.  */
 > +
 > +static void
 > +gdbpy_free_debug_method_worker_data (
 > +   const struct extension_language_defn *extlang, void *data)

Fix indentation (here and elsewhere below).

 > +{
 > +  struct gdbpy_worker_data *worker_data = data;
 > +
 > +  Py_XDECREF (worker_data->worker);
 > +  Py_XDECREF (worker_data->this_type);
 > +  xfree (worker_data);
 > +}
 > +
 > +/* Implementation of clone_debug_method_worker_data for Python.  */
 > +
 > +static void *
 > +gdbpy_clone_debug_method_worker_data (
 > +   const struct extension_language_defn *extlang, void *data)
 > +{
 > +  struct gdbpy_worker_data *worker_data = data, *new_data;
 > +
 > +  new_data = XCNEW (struct gdbpy_worker_data); 
 > +  new_data->worker = worker_data->worker;
 > +  new_data->this_type = worker_data->this_type;
 > +  Py_XINCREF (new_data->worker);
 > +  Py_XINCREF (new_data->this_type);
 > +
 > +  return new_data;
 > +}
 > +
 > +/* Invoke the "match" method of the MATCHER and return a new reference
 > +   to the result.  Returns NULL on error.  */
 > +
 > +static PyObject *
 > +invoke_match_method (PyObject *matcher, PyObject *py_obj_type,
 > +		     const char *debugmethod_name)
 > +{
 > +  PyObject *py_debugmethod_name;
 > +  PyObject *match_method, *enabled_field, *match_result;
 > +  struct cleanup *cleanups;
 > +  int enabled;
 > +
 > +  if (debugmethod_name == NULL)
 > +    return NULL;

When would this be called with a NULL method name?
All else being equal, it would be preferable to disallow NULL and assert debugmethod_name != NULL here.

 > +
 > +  cleanups = make_cleanup (null_cleanup, NULL);
 > +
 > +  enabled_field = PyObject_GetAttrString (matcher, enabled_field_name);
 > +  if (enabled_field == NULL)
 > +    {
 > +      do_cleanups (cleanups);
 > +

The blank line here feels out of place.

 > +      return NULL;
 > +    }
 > +  make_cleanup_py_decref (enabled_field);
 > +
 > +  enabled = PyObject_IsTrue (enabled_field);
 > +  if (enabled == -1)
 > +    {
 > +      do_cleanups (cleanups);
 > +

Ditto, remove blank line (here and elsewhere below).
[If there was more to this clause than just a do_cleanups and return, blank lines can be useful.  But this case is too simple.]

 > +      return NULL;
 > +    }
 > +  if (enabled == 0)
 > +    {
 > +      /* Return 'None' if the matcher is not enabled.  */
 > +      do_cleanups (cleanups);
 > +      Py_INCREF (Py_None);
 > +
 > +      return Py_None;

Use Py_RETURN_NONE.

 > +    }
 > +
 > +  match_method = PyObject_GetAttrString (matcher, match_method_name);
 > +  if (match_method == NULL)
 > +    {
 > +      do_cleanups (cleanups);

I'm not sure any changes are required, but there's an implicit contract between this function and the caller that if NULL is returned then the python error status is correct.  What if some cleanup corrupts that?

 > +
 > +      return NULL;
 > +    }
 > +  make_cleanup_py_decref (match_method);
 > +  if (!PyCallable_Check (match_method))

The pretty-printer code doesn't do a callable check.
What happens if we remove this check?

 > +    {
 > +      warning (_("Attribute '%s' of a registered Python debug method matcher "
 > +		 "is not a callable."), match_method_name);
 > +      do_cleanups (cleanups);
 > +
 > +      return NULL;
 > +    }
 > +
 > +  py_debugmethod_name = PyString_FromString (debugmethod_name);
 > +  if (py_debugmethod_name == NULL)
 > +    {
 > +      do_cleanups (cleanups);
 > +
 > +      return NULL;
 > +    }
 > +  make_cleanup_py_decref (py_debugmethod_name);
 > +
 > +  match_result = PyObject_CallMethodObjArgs (matcher,
 > +					     py_match_method_name,
 > +					     py_obj_type,
 > +					     py_debugmethod_name,
 > +					     NULL);
 > +  if (match_result == NULL)
 > +    gdbpy_print_stack ();

The caller will call gdbpy_print_stack, right?

 > +  do_cleanups (cleanups);
 > +
 > +  return match_result;
 > +}
 > +
 > +/* Implementation of get_matching_debug_method_workers for Python.  */
 > +
 > +static enum ext_lang_rc
 > +gdbpy_get_matching_debug_method_workers (
 > +   const struct extension_language_defn *extlang,
 > +   struct type *obj_type, const char *method_name,
 > +   debug_method_worker_vec **dm_vec)
 > +{
 > +  struct cleanup *cleanups;
 > +  struct objfile *objfile;
 > +  VEC (debug_method_worker_p) *worker_vec = NULL;
 > +  PyObject *py_type, *py_progspace;
 > +  PyObject *py_debugmethod_matcher_list = NULL, *list_iter, *matcher;
 > +
 > +  if (obj_type == NULL || method_name == NULL)
 > +    return EXT_LANG_RC_ERROR;

When would obj_type or method_name be NULL?
The documentation for this API function doesn't mention this possibility.
If the caller should never pass NULL (which would be my preference) for these let's make these an assert fail.

 > +
 > +  cleanups = ensure_python_env (get_current_arch (), current_language);
 > +
 > +  py_type = type_to_type_object (obj_type);
 > +  if (py_type == NULL)
 > +    {
 > +      gdbpy_print_stack ();
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +  make_cleanup_py_decref (py_type);
 > +
 > +  /* Create an empty list of debug methods.  */
 > +  py_debugmethod_matcher_list = PyList_New (0);
 > +  if (py_debugmethod_matcher_list == NULL)
 > +    {
 > +      gdbpy_print_stack ();
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +
 > +  /* Gather debug method matchers registered with the object files.  */
 > +  ALL_OBJFILES (objfile)
 > +    {
 > +      PyObject *py_objfile = objfile_to_objfile_object (objfile);
 > +      PyObject *objfile_matchers, *temp = py_debugmethod_matcher_list;
 > +
 > +      if (py_objfile == NULL)
 > +	{
 > +	  gdbpy_print_stack ();
 > +	  Py_DECREF (py_debugmethod_matcher_list);
 > +	  do_cleanups (cleanups);
 > +
 > +	  return EXT_LANG_RC_ERROR;
 > +	}
 > +
 > +      objfile_matchers = objfpy_get_debugmethod_matchers (py_objfile, NULL);
 > +      py_debugmethod_matcher_list = PySequence_Concat (temp,
 > +						       objfile_matchers);
 > +      Py_DECREF (temp);
 > +      Py_XDECREF (objfile_matchers);

An XDECREF doesn't feel necessary here.  Is it?

 > +      if (py_debugmethod_matcher_list == NULL)
 > +	{
 > +	  gdbpy_print_stack ();
 > +	  do_cleanups (cleanups);
 > +
 > +	  return EXT_LANG_RC_ERROR;
 > +	}
 > +    }
 > +
 > +  /* Gather debug methods matchers registered with the current program
 > +     space.  */
 > +  py_progspace = pspace_to_pspace_object (current_program_space);
 > +  if (py_progspace != NULL)
 > +    {
 > +      PyObject *temp = py_debugmethod_matcher_list;
 > +      PyObject *pspace_matchers = pspy_get_debugmethod_matchers (py_progspace,
 > +								 NULL);
 > +
 > +      py_debugmethod_matcher_list = PySequence_Concat (temp, pspace_matchers);
 > +      Py_DECREF (temp);
 > +      Py_XDECREF (pspace_matchers);

Ditto.  Is XDECREF necessary here?

 > +      if (py_debugmethod_matcher_list == NULL)
 > +	{
 > +	  gdbpy_print_stack ();
 > +	  do_cleanups (cleanups);
 > +
 > +	  return EXT_LANG_RC_ERROR;
 > +	}
 > +    }
 > +  else
 > +    {
 > +      gdbpy_print_stack ();
 > +      Py_DECREF (py_debugmethod_matcher_list);
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +
 > +  /* Gather debug method matchers registered globally.  */
 > +  if (gdb_python_module != NULL
 > +      && PyObject_HasAttrString (gdb_python_module, matchers_attr_str))
 > +    {
 > +      PyObject *gdb_matchers;
 > +      PyObject *temp = py_debugmethod_matcher_list;
 > +
 > +      gdb_matchers = PyObject_GetAttrString (gdb_python_module,
 > +					     matchers_attr_str);
 > +      if (gdb_matchers != NULL)
 > +	{
 > +	  py_debugmethod_matcher_list = PySequence_Concat (temp,
 > +							   gdb_matchers);
 > +	  Py_DECREF (temp);
 > +	  Py_DECREF (gdb_matchers);
 > +	  if (py_debugmethod_matcher_list == NULL)
 > +	    {
 > +	      gdbpy_print_stack ();
 > +	      do_cleanups (cleanups);
 > +
 > +	      return EXT_LANG_RC_ERROR;
 > +	    }
 > +	}
 > +      else
 > +	{
 > +	  gdbpy_print_stack ();
 > +	  Py_DECREF (py_debugmethod_matcher_list);
 > +	  do_cleanups (cleanups);
 > +
 > +	  return EXT_LANG_RC_ERROR;
 > +	}
 > +    }
 > +
 > +  /* Safe to make a cleanup for py_debugmethod_matcher_list now as it
 > +     will not change any more.  */
 > +  make_cleanup_py_decref (py_debugmethod_matcher_list);

There's no need to do any optimization at this point,
but I wonder if at some point it would be better to invoke
the match method on each loci rather than build one big list
and then iterate over it.

 > +
 > +  list_iter = PyObject_GetIter (py_debugmethod_matcher_list);
 > +  if (list_iter == NULL)
 > +    {
 > +      gdbpy_print_stack ();
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +  while ((matcher = PyIter_Next (list_iter)))

Add != NULL check.

 > +    {
 > +      PyObject *match_result = invoke_match_method (matcher, py_type,
 > +						    method_name);
 > +
 > +      if (match_result == NULL)
 > +	{
 > +	  gdbpy_print_stack ();
 > +	  Py_DECREF (matcher);
 > +	  do_cleanups (cleanups);
 > +
 > +	  return EXT_LANG_RC_ERROR;
 > +	}
 > +      if (match_result == Py_None)
 > +	; /* This means there was no match.  */
 > +      else if (PySequence_Check (match_result))
 > +	{
 > +	  PyObject *iter = PyObject_GetIter (match_result);
 > +	  PyObject *py_worker;
 > +
 > +	  if (iter == NULL)
 > +	    {
 > +	      gdbpy_print_stack ();
 > +	      Py_DECREF (matcher);
 > +	      Py_DECREF (match_result);
 > +	      do_cleanups (cleanups);
 > +
 > +	      return EXT_LANG_RC_ERROR;
 > +	    }
 > +	  while ((py_worker = PyIter_Next (iter)))

Add != NULL check.

 > +	    {
 > +	      struct debug_method_worker *worker;
 > +
 > +	      worker = new_python_debug_method_worker (py_worker, py_type);
 > +	      VEC_safe_push (debug_method_worker_p, worker_vec, worker);
 > +	      Py_DECREF (worker);
 > +	    }
 > +	  Py_DECREF (iter);
 > +	  /* Report any error that could have occurred while iterating.  */
 > +	  if (PyErr_Occurred ())
 > +	    {
 > +	      gdbpy_print_stack ();
 > +	      Py_DECREF (matcher);
 > +	      Py_DECREF (match_result);
 > +	      do_cleanups (cleanups);
 > +
 > +	      return EXT_LANG_RC_ERROR;
 > +	    }
 > +	}
 > +      else
 > +	{
 > +	  struct debug_method_worker *worker;
 > +
 > +	  worker = new_python_debug_method_worker (match_result, py_type);
 > +	  VEC_safe_push (debug_method_worker_p, worker_vec, worker);
 > +	}
 > +
 > +      Py_XDECREF (match_result);
 > +      Py_DECREF (matcher);
 > +    }
 > +  Py_DECREF (list_iter);
 > +  /* Report any error that could have occurred while iterating.  */
 > +  if (PyErr_Occurred ())
 > +    {
 > +      gdbpy_print_stack ();
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +
 > +  do_cleanups (cleanups);
 > +  *dm_vec = worker_vec;
 > +
 > +  return EXT_LANG_RC_OK;
 > +}
 > +
 > +/* Implementation of get_debug_method_argtypes for Python.  */
 > +
 > +static enum ext_lang_rc
 > +gdbpy_get_debug_method_argtypes (
 > +   const struct extension_language_defn *extlang,
 > +   struct debug_method_worker *worker,

[Writing this down for reference sake. I'm not arguing for changes, per se.]
struct debug_method_worker contains an extension_language_defn *, so there's no need to pass extlang as an arg here.
IOW this function is more like a method of debug_method_worker than extension_language_defn.
Given the absence of c++ going this route would require splitting debug_method_worker into debug_method_worker_defn and debug_method_worker_ops (or some such), so I don't mind leaving things as is.
OTOH, I see there are two such methods that are more methods of debug_method_worker than ext_lang.

 > +   int *nargs,
 > +   struct type ***argtypes)
 > +{
 > +  struct gdbpy_worker_data *worker_data = worker->data;
 > +  PyObject *py_worker = worker_data->worker;
 > +  PyObject *get_argtypes_method;
 > +  PyObject *py_argtype_list, *list_iter = NULL, *item;
 > +  struct cleanup *cleanups;
 > +  struct type **type_array, *obj_type;
 > +  int i = 1, arg_count;
 > +
 > +  /* Set nargs to 0 so that any premature return from this function returns
 > +     0 arg types.  */
 > +  *nargs = 0;
 > +
 > +  cleanups = ensure_python_env (get_current_arch (), current_language);
 > +
 > +  get_argtypes_method =  PyObject_GetAttrString (py_worker,
 > +						 get_argtypes_method_name);
 > +  if (get_argtypes_method == NULL)
 > +    {
 > +      gdbpy_print_stack ();
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +  make_cleanup_py_decref (get_argtypes_method);
 > +
 > +  if (!PyCallable_Check (get_argtypes_method))
 > +    {

Again, do we need to call PyCallable_Check here?
[I honestly don't know.  I don't see other calls in py-*.c.]
Plus I would definitely flag this as an error.
I realize you return EXT_LANG_RC_ERROR, but the call to warning() is wrong.
I suspect you can just delete this code, and let PyObject_CallMethodObjArgs flag the error.

 > +      warning (_("Attribute '%s' of a registered Python debug method is not "
 > +		 "callable."), get_argtypes_method_name);
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +
 > +  py_argtype_list = PyObject_CallMethodObjArgs (py_worker,
 > +						py_get_argtypes_method_name,
 > +						NULL);
 > +  if (py_argtype_list == NULL)
 > +    {
 > +      gdbpy_print_stack ();
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +  make_cleanup_py_decref (py_argtype_list);
 > +  if (py_argtype_list == Py_None)
 > +    arg_count = 0;
 > +  else if (PySequence_Check (py_argtype_list))
 > +    {
 > +      arg_count = PySequence_Size (py_argtype_list);
 > +      if (arg_count == -1)
 > +	{
 > +	  gdbpy_print_stack ();
 > +	  do_cleanups (cleanups);
 > +
 > +	  return EXT_LANG_RC_ERROR;
 > +	}
 > +
 > +      list_iter = PyObject_GetIter (py_argtype_list);
 > +      if (list_iter == NULL)
 > +	{
 > +	  gdbpy_print_stack ();
 > +	  do_cleanups (cleanups);
 > +
 > +	  return EXT_LANG_RC_ERROR;
 > +	}
 > +      make_cleanup_py_decref (list_iter);
 > +    }
 > +  else
 > +    arg_count = 1;
 > +
 > +  /* Include the 'this' argument in the size.  */
 > +  type_array = XCNEWVEC (struct type *, arg_count + 1);
 > +  i = 1;
 > +  if (list_iter != NULL)
 > +    {
 > +      while ((item = PyIter_Next (list_iter)))

Add != NULL check.

 > +	{
 > +	  struct type *arg_type = type_object_to_type (item);
 > +
 > +	  Py_DECREF (item);
 > +	  if (arg_type == NULL)
 > +	    {
 > +	      PyErr_SetString (PyExc_TypeError,
 > +			       _("Arg type returned by the get_argtypes method "
 > +				 "of a debug method worker object is not a "
 > +				 "gdb.Type object."));
 > +	      break;
 > +	    }
 > +
 > +	  type_array[i] = arg_type;
 > +	  i++;
 > +	}
 > +    }
 > +  else if (arg_count == 1)
 > +    {
 > +      /* py_argtype_list is not actually a list but a single gdb.Type
 > +	 object.  */
 > +      struct type *arg_type = type_object_to_type (py_argtype_list);
 > +
 > +      if (arg_type == NULL)
 > +	PyErr_SetString (PyExc_TypeError,
 > +			 _("Arg type returned by the get_argtypes method "
 > +			   "of a debug method worker object is not a gdb.Type "
 > +			   "object."));
 > +      else
 > +	{
 > +	  type_array[1] = arg_type;
 > +	  i++;
 > +	}
 > +    }
 > +  if (PyErr_Occurred ())
 > +    {
 > +      gdbpy_print_stack ();
 > +      do_cleanups (cleanups);
 > +      xfree (type_array);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +
 > +  /* Add the type of 'this' as the first argument.  */
 > +  obj_type = type_object_to_type (worker_data->this_type);
 > +  type_array[0] = make_cv_type (1, 0, lookup_pointer_type (obj_type), NULL);
 > +  *nargs = i;
 > +  *argtypes = type_array;
 > +  do_cleanups (cleanups);
 > +
 > +  return EXT_LANG_RC_OK;
 > +}
 > +
 > +/* Implementation of invoke_debug_method for Python.  */
 > +
 > +static enum ext_lang_rc
 > +gdbpy_invoke_debug_method (
 > +   const struct extension_language_defn *extlang,
 > +   struct debug_method_worker *worker,
 > +   struct value *obj, struct value **args, int nargs,
 > +   struct value **result)
 > +{
 > +  int i;
 > +  struct cleanup *cleanups;
 > +  PyObject *py_value_obj, *py_arg_tuple, *py_result;
 > +  PyObject *invoke_method;
 > +  struct type *obj_type, *this_type;
 > +  struct value *res = NULL;
 > +  struct gdbpy_worker_data *worker_data = worker->data;
 > +  PyObject *debugmethod_worker = worker_data->worker;
 > +
 > +  cleanups = ensure_python_env (get_current_arch (), current_language);
 > +
 > +  invoke_method =  PyObject_GetAttrString (debugmethod_worker,
 > +					   invoke_method_name);
 > +  if (invoke_method == NULL)
 > +    {
 > +      gdbpy_print_stack ();
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +  make_cleanup_py_decref (invoke_method);
 > +
 > +  if (!PyCallable_Check (invoke_method))

Can we delete this callable check?

 > +    {
 > +      warning (_("Attribute '%s' of a registered Python debug method is not "
 > +		 "callable."), invoke_method_name);
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +
 > +  obj_type = check_typedef (value_type (obj));
 > +  this_type = check_typedef (type_object_to_type (worker_data->this_type));
 > +  if (TYPE_CODE (obj_type) == TYPE_CODE_PTR)
 > +    {
 > +      struct type *this_ptr = lookup_pointer_type (this_type);
 > +
 > +      if (!types_equal (obj_type, this_ptr))
 > +	obj = value_cast (this_ptr, obj);
 > +    }
 > +  else if (TYPE_CODE (obj_type) == TYPE_CODE_REF)
 > +    {
 > +      struct type *this_ref = lookup_reference_type (this_type);
 > +
 > +      if (!types_equal (obj_type, this_ref))
 > +	obj = value_cast (this_ref, obj);
 > +    }
 > +  else
 > +    {
 > +      if (!types_equal (obj_type, this_type))
 > +	obj = value_cast (this_type, obj);
 > +    }

I haven't checked the testcase, but it would be good if all three of these clauses were tested.

 > +  py_value_obj = value_to_value_object (obj);
 > +  if (py_value_obj == NULL)
 > +    {
 > +      gdbpy_print_stack ();
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +  make_cleanup_py_decref (py_value_obj);
 > +
 > +  py_arg_tuple = PyTuple_New (nargs);
 > +  if (py_arg_tuple == NULL)
 > +    {
 > +      gdbpy_print_stack ();
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +  make_cleanup_py_decref (py_arg_tuple);
 > +
 > +  for (i = 0; i < nargs; i++)
 > +    {
 > +      PyObject *py_value_arg = value_to_value_object (args[i]);
 > +
 > +      if (py_value_arg == NULL)
 > +	{
 > +	  gdbpy_print_stack ();
 > +	  do_cleanups (cleanups);
 > +
 > +	  return EXT_LANG_RC_ERROR;
 > +	}
 > +
 > +      PyTuple_SET_ITEM (py_arg_tuple, i, py_value_arg);
 > +    }
 > +
 > +  py_result = PyObject_CallMethodObjArgs (debugmethod_worker,
 > +					  py_invoke_method_name,
 > +					  py_value_obj,
 > +					  py_arg_tuple, NULL);
 > +  if (py_result == NULL)
 > +    {
 > +      gdbpy_print_stack ();
 > +      do_cleanups (cleanups);
 > +
 > +      return EXT_LANG_RC_ERROR;
 > +    }
 > +  make_cleanup_py_decref (py_result);
 > +
 > +  /* Check that a debug method did not return None.  */
 > +  if (py_result != Py_None)
 > +    {
 > +      res = convert_value_from_python (py_result);
 > +      if (res == NULL)
 > +	{
 > +	  gdbpy_print_stack ();
 > +	  do_cleanups (cleanups);
 > +
 > +	  return EXT_LANG_RC_ERROR;
 > +	}
 > +    }
 > +  else
 > +    res = allocate_value (lookup_typename (python_language, python_gdbarch,
 > +					   "void", NULL, 0));
 > +
 > +  *result = res;
 > +  do_cleanups (cleanups);
 > +
 > +  return EXT_LANG_RC_OK;
 > +}
 > +
 > +/* v-table of extension_language_debug_method_ops for Python.  */
 > +
 > +struct extension_language_debug_method_ops python_debug_method_ops = {
 > +  gdbpy_clone_debug_method_worker_data,
 > +  gdbpy_free_debug_method_worker_data,
 > +  gdbpy_get_matching_debug_method_workers,
 > +  gdbpy_get_debug_method_argtypes,
 > +  gdbpy_invoke_debug_method
 > +};
 > +
 > +/* Creates a new Python debug_method_worker object.
 > +   The new object has data of type 'struct gdbpy_worker_data' composed
 > +   with the components PY_WORKER and THIS_TYPE.  */
 > +
 > +static struct debug_method_worker *
 > +new_python_debug_method_worker (PyObject *py_worker, PyObject *this_type)
 > +{
 > +  struct gdbpy_worker_data *data = XCNEW (struct gdbpy_worker_data);
 > +
 > +  data->worker = py_worker;
 > +  data->this_type = this_type;
 > +  Py_XINCREF (py_worker);
 > +  Py_XINCREF (this_type);

Is XINCREF needed (over just INCREF)?
It suggested the args may be NULL, but that doesn't feel right.

 > +
 > +  return new_debug_method_worker (&extension_language_python, data);
 > +}
 > +
 > +int
 > +gdbpy_initialize_debugmethods (void)
 > +{
 > +  py_match_method_name = PyString_FromString (match_method_name);
 > +  if (py_match_method_name == NULL)
 > +    return -1;
 > +
 > +  py_invoke_method_name = PyString_FromString (invoke_method_name);
 > +  if (py_invoke_method_name == NULL)
 > +    return -1;
 > +
 > +  py_get_argtypes_method_name = PyString_FromString (get_argtypes_method_name);
 > +  if (py_get_argtypes_method_name == NULL)
 > +    return -1;
 > +
 > +  return 1;
 > +}
 > +
 > +#endif /* HAVE_PYTHON */
 > diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
 > index 97fb0be..fd4254a 100644
 > --- a/gdb/python/py-objfile.c
 > +++ b/gdb/python/py-objfile.c
 > @@ -37,6 +37,9 @@ typedef struct
 >    PyObject *frame_filters;
 >    /* The type-printer list.  */
 >    PyObject *type_printers;
 > +
 > +  /* The debug method matcher list.  */
 > +  PyObject *debugmethod_matchers;
 >  } objfile_object;
 >  
 >  static PyTypeObject objfile_object_type
 > @@ -67,6 +70,7 @@ objfpy_dealloc (PyObject *o)
 >    Py_XDECREF (self->printers);
 >    Py_XDECREF (self->frame_filters);
 >    Py_XDECREF (self->type_printers);
 > +  Py_XDECREF (self->debugmethod_matchers);
 >    Py_TYPE (self)->tp_free (self);
 >  }
 >  
 > @@ -99,6 +103,13 @@ objfpy_new (PyTypeObject *type, PyObject *args, PyObject *keywords)
 >  	  Py_DECREF (self);
 >  	  return NULL;
 >  	}
 > +
 > +      self->debugmethod_matchers = PyList_New (0);
 > +      if (self->debugmethod_matchers == NULL)
 > +	{
 > +	  Py_DECREF (self);
 > +	  return NULL;
 > +	}
 >      }
 >    return (PyObject *) self;
 >  }
 > @@ -193,6 +204,17 @@ objfpy_get_type_printers (PyObject *o, void *ignore)
 >    return self->type_printers;
 >  }
 >  
 > +/* Get the 'debugmethod_matchers' attribute.  */
 > +
 > +PyObject *
 > +objfpy_get_debugmethod_matchers (PyObject *o, void *ignore)
 > +{
 > +  objfile_object *self = (objfile_object *) o;
 > +
 > +  Py_INCREF (self->debugmethod_matchers);
 > +  return self->debugmethod_matchers;
 > +}
 > +
 >  /* Set the 'type_printers' attribute.  */
 >  
 >  static int
 > @@ -292,6 +314,13 @@ objfile_to_objfile_object (struct objfile *objfile)
 >  	      return NULL;
 >  	    }
 >  
 > +	  object->debugmethod_matchers = PyList_New (0);
 > +	  if (object->debugmethod_matchers == NULL)
 > +	    {
 > +	      Py_DECREF (object);
 > +	      return NULL;
 > +	    }
 > +
 >  	  set_objfile_data (objfile, objfpy_objfile_data_key, object);
 >  	}
 >      }
 > @@ -333,6 +362,8 @@ static PyGetSetDef objfile_getset[] =
 >      objfpy_set_frame_filters, "Frame Filters.", NULL },
 >    { "type_printers", objfpy_get_type_printers, objfpy_set_type_printers,
 >      "Type printers.", NULL },
 > +  { "debugmethod_matchers", objfpy_get_debugmethod_matchers, NULL,

debug_method_matchers

 > +    "Debug methods.", NULL },
 >    { NULL }
 >  };
 >  
 > diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
 > index cda5a86..c65d863 100644
 > --- a/gdb/python/py-progspace.c
 > +++ b/gdb/python/py-progspace.c
 > @@ -39,6 +39,9 @@ typedef struct
 >    PyObject *frame_filters;
 >    /* The type-printer list.  */
 >    PyObject *type_printers;
 > +
 > +  /* The debug method list.  */
 > +  PyObject *debugmethod_matchers;
 >  } pspace_object;
 >  
 >  static PyTypeObject pspace_object_type
 > @@ -75,6 +78,7 @@ pspy_dealloc (PyObject *self)
 >    Py_XDECREF (ps_self->printers);
 >    Py_XDECREF (ps_self->frame_filters);
 >    Py_XDECREF (ps_self->type_printers);
 > +  Py_XDECREF (ps_self->debugmethod_matchers);
 >    Py_TYPE (self)->tp_free (self);
 >  }
 >  
 > @@ -107,6 +111,13 @@ pspy_new (PyTypeObject *type, PyObject *args, PyObject *keywords)
 >  	  Py_DECREF (self);
 >  	  return NULL;
 >  	}
 > +
 > +      self->debugmethod_matchers = PyList_New (0);
 > +      if (self->debugmethod_matchers == NULL)
 > +	{
 > +	  Py_DECREF (self);
 > +	  return NULL;
 > +	}
 >      }
 >    return (PyObject *) self;
 >  }
 > @@ -201,6 +212,17 @@ pspy_get_type_printers (PyObject *o, void *ignore)
 >    return self->type_printers;
 >  }
 >  
 > +/* Get the 'debugmethod_matchers' attribute.  */
 > +
 > +PyObject *
 > +pspy_get_debugmethod_matchers (PyObject *o, void *ignore)
 > +{
 > +  pspace_object *self = (pspace_object *) o;
 > +
 > +  Py_INCREF (self->debugmethod_matchers);
 > +  return self->debugmethod_matchers;
 > +}
 > +
 >  /* Set the 'type_printers' attribute.  */
 >  
 >  static int
 > @@ -288,6 +310,13 @@ pspace_to_pspace_object (struct program_space *pspace)
 >  	      return NULL;
 >  	    }
 >  
 > +	  object->debugmethod_matchers = PyList_New (0);
 > +	  if (object->debugmethod_matchers == NULL)
 > +	    {
 > +	      Py_DECREF (object);
 > +	      return NULL;
 > +	    }
 > +
 >  	  set_program_space_data (pspace, pspy_pspace_data_key, object);
 >  	}
 >      }
 > @@ -320,6 +349,8 @@ static PyGetSetDef pspace_getset[] =
 >      "Frame filters.", NULL },
 >    { "type_printers", pspy_get_type_printers, pspy_set_type_printers,
 >      "Type printers.", NULL },
 > +  { "debugmethod_matchers", pspy_get_debugmethod_matchers, NULL,

debug_method_matchers

 > +    "Debug methods.", NULL },
 >    { NULL }
 >  };
 >  
 > diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
 > index 07650f7..fe1f479 100644
 > --- a/gdb/python/python-internal.h
 > +++ b/gdb/python/python-internal.h
 > @@ -345,11 +345,13 @@ PyObject *pspace_to_pspace_object (struct program_space *)
 >      CPYCHECKER_RETURNS_BORROWED_REF;
 >  PyObject *pspy_get_printers (PyObject *, void *);
 >  PyObject *pspy_get_frame_filters (PyObject *, void *);
 > +PyObject *pspy_get_debugmethod_matchers (PyObject *, void *);
 >  
 >  PyObject *objfile_to_objfile_object (struct objfile *)
 >      CPYCHECKER_RETURNS_BORROWED_REF;
 >  PyObject *objfpy_get_printers (PyObject *, void *);
 >  PyObject *objfpy_get_frame_filters (PyObject *, void *);
 > +PyObject *objfpy_get_debugmethod_matchers (PyObject *, void *);
 >  
 >  PyObject *gdbarch_to_arch_object (struct gdbarch *gdbarch);
 >  
 > @@ -430,6 +432,8 @@ int gdbpy_initialize_new_objfile_event (void)
 >    CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 >  int gdbpy_initialize_arch (void)
 >    CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 > +int gdbpy_initialize_debugmethods (void)
 > +  CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 >  
 >  struct cleanup *make_cleanup_py_decref (PyObject *py);
 >  struct cleanup *make_cleanup_py_xdecref (PyObject *py);
 > diff --git a/gdb/python/python.c b/gdb/python/python.c
 > index cbfa73a..50e3020 100644
 > --- a/gdb/python/python.c
 > +++ b/gdb/python/python.c
 > @@ -60,6 +60,8 @@ static const char *gdbpy_should_print_stack = python_excp_message;
 >  /* Forward decls, these are defined later.  */
 >  static const struct extension_language_script_ops python_extension_script_ops;
 >  static const struct extension_language_ops python_extension_ops;
 > +/* This is defined in py-debugmethods.c.  */
 > +extern const struct extension_language_debug_method_ops python_debug_method_ops;
 >  #endif
 >  
 >  /* The main struct describing GDB's interface to the Python
 > @@ -77,6 +79,7 @@ const struct extension_language_defn extension_language_python =
 >  
 >  #ifdef HAVE_PYTHON
 >    &python_extension_script_ops,
 > +  &python_debug_method_ops,
 >    &python_extension_ops
 >  #else
 >    NULL,
 > @@ -1752,7 +1755,8 @@ message == an error message without a stack will be printed."),
 >        || gdbpy_initialize_exited_event () < 0
 >        || gdbpy_initialize_thread_event () < 0
 >        || gdbpy_initialize_new_objfile_event ()  < 0
 > -      || gdbpy_initialize_arch () < 0)
 > +      || gdbpy_initialize_arch () < 0
 > +      || gdbpy_initialize_debugmethods () < 0)
 >      goto fail;
 >  
 >    gdbpy_to_string_cst = PyString_FromString ("to_string");
 > diff --git a/gdb/testsuite/gdb.python/py-debugmethods.cc b/gdb/testsuite/gdb.python/py-debugmethods.cc
 > new file mode 100644
 > index 0000000..e71d5bc
 > --- /dev/null
 > +++ b/gdb/testsuite/gdb.python/py-debugmethods.cc
 > @@ -0,0 +1,192 @@
 > +/* This testcase is part of GDB, the GNU debugger.
 > +
 > +   Copyright 2014 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 <iostream>
 > +
 > +using namespace std;
 > +
 > +namespace dop
 > +{
 > +
 > +class A
 > +{
 > + public:
 > +  int a;
 > +  int array [10];
 > +  virtual ~A ();
 > +  int operator+ (const A &obj);
 > +  virtual int operator- (const A &obj);
 > +  virtual int geta ();
 > +};
 > +
 > +A::~A () { }
 > +
 > +int
 > +A::operator+ (const A &obj)
 > +{
 > +  cout << "From CC <A_plus_A>:" << endl;
 > +  return a + obj.a;
 > +}
 > +
 > +int A::operator- (const A &obj)
 > +{
 > +  cout << "From CC <A_minus_A>:" << endl;
 > +  return a - obj.a;
 > +}
 > +
 > +int A::geta (void)
 > +{
 > +  cout << "From CC geta:" << endl;
 > +  return a;
 > +}
 > +
 > +class B : public A
 > +{
 > + public:
 > +  virtual int operator* (const B &obj);
 > +};
 > +
 > +int
 > +B::operator* (const B &obj)
 > +{
 > +  cout << "From CC <B_star_B>:" << endl;
 > +  return a * obj.a;
 > +}
 > +
 > +typedef B Bt;
 > +
 > +typedef Bt Btt;
 > +
 > +class C : public Bt
 > +{
 > + public:
 > +  virtual ~C();
 > +};
 > +
 > +C::~C () { }
 > +
 > +class D : public B
 > +{
 > + public:
 > +  /* This class overrides the virtual operator* defined in B.  The
 > +     associated Python script replaces B::operator* but not D::operator*.
 > +     Hence, if we have a reference pointing to an instance of D, the C++
 > +     version of D::operator* should be invoked and not the Python version
 > +     B::operator* even after loading the python script.  */
 > +  virtual int operator* (const B &obj);
 > +};
 > +
 > +int
 > +D::operator* (const B &obj)
 > +{
 > +  cout << "From CC <D_star_D>:" << endl;
 > +  return a * obj.a;
 > +}
 > +
 > +class E : public A
 > +{
 > + public:
 > +  /* This class has a member named 'a', while the base class also has a
 > +     member named 'a'.  When one invokes A::geta(), A::a should be
 > +     returned and not E::a as the 'geta' method is defined on class 'A'.
 > +     This class tests this aspect of debug methods.  */
 > +  int a;
 > +};
 > +
 > +template <typename T>
 > +class G
 > +{
 > + public:
 > +  template <typename T1>
 > +  int size_diff ();
 > +
 > +  template <int M>
 > +  int size_mul ();
 > +
 > +  template <typename T1>
 > +  T mul(const T1 t1);
 > +
 > + public:
 > +  T t;
 > +};
 > +
 > +template <typename T>
 > +template <typename T1>
 > +int
 > +G<T>::size_diff ()
 > +{
 > +  cout << "From CC G<>::size_diff:" << endl;
 > +  return sizeof (T1) - sizeof (T);
 > +}
 > +
 > +template <typename T>
 > +template <int M>
 > +int
 > +G<T>::size_mul ()
 > +{
 > +  cout << "From CC G<>::size_mul:" << endl;
 > +  return M * sizeof (T);
 > +}
 > +
 > +template <typename T>
 > +template <typename T1>
 > +T
 > +G<T>::mul (const T1 t1)
 > +{
 > +  cout << "From CC G<>::mul:" << endl;
 > +  return t1 * t;
 > +}
 > +
 > +}
 > +
 > +using namespace dop;
 > +
 > +int main(void)
 > +{
 > +  A a1, a2;
 > +  a1.a = 5;
 > +  a2.a = 10;
 > +  C c1;
 > +  c1.a = 20;
 > +  B b1;
 > +  b1.a = 30;
 > +  D d1;
 > +  d1.a = 50;
 > +  Bt bt;
 > +  bt.a = 40;
 > +  A &ref_c = c1;
 > +  B &ref_d = d1;
 > +  Btt btt;
 > +  btt.a = -5;
 > +  G<int> g, *g_ptr;
 > +  g.t = 5;
 > +  g_ptr = &g;
 > +  E e;
 > +  e.a = 1000;
 > +  e.A::a = 100;
 > +
 > +  int diff = g.size_diff<float> ();
 > +  int smul = g.size_mul<2> ();
 > +  int mul = g.mul (1.0);
 > +
 > +  for (int i = 0; i < 10; i++)
 > +    {
 > +      a1.array[i] = a2.array[i] = i;
 > +    }
 > +
 > +  return 0; /* Break here.  */ 
 > +}
 > diff --git a/gdb/testsuite/gdb.python/py-debugmethods.exp b/gdb/testsuite/gdb.python/py-debugmethods.exp
 > new file mode 100644
 > index 0000000..d54af6e
 > --- /dev/null
 > +++ b/gdb/testsuite/gdb.python/py-debugmethods.exp
 > @@ -0,0 +1,125 @@
 > +# Copyright 2014 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/>.
 > +
 > +# This file is part of the GDB testsuite.  It tests the debug methods
 > +# feature in the Python extension language.
 > +
 > +load_lib gdb-python.exp
 > +
 > +if { [skip_cplus_tests] } { continue }
 > +
 > +standard_testfile py-debugmethods.cc
 > +
 > +if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug c++}]} {
 > +    return -1
 > +}
 > +
 > +# Skip all tests if Python scripting is not enabled.
 > +if { [skip_python_tests] } { continue }
 > +
 > +if ![runto_main] {
 > +   return -1
 > +}
 > +
 > +set debug_methods_script "${srcdir}/${subdir}/${testfile}.py"

This is insufficient for cross testing (build != host).
There's no reason to not support cross testing here, it's easy enough.
grep for gdb_remote_download in py-prettyprint.exp.

 > +
 > +gdb_breakpoint [gdb_get_line_number "Break here."]
 > +gdb_continue_to_breakpoint "Break here" ".*Break here.*"
 > +
 > +# Tests before loading the debug methods.
 > +gdb_test "p a1 + a2" "From CC <A_plus_A>.*15" "Before: a1 + a2"
 > +gdb_test "p a2 - a1" "From CC <A_minus_A>.*5" "Before: a1 - a2"
 > +gdb_test "p b1 - a1" "From CC <A_minus_A>.*25" "Before: b1 - a1"
 > +gdb_test "p c1 - a1" "From CC <A_minus_A>.*15" "Before: c1 - a1"
 > +gdb_test "p c1 - c1" "From CC <A_minus_A>.*0" "Before: c1 - c1"
 > +gdb_test "p a1.geta()" "From CC geta.*5" "Before: a1.geta()"
 > +gdb_test "p ref_c - a1" "From CC <A_minus_A>.*15" "Before: ref_c - a1"
 > +gdb_test "p ref_c - c1" "From CC <A_minus_A>.*0" "Before: ref_c - c1"
 > +gdb_test "p b1 * b1" "From CC <B_star_B>.*900" "Before: b1 * b1"
 > +gdb_test "p ref_c * b1" "No symbol.*" "Before: ref_c * b1"
 > +gdb_test "p ref_d * b1" "From CC <D_star_D>.*1500" "Before: ref_d * b1"
 > +gdb_test "p bt * c1" "From CC <B_star_B>.*800" "Before: bt * c1"
 > +gdb_test "p ++a1" "No symbol.*" "Before: ++a1"
 > +gdb_test "p a1.getarrayind(5)" "Couldn't find method.*" \
 > +  "Before: a1.getarrayind(5)"
 > +gdb_test "p e.geta()" "From CC geta.*100" "Before: e.geta()"
 > +gdb_test "p g.size_diff<float>()" "From CC G<>::size_diff.*" \
 > +  "Before: g.size_diff<float>()"
 > +gdb_test "p g.size_diff<unsigned long>()" "Couldn't find method.*" \
 > +  "Before: g.size_diff<unsigned long>()"
 > +gdb_test "p g.size_mul<2>()" "From CC G<>::size_mul.*" \
 > +  "Before: g.size_mul<2>()"
 > +gdb_test "p g.size_mul<5>()" "Couldn't find method.*" \
 > +  "Before: g.size_mul<5>()"
 > +gdb_test "p g.mul<double>(2.0)" "From CC G<>::mul.*" \
 > +  "Before: g.mul<double>(2.0)"
 > +gdb_test "p g.mul<char>('a')" "Couldn't find method.*" \
 > +  "Before: g.mul<char>('a')"
 > +
 > +# Load the script which adds the debug methods.
 > +gdb_test_no_output "source ${debug_methods_script}" "load the script file"
 > +
 > +# Tests after loading debug methods.
 > +gdb_test "p a1 + a2" "From Python <A_plus_A>.*15" "After: a1 + a2"
 > +gdb_test "p a2 - a1" "From CC <A_minus_A>.*5" "After: a1 - a2"
 > +gdb_test "p b1 - a1" "From CC <A_minus_A>.*25" "After: b1 - a2"
 > +gdb_test "p c1 - a1" "From CC <A_minus_A>.*15" "After: c1 - a1"
 > +gdb_test "p c1 - c1" "From Python <C_minus_C>.*0" "After: c1 - c1"
 > +gdb_test "p a1.geta()" "From Python <A_geta>.*5" "After: a1.geta()"
 > +gdb_test "p ref_c - a1" "From CC <A_minus_A>.*15" "After: ref_c - a1"
 > +gdb_test "p ref_c - c1" "From Python <C_minus_C>.*0" "After: ref_c - c1"
 > +gdb_test "p b1 * b1" "From Python <B_star_B>.*900" "After: b1 * b1"
 > +gdb_test "p ref_c * b1" "No symbol.*" "After: ref_c * b1 failure"
 > +gdb_test "p ref_d * b1" "From CC <D_star_D>.*1500" "After: ref_d * b1"
 > +gdb_test "p bt * c1" "From Python <B_star_B>.*800" "After: bt * c1"
 > +gdb_test "p ++a1" "From Python <plus_plus_A>.*6" "After: ++a1"
 > +gdb_test "p a1.getarrayind(5)" "From Python <A_getarrayind>.*5" \
 > +  "After: a1.getarrayind(5)"
 > +gdb_test "p e.geta()" "From Python <A_geta>.*100" "After: e.geta()"
 > +gdb_test "p g.size_diff<float>  ()" "From Python G<>::size_diff.*" \
 > +  "After: g.size_diff<float>()"
 > +gdb_test "p g.size_diff<  unsigned long  >()" "From Python G<>::size_diff.*" \
 > +  "After: g.size_diff<unsigned long>()"
 > +gdb_test "p g.size_mul<2>()" "From Python G<>::size_mul.*" \
 > +  "After: g.size_mul<2>()"
 > +gdb_test "p g.size_mul<  5  >()" "From Python G<>::size_mul.*" \
 > +  "After: g.size_mul<  5  >()"
 > +gdb_test "p g.mul<double>(2.0)" "From Python G<>::mul.*" \
 > +  "After: g.mul<double>(2.0)"
 > +gdb_test "p g.mul<char>('a')" "From Python G<>::mul.*" \
 > +gdb_test "p g_ptr->mul<char>('a')" "From Python G<>::mul.*" \
 > +  "After: g->mul<char>('a')"
 > +
 > +# Tests for 'disable/enable debugmethod' command.
 > +gdb_test_no_output "disable debugmethod .*debugmethods G_methods" \
 > +  "Disable G_methods"
 > +gdb_test "p g.mul<char>('a')" "Couldn't find method.*" \
 > +  "g.mul<char>('a') after disabling G_methods"
 > +gdb_test_no_output "enable debugmethod .*debugmethods G_methods" \
 > +  "Enable G_methods"
 > +gdb_test "p g.mul<char>('a')" "From Python G<>::mul.*" \
 > +  "After enabling G_methods"
 > +gdb_test_no_output "disable debugmethod .*debugmethods G_methods;mul" \
 > +  "Disable G_methods;mul"
 > +gdb_test "p g.mul<char>('a')" "Couldn't find method.*" \
 > +  "g.mul<char>('a') after disabling G_methods;mul"
 > +gdb_test_no_output "enable debugmethod .*debugmethods G_methods;mul" \
 > +  "Enable G_methods;mul"
 > +gdb_test "p g.mul<char>('a')" "From Python G<>::mul.*" \
 > +  "After enabling G_methods;mul"
 > +
 > +# Test for 'info debug-methods' command
 > +gdb_test "info debugmethod global plus" "global.*plus_plus_A" \
 > +  "info debug-method global plus"
 > diff --git a/gdb/testsuite/gdb.python/py-debugmethods.py b/gdb/testsuite/gdb.python/py-debugmethods.py
 > new file mode 100644
 > index 0000000..2494914
 > --- /dev/null
 > +++ b/gdb/testsuite/gdb.python/py-debugmethods.py
 > @@ -0,0 +1,185 @@
 > +# Copyright 2014 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/>.
 > +
 > +# This file is part of the GDB testsuite.  It implements debug methods
 > +# in the Python extension language.
 > +
 > +import gdb
 > +import re
 > +
 > +from gdb.debugmethods import DebugMethod, DebugMethodMatcher, DebugMethodWorker
 > +from gdb.debugmethods import SimpleDebugMethodMatcher
 > +
 > +def A_plus_A(obj, opr):
 > +  print ('From Python <A_plus_A>:')
 > +  return obj['a'] + opr['a']
 > +
 > +def plus_plus_A(obj):
 > +  print ('From Python <plus_plus_A>:')
 > +  return obj['a'] + 1
 > +
 > +def C_minus_C(obj, opr):
 > +  # This function is not defined for objects of class C in the associated C++
 > +  # file.  However, C is derived from A which has a virtual operator- method.
 > +  # Hence, if an operator '-' is used on a reference pointing to 'C' after
 > +  # loading this script, then this Python version should be invoked.
 > +  print ('From Python <C_minus_C>:')
 > +  return obj['a'] - opr['a']
 > +
 > +def B_star_B(obj, opr):
 > +  print ('From Python <B_star_B>:')
 > +  return obj['a'] * opr['a']
 > +
 > +def A_geta(obj):
 > +  print ('From Python <A_geta>:')
 > +  return obj['a']
 > +
 > +def A_getarrayind(obj, index):
 > +  print 'From Python <A_getarrayind>:'
 > +  return obj['array'][index]
 > +
 > +
 > +type_A = gdb.parse_and_eval('(dop::A *) 0').type.target()
 > +type_B = gdb.parse_and_eval('(dop::B *) 0').type.target()
 > +type_C = gdb.parse_and_eval('(dop::C *) 0').type.target()
 > +type_int = gdb.parse_and_eval('(int *) 0').type.target()
 > +
 > +
 > +class G_size_diff_worker(DebugMethodWorker):
 > +    def __init__(self, class_template_type, method_template_type):
 > +        self._class_template_type = class_template_type
 > +        self._method_template_type = method_template_type
 > +
 > +    def get_argtypes(self):
 > +        pass
 > +
 > +    def invoke(self, obj, args):
 > +        print ('From Python G<>::size_diff()')
 > +        return (self._method_template_type.sizeof -
 > +                self._class_template_type.sizeof)
 > +
 > +
 > +class G_size_mul_worker(DebugMethodWorker):
 > +    def __init__(self, class_template_type, method_template_val):
 > +        self._class_template_type = class_template_type
 > +        self._method_template_val = method_template_val
 > +
 > +    def get_argtypes(self):
 > +        pass
 > +
 > +    def invoke(self, obj, args):
 > +        print ('From Python G<>::size_mul()')
 > +        return self._class_template_type.sizeof * self._method_template_val
 > +
 > +
 > +class G_mul_worker(DebugMethodWorker):
 > +    def __init__(self, class_template_type, method_template_type):
 > +        self._class_template_type = class_template_type
 > +        self._method_template_type = method_template_type
 > +
 > +    def get_argtypes(self):
 > +        return self._method_template_type
 > +
 > +    def invoke(self, obj, args):
 > +        print ('From Python G<>::mul()')
 > +        return obj['t'] * args[0]
 > +
 > +
 > +class G_methods_matcher(DebugMethodMatcher):
 > +    def __init__(self):
 > +        DebugMethodMatcher.__init__(self, 'G_methods')
 > +        self.methods = [DebugMethod('size_diff'),
 > +                        DebugMethod('size_mul'),
 > +                        DebugMethod('mul')]
 > +
 > +    def _is_enabled(self, name):
 > +        for method in self.methods:
 > +            if method.name == name and method.enabled:
 > +                return True
 > +
 > +    def match(self, class_type, method_name):
 > +        class_tag = class_type.unqualified().tag
 > +        if not re.match('^dop::G<[ ]*[_a-zA-Z][ _a-zA-Z0-9]*>$',
 > +                        class_tag):
 > +            return None
 > +        t_name = class_tag[7:-1]
 > +        try:
 > +            t_type = gdb.lookup_type(t_name)
 > +        except Exception:

The general rule is to only watch for specific exceptions that are expected so that bugs aren't masked.
In this case I don't have as strong an opinion, but IWBN.

 > +            return None
 > +        if re.match('^size_diff<[ ]*[_a-zA-Z][ _a-zA-Z0-9]*>$', method_name):
 > +            if not self._is_enabled('size_diff'):
 > +                return None
 > +            t1_name = method_name[10:-1]
 > +            try:
 > +                t1_type = gdb.lookup_type(t1_name)
 > +                return G_size_diff_worker(t_type, t1_type)
 > +            except Exception:
 > +                return None
 > +        if re.match('^size_mul<[ ]*[0-9]+[ ]*>$', method_name):
 > +            if not self._is_enabled('size_mul'):
 > +                return None
 > +            m_val = int(method_name[9:-1])
 > +            return G_size_mul_worker(t_type, m_val)
 > +        if re.match('^mul<[ ]*[_a-zA-Z][ _a-zA-Z0-9]*>$', method_name):
 > +            if not self._is_enabled('mul'):
 > +                return None
 > +            t1_name = method_name[4:-1]
 > +            try:
 > +                t1_type = gdb.lookup_type(t1_name)
 > +                return G_mul_worker(t_type, t1_type)
 > +            except Exception:
 > +                return None
 > +
 > +
 > +global_dm_list = [
 > +    SimpleDebugMethodMatcher('A_plus_A',
 > +                             '^dop::A$',
 > +                             'operator\+',
 > +                             A_plus_A,
 > +                             # This is a replacement, hence match the arg type
 > +                             # exactly!
 > +                             type_A.const().reference()),
 > +    SimpleDebugMethodMatcher('plus_plus_A',
 > +                             '^dop::A$',
 > +                             'operator\+\+',
 > +                             plus_plus_A),
 > +    SimpleDebugMethodMatcher('C_minus_C',
 > +                             '^dop::C$',
 > +                             'operator\-',
 > +                             C_minus_C,
 > +                             type_C),
 > +    SimpleDebugMethodMatcher('B_star_B',
 > +                             '^dop::B$',
 > +                             'operator\*',
 > +                             B_star_B,
 > +                             # This is a replacement, hence match the arg type
 > +                             # exactly!
 > +                             type_B.const().reference()),
 > +    SimpleDebugMethodMatcher('A_geta',
 > +                             '^dop::A$',
 > +                             '^geta$',
 > +                             A_geta),
 > +    SimpleDebugMethodMatcher('A_getarrayind',
 > +                             '^dop::A$',
 > +                             '^getarrayind$',
 > +                             A_getarrayind,
 > +                             type_int),
 > +]
 > +
 > +for matcher in global_dm_list:
 > +    gdb.debugmethods.register_debugmethod_matcher(gdb, matcher)
 > +gdb.debugmethods.register_debugmethod_matcher(gdb.current_progspace(),
 > +                                              G_methods_matcher())
 > diff --git a/gdb/valarith.c b/gdb/valarith.c
 > index 8e863e3..c39d88c 100644
 > --- a/gdb/valarith.c
 > +++ b/gdb/valarith.c
 > @@ -30,6 +30,7 @@
 >  #include <math.h>
 >  #include "infcall.h"
 >  #include "exceptions.h"
 > +#include "extension.h"
 >  
 >  /* Define whether or not the C operator '/' truncates towards zero for
 >     differently signed operands (truncation direction is undefined in C).  */
 > @@ -285,21 +286,27 @@ unop_user_defined_p (enum exp_opcode op, struct value *arg1)
 >     explicitly, and perform correct overload resolution in all of the above
 >     situations or combinations thereof.  */

The function comment needs updating.

 >  
 > -static struct value *
 > +static void
 >  value_user_defined_cpp_op (struct value **args, int nargs, char *operator,
 > -                           int *static_memfuncp)
 > +			   struct value **src_fn,
 > +			   struct debug_method_worker **dm_worker,
 > +			   int *static_memfuncp)
 >  {
 >  
 >    struct symbol *symp = NULL;
 >    struct value *valp = NULL;
 > +  struct debug_method_worker *worker = NULL;
 >  
 >    find_overload_match (args, nargs, operator, BOTH /* could be method */,
 > -                       &args[0] /* objp */,
 > -                       NULL /* pass NULL symbol since symbol is unknown */,
 > -                       &valp, &symp, static_memfuncp, 0);
 > +		       &args[0] /* objp */,
 > +		       NULL /* pass NULL symbol since symbol is unknown */,
 > +		       &valp, &symp, &worker, static_memfuncp, 0);
 >  
 >    if (valp)
 > -    return valp;
 > +    {
 > +      *src_fn = valp;
 > +      return;
 > +    }
 >  
 >    if (symp)
 >      {
 > @@ -307,7 +314,14 @@ value_user_defined_cpp_op (struct value **args, int nargs, char *operator,
 >           expect a reference as its first argument
 >           rather the explicit structure.  */
 >        args[0] = value_ind (args[0]);
 > -      return value_of_variable (symp, 0);
 > +      *src_fn = value_of_variable (symp, 0);
 > +      return;
 > +    }
 > +
 > +  if (worker)
 > +    {
 > +      *dm_worker = worker;
 > +      return;
 >      }
 >  
 >    error (_("Could not find %s."), operator);
 > @@ -316,19 +330,23 @@ value_user_defined_cpp_op (struct value **args, int nargs, char *operator,
 >  /* Lookup user defined operator NAME.  Return a value representing the
 >     function, otherwise return NULL.  */

The function comment needs updating.

 >  
 > -static struct value *
 > +static void
 >  value_user_defined_op (struct value **argp, struct value **args, char *name,
 > -                       int *static_memfuncp, int nargs)
 > +		       int *static_memfuncp, int nargs,
 > +		       struct value **src_fn,
 > +		       struct debug_method_worker **dm_worker)
 >  {
 >    struct value *result = NULL;
 >  
 >    if (current_language->la_language == language_cplus)
 > -    result = value_user_defined_cpp_op (args, nargs, name, static_memfuncp);
 > +    value_user_defined_cpp_op (args, nargs, name, src_fn, dm_worker,
 > +			       static_memfuncp);
 >    else
 > -    result = value_struct_elt (argp, args, name, static_memfuncp,
 > -                               "structure");
 > -
 > -  return result;
 > +    {
 > +      result = value_struct_elt (argp, args, name, static_memfuncp,
 > +				 "structure");
 > +      *src_fn = result;
 > +    }
 >  }
 >  
 >  /* We know either arg1 or arg2 is a structure, so try to find the right
 > @@ -345,6 +363,7 @@ value_x_binop (struct value *arg1, struct value *arg2, enum exp_opcode op,
 >  	       enum exp_opcode otherop, enum noside noside)
 >  {
 >    struct value **argvec;
 > +  struct debug_method_worker *dm_worker = NULL;
 >    char *ptr;
 >    char tstr[13];
 >    int static_memfuncp;
 > @@ -359,6 +378,7 @@ value_x_binop (struct value *arg1, struct value *arg2, enum exp_opcode op,
 >      error (_("Can't do that binary op on that type"));	/* FIXME be explicit */
 >  
 >    argvec = (struct value **) alloca (sizeof (struct value *) * 4);
 > +  argvec[0] = NULL;
 >    argvec[1] = value_addr (arg1);
 >    argvec[2] = arg2;
 >    argvec[3] = 0;
 > @@ -471,8 +491,8 @@ value_x_binop (struct value *arg1, struct value *arg2, enum exp_opcode op,
 >        error (_("Invalid binary operation specified."));
 >      }
 >  
 > -  argvec[0] = value_user_defined_op (&arg1, argvec + 1, tstr,
 > -                                     &static_memfuncp, 2);
 > +  value_user_defined_op (&arg1, argvec + 1, tstr, &static_memfuncp, 2,
 > +			 &argvec[0], &dm_worker);
 >  
 >    if (argvec[0])
 >      {
 > @@ -492,6 +512,14 @@ value_x_binop (struct value *arg1, struct value *arg2, enum exp_opcode op,
 >        return call_function_by_hand (argvec[0], 2 - static_memfuncp,
 >  				    argvec + 1);
 >      }
 > +  if (dm_worker)

Add != NULL check.

 > +    {
 > +      struct value *ret_val = invoke_debug_method (dm_worker, arg1, &arg2, 1);
 > +
 > +      xfree (dm_worker);
 > +
 > +      return ret_val;
 > +    }
 >    throw_error (NOT_FOUND_ERROR,
 >                 _("member function %s not found"), tstr);
 >  #ifdef lint
 > @@ -510,6 +538,7 @@ value_x_unop (struct value *arg1, enum exp_opcode op, enum noside noside)
 >  {
 >    struct gdbarch *gdbarch = get_type_arch (value_type (arg1));
 >    struct value **argvec;
 > +  struct debug_method_worker *dm_worker;
 >    char *ptr;
 >    char tstr[13], mangle_tstr[13];
 >    int static_memfuncp, nargs;
 > @@ -523,6 +552,7 @@ value_x_unop (struct value *arg1, enum exp_opcode op, enum noside noside)
 >      error (_("Can't do that unary op on that type"));	/* FIXME be explicit */
 >  
 >    argvec = (struct value **) alloca (sizeof (struct value *) * 4);
 > +  argvec[0] = NULL;
 >    argvec[1] = value_addr (arg1);
 >    argvec[2] = 0;
 >  
 > @@ -574,8 +604,8 @@ value_x_unop (struct value *arg1, enum exp_opcode op, enum noside noside)
 >        error (_("Invalid unary operation specified."));
 >      }
 >  
 > -  argvec[0] = value_user_defined_op (&arg1, argvec + 1, tstr,
 > -                                     &static_memfuncp, nargs);
 > +  value_user_defined_op (&arg1, argvec + 1, tstr, &static_memfuncp, nargs,
 > +			 &argvec[0], &dm_worker);
 >  
 >    if (argvec[0])
 >      {
 > @@ -594,6 +624,14 @@ value_x_unop (struct value *arg1, enum exp_opcode op, enum noside noside)
 >  	  return value_zero (return_type, VALUE_LVAL (arg1));
 >  	}
 >        return call_function_by_hand (argvec[0], nargs, argvec + 1);
 > +    } 
 > +  if (dm_worker)

Add != NULL check.

 > +    {
 > +      struct value *ret_val = invoke_debug_method (dm_worker, arg1, NULL, 0);
 > +
 > +      xfree (dm_worker);
 > +
 > +      return ret_val;
 >      }
 >    throw_error (NOT_FOUND_ERROR,
 >                 _("member function %s not found"), tstr);
 > diff --git a/gdb/valops.c b/gdb/valops.c
 > index 898401d..4feebd4 100644
 > --- a/gdb/valops.c
 > +++ b/gdb/valops.c
 > @@ -42,6 +42,7 @@
 >  #include "observer.h"
 >  #include "objfiles.h"
 >  #include "exceptions.h"
 > +#include "extension.h"
 >  
 >  extern unsigned int overload_debug;
 >  /* Local functions.  */
 > @@ -70,8 +71,8 @@ int find_oload_champ_namespace_loop (struct value **, int,
 >  				     const int no_adl);
 >  
 >  static int find_oload_champ (struct value **, int, int, int,
 > -			     struct fn_field *, struct symbol **,
 > -			     struct badness_vector **);
 > +			     struct fn_field *, VEC (debug_method_worker_p) *,
 > +			     struct symbol **, struct badness_vector **);
 >  
 >  static int oload_method_static (int, struct fn_field *, int);
 >  
 > @@ -98,9 +99,10 @@ static CORE_ADDR allocate_space_in_inferior (int);
 >  
 >  static struct value *cast_into_complex (struct type *, struct value *);
 >  
 > -static struct fn_field *find_method_list (struct value **, const char *,
 > -					  int, struct type *, int *,
 > -					  struct type **, int *);
 > +static void find_method_list (struct value **, const char *,
 > +			      int, struct type *, struct fn_field **, int *,
 > +			      VEC (debug_method_worker_p) **,
 > +			      struct type **, int *);
 >  
 >  void _initialize_valops (void);
 >  
 > @@ -2293,52 +2295,76 @@ value_struct_elt_bitpos (struct value **argp, int bitpos, struct type *ftype,
 >  
 >  /* Search through the methods of an object (and its bases) to find a
 >     specified method.  Return the pointer to the fn_field list of
 > -   overloaded instances.

This comment needs further updating as the result is "void".

 > +   overloaded instances defined in the source language.  If available
 > +   and matching, a vector matching/overloaded debug methods defined in
 > +   extension languages are also returned.
 >  
 >     Helper function for value_find_oload_list.
 >     ARGP is a pointer to a pointer to a value (the object).
 >     METHOD is a string containing the method name.
 >     OFFSET is the offset within the value.
 >     TYPE is the assumed type of the object.
 > +   FN_LIST The pointer to matching overloaded instances defined in
 > +      source language.
 >     NUM_FNS is the number of overloaded instances.
 > +   EXT_FN_VEC The vector matching debug methods defined in extension
 > +      languages.
 >     BASETYPE is set to the actual type of the subobject where the
 >        method is found.
 >     BOFFSET is the offset of the base subobject where the method is found.  */
 >  
 > -static struct fn_field *
 > +static void
 >  find_method_list (struct value **argp, const char *method,
 > -		  int offset, struct type *type, int *num_fns,
 > +		  int offset, struct type *type,
 > +		  struct fn_field **fn_list, int *num_fns,
 > +		  VEC (debug_method_worker_p) **dm_worker_vec,
 >  		  struct type **basetype, int *boffset)
 >  {
 >    int i;
 > -  struct fn_field *f;
 > -  CHECK_TYPEDEF (type);
 > +  struct fn_field *f = NULL;
 >  
 > -  *num_fns = 0;
 > +  CHECK_TYPEDEF (type);
 >  
 >    /* First check in object itself.  */
 > -  for (i = TYPE_NFN_FIELDS (type) - 1; i >= 0; i--)
 > -    {
 > -      /* pai: FIXME What about operators and type conversions?  */
 > -      const char *fn_field_name = TYPE_FN_FIELDLIST_NAME (type, i);
 >  
 > -      if (fn_field_name && (strcmp_iw (fn_field_name, method) == 0))
 > -	{
 > -	  int len = TYPE_FN_FIELDLIST_LENGTH (type, i);
 > -	  struct fn_field *f = TYPE_FN_FIELDLIST1 (type, i);
 > +  if (fn_list && !(*fn_list))

Wrap the for () loop in { }.

 > +    for (i = TYPE_NFN_FIELDS (type) - 1; i >= 0; i--)
 > +      {
 > +	/* pai: FIXME What about operators and type conversions?  */
 > +	const char *fn_field_name = TYPE_FN_FIELDLIST_NAME (type, i);
 > +  
 > +	if (fn_field_name && (strcmp_iw (fn_field_name, method) == 0))
 > +  	{
 > +  	  int len = TYPE_FN_FIELDLIST_LENGTH (type, i);
 > +  	  f = TYPE_FN_FIELDLIST1 (type, i);
 > +	  *fn_list = f;
 > +  
 > +  	  *num_fns = len;
 > +  	  *basetype = type;
 > +  	  *boffset = offset;
 > +  
 > +  	  /* Resolve any stub methods.  */
 > +  	  check_stub_method_group (type, i);
 > +  
 > +	  break;
 > +  	}
 > +      }
 >  
 > -	  *num_fns = len;
 > -	  *basetype = type;
 > -	  *boffset = offset;
 > +  if (dm_worker_vec)
 > +    {
 > +      VEC (debug_method_worker_p) *worker_vec = NULL, *new_vec = NULL;
 >  
 > -	  /* Resolve any stub methods.  */
 > -	  check_stub_method_group (type, i);
 > +      worker_vec = get_matching_debug_method_workers (type, method);
 > +      new_vec = VEC_merge (debug_method_worker_p, *dm_worker_vec, worker_vec);
 >  
 > -	  return f;
 > -	}
 > +      VEC_free (debug_method_worker_p, *dm_worker_vec);
 > +      VEC_free (debug_method_worker_p, worker_vec);
 > +      *dm_worker_vec = new_vec;
 >      }
 >  
 > -  /* Not found in object, check in base subobjects.  */
 > +  /* If source methods are not found in current class, look for them in the
 > +     base classes.  We have to go through the base classes to gather extension
 > +     methods anyway.  */
 >    for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--)
 >      {
 >        int base_offset;
 > @@ -2355,13 +2381,11 @@ find_method_list (struct value **argp, const char *method,
 >  	{
 >  	  base_offset = TYPE_BASECLASS_BITPOS (type, i) / 8;
 >  	}
 > -      f = find_method_list (argp, method, base_offset + offset,
 > -			    TYPE_BASECLASS (type, i), num_fns, 
 > -			    basetype, boffset);
 > -      if (f)
 > -	return f;
 > +      
 > +      find_method_list (argp, method, base_offset + offset,
 > +			TYPE_BASECLASS (type, i), fn_list, num_fns,
 > +			dm_worker_vec, basetype, boffset);
 >      }
 > -  return NULL;
 >  }
 >  
 >  /* Return the list of overloaded methods of a specified name.
 > @@ -2369,14 +2393,20 @@ find_method_list (struct value **argp, const char *method,
 >     ARGP is a pointer to a pointer to a value (the object).
 >     METHOD is the method name.
 >     OFFSET is the offset within the value contents.
 > +   FN_LIST The pointer to matching overloaded instances defined in
 > +      source language.
 >     NUM_FNS is the number of overloaded instances.
 > +   DM_WORKER_VEC The vector of matching debug method workers defined in
 > +      extension languages.
 >     BASETYPE is set to the type of the base subobject that defines the
 >        method.
 >     BOFFSET is the offset of the base subobject which defines the method.  */

The function comment needs further updating (I think) now that
the result is "void".

 >  
 > -static struct fn_field *
 > +static void
 >  value_find_oload_method_list (struct value **argp, const char *method,
 > -			      int offset, int *num_fns, 
 > +                              int offset, struct fn_field **fn_list,
 > +                              int *num_fns,
 > +                              VEC (debug_method_worker_p) **dm_worker_vec,
 >  			      struct type **basetype, int *boffset)
 >  {
 >    struct type *t;
 > @@ -2398,8 +2428,34 @@ value_find_oload_method_list (struct value **argp, const char *method,
 >      error (_("Attempt to extract a component of a "
 >  	     "value that is not a struct or union"));
 >  
 > -  return find_method_list (argp, method, 0, t, num_fns, 
 > -			   basetype, boffset);
 > +  /* Clear the lists.  */
 > +  if (fn_list)
 > +    {
 > +      *fn_list = NULL;
 > +      *num_fns = 0;
 > +    }
 > +  if (dm_worker_vec)
 > +    *dm_worker_vec = VEC_alloc (debug_method_worker_p, 1);
 > +
 > +  find_method_list (argp, method, 0, t, fn_list, num_fns, dm_worker_vec,
 > +		    basetype, boffset);
 > +}
 > +
 > +/* Return the dynamic type of OBJ.  */

"or NULL if it doesn't have one." (or some such)

 > +
 > +static struct type *
 > +value_has_indirect_dynamic_type (struct value *obj)
 > +{
 > +  struct type *stype, *dtype, *dtype_ind;
 > +
 > +  stype = check_typedef (TYPE_TARGET_TYPE (value_type (obj)));
 > +  dtype_ind = value_rtti_indirect_type (obj, NULL, NULL, NULL);
 > +  dtype = dtype_ind ? check_typedef (TYPE_TARGET_TYPE (dtype_ind)) : stype;
 > +
 > +  if (class_types_same_p (stype, dtype))
 > +    return NULL;
 > +  else
 > +    return dtype_ind;
 >  }
 >  
 >  /* Given an array of arguments (ARGS) (which includes an
 > @@ -2427,9 +2483,11 @@ value_find_oload_method_list (struct value **argp, const char *method,
 >     Return value is an integer: 0 -> good match, 10 -> debugger applied
 >     non-standard coercions, 100 -> incompatible.
 >  
 > -   If a method is being searched for, VALP will hold the value.
 > -   If a non-method is being searched for, SYMP will hold the symbol 
 > -   for it.
 > +   If the best match is a debug function/method implemented in an extension
 > +   language, then EXT_FN will hold the matching function/method.  Otherwise,
 > +   VALP will hold the value if a method is being searched for, or SYMP will
 > +   hold the symbol for the matching function if a non-method is being
 > +   searched for.

Code or comment needs updating.  EXT_FN vs dm_worker.

 >  
 >     If a method is being searched for, and it is a static method,
 >     then STATICP will point to a non-zero value.
 > @@ -2447,6 +2505,7 @@ find_overload_match (struct value **args, int nargs,
 >  		     const char *name, enum oload_search_type method,
 >  		     struct value **objp, struct symbol *fsym,
 >  		     struct value **valp, struct symbol **symp, 
 > +		     struct debug_method_worker **dm_worker,
 >  		     int *staticp, const int no_adl)
 >  {
 >    struct value *obj = (objp ? *objp : NULL);
 > @@ -2454,16 +2513,24 @@ find_overload_match (struct value **args, int nargs,
 >    /* Index of best overloaded function.  */
 >    int func_oload_champ = -1;
 >    int method_oload_champ = -1;
 > +  int src_method_oload_champ = -1;

It would get good to add a comment here describing what ..._bkp is for, it's not readily clear.
Plus, IIUC, _orig might be a better suffix.

 > +  int src_method_oload_champ_bkp = -1;
 > +  int ext_method_oload_champ = -1;
 > +  int src_and_ext_equal = 0;
 >  
 >    /* The measure for the current best match.  */
 >    struct badness_vector *method_badness = NULL;
 >    struct badness_vector *func_badness = NULL;
 > +  struct badness_vector *ext_method_badness = NULL;
 > +  struct badness_vector *src_method_badness = NULL;
 >  
 >    struct value *temp = obj;
 >    /* For methods, the list of overloaded methods.  */
 >    struct fn_field *fns_ptr = NULL;
 >    /* For non-methods, the list of overloaded function symbols.  */
 >    struct symbol **oload_syms = NULL;
 > +  /* For extension functions, the VEC of extension function descriptors.  */
 > +  VEC (debug_method_worker_p) *dm_worker_vec = NULL;
 >    /* Number of overloaded instances being considered.  */
 >    int num_fns = 0;
 >    struct type *basetype = NULL;
 > @@ -2475,6 +2542,8 @@ find_overload_match (struct value **args, int nargs,
 >    const char *func_name = NULL;
 >    enum oload_classification match_quality;
 >    enum oload_classification method_match_quality = INCOMPATIBLE;
 > +  enum oload_classification src_method_match_quality = INCOMPATIBLE;
 > +  enum oload_classification ext_method_match_quality = INCOMPATIBLE;
 >    enum oload_classification func_match_quality = INCOMPATIBLE;
 >  
 >    /* Get the list of overloaded methods or functions.  */
 > @@ -2503,12 +2572,12 @@ find_overload_match (struct value **args, int nargs,
 >  	}
 >  
 >        /* Retrieve the list of methods with the name NAME.  */
 > -      fns_ptr = value_find_oload_method_list (&temp, name, 
 > -					      0, &num_fns, 
 > -					      &basetype, &boffset);
 > +      value_find_oload_method_list (&temp, name, 0, &fns_ptr, &num_fns,
 > +				    dm_worker ? &dm_worker_vec : NULL,
 > +				    &basetype, &boffset);
 >        /* If this is a method only search, and no methods were found
 >           the search has faild.  */
 > -      if (method == METHOD && (!fns_ptr || !num_fns))
 > +      if (method == METHOD && (!fns_ptr || !num_fns) && !dm_worker_vec)
 >  	error (_("Couldn't find method %s%s%s"),
 >  	       obj_type_name,
 >  	       (obj_type_name && *obj_type_name) ? "::" : "",
 > @@ -2519,18 +2588,82 @@ find_overload_match (struct value **args, int nargs,
 >        if (fns_ptr)
 >  	{
 >  	  gdb_assert (TYPE_DOMAIN_TYPE (fns_ptr[0].type) != NULL);
 > -	  method_oload_champ = find_oload_champ (args, nargs, method,
 > -	                                         num_fns, fns_ptr,
 > -	                                         oload_syms, &method_badness);
 > -
 > -	  method_match_quality =
 > -	      classify_oload_match (method_badness, nargs,
 > -	                            oload_method_static (method, fns_ptr,
 > -	                                                 method_oload_champ));
 > +	  src_method_oload_champ = find_oload_champ (args, nargs, method,
 > +						     num_fns, fns_ptr, NULL,
 > +						     oload_syms,
 > +						     &src_method_badness);
 > +
 > +	  src_method_match_quality =

The rule is the = goes on the next line.

 > +	      classify_oload_match (
 > +		  src_method_badness, nargs,
 > +		  oload_method_static (method, fns_ptr,
 > +				       src_method_oload_champ));
 > +
 > +	  make_cleanup (xfree, src_method_badness);
 > +	}
 >  
 > -	  make_cleanup (xfree, method_badness);
 > +      if (dm_worker && VEC_length (debug_method_worker_p, dm_worker_vec))
 > +	{
 > +	  ext_method_oload_champ = find_oload_champ (args, nargs, method,
 > +						     0, NULL, dm_worker_vec,
 > +						     NULL, &ext_method_badness);
 > +	  ext_method_match_quality = classify_oload_match (ext_method_badness,
 > +							   nargs, 0);
 > +	  make_cleanup (xfree, ext_method_badness);
 > +	  make_debug_method_worker_vec_cleanup (dm_worker_vec);
 >  	}
 >  
 > +      if (src_method_oload_champ >= 0 && ext_method_oload_champ >= 0)
 > +	{
 > +	  switch (compare_badness (ext_method_badness, src_method_badness))
 > +	    {
 > +	      case 0: /* Src method and ext method are equally good.  */
 > +		src_and_ext_equal = 1;

Missing break?
If fall-through is intentional a comment is required: /* FALLTHROUGH */
[spelling of the comment taken from that used throughout gdb]

 > +	      case 1: /* Src method and ext method are incompatible  */

Period at end of sentence.

 > +		/* if ext method match is not standard, then let source method
 > +		   win.  */

s/if/If/

 > +		if (ext_method_match_quality != STANDARD)
 > +		  {
 > +		    method_oload_champ = src_method_oload_champ;
 > +		    method_badness = src_method_badness;
 > +		    ext_method_oload_champ = -1;
 > +		    method_match_quality = src_method_match_quality;
 > +		    break;
 > +		  }

Should break be outside "if", or is this an intentional fall-through?

 > +	      case 2: /* Ext method is champion.  */
 > +		method_oload_champ = ext_method_oload_champ;
 > +		method_badness = ext_method_badness;
 > +		src_method_oload_champ_bkp = src_method_oload_champ;
 > +		src_method_oload_champ = -1;
 > +		method_match_quality = ext_method_match_quality;
 > +		break;
 > +	      case 3: /* Src method is champion.  */
 > +		method_oload_champ = src_method_oload_champ;
 > +		method_badness = src_method_badness;
 > +		ext_method_oload_champ = -1;
 > +		method_match_quality = src_method_match_quality;
 > +		break;
 > +	      default:
 > +		error (_("Internal error: unexpected overload comparison "
 > +			 "result"));

Use gdb_assert_not_reached.

 > +		break;
 > +	    }
 > +	}
 > +      else
 > +	{

Add comment here saying we intentionally pick the ext method over the src one.
[That is the intent, right?]

 > +	  if (src_method_oload_champ >= 0)
 > +	    {
 > +	      method_oload_champ = src_method_oload_champ;
 > +	      method_badness = src_method_badness;
 > +	      method_match_quality = src_method_match_quality;
 > +	    }
 > +	  if (ext_method_oload_champ >= 0)
 > +	    {
 > +	      method_oload_champ = ext_method_oload_champ;
 > +	      method_badness = ext_method_badness;
 > +	      method_match_quality = ext_method_match_quality;
 > +	    }
 > +	}
 >      }
 >  
 >    if (method == NON_METHOD || method == BOTH)
 > @@ -2673,21 +2806,6 @@ find_overload_match (struct value **args, int nargs,
 >  		 func_name);
 >      }
 >  
 > -  if (staticp != NULL)
 > -    *staticp = oload_method_static (method, fns_ptr, method_oload_champ);
 > -
 > -  if (method_oload_champ >= 0)
 > -    {
 > -      if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ))
 > -	*valp = value_virtual_fn_field (&temp, fns_ptr, method_oload_champ,
 > -					basetype, boffset);
 > -      else
 > -	*valp = value_fn_field (&temp, fns_ptr, method_oload_champ,
 > -				basetype, boffset);
 > -    }
 > -  else
 > -    *symp = oload_syms[func_oload_champ];
 > -
 >    if (objp)
 >      {
 >        struct type *temp_type = check_typedef (value_type (temp));
 > @@ -2697,10 +2815,66 @@ find_overload_match (struct value **args, int nargs,
 >  	  && (TYPE_CODE (objtype) == TYPE_CODE_PTR
 >  	      || TYPE_CODE (objtype) == TYPE_CODE_REF))
 >  	{
 > -	  temp = value_addr (temp);
 > +	  *objp = value_addr (temp);
 > +	}
 > +      else
 > +	*objp = temp;
 > +    }
 > +
 > +  if (staticp != NULL)
 > +    *staticp = oload_method_static (method, fns_ptr, method_oload_champ);
 > +
 > +  if (method_oload_champ >= 0)
 > +    {
 > +      if (src_method_oload_champ >= 0)

This is confusing with the above section of code where I think we're intentionally picking the ext method over the src one.
Here we do the opposite.
IOW, if (src_method_oload_champ >= 0 && ext_method_oload_champ >= 0),
above we'll set method_oload_champ to ext_method_oload_champ,
but here we choose the src method.

Plus it would be cleaner (I think!) to have one copy of the value_has_indirect_dynamic_type handling code.
Plus, it's confusing that we've decided the src method is the oload champ, but we make a recursive call in which it seems that an ext method could win.  It may be that things are ok as is, but a comment is needed here to help clarify what's going on here.

 > +	{
 > +	  if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ))
 > +	    {
 > +	      struct type *dtype;
 > +      
 > +	      dtype = value_has_indirect_dynamic_type (args[0]);
 > +	      if (dtype)
 > +		{
 > +		  args[0] = value_cast (dtype, args[0]);

Is value_cast sufficient here?
Seems like value_dynamic_cast may be necessary.

 > +		  do_cleanups (all_cleanups);
 > +		  return find_overload_match (args, nargs, name, method,
 > +					      objp, fsym, valp, symp, dm_worker,
 > +					      staticp, no_adl);
 > +		}
 > +	      else
 > +		*valp = value_virtual_fn_field (&temp, fns_ptr,
 > +						method_oload_champ,
 > +						basetype, boffset);
 > +	    }
 > +	  else
 > +	    *valp = value_fn_field (&temp, fns_ptr, method_oload_champ,
 > +				    basetype, boffset);
 > +	}
 > +      else
 > +	{
 > +	  if (src_and_ext_equal 
 > +	      && TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, src_method_oload_champ_bkp))
 > +	    {
 > +	      struct type *dtype;
 > +      
 > +	      dtype = value_has_indirect_dynamic_type (args[0]);
 > +	      if (dtype)
 > +		{
 > +		  args[0] = value_cast (dtype, args[0]);

value_cast -> value_dynamic_cast?

 > +		  do_cleanups (all_cleanups);
 > +		  return find_overload_match (args, nargs, name, method,
 > +					      objp, fsym, valp, symp, dm_worker,
 > +					      staticp, no_adl);
 > +		}
 > +	    }
 > +
 > +	  *dm_worker = clone_debug_method_worker (
 > +	     VEC_index (debug_method_worker_p, dm_worker_vec,
 > +			ext_method_oload_champ));
 >  	}
 > -      *objp = temp;
 >      }
 > +  else
 > +    *symp = oload_syms[func_oload_champ];
 >  
 >    do_cleanups (all_cleanups);
 >  
 > @@ -2835,7 +3009,7 @@ find_oload_champ_namespace_loop (struct value **args, int nargs,
 >      ++num_fns;
 >  
 >    new_oload_champ = find_oload_champ (args, nargs, 0, num_fns,
 > -				      NULL, new_oload_syms,
 > +				      NULL, NULL, new_oload_syms,
 >  				      &new_oload_champ_bv);
 >  
 >    /* Case 1: We found a good match.  Free earlier matches (if any),
 > @@ -2873,7 +3047,9 @@ find_oload_champ_namespace_loop (struct value **args, int nargs,
 >  
 >  /* Look for a function to take NARGS args of ARGS.  Find
 >     the best match from among the overloaded methods or functions
 > -   (depending on METHOD) given by FNS_PTR or OLOAD_SYMS, respectively.
 > +   given by FNS_PTR or OLOAD_SYMS or DM_WORKER_VEC, respectively.  If
 > +   DM_WORKER_VEC is NULL, then METHOD indicates whether to use FNS_PTR
 > +   or OLOAD_SYMS to find the best match.

Note that an empty VEC is legitimately represented as NULL (and it would be confusing to not allow for that).
Thus the sentence "If DM_WORKER_VEC is NULL ..." is unnecessary.
Its presence feels like you're intending NULL to mean something else.

 >     The number of methods/functions in the list is given by NUM_FNS.
 >     Return the index of the best match; store an indication of the
 >     quality of the match in OLOAD_CHAMP_BV.
 > @@ -2883,10 +3059,12 @@ find_oload_champ_namespace_loop (struct value **args, int nargs,
 >  static int
 >  find_oload_champ (struct value **args, int nargs, int method,
 >  		  int num_fns, struct fn_field *fns_ptr,
 > +		  VEC (debug_method_worker_p) *dm_worker_vec,
 >  		  struct symbol **oload_syms,
 >  		  struct badness_vector **oload_champ_bv)
 >  {
 >    int ix;
 > +  int dm_worker_vec_n = 0;
 >    /* A measure of how good an overloaded instance is.  */
 >    struct badness_vector *bv;
 >    /* Index of best overloaded function.  */
 > @@ -2896,18 +3074,30 @@ find_oload_champ (struct value **args, int nargs, int method,
 >    /* 0 => no ambiguity, 1 => two good funcs, 2 => incomparable funcs.  */
 >  
 >    *oload_champ_bv = NULL;
 > +  if (dm_worker_vec != NULL)
 > +    dm_worker_vec_n = VEC_length (debug_method_worker_p, dm_worker_vec);
 > +  else
 > +    dm_worker_vec_n = 0;

The check for NULL is not necessary here, VEC_length can handle NULL.

 >  
 >    /* Consider each candidate in turn.  */
 > -  for (ix = 0; ix < num_fns; ix++)
 > +  for (ix = 0; (ix < num_fns) || (ix < dm_worker_vec_n); ix++)
 >      {
 >        int jj;
 > -      int static_offset = oload_method_static (method, fns_ptr, ix);
 > +      int static_offset = 0;
 >        int nparms;
 >        struct type **parm_types;
 > +      struct debug_method_worker *worker = NULL;
 > +
 > +      if (dm_worker_vec)
 > +	worker = VEC_index (debug_method_worker_p, dm_worker_vec, ix);
 >  
 >        if (method)
 >  	{
 > -	  nparms = TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr, ix));
 > +	  if (fns_ptr)
 > +	    {
 > +	      static_offset = oload_method_static (method, fns_ptr, ix);
 > +	      nparms = TYPE_NFIELDS (TYPE_FN_FIELD_TYPE (fns_ptr, ix));
 > +	    }
 >  	}
 >        else
 >  	{
 > @@ -2916,13 +3106,18 @@ find_oload_champ (struct value **args, int nargs, int method,
 >  	}
 >  
 >        /* Prepare array of parameter types.  */
 > -      parm_types = (struct type **) 
 > -	xmalloc (nparms * (sizeof (struct type *)));
 > -      for (jj = 0; jj < nparms; jj++)
 > -	parm_types[jj] = (method
 > -			  ? (TYPE_FN_FIELD_ARGS (fns_ptr, ix)[jj].type)
 > -			  : TYPE_FIELD_TYPE (SYMBOL_TYPE (oload_syms[ix]), 
 > -					     jj));
 > +      if (fns_ptr || oload_syms)
 > +	{
 > +	  parm_types = (struct type **) 
 > +	    xmalloc (nparms * (sizeof (struct type *)));
 > +	  for (jj = 0; jj < nparms; jj++)
 > +	    parm_types[jj] = (method
 > +			      ? (TYPE_FN_FIELD_ARGS (fns_ptr, ix)[jj].type)
 > +			      : TYPE_FIELD_TYPE (SYMBOL_TYPE (oload_syms[ix]), 
 > +			      jj));
 > +	}
 > +      else
 > +	parm_types = get_debug_method_argtypes (worker, &nparms);

What if worker is NULL?
Plus maybe I was wrong above: Here we prefer a src method over an ext method.
Clarity is needed.

 >  
 >        /* Compare parameter types to supplied argument types.  Skip
 >           THIS for static methods.  */
 > diff --git a/gdb/value.h b/gdb/value.h
 > index f846669..87bd109 100644
 > --- a/gdb/value.h
 > +++ b/gdb/value.h
 > @@ -31,6 +31,7 @@ struct type;
 >  struct ui_file;
 >  struct language_defn;
 >  struct value_print_options;
 > +struct debug_method_worker;
 >  
 >  /* The structure which defines the type of a value.  It should never
 >     be possible for a program lval value to survive over a call to the
 > @@ -690,6 +691,7 @@ extern int find_overload_match (struct value **args, int nargs,
 >  				enum oload_search_type method,
 >  				struct value **objp, struct symbol *fsym,
 >  				struct value **valp, struct symbol **symp,
 > +				struct debug_method_worker **dm_worker,
 >  				int *staticp, const int no_adl);
 >  
 >  extern struct value *value_field (struct value *arg1, int fieldno);

That's it for this pass of my review.
I still want to apply the patch and dig a bit deeper in places.
I'll get to that next week.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]