Re: -file-list-exec-source-files

Bob Rossi
2004-04-27T00:05:36Z

On Thu, Apr 22, 2004 at 11:40:31AM -0400, Elena Zannoni wrote:
> Eli Zaretskii writes:
> > > Date: Tue, 20 Apr 2004 10:10:01 -0400
> > > From: Bob Rossi <bob@brasko.net>
> > >
> > > Honestly, will someone please review this patch.
> > >
> > > Seriously, why is no one looking at this patch?
> > > It's been over 2 months now!
> >
> > Bob, I suggest that you submit another patch that takes into account
> > all the comments, mainly by Elena and Jason, that still stand after
> > the discussions back then. Right now, your followups are just
> > reiterations of Elena's comments and your counter-comments; it's hard
> > to look at the patch whose many parts are invalidated by discussions.
> >
> > If I missed a newer patch you've already sent, please tell me its URL
> > in the archives, and I will look at it.
> >
> > In any case, I think Elena should have the final word, since she is
> > both responsible for gdb/mi and was the original reviewer of your
> > submission. But I will try to help her by taking another look at your
> > code.
> >
>
> Thank you Eli for helping in this, I appreciate the help. Bob, please
> run gdb_indent.sh (found in the gdb source directory) on your code to
> make it conform to gnu standards. Also address the issues you feel
> comfortable fixing. Once that's done we can have another look.

Thanks, here is a new patch. The testsuite is still passing with this
change.

I hopefully fixed the formatting problems and most of the other issues
you brought up. The only 2 known issues are the
1. dwarf2 specs about :pathname or hostname:pathname
2. comments about source.c

Here were your comments with my respone,

> this part I am not clear about.

Ok, Ok. I thought about this a lot. I think I made the best decision and
can describe why.

A few assumptions are in order. In order to get the fullname (abs path)
to a file, GDB need's three things. The directory the file was compiled
in (dirname), the filename in question (filename) and a list of paths
to search.

> There is already a function called source_full_path_of() would it help
> if you used it?

The function source_full_path_of does not take into account 'dirname'.
It calls openp, which is not capable of finding the fullname of a file,
since it doesn't understand what dirname is. Basically, I don't even
think this function (source_full_path_of) is "truly" capable of
finding the fullpath to a file. However, instead of removing it,
I left it, since caller's of this function might be using for something
I know nothing about.

> What is the difference between find_and_open_source and
> open_source_file? I.e. why did you need to introduce it. I think it's
> not clear just from your comments about the file possibly baing moved
> around.

open_source_file was left around for backwards compatibility. The unit
source.c was used to calling a function, with just passing the symtab,
and getting back the symtab with a valid fullname. I could remove all
occurences of this function and replace it with symtab_to_fullname.

> I am a bit worried about the substitution of symtab_to_filename with
> symtab_to_fullname. The former returns null only if there is no
> symtab. The latter returns null when there is no symtab OR when it
> cannot find the file. So the behavior is slightly different.

I basically think that the call -file-list-exec-source-files shouldn't
'cache' it's results. GDB looks for each file, every time it is
requested to get the fullname. This is because, the user could have
changed the path, or moved/deleted the file. I don't think GDB should
just return the filename instead, of the fullname.

So, if find_and_open_source couldn't "find and open the source file", it
returns NULL. Also, as a side effect, fullname in the [ps]ymtab also
get's set to NULL.

The testsuite didn't seem to have a problem with this, and I think it
makes sense to not trick the caller into having results when it couldn't
find any.

If the caller really wanted this functionality,
return s->filename; /* File not found. Just use short name */
I believe it should be the caller's responsibility.

if ( symtab_to_fullname ( s ) == NULL )
/* use symtab->filename */
else
/* use symtab->fullname */

It doesn't really make sense to return the filename and not state that
it is not really the fullname. Also, if the caller tries to access
s->fullname, it will not be successful, because the file simply isn't
there.

Bob Rossi

Index: gdb/dbxread.c
===================================================================
RCS file: /cvs/src/src/gdb/dbxread.c,v
retrieving revision 1.64
diff -w -u -r1.64 dbxread.c
--- gdb/dbxread.c 14 Feb 2004 15:46:32 -0000 1.64
+++ gdb/dbxread.c 26 Apr 2004 23:54:42 -0000
@@ -1463,6 +1463,7 @@
static int prev_so_symnum = -10;
static int first_so_symnum;
char *p;
+ static char *dirname_nso;
int prev_textlow_not_set;

valu = nlist.n_value + ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
@@ -1520,18 +1521,27 @@

p = strrchr (namestring, '/');
if (p && *(p + 1) == '\000')
- continue; /* Simply ignore directory name SOs */
+ {
+ /* Save the directory name SOs locally, then save it into
+ the psymtab when it's created below. */
+ dirname_nso = namestring;
+ continue;
+ }

/* Some other compilers (C++ ones in particular) emit useless
SOs for non-existant .c files. We ignore all subsequent SOs that
immediately follow the first. */

if (!pst)
+ {
pst = start_psymtab (objfile,
namestring, valu,
first_so_symnum * symbol_size,
objfile->global_psymbols.next,
objfile->static_psymbols.next);
+ pst->dirname = dirname_nso;
+ dirname_nso = NULL;
+ }
continue;
}

Index: gdb/defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.144
diff -w -u -r1.144 defs.h
--- gdb/defs.h 21 Apr 2004 23:52:20 -0000 1.144
+++ gdb/defs.h 26 Apr 2004 23:54:44 -0000
@@ -616,8 +616,6 @@

extern void init_last_source_visited (void);

-extern char *symtab_to_filename (struct symtab *);
-
/* From exec.c */

extern void exec_set_section_offsets (bfd_signed_vma text_off,
Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.148
diff -w -u -r1.148 dwarf2read.c
--- gdb/dwarf2read.c 19 Apr 2004 18:20:50 -0000 1.148
+++ gdb/dwarf2read.c 26 Apr 2004 23:54:52 -0000
@@ -378,6 +378,7 @@
sometimes DW_TAG_MIPS_linkage_name or a string computed in some
other fashion. */
char *name;
+ char *dirname;

/* The scope to prepend to our children. This is generally
allocated on the comp_unit_obstack, so will disappear
@@ -1272,6 +1273,8 @@
objfile->global_psymbols.next,
objfile->static_psymbols.next);

+ pst->dirname = xstrdup ( comp_unit_die.dirname );
+
pst->read_symtab_private = (char *)
obstack_alloc (&objfile->objfile_obstack, sizeof (struct dwarf2_pinfo));
DWARF_INFO_OFFSET (pst) = beg_of_comp_unit - dwarf2_per_objfile->info_buffer;
@@ -4771,6 +4774,10 @@
/* Prefer DW_AT_MIPS_linkage_name over DW_AT_name. */
if (part_die->name == NULL)
part_die->name = DW_STRING (&attr);
+ break;
+ case DW_AT_comp_dir:
+ if (part_die->dirname == NULL)
+ part_die->dirname = DW_STRING (&attr);
break;
case DW_AT_MIPS_linkage_name:
part_die->name = DW_STRING (&attr);
Index: gdb/source.c
===================================================================
RCS file: /cvs/src/src/gdb/source.c,v
retrieving revision 1.50
diff -w -u -r1.50 source.c
--- gdb/source.c 28 Feb 2004 18:04:37 -0000 1.50
+++ gdb/source.c 26 Apr 2004 23:54:54 -0000
@@ -805,30 +805,45 @@
return 1;
}

-
-/* Open a source file given a symtab S. Returns a file descriptor or
- negative number for error. */
-
+/* This function is capable of finding the absolute path to a
+ source file, and opening it, provided you give it an
+ OBJFILE and FILENAME. Both the DIRNAME and FULLNAME are only
+ added suggestions on where to find the file.
+
+ OBJFILE should be the objfile associated with a psymtab or symtab.
+ FILENAME should be the filename to open.
+ DIRNAME is the compilation directory of a particular source file.
+ Only some debug formats provide this info.
+ FULLNAME can be the last known absolute path to the file in question.
+
+ On Success
+ A valid file descriptor is returned. ( the return value is positive )
+ FULLNAME is set to the absolute path to the file just opened.
+
+ On Failure
+ A non valid file descriptor is returned. ( the return value is negitive )
+ FULLNAME is set to NULL. */
int
-open_source_file (struct symtab *s)
+find_and_open_source (struct objfile *objfile,
+ const char *filename,
+ const char *dirname, char **fullname)
{
char *path = source_path;
const char *p;
int result;
- char *fullname;

/* Quick way out if we already know its full name */
- if (s->fullname)
+ if (*fullname)
{
- result = open (s->fullname, OPEN_MODE);
+ result = open (*fullname, OPEN_MODE);
if (result >= 0)
return result;
/* Didn't work -- free old one, try again. */
- xmfree (s->objfile->md, s->fullname);
- s->fullname = NULL;
+ xmfree (objfile->md, *fullname);
+ *fullname = NULL;
}

- if (s->dirname != NULL)
+ if (dirname != NULL)
{
/* Replace a path entry of $cdir with the compilation directory name */
#define cdir_len 5
@@ -841,60 +856,106 @@
int len;

path = (char *)
- alloca (strlen (source_path) + 1 + strlen (s->dirname) + 1);
+ alloca (strlen (source_path) + 1 + strlen (dirname) + 1);
len = p - source_path;
strncpy (path, source_path, len); /* Before $cdir */
- strcpy (path + len, s->dirname); /* new stuff */
+ strcpy (path + len, dirname); /* new stuff */
strcat (path + len, source_path + len + cdir_len); /* After $cdir */
}
}

- result = openp (path, 0, s->filename, OPEN_MODE, 0, &s->fullname);
+ result = openp (path, 0, filename, OPEN_MODE, 0, fullname);
if (result < 0)
{
/* Didn't work. Try using just the basename. */
- p = lbasename (s->filename);
- if (p != s->filename)
- result = openp (path, 0, p, OPEN_MODE, 0, &s->fullname);
+ p = lbasename (filename);
+ if (p != filename)
+ result = openp (path, 0, p, OPEN_MODE, 0, fullname);
}

if (result >= 0)
{
- fullname = s->fullname;
- s->fullname = mstrsave (s->objfile->md, s->fullname);
- xfree (fullname);
+ char *tmp_fullname;
+ tmp_fullname = *fullname;
+ *fullname = mstrsave (objfile->md, *fullname);
+ xfree (tmp_fullname);
}
return result;
}

-/* Return the path to the source file associated with symtab. Returns NULL
- if no symtab. */
+/* Open a source file given a symtab S. Returns a file descriptor or
+ negative number for error.
+
+ This function is a convience function to find_and_open_source. */
+
+int
+open_source_file (struct symtab *s)
+{
+ if (!s)
+ return -1;
+
+ return find_and_open_source (s->objfile, s->filename, s->dirname,
+ &s->fullname);
+}
+
+/* Finds the fullname that a symtab represents.
+
+ If this functions finds the fullname, it will save it in ps->fullname
+ and it will also return the value.

+ If this function fails to find the file that this symtab represents,
+ NULL will be returned and ps->fullname will be set to NULL. */
char *
-symtab_to_filename (struct symtab *s)
+symtab_to_fullname (struct symtab *s)
{
- int fd;
+ int r;

if (!s)
return NULL;

- /* If we've seen the file before, just return fullname. */
+ /* Don't check s->fullname here, the file could have been
+ deleted/moved/..., look for it again */
+ r =
+ find_and_open_source (s->objfile, s->filename, s->dirname, &s->fullname);

- if (s->fullname)
+ if (r)
+ {
+ close (r);
return s->fullname;
+ }

- /* Try opening the file to setup fullname */
+ return NULL;
+}

- fd = open_source_file (s);
- if (fd < 0)
- return s->filename; /* File not found. Just use short name */
+/* Finds the fullname that a partial_symtab represents.

- /* Found the file. Cleanup and return the full name */
+ If this functions finds the fullname, it will save it in ps->fullname
+ and it will also return the value.

- close (fd);
- return s->fullname;
+ If this function fails to find the file that this partial_symtab represents,
+ NULL will be returned and ps->fullname will be set to NULL. */
+char *
+psymtab_to_fullname (struct partial_symtab *ps)
+{
+ int r;
+
+ if (!ps)
+ return NULL;
+
+ /* Don't check ps->fullname here, the file could have been
+ deleted/moved/..., look for it again */
+ r =
+ find_and_open_source (ps->objfile, ps->filename, ps->dirname,
+ &ps->fullname);
+
+ if (r)
+ {
+ close (r);
+ return ps->fullname;
}
\f
+ return NULL;
+}

/* Create and initialize the table S->line_charpos that records
the positions of the lines in the source file, which is assumed
Index: gdb/source.h
===================================================================
RCS file: /cvs/src/src/gdb/source.h,v
retrieving revision 1.4
diff -w -u -r1.4 source.h
--- gdb/source.h 12 Apr 2003 17:41:25 -0000 1.4
+++ gdb/source.h 26 Apr 2004 23:54:54 -0000
@@ -27,6 +27,9 @@
negative number for error. */
extern int open_source_file (struct symtab *s);

+extern char* psymtab_to_fullname ( struct partial_symtab *ps );
+extern char* symtab_to_fullname ( struct symtab *s );
+
/* Create and initialize the table S->line_charpos that records the
positions of the lines in the source file, which is assumed to be
open on descriptor DESC. All set S->nlines to the number of such
Index: gdb/symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.129
diff -w -u -r1.129 symtab.c
--- gdb/symtab.c 8 Apr 2004 21:18:13 -0000 1.129
+++ gdb/symtab.c 26 Apr 2004 23:54:58 -0000
@@ -181,7 +181,7 @@

if (full_path != NULL)
{
- const char *fp = symtab_to_filename (s);
+ const char *fp = symtab_to_fullname (s);
if (FILENAME_CMP (full_path, fp) == 0)
{
return s;
@@ -190,7 +190,7 @@

if (real_path != NULL)
{
- char *rp = gdb_realpath (symtab_to_filename (s));
+ char *rp = gdb_realpath (symtab_to_fullname (s));
make_cleanup (xfree, rp);
if (FILENAME_CMP (real_path, rp) == 0)
{
Index: gdb/symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.90
diff -w -u -r1.90 symtab.h
--- gdb/symtab.h 8 Apr 2004 21:18:13 -0000 1.90
+++ gdb/symtab.h 26 Apr 2004 23:55:01 -0000
@@ -877,6 +877,10 @@

char *fullname;

+ /* Directory in which it was compiled, or NULL if we don't know. */
+
+ char *dirname;
+
/* Information about the object file from which symbols should be read. */

struct objfile *objfile;
Index: gdb/doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.202
diff -w -u -r1.202 gdb.texinfo
--- gdb/doc/gdb.texinfo 28 Mar 2004 12:22:55 -0000 1.202
+++ gdb/doc/gdb.texinfo 26 Apr 2004 23:55:26 -0000
@@ -16805,14 +16805,24 @@

List the source files for the current executable.

+It will always output the filename, but only when GDB can find the absolute
+path to a source file, will it output the fullname.
+
@subsubheading @value{GDBN} Command

There's no @value{GDBN} command which directly corresponds to this one.
@code{gdbtk} has an analogous command @samp{gdb_listfiles}.

@subsubheading Example
-N.A.
-
+@smallexample
+(@value{GDBP})
+-file-list-exec-source-files
+^done,files=[
+@{file=foo.c,fullname=/home/foo.c@},
+@{file=/home/bar.c,fullname=/home/bar.c@},
+@{file=gdb_could_not_find_fullpath.c@}]
+(@value{GDBP})
+@end smallexample

@subheading The @code{-file-list-shared-libraries} Command
@findex -file-list-shared-libraries
Index: gdb/mi/mi-cmd-file.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-file.c,v
retrieving revision 1.1
diff -w -u -r1.1 mi-cmd-file.c
--- gdb/mi/mi-cmd-file.c 2 Apr 2003 22:10:35 -0000 1.1
+++ gdb/mi/mi-cmd-file.c 26 Apr 2004 23:55:26 -0000
@@ -25,6 +25,7 @@
#include "ui-out.h"
#include "symtab.h"
#include "source.h"
+#include "objfiles.h"

/* Return to the client the absolute path and line number of the
current file being executed. */
@@ -39,7 +40,6 @@
if ( !mi_valid_noargs("mi_cmd_file_list_exec_source_file", argc, argv) )
error ("mi_cmd_file_list_exec_source_file: Usage: No args");

-
/* Set the default file and line, also get them */
set_default_source_symtab_and_line();
st = get_current_source_symtab_and_line();
@@ -51,17 +51,68 @@
error ("mi_cmd_file_list_exec_source_file: No symtab");

/* Extract the fullname if it is not known yet */
- if (st.symtab->fullname == NULL)
- symtab_to_filename (st.symtab);
-
- /* We may not be able to open the file (not available). */
- if (st.symtab->fullname == NULL)
- error ("mi_cmd_file_list_exec_source_file: File not found");
+ symtab_to_fullname (st.symtab);

/* Print to the user the line, filename and fullname */
ui_out_field_int (uiout, "line", st.line);
ui_out_field_string (uiout, "file", st.symtab->filename);
+
+ /* We may not be able to open the file (not available). */
+ if (st.symtab->fullname)
ui_out_field_string (uiout, "fullname", st.symtab->fullname);
+
+ return MI_CMD_DONE;
+}
+
+enum mi_cmd_result
+mi_cmd_file_list_exec_source_files (char *command, char **argv, int argc)
+{
+ struct symtab *s;
+ struct partial_symtab *ps;
+ struct objfile *objfile;
+
+ if (!mi_valid_noargs ("mi_cmd_file_list_exec_source_files", argc, argv))
+ error ("mi_cmd_file_list_exec_source_files: Usage: No args");
+
+ /* Print the table header */
+ ui_out_begin (uiout, ui_out_type_list, "files");
+
+ /* Look at all of the symtabs */
+ ALL_SYMTABS (objfile, s)
+ {
+ ui_out_begin (uiout, ui_out_type_tuple, NULL);
+
+ ui_out_field_string (uiout, "file", s->filename);
+
+ /* Extract the fullname if it is not known yet */
+ symtab_to_fullname (s);
+
+ if (s->fullname)
+ ui_out_field_string (uiout, "fullname", s->fullname);
+
+ ui_out_end (uiout, ui_out_type_tuple);
+ }
+
+ /* Look at all of the psymtabs */
+ ALL_PSYMTABS (objfile, ps)
+ {
+ if (!ps->readin)
+ {
+ ui_out_begin (uiout, ui_out_type_tuple, NULL);
+
+ ui_out_field_string (uiout, "file", ps->filename);
+
+ /* Extract the fullname if it is not known yet */
+ psymtab_to_fullname (ps);
+
+ if (ps->fullname)
+ ui_out_field_string (uiout, "fullname", ps->fullname);
+
+ ui_out_end (uiout, ui_out_type_tuple);
+ }
+ }
+
+ ui_out_end (uiout, ui_out_type_list);

return MI_CMD_DONE;
}
Index: gdb/mi/mi-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.c,v
retrieving revision 1.14
diff -w -u -r1.14 mi-cmds.c
--- gdb/mi/mi-cmds.c 4 Aug 2003 23:18:50 -0000 1.14
+++ gdb/mi/mi-cmds.c 26 Apr 2004 23:55:26 -0000
@@ -81,7 +81,7 @@
{ "file-exec-file", { "exec-file", 1 }, NULL, NULL },
{ "file-list-exec-sections", { NULL, 0 }, NULL, NULL },
{ "file-list-exec-source-file", { NULL, 0 }, 0, mi_cmd_file_list_exec_source_file},
- { "file-list-exec-source-files", { NULL, 0 }, NULL, NULL },
+ { "file-list-exec-source-files", { NULL, 0 }, NULL, mi_cmd_file_list_exec_source_files },
{ "file-list-shared-libraries", { NULL, 0 }, NULL, NULL },
{ "file-list-symbol-files", { NULL, 0 }, NULL, NULL },
{ "file-symbol-file", { "symbol-file", 1 }, NULL, NULL },
Index: gdb/mi/mi-cmds.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
retrieving revision 1.12
diff -w -u -r1.12 mi-cmds.h
--- gdb/mi/mi-cmds.h 24 Jan 2004 04:21:43 -0000 1.12
+++ gdb/mi/mi-cmds.h 26 Apr 2004 23:55:26 -0000
@@ -87,6 +87,7 @@
extern mi_cmd_args_ftype mi_cmd_exec_until;
extern mi_cmd_args_ftype mi_cmd_exec_interrupt;
extern mi_cmd_argv_ftype mi_cmd_file_list_exec_source_file;
+extern mi_cmd_argv_ftype mi_cmd_file_list_exec_source_files;
extern mi_cmd_argv_ftype mi_cmd_gdb_exit;
extern mi_cmd_argv_ftype mi_cmd_interpreter_exec;
extern mi_cmd_argv_ftype mi_cmd_stack_info_depth;
Index: gdb/testsuite/gdb.mi/mi2-file.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi2-file.exp,v
retrieving revision 1.1
diff -w -u -r1.1 mi2-file.exp
--- gdb/testsuite/gdb.mi/mi2-file.exp 7 Aug 2003 17:47:42 -0000 1.1
+++ gdb/testsuite/gdb.mi/mi2-file.exp 26 Apr 2004 23:55:27 -0000
@@ -47,7 +47,7 @@
mi_gdb_reinitialize_dir $srcdir/$subdir
mi_gdb_load ${binfile}

-proc test_tbreak_creation_and_listing {} {
+proc test_file_list_exec_source_file {} {
global srcfile
global srcdir
global subdir
@@ -59,7 +59,17 @@
"request path info of current source file (${srcfile})"
}

-test_tbreak_creation_and_listing
+proc test_file_list_exec_source_files {} {
+ global srcfile
+
+ # get the path and absolute path to the current executable
+ mi_gdb_test "222-file-list-exec-source-files" \
+ "222\\\^done,files=\\\[\{file=\".*/${srcfile}\",fullname=\"/.*/${srcfile}\"\},\{file=\".*\"\},\{file=\".*\"\},\{file=\".*\"\},\{file=\".*\"\}\\\]" \
+ "Getting a list of source files."
+}
+
+test_file_list_exec_source_file
+test_file_list_exec_source_files

mi_gdb_exit
return 0