[RFA] patch for DW_AT_comp_dir/DW_AT_name vs .debug_line inconsistencies

Doug Evans
2008-01-05T22:18:41Z

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

Ref: http://sourceware.org/ml/gdb/2008-01/msg00007.html

[Also ref: http://sourceware.org/ml/gdb-patches/2007-11/msg00314.html
but I'm agnostic as to whether gdb should handle #line like this. The
testcases in this patch use #line for convenience only.]

How about this?
Aleksandar, does this work for you? [you'll need to still account for
IS_ABSOLUTE_PATH issues I suspect]

This patch has dwarf_decode_lines prescan the .debug_line info for
files that match DW_AT_name of the main source file, and passes that
to start_subfile instead of what's recorded in .debug_line. This lets
start_subfile get a match with the initial subfile created by
start_symtab.

I took an easy out in scanning for a match, I just pick the first. I
can add the requisite code if folks think this is the way to go.

No regressions in the testsuite (i686-linux).

[-- Attachment #2: gdb-080105-dwarf-decode-lines-1.patch --]
[-- Type: application/octet-stream, Size: 23294 bytes --]

2008-01-05 Doug Evans <dje@google.com>

Handle .debug_line inconsistency with DW_AT_comp_dir/DW_AT_name.
* dwarf2read.c (dwarf2_start_subfile): Delete.
(dwarf_build_subfile_names,dwarf_free_subfile_names): New fns.
(dwarf_decode_lines): New arg cu_file_name, all callers updated.
Build table of subfile names at the start, and then pass the computed
name to start_subfile.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.247
diff -u -p -u -p -r1.247 dwarf2read.c
--- dwarf2read.c 3 Jan 2008 17:29:55 -0000 1.247
+++ dwarf2read.c 5 Jan 2008 22:07:51 -0000
@@ -845,10 +845,13 @@ static struct line_header *(dwarf_decode
(unsigned int offset,
bfd *abfd, struct dwarf2_cu *cu));

-static void dwarf_decode_lines (struct line_header *, char *, bfd *,
- struct dwarf2_cu *, struct partial_symtab *);
+static char **dwarf_build_subfile_names (struct line_header *lh,
+ char *cu_file_name, char *comp_dir);
+
+static void dwarf_free_subfile_names (void *names);

-static void dwarf2_start_subfile (char *, char *, char *);
+static void dwarf_decode_lines (struct line_header *, char *, char *, bfd *,
+ struct dwarf2_cu *, struct partial_symtab *);

static struct symbol *new_symbol (struct die_info *, struct type *,
struct dwarf2_cu *);
@@ -1390,7 +1393,7 @@ dwarf2_build_include_psymtabs (struct dw
if (lh == NULL)
return; /* No linetable, so no includes. */

- dwarf_decode_lines (lh, NULL, abfd, cu, pst);
+ dwarf_decode_lines (lh, NULL, NULL, abfd, cu, pst);

free_line_header (lh);
}
@@ -2858,7 +2861,7 @@ read_file_scope (struct die_info *die, s
{
cu->line_header = line_header;
make_cleanup (free_cu_line_header, cu);
- dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL);
+ dwarf_decode_lines (line_header, name, comp_dir, abfd, cu, NULL);
}
}

@@ -6791,14 +6794,188 @@ check_cu_functions (CORE_ADDR address, s
return fn->lowpc;
}

+/* Subroutine of dwarf_decode_lines to simplify it.
+ Given line_header LH, compute the table of associated path names of files
+ to pass to start_subfile. The resulting table is indexed using the same
+ file number for lh->file_names. Space for the resulting table and strings
+ are allocated separately, and can be freed with free_line_header_files.
+ CU_FILE_NAME is the DW_AT_name entry of the CU.
+ COMP_DIR is the DW_AT_comp_dir entry of the CU.
+
+ Background:
+
+ Each DW_TAG_compile_unit DIE has a pair of attributes DW_AT_name and
+ DW_AT_comp_dir. Also within this DIE is a DW_AT_stmt_list attribute that
+ is used to fetch line number information. See the Dwarf2 or Dwarf3
+ Standards for complete details. For our purposes here we are only
+ interested in file names and their paths. Constructing a complete path
+ name for a file uses up to three pieces of information: the file name as
+ specified in .debug_line, the file's directory as specified in .debug_line,
+ and the compilation directory. The latter is used when directory name
+ + file name yield a relative path name.
+
+ Constructing the full path name of a file is straightforward. We assume
+ the compilation directory and file name are specified. The file's
+ directory may or may not be specified. Typically the file name, as
+ encoded in the .debug_line entry, is just the file name with no path
+ information, but we don't assume that here.
+
+ [in c++ parlance]
+ string
+ build_full_path (string comp_dir, string file_dir, string file_name)
+ {
+ string full_path = file_name;
+ if (! absolute_path_p (full_path))
+ {
+ if (file_dir.length () != 0)
+ full_path = file_dir + "/" + full_path;
+ if (! absolute_path_p (full_path))
+ full_path = comp_dir + "/" + full_path;
+ }
+ return full_path;
+ }
+
+ start_subfile takes care of prepending comp_dir when necessary so we only
+ have to take care of prepending file_dir.
+
+ Constructing path names correctly is important because GDB (effectively)
+ uses strcmp to decide when file paths are equivalent, and we need to make
+ sure a file's line number information gets associated with the correct file.
+
+ Using the `list' example from the GDB testsuite, which resides in
+ /srcdir and compiling it with Irix6.2 cc in /compdir using a filename
+ of /srcdir/list0.c yields the following debugging information for list0.c:
+
+ DW_AT_name: /srcdir/list0.c
+ DW_AT_comp_dir: /compdir
+ files.files[0].name: list0.h
+ files.files[0].dir: /srcdir
+ files.files[1].name: list0.c
+ files.files[1].dir: /srcdir
+
+ The line number information for list0.c has to end up in a single
+ subfile, so that `break /srcdir/list0.c:1' works as expected.
+ start_subfile will ensure that this happens provided that we pass the
+ concatenation of files.files[1].dir and files.files[1].name as the
+ subfile's name.
+
+ Up until now everything has been straightforward.
+ Here's the part that requires extra effort:
+
+ For the case of the main source file's line number information,
+ what if build_full_path (DW_AT_comp_dir, "", DW_AT_name)
+ != build_full_path (DW_AT_comp_dir, file_dir, file_name) ?
+
+ An example is:
+ DW_AT_comp_dir = /test/src/obj
+ DW_AT_name = /test/src/main.cc
+ file_dir = ..
+ file_name = main.cc
+
+ start_symtab will first call start_subfile to create a psymtab for
+ /test/src/main.cc. Later .debug_line info will be read and the line number
+ info for main.cc will be for file /test/src/obj/../main.cc. start_subfile
+ will decide this is a different file than /test/src/main.cc and GDB will
+ lose track of the line number information for this file. This will cause
+ "break main.cc:10" to get the error "No line 10 in main.cc."
+
+ One potential solution to this problem is to have start_subfile canonicalize
+ path names before comparing them. I.e., convert /test/src/obj/../main.cc
+ to /test/src/main.cc and then do the comparison. However this doesn't
+ necessarily work if obj is a symlink.
+
+ The solution chosen here is to watch for file names in .debug_line info
+ that might match DW_AT_name. If it does match then we record DW_AT_name
+ as the value to pass to start_subfile thus ensuring a match.
+ This could be done by checking right before each call to start_subfile.
+ However, this can't handle the case where there is another file that
+ matches basename (DW_AT_name) but is not the same file, we need to examine
+ all the files first and pick the best match.
+ Whether it is worth it to handle this case is debatable, but preprocessing
+ the file names allows us to catch this case.
+ The Dwarf Standard makes no guarantees of the order of files in .debug_line
+ so we can't, for example, just pick the first one.
+*/
+
+static char **
+dwarf_build_subfile_names (struct line_header *lh,
+ char *cu_file_name, char *comp_dir)
+{
+ char **names = (char **) xmalloc ((lh->num_file_names + 1) * sizeof (char*));
+ const char *cu_file_basename = lbasename (cu_file_name);
+ /* Set to 1 when a match to FILE_NAME is found in LH. */
+ int match_found = 0;
+ int i;
+
+ for (i = 0; i < lh->num_file_names; ++i)
+ {
+ struct file_entry *fe = &lh->file_names[i];
+ char *fname = fe->name;
+ const char *bname = lbasename (fname);
+
+ /* ??? For now we just pick the first file in LH that matches
+ FILE_NAME. */
+
+ if (! match_found && strcmp (bname, cu_file_basename) == 0)
+ {
+ names[i] = xstrdup (cu_file_name);
+ match_found = 1;
+ }
+ else
+ {
+ char *dir = NULL;
+
+ /* A directory index of 0 means use comp_dir.
+ Otherwise it is an origin-1 based index into INCLUDE_DIRS.
+ start_subfile will prepend comp_dir so we don't need to do that
+ here (and if both file and comp_dir are relative paths we
+ mustn't!). */
+ if (fe->dir_index)
+ dir = lh->include_dirs[fe->dir_index - 1];
+
+ /* Note that the Dwarf3 standard says (speaking of filenames in line
+ information): ``The directory index is ignored for file names
+ that represent full path names''. Thus ignoring dirname in the
+ `else' branch below isn't an issue. */
+
+ if (!IS_ABSOLUTE_PATH (fname) && dir != NULL)
+ names[i] = concat (dir, SLASH_STRING, fname, NULL);
+ else
+ names[i] = xstrdup (fname);
+ }
+ }
+
+ names[lh->num_file_names] = NULL;
+
+ return names;
+}
+
+/* Utility to free the result of dwarf_build_subfile_names.
+ VNAMES is a void* because this function is an argument to make_cleanup.
+ ??? We could just arrange to use free_argv. Too much of a hack? */
+
+static void
+dwarf_free_subfile_names (void *vnames)
+{
+ int i;
+ char **names = (char **) vnames;
+
+ for (i = 0; names[i] != NULL; ++i)
+ xfree (names[i]);
+ xfree (names);
+}
+
/* Decode the Line Number Program (LNP) for the given line_header
structure and CU. The actual information extracted and the type
of structures created from the LNP depends on the value of PST.

1. If PST is NULL, then this procedure uses the data from the program
to create all necessary symbol tables, and their linetables.
- The compilation directory of the file is passed in COMP_DIR,
- and must not be NULL.
+ The name of the file (DW_AT_name) is passed in CU_FILE_NAME,
+ and must not be NULL. It is used to help resolve ambiguities in
+ the .decode_line info.
+ The compilation directory of the file (DW_AT_comp_dir) is passed
+ in COMP_DIR, and must not be NULL.

2. If PST is not NULL, this procedure reads the program to determine
the list of files included by the unit represented by PST, and
@@ -6807,11 +6984,16 @@ check_cu_functions (CORE_ADDR address, s
is not used to compute the full name of the symtab, and therefore
omitting it when building the partial symtab does not introduce
the potential for inconsistency - a partial symtab and its associated
- symbtab having a different fullname -). */
+ symbtab having a different fullname). CU_FILE_NAME is not used in this
+ case either.
+
+ NOTE: lh->include_dirs and lh->file_names are 0-based, but the
+ directory and file name numbers in the statement program are 1-based. */

static void
-dwarf_decode_lines (struct line_header *lh, char *comp_dir, bfd *abfd,
- struct dwarf2_cu *cu, struct partial_symtab *pst)
+dwarf_decode_lines (struct line_header *lh, char *cu_file_name, char *comp_dir,
+ bfd *abfd, struct dwarf2_cu *cu,
+ struct partial_symtab *pst)
{
gdb_byte *line_ptr, *extended_end;
gdb_byte *line_end;
@@ -6821,6 +7003,14 @@ dwarf_decode_lines (struct line_header *
struct objfile *objfile = cu->objfile;
const int decode_for_pst_p = (pst != NULL);
struct subfile *last_subfile = NULL, *first_subfile = current_subfile;
+ char **subfile_names = NULL;
+
+ /* Build table of names to pass to start_subfile. */
+ if (! decode_for_pst_p)
+ {
+ subfile_names = dwarf_build_subfile_names (lh, cu_file_name, comp_dir);
+ make_cleanup (dwarf_free_subfile_names, subfile_names);
+ }

baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));

@@ -6842,16 +7032,9 @@ dwarf_decode_lines (struct line_header *
if (!decode_for_pst_p && lh->num_file_names >= file)
{
/* Start a subfile for the current file of the state machine. */
- /* lh->include_dirs and lh->file_names are 0-based, but the
- directory and file name numbers in the statement program
- are 1-based. */
- struct file_entry *fe = &lh->file_names[file - 1];
- char *dir = NULL;
-
- if (fe->dir_index)
- dir = lh->include_dirs[fe->dir_index - 1];
-
- dwarf2_start_subfile (fe->name, dir, comp_dir);
+ /* File numbers in the statement program are 1-based. */
+ char *fname = subfile_names[file - 1];
+ start_subfile (fname, comp_dir);
}

/* Decode the table. */
@@ -6979,27 +7162,17 @@ dwarf_decode_lines (struct line_header *
break;
case DW_LNS_set_file:
{
- /* The arrays lh->include_dirs and lh->file_names are
- 0-based, but the directory and file name numbers in
- the statement program are 1-based. */
- struct file_entry *fe;
- char *dir = NULL;
-
file = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
line_ptr += bytes_read;
if (lh->num_file_names < file)
dwarf2_debug_line_missing_file_complaint ();
- else
- {
- fe = &lh->file_names[file - 1];
- if (fe->dir_index)
- dir = lh->include_dirs[fe->dir_index - 1];
- if (!decode_for_pst_p)
- {
- last_subfile = current_subfile;
- dwarf2_start_subfile (fe->name, dir, comp_dir);
- }
- }
+ else if (!decode_for_pst_p)
+ {
+ /* File numbers in the statement program are 1-based. */
+ char *fname = subfile_names[file - 1];
+ last_subfile = current_subfile;
+ start_subfile (fname, comp_dir);
+ }
}
break;
case DW_LNS_set_column:
@@ -7082,15 +7255,12 @@ dwarf_decode_lines (struct line_header *
line numbers). */

int i;
- struct file_entry *fe;

for (i = 0; i < lh->num_file_names; i++)
{
- char *dir = NULL;
- fe = &lh->file_names[i];
- if (fe->dir_index)
- dir = lh->include_dirs[fe->dir_index - 1];
- dwarf2_start_subfile (fe->name, dir, comp_dir);
+ struct file_entry *fe = &lh->file_names[i];
+
+ start_subfile (subfile_names[i], comp_dir);

/* Skip the main file; we don't need it, and it must be
allocated last, so that it will show up before the
@@ -7106,56 +7276,6 @@ dwarf_decode_lines (struct line_header *
}
}

-/* Start a subfile for DWARF. FILENAME is the name of the file and
- DIRNAME the name of the source directory which contains FILENAME
- or NULL if not known. COMP_DIR is the compilation directory for the
- linetable's compilation unit or NULL if not known.
- This routine tries to keep line numbers from identical absolute and
- relative file names in a common subfile.
-
- Using the `list' example from the GDB testsuite, which resides in
- /srcdir and compiling it with Irix6.2 cc in /compdir using a filename
- of /srcdir/list0.c yields the following debugging information for list0.c:
-
- DW_AT_name: /srcdir/list0.c
- DW_AT_comp_dir: /compdir
- files.files[0].name: list0.h
- files.files[0].dir: /srcdir
- files.files[1].name: list0.c
- files.files[1].dir: /srcdir
-
- The line number information for list0.c has to end up in a single
- subfile, so that `break /srcdir/list0.c:1' works as expected.
- start_subfile will ensure that this happens provided that we pass the
- concatenation of files.files[1].dir and files.files[1].name as the
- subfile's name. */
-
-static void
-dwarf2_start_subfile (char *filename, char *dirname, char *comp_dir)
-{
- char *fullname;
-
- /* While reading the DIEs, we call start_symtab(DW_AT_name, DW_AT_comp_dir).
- `start_symtab' will always pass the contents of DW_AT_comp_dir as
- second argument to start_subfile. To be consistent, we do the
- same here. In order not to lose the line information directory,
- we concatenate it to the filename when it makes sense.
- Note that the Dwarf3 standard says (speaking of filenames in line
- information): ``The directory index is ignored for file names
- that represent full path names''. Thus ignoring dirname in the
- `else' branch below isn't an issue. */
-
- if (!IS_ABSOLUTE_PATH (filename) && dirname != NULL)
- fullname = concat (dirname, SLASH_STRING, filename, (char *)NULL);
- else
- fullname = filename;
-
- start_subfile (fullname, comp_dir);
-
- if (fullname != filename)
- xfree (fullname);
-}
-
static void
var_decode_location (struct attribute *attr, struct symbol *sym,
struct dwarf2_cu *cu)
Index: testsuite/gdb.base/hashline1.exp
===================================================================
RCS file: testsuite/gdb.base/hashline1.exp
diff -N testsuite/gdb.base/hashline1.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/hashline1.exp 5 Jan 2008 22:07:51 -0000
@@ -0,0 +1,60 @@
+# Copyright 2007 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.
+
+# Test loading of line number information with absolute path in #line, bug 2360.
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+set prms_id 2360
+set bug_id 0
+
+# srcfile is in objdir because we need to machine generate it in order
+# to get correct the correct path in the #line directive.
+
+set testfile "hashline1"
+set srcfile "${testfile}.c"
+set binfile "${objdir}/${subdir}/${testfile}"
+
+set fd [open ${objdir}/${subdir}/${srcfile} w]
+puts $fd "#line 2 \"[pwd]/${subdir}/${srcfile}\""
+puts $fd "int main () { return 0; } /* set breakpoint here */"
+close $fd
+
+# The choice of path name for the source file is important in order to
+# trigger the bug. Using ${objdir}/${subdir}/${srcfile} here won't trigger
+# the bug.
+if { [gdb_compile "./${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested hashline1.exp
+ return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "set breakpoint here" ${objdir}/${subdir}/${srcfile}]
+
+# Try to set a breakpoint on the specified file location.
+
+#gdb_test "set substitute-path [pwd]/. [pwd]" "" ""
+
+gdb_test "break $srcfile:$bp_location" \
+ "Breakpoint.*at.* file .*$srcfile, line.*" \
+ "set breakpoint"
Index: testsuite/gdb.base/hashline2.exp
===================================================================
RCS file: testsuite/gdb.base/hashline2.exp
diff -N testsuite/gdb.base/hashline2.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/hashline2.exp 5 Jan 2008 22:07:51 -0000
@@ -0,0 +1,58 @@
+# Copyright 2007 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.
+
+# Test loading of line number information with an absolute path with extra
+# /'s in #line, bug 2360.
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+set prms_id 2360
+set bug_id 0
+
+# srcfile is in objdir because we need to machine generate it in order
+# to get correct the correct path in the #line directive.
+
+set testfile "hashline2"
+set srcfile "${testfile}.c"
+set binfile "${objdir}/${subdir}/${testfile}"
+
+set fd [open ${objdir}/${subdir}/${srcfile} w]
+puts $fd "#line 2 \"///[pwd]/${subdir}/${srcfile}\""
+puts $fd "int main () { return 0; } /* set breakpoint here */"
+close $fd
+
+if { [gdb_compile "${objdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested hashline1.exp
+ return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "set breakpoint here" ${objdir}/${subdir}/${srcfile}]
+
+# Try to set a breakpoint on the specified file location.
+
+#gdb_test "set substitute-path ///[pwd] [pwd]" "" ""
+
+gdb_test "break $srcfile:$bp_location" \
+ "Breakpoint.*at.* file .*$srcfile, line.*" \
+ "set breakpoint"
Index: testsuite/gdb.base/hashline3.exp
===================================================================
RCS file: testsuite/gdb.base/hashline3.exp
diff -N testsuite/gdb.base/hashline3.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/hashline3.exp 5 Jan 2008 22:07:51 -0000
@@ -0,0 +1,57 @@
+# Copyright 2007 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.
+
+# Test loading of line number information with relative path in #line, bug 2360.
+
+if $tracelevel then {
+ strace $tracelevel
+}
+
+set prms_id 2360
+set bug_id 0
+
+# srcfile is in objdir because we need to machine generate it in order
+# to get correct the correct path in the #line directive.
+
+set testfile "hashline3"
+set srcfile "${testfile}.c"
+set binfile "${objdir}/${subdir}/${testfile}"
+
+set fd [open ${objdir}/${subdir}/${srcfile} w]
+puts $fd "#line 2 \"./${subdir}/${srcfile}\""
+puts $fd "int main () { return 0; } /* set breakpoint here */"
+close $fd
+
+if { [gdb_compile "${objdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+ untested hashline1.exp
+ return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "set breakpoint here" ${objdir}/${subdir}/${srcfile}]
+
+# Try to set a breakpoint on the specified file location.
+
+#gdb_test "set substitute-path ${objdir}/. ${objdir}" "" ""