Re: [PATCH] Add a find_source hook for extension languages to locate missing source files.

[Yao Qi <qiyaoltc at gmail dot com>]

On Wed, May 6, 2015 at 8:58 PM, Ted Mielczarek <ted@mielczarek.org> wrote:

Hi Ted,
sorry for delay.  In general, your patch doesn't have changelog entry.  Take
a look at https://sourceware.org/gdb/wiki/ContributionChecklist  Also, you
need copyright assignment.  See "6. FSF copyright Assignment" in the
checklist above.

> diff --git a/gdb/extension.c b/gdb/extension.c
> index dac203b..e0c8b01 100644
> --- a/gdb/extension.c
> +++ b/gdb/extension.c
> @@ -1060,6 +1060,43 @@ ext_lang_before_prompt (const char *current_gdb_prompt)
>      }
>  }
>
> +/* Iterate over the extension languages giving them a chance to
> +   locate the source file in FILENAME.  The first one to return
> +   a result wins, and no further languages are tried.
> +   If there was an error, or if no extension succeeds, then NULL is returned.
> +*/

Nit, move "*/" to the previous line.

> +
> +char *
> +ext_lang_find_source (const char* filename)
> +{
> +  int i;
> +  const struct extension_language_defn *extlang;
> +
> +  ALL_ENABLED_EXTENSION_LANGUAGES (i, extlang)
> +    {
> +      char *result = NULL;
> +      enum ext_lang_rc rc;
> +
> +      if (extlang->ops->find_source == NULL)
> +       continue;
> +      rc = extlang->ops->find_source (extlang, filename, &result);
> +      switch (rc)
> +       {
> +       case EXT_LANG_RC_OK:
> +         gdb_assert (result != NULL);

As documented in doc/python.texi, result should be an absolute path, and
its base name should be same as filename's.  Let us add two gdb_assert
to check these properties.

> +         return result;
> +       case EXT_LANG_RC_ERROR:
> +         return NULL;
> +       case EXT_LANG_RC_NOP:
> +         break;
> +       default:
> +         gdb_assert_not_reached ("bad return from find_source");
> +       }
> +    }
> +
> +  return NULL;
> +}
> +
>  extern initialize_file_ftype _initialize_extension;
>

> +/* This is the extension_language_ops.find_source "method".  */
> +
> +static enum ext_lang_rc
> +gdbpy_find_source_hook (const struct extension_language_defn *extlang,
> +                       const char *filename,
> +                       char **found_filename)

The indentation looks odd.

>
>
>  /* Printing.  */
> diff --git a/gdb/source.c b/gdb/source.c
> index fbec0f1..8bee855 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -20,6 +20,7 @@
>  #include "arch-utils.h"
>  #include "symtab.h"
>  #include "expression.h"
> +#include "extension.h"
>  #include "language.h"
>  #include "command.h"
>  #include "source.h"
> @@ -1100,6 +1101,25 @@ find_and_open_source (const char *filename,
>                         OPEN_MODE, fullname);
>      }
>
> +  if (result < 0)
> +    {
> +      /* Didn't work.  Ask extension languages if they can find it.  */
> +      char *found_filename = ext_lang_find_source (filename);
> +
> +      if (found_filename != NULL)
> +        {
> +         result = gdb_open_cloexec (found_filename, OPEN_MODE, 0);
> +         if (result >= 0)
> +           {
> +             *fullname = found_filename;
> +           }
> +         else
> +           {
> +             make_cleanup (xfree, found_filename);

If there is something wrong, we can just xfree foud_filename immediately.  Don't
have to append the xfree in to the cleanup chain.

> diff --git a/gdb/testsuite/gdb.python/py-find-source-hook.c b/gdb/testsuite/gdb.python/py-find-source-hook.c
> new file mode 100644
> index 0000000..50db568
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-find-source-hook.c
> @@ -0,0 +1,20 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2011-2015 Free Software Foundation, Inc.

2016

> +
> +   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/>.  */
> +
> +
> +int main (int argc, char** argv) { return 0; }
> +
> diff --git a/gdb/testsuite/gdb.python/py-find-source-hook.exp b/gdb/testsuite/gdb.python/py-find-source-hook.exp
> new file mode 100644
> index 0000000..a63b0f8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-find-source-hook.exp
> @@ -0,0 +1,96 @@
> +# Copyright (C) 2015 Free Software Foundation, Inc.
> +

2016

> +# 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/>.
> +
> +if [target_info exists use_gdb_stub] {
> +    return 0
> +}

Why do you skip the test like this?

> +
> +load_lib gdb-python.exp
> +
> +standard_testfile
> +
> +set orig_srcfile  $srcdir/$subdir/$testfile.c
> +# Copy the source file to the test directory so
> +# we can remove it to test the find_source hook.
> +set srcfile  [standard_output_file $testfile.c]
> +set result [catch "exec cp $orig_srcfile $srcfile" output]
> +if {$result == 1} {
> +    return -1
> +}

It doesn't work in remote-host testing, in which gdb is run on a different
machine other than the machine gdb is built.

The rest of test should be reviewed, but I can not finish the review now.
I am in a travel now, and hopefully I can finish the review next week.

-- 
Yao (齐尧)