Patch: Handle relative paths in .debug_line

Patch: Handle relative paths in .debug_line

[Bryce McKinlay]

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

When debugging mainline gcc, GDB has a problem locating source for files 
which are only referenced from .debug_line. For example: debugging gcj, 
and trying to put a breakpoint on the parse.y file, I get the following:

Breakpoint 1, lookup_method_invoke (lc=1, cl=0xf6dee5f0, class=0xf6def244,
    name=0xf6dee550, arg_list=0xf6e574b0) at parse.y:10956
10956   parse.y: No such file or directory.
        in parse.y
(gdb) info source
Current source file is parse.y
Compilation directory is ../../gcc/java
Source language is c.
Compiled with unknown debugging format.
Does not include preprocessor macro info.

This is because the contents of the directory table in .debug_line can 
be paths relative to the DW_AT_comp_dir:


 The Directory Table:
  ../../gcc/java
  java
  /usr/include/bits
  .
  ../../gcc
  ../../gcc/../libcpp/include
  ../../gcc/config/i386
  ../../gcc/../include
  /local/gcc-clean/lib/gcc/i686-pc-linux-gnu/3.5.0/include
  /usr/include

 The File Name Table:
  Entry    Dir    Time    Size    Name
  1    1    0    0    keyword.h
  2    1    0    0    lex.c
  3    2    0    0    parse.c
  4    1    0    0    parse.y
... etc...

     DW_AT_comp_dir    : (indirect string, offset: 0x658c): 
/home/mckinlay/cvs/gcc-really-clean/build/gcc   

This patch fixes the problem by appending the path from the directory 
table, if it is not absolute already, to the DW_AT_comp_dir path when 
creating subfiles from the .debug_line info.

OK to commit?

Bryce



[-- Attachment #2: gdb-subfile-compdir.patch --]
[-- Type: text/x-patch, Size: 4315 bytes --]

2004-07-22  Bryce McKinlay  <mckinlay@redhat.com>

	* dwarf2read.c (dwarf_decode_lines): Pass comp_dir to
	dwarf2_start_subfile.
	(dwarf2_start_subfile): New comp_dir parameter.	Handle relative paths
	in .debug_line by appending them to comp_dir. Use make_cleanup to
	free strings used in directory concatenation.
	
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.156
diff -u -r1.156 dwarf2read.c
--- dwarf2read.c	6 Jul 2004 19:29:30 -0000	1.156
+++ dwarf2read.c	22 Jul 2004 23:07:41 -0000
@@ -762,7 +762,7 @@
 static void dwarf_decode_lines (struct line_header *, char *, bfd *,
 				struct dwarf2_cu *, struct partial_symtab *);
 
-static void dwarf2_start_subfile (char *, char *);
+static void dwarf2_start_subfile (char *, char *, char *);
 
 static struct symbol *new_symbol (struct die_info *, struct type *,
 				  struct dwarf2_cu *);
@@ -5951,12 +5951,10 @@
 	     directory and file name numbers in the statement program
 	     are 1-based.  */
           struct file_entry *fe = &lh->file_names[file - 1];
-          char *dir;
+          char *dir = NULL;
           if (fe->dir_index)
             dir = lh->include_dirs[fe->dir_index - 1];
-          else
-            dir = comp_dir;
-	  dwarf2_start_subfile (fe->name, dir);
+	  dwarf2_start_subfile (fe->name, dir, comp_dir);
 	}
 
       /* Decode the table. */
@@ -6044,17 +6042,15 @@
                    but the directory and file name numbers in the
                    statement program are 1-based.  */
                 struct file_entry *fe;
-                char *dir;
+                char *dir = NULL;
                 file = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
                 line_ptr += bytes_read;
                 fe = &lh->file_names[file - 1];
                 fe->included_p = 1;
                 if (fe->dir_index)
                   dir = lh->include_dirs[fe->dir_index - 1];
-                else
-                  dir = comp_dir;
                 if (!decode_for_pst_p)
-                  dwarf2_start_subfile (fe->name, dir);
+                  dwarf2_start_subfile (fe->name, dir, comp_dir);
               }
 	      break;
 	    case DW_LNS_set_column:
@@ -6112,7 +6108,8 @@
 
 /* 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.
+   or NULL if not known.  COMP_DIR is the value of DW_AT_comp_dir.  If
+   DIRNAME specifies a relative path, it is appended to COMP_DIR.
    This routine tries to keep line numbers from identical absolute and
    relative file names in a common subfile.
 
@@ -6131,28 +6128,44 @@
    subfile, so that `break /srcdir/list0.c:1' works as expected.  */
 
 static void
-dwarf2_start_subfile (char *filename, char *dirname)
+dwarf2_start_subfile (char *filename, char *dirname, char *comp_dir)
 {
+  char *fullname;
+  struct subfile *subfile;
+  struct cleanup *back_to = make_cleanup (null_cleanup, 0);
+
+  /* If we have a relative dirname, append comp_dir to it.  */
+  if (dirname != NULL && !IS_ABSOLUTE_PATH (dirname))
+    {
+      dirname = concat (comp_dir, "/", dirname, NULL);
+      make_cleanup (xfree, dirname);
+    }
+  else if (dirname == NULL)
+    dirname = comp_dir;
+
   /* If the filename isn't absolute, try to match an existing subfile
      with the full pathname.  */
 
-  if (!IS_ABSOLUTE_PATH (filename) && dirname != NULL)
+  if (!IS_ABSOLUTE_PATH (filename))
     {
-      struct subfile *subfile;
-      char *fullname = concat (dirname, "/", filename, NULL);
+      fullname = concat (dirname, "/", filename, NULL);
+      make_cleanup (xfree, fullname);
+    }
+  else
+    fullname = filename;
 
-      for (subfile = subfiles; subfile; subfile = subfile->next)
+  for (subfile = subfiles; subfile; subfile = subfile->next)
+    {
+      if (FILENAME_CMP (subfile->name, fullname) == 0)
 	{
-	  if (FILENAME_CMP (subfile->name, fullname) == 0)
-	    {
-	      current_subfile = subfile;
-	      xfree (fullname);
-	      return;
-	    }
+	  current_subfile = subfile;
+	  do_cleanups (back_to);
+	  return;
 	}
-      xfree (fullname);
     }
+
   start_subfile (filename, dirname);
+  do_cleanups (back_to);
 }
 
 static void

Re: Patch: Handle relative paths in .debug_line

[Jim Blandy]

Thanks for this patch!

In the revised dwarf2_start_subfile:

- It looks to me as if the new code doesn't handle the case where both
  dirname and comp_dir are null.

- It seems to me that, when fullname == filename, the loop over the
  subfiles duplicates the loop at the top of start_subfile.  So that
  loop should remain conditional.  Its comment should be moved and
  adjusted, too.

Re: Patch: Handle relative paths in .debug_line

[Bryce McKinlay]

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

Jim Blandy wrote:

>Thanks for this patch!
>
>In the revised dwarf2_start_subfile:
>
>- It looks to me as if the new code doesn't handle the case where both
>  dirname and comp_dir are null.
>
>- It seems to me that, when fullname == filename, the loop over the
>  subfiles duplicates the loop at the top of start_subfile.  So that
>  loop should remain conditional.  Its comment should be moved and
>  adjusted, too.
>
>  
>
Here's a revised patch that should address this. OK to commit?

Regards

Bryce



[-- Attachment #2: gdb-subfile-compdir-2.patch --]
[-- Type: text/x-patch, Size: 3618 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.156
diff -u -r1.156 dwarf2read.c
--- dwarf2read.c	6 Jul 2004 19:29:30 -0000	1.156
+++ dwarf2read.c	23 Jul 2004 23:54:07 -0000
@@ -762,7 +762,7 @@
 static void dwarf_decode_lines (struct line_header *, char *, bfd *,
 				struct dwarf2_cu *, struct partial_symtab *);
 
-static void dwarf2_start_subfile (char *, char *);
+static void dwarf2_start_subfile (char *, char *, char *);
 
 static struct symbol *new_symbol (struct die_info *, struct type *,
 				  struct dwarf2_cu *);
@@ -5951,12 +5951,10 @@
 	     directory and file name numbers in the statement program
 	     are 1-based.  */
           struct file_entry *fe = &lh->file_names[file - 1];
-          char *dir;
+          char *dir = NULL;
           if (fe->dir_index)
             dir = lh->include_dirs[fe->dir_index - 1];
-          else
-            dir = comp_dir;
-	  dwarf2_start_subfile (fe->name, dir);
+	  dwarf2_start_subfile (fe->name, dir, comp_dir);
 	}
 
       /* Decode the table. */
@@ -6044,17 +6042,15 @@
                    but the directory and file name numbers in the
                    statement program are 1-based.  */
                 struct file_entry *fe;
-                char *dir;
+                char *dir = NULL;
                 file = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
                 line_ptr += bytes_read;
                 fe = &lh->file_names[file - 1];
                 fe->included_p = 1;
                 if (fe->dir_index)
                   dir = lh->include_dirs[fe->dir_index - 1];
-                else
-                  dir = comp_dir;
                 if (!decode_for_pst_p)
-                  dwarf2_start_subfile (fe->name, dir);
+                  dwarf2_start_subfile (fe->name, dir, comp_dir);
               }
 	      break;
 	    case DW_LNS_set_column:
@@ -6112,7 +6108,8 @@
 
 /* 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.
+   or NULL if not known.  COMP_DIR is the value of DW_AT_comp_dir.  If
+   DIRNAME specifies a relative path, it is appended to COMP_DIR.
    This routine tries to keep line numbers from identical absolute and
    relative file names in a common subfile.
 
@@ -6131,8 +6128,19 @@
    subfile, so that `break /srcdir/list0.c:1' works as expected.  */
 
 static void
-dwarf2_start_subfile (char *filename, char *dirname)
+dwarf2_start_subfile (char *filename, char *dirname, char *comp_dir)
 {
+  struct cleanup *back_to = make_cleanup (null_cleanup, 0);
+
+  /* If we have a relative dirname, append it to comp_dir.  */
+  if (dirname != NULL && !IS_ABSOLUTE_PATH (dirname))
+    {
+      dirname = concat (comp_dir, "/", dirname, NULL);
+      make_cleanup (xfree, dirname);
+    }
+  else if (dirname == NULL)
+    dirname = comp_dir;
+
   /* If the filename isn't absolute, try to match an existing subfile
      with the full pathname.  */
 
@@ -6140,19 +6148,20 @@
     {
       struct subfile *subfile;
       char *fullname = concat (dirname, "/", filename, NULL);
+      make_cleanup (xfree, fullname);
 
       for (subfile = subfiles; subfile; subfile = subfile->next)
 	{
 	  if (FILENAME_CMP (subfile->name, fullname) == 0)
 	    {
 	      current_subfile = subfile;
-	      xfree (fullname);
+	      do_cleanups (back_to);
 	      return;
 	    }
 	}
-      xfree (fullname);
     }
   start_subfile (filename, dirname);
+  do_cleanups (back_to);
 }
 
 static void

Re: Patch: Handle relative paths in .debug_line

[Jim Blandy]

Bryce McKinlay <mckinlay@redhat.com> writes:

> Jim Blandy wrote:
> 
> >Thanks for this patch!
> >
> >In the revised dwarf2_start_subfile:
> >
> >- It looks to me as if the new code doesn't handle the case where both
> >  dirname and comp_dir are null.
> >
> >- It seems to me that, when fullname == filename, the loop over the
> >  subfiles duplicates the loop at the top of start_subfile.  So that
> >  loop should remain conditional.  Its comment should be moved and
> >  adjusted, too.
> >
> >
> Here's a revised patch that should address this. OK to commit?

Almost --- dwarf2_start_subfile still passes comp_dir to concat
without checking whether it's NULL.

Re: Patch: Handle relative paths in .debug_line

[Bryce McKinlay]

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

Jim Blandy wrote:

>Bryce McKinlay <mckinlay@redhat.com> writes:
>
>  
>
>>Jim Blandy wrote:
>>
>>    
>>
>>>Thanks for this patch!
>>>
>>>In the revised dwarf2_start_subfile:
>>>
>>>- It looks to me as if the new code doesn't handle the case where both
>>> dirname and comp_dir are null.
>>>
>>>- It seems to me that, when fullname == filename, the loop over the
>>> subfiles duplicates the loop at the top of start_subfile.  So that
>>> loop should remain conditional.  Its comment should be moved and
>>> adjusted, too.
>>>
>>>
>>>      
>>>
>>Here's a revised patch that should address this. OK to commit?
>>    
>>
>
>Almost --- dwarf2_start_subfile still passes comp_dir to concat
>without checking whether it's NULL.
>  
>
Fixed in the following patch. OK?

Bryce


[-- Attachment #2: gdb-subfile-compdir-3.patch --]
[-- Type: text/x-patch, Size: 3953 bytes --]

2004-07-29  Bryce McKinlay  <mckinlay@redhat.com>

	* dwarf2read.c (dwarf_decode_lines): Pass comp_dir to
	dwarf2_start_subfile.
	(dwarf2_start_subfile): New comp_dir parameter.	Handle relative paths
	in .debug_line by appending them to comp_dir. Use make_cleanup to
	free strings used in directory concatenation.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.156
diff -u -r1.156 dwarf2read.c
--- dwarf2read.c	6 Jul 2004 19:29:30 -0000	1.156
+++ dwarf2read.c	29 Jul 2004 20:52:10 -0000
@@ -762,7 +762,7 @@
 static void dwarf_decode_lines (struct line_header *, char *, bfd *,
 				struct dwarf2_cu *, struct partial_symtab *);
 
-static void dwarf2_start_subfile (char *, char *);
+static void dwarf2_start_subfile (char *, char *, char *);
 
 static struct symbol *new_symbol (struct die_info *, struct type *,
 				  struct dwarf2_cu *);
@@ -5951,12 +5951,10 @@
 	     directory and file name numbers in the statement program
 	     are 1-based.  */
           struct file_entry *fe = &lh->file_names[file - 1];
-          char *dir;
+          char *dir = NULL;
           if (fe->dir_index)
             dir = lh->include_dirs[fe->dir_index - 1];
-          else
-            dir = comp_dir;
-	  dwarf2_start_subfile (fe->name, dir);
+	  dwarf2_start_subfile (fe->name, dir, comp_dir);
 	}
 
       /* Decode the table. */
@@ -6044,17 +6042,15 @@
                    but the directory and file name numbers in the
                    statement program are 1-based.  */
                 struct file_entry *fe;
-                char *dir;
+                char *dir = NULL;
                 file = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
                 line_ptr += bytes_read;
                 fe = &lh->file_names[file - 1];
                 fe->included_p = 1;
                 if (fe->dir_index)
                   dir = lh->include_dirs[fe->dir_index - 1];
-                else
-                  dir = comp_dir;
                 if (!decode_for_pst_p)
-                  dwarf2_start_subfile (fe->name, dir);
+                  dwarf2_start_subfile (fe->name, dir, comp_dir);
               }
 	      break;
 	    case DW_LNS_set_column:
@@ -6112,7 +6108,8 @@
 
 /* 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.
+   or NULL if not known.  COMP_DIR is the value of DW_AT_comp_dir.  If
+   DIRNAME specifies a relative path, it is appended to COMP_DIR.
    This routine tries to keep line numbers from identical absolute and
    relative file names in a common subfile.
 
@@ -6131,8 +6128,19 @@
    subfile, so that `break /srcdir/list0.c:1' works as expected.  */
 
 static void
-dwarf2_start_subfile (char *filename, char *dirname)
+dwarf2_start_subfile (char *filename, char *dirname, char *comp_dir)
 {
+  struct cleanup *back_to = make_cleanup (null_cleanup, 0);
+
+  /* If we have a relative dirname, append it to comp_dir.  */
+  if (comp_dir != NULL && dirname != NULL && !IS_ABSOLUTE_PATH (dirname))
+    {
+      dirname = concat (comp_dir, "/", dirname, NULL);
+      make_cleanup (xfree, dirname);
+    }
+  else if (dirname == NULL)
+    dirname = comp_dir;
+
   /* If the filename isn't absolute, try to match an existing subfile
      with the full pathname.  */
 
@@ -6140,19 +6148,20 @@
     {
       struct subfile *subfile;
       char *fullname = concat (dirname, "/", filename, NULL);
+      make_cleanup (xfree, fullname);
 
       for (subfile = subfiles; subfile; subfile = subfile->next)
 	{
 	  if (FILENAME_CMP (subfile->name, fullname) == 0)
 	    {
 	      current_subfile = subfile;
-	      xfree (fullname);
+	      do_cleanups (back_to);
 	      return;
 	    }
 	}
-      xfree (fullname);
     }
   start_subfile (filename, dirname);
+  do_cleanups (back_to);
 }
 
 static void

Re: Patch: Handle relative paths in .debug_line

[Jim Blandy]

Bryce McKinlay <mckinlay@redhat.com> writes:
> Jim Blandy wrote:
> >Almost --- dwarf2_start_subfile still passes comp_dir to concat
> >without checking whether it's NULL.
> >
> Fixed in the following patch. OK?

Looks better!

You've done before-and-after runs of the test suite, right?  If there
are no regressions there, please feel free to commit. 

Are there any tests currently in the test suite that catch this bug?

Re: Patch: Handle relative paths in .debug_line

[Bryce McKinlay]

Jim Blandy wrote:

>Looks better!
>
>You've done before-and-after runs of the test suite, right?  If there
>are no regressions there, please feel free to commit. 
>  
>

I get an error when I try to run the test suite:

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file 
for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for 
target.
Using ../../../gdb/testsuite/config/unix.exp as tool-and-target-specific 
interface file.
Running ../../../gdb/testsuite/gdb.ada/null_record.exp ...
Running ../../../gdb/testsuite/gdb.arch/altivec-abi.exp ...
Running ../../../gdb/testsuite/gdb.arch/altivec-regs.exp ...
Running ../../../gdb/testsuite/gdb.arch/e500-abi.exp ...
Running ../../../gdb/testsuite/gdb.arch/e500-prologue.exp ...
Running ../../../gdb/testsuite/gdb.arch/e500-regs.exp ...
Running ../../../gdb/testsuite/gdb.arch/gdb1291.exp ...
Running ../../../gdb/testsuite/gdb.arch/gdb1431.exp ...
Running ../../../gdb/testsuite/gdb.arch/gdb1558.exp ...
Running ../../../gdb/testsuite/gdb.arch/i386-prologue.exp ...
ERROR: (DejaGnu) proc "setup_kfail *-*-* gdb/1718" does not exist.
The error code is NONE
The info on the error is:
close: spawn id exp6 not open
    while executing
"close -i exp6"
    invoked from within
"catch "close -i $spawn_id""

                === gdb Summary ===

# of expected passes            5
# of unsupported tests          1
/local/cvs/src/build/gdb/testsuite/../../gdb/gdb version  2004-07-31-cvs -nx


This is on i686 FC2, dejagnu-1.4.2-11.  Any tricks I should know about?

Regards

Bryce

Re: Patch: Handle relative paths in .debug_line

[Andrew Cagney]

> ERROR: (DejaGnu) proc "setup_kfail *-*-* gdb/1718" does not exist.
> The error code is NONE
> The info on the error is:
> close: spawn id exp6 not open
>    while executing
> "close -i exp6"
>    invoked from within
> "catch "close -i $spawn_id""

Your dejagnu is too old :-(

You can fudge things by checking out / building src/dejagnu (`cd src && 
cvs update -d dejagnu`), just keep in mind that src/dejagnu is 
technically past its removal date :-)

Andrew

Re: Patch: Handle relative paths in .debug_line

[Michael Chastain]

Andrew Cagney <cagney@gnu.org> wrote:
> Your dejagnu is too old :-(

That is the problem all right.  I need to write some more doco.
In the interim, here is a brain dump for Bryce.

You need three packages to test Gdb: TCL, Expect, and DejaGnu.  TCL
provides the TCL library.  Expect provides the 'expect' executable,
which is linked with the TCL library.  And DejaGnu provides the
'runtest' shell script, which needs the 'expect' executable.

You can find the version of these packages by running:

  % runtest --version
  WARNING: Couldn't find the global config file.
  Expect version is       5.41.0
  Tcl version is          8.4
  Framework version is    1.4.4

The 'framework version' is the DejaGnu version.
The TCL version is misreported, I've got TCL 8.4.6,
but 'runtest --version' is reporting only two digits of it.

To test Gdb, you need these versions: TCL 8.4.1 or later, 
Expect 5.27 or later, and DejaGnu 1.4.3 or later.
In particular, if you get an error on proc setup_kfail,
that means your Dejagnu is too old.

You can download these packages from:

  TCL	   ftp://ftp2.sourceforge.net/pub/sourceforge/t/tc/tcl/tcl8.4.6-src.tar.gz
  Expect   http://expect.nist.gov/src/expect-5.41.0.tar.gz
  DejaGnu  ftp://ftp.gnu.org/pub/gnu/dejagnu/dejagnu-1.4.4.tar.gz

To build TCL:

  Unpack the source.
  Make build and install directories.
  cd $dir_build
  $dir_source/unix/configure --prefix="$dir_install" --enable-gcc --disable-shared
  make all
  make install
  cd $dir_install/bin
  ln -s tclsh8.4 tclsh

The '--prefix' lets you keep your own private install trees without
  interfering with /usr/bin or /usr/local/bin.
Use '--enable-gcc' if you are compiling with gcc.
The '--disable-shared' is optional but I always use it.
The 'ln -s' at the end is needed by DejaGnu.

To build Expect:

  Unpack the source.
  Make build and install directories.
  PATH="$dir_install_tcl/bin":"$PATH"
  export PATH
  cd $dir_build
  $dir_source/configure --prefix="$dir_install" --disable-shared --with-tcl="$dir_install_tcl/lib" --with-tclconfig="$dir_install_tcl/lib" --with-tclinclude="$dir_source_tcl/generic"
  make all
  make install

Expect needs to be configured with the TCL library that you just built.
--disable-shared is optional, as before.
"$dir_install_tcl" is the "$dir_install" where you installed TCL.
"$dir_source_tcl" is the "$dir_source" where you unpacked the TCL source.
[Hmmm, that could probably be "$dir_install_tcl/include" instead of
"$dir_source_tcl/generic", I need to work on my own script].

To build DejaGnu:

  Unpack the source.
  Make build and install directories.
  PATH="$dir_install_expect/bin":"$dir_install_tcl/bin":"$PATH"
  export PATH
  cd $dir_build
  $dir_source/configure --prefix="$dir_install" --with-tclinclude="$dir_install_tcl/bin"
  make all
  make install

DejaGnu needs to find the directory where you installed Expect,
so $dir_install_expect/bin has to be in your path.  I also throw
in the directory where TCL is installed, although TCL is mostly
a library.

To test GDB:

  PATH="$dir_install_dejagnu/bin":"$dir_install_expect/bin":$PATH
  runtest --version
  make check

Hope this helps,

Michael C

Re: Patch: Handle relative paths in .debug_line

[Andrew Cagney]

Did this get closed / committed?

> Bryce McKinlay <mckinlay@redhat.com> writes:
> 
>>> Jim Blandy wrote:
>>
>>>> >Almost --- dwarf2_start_subfile still passes comp_dir to concat
>>>> >without checking whether it's NULL.
>>>> >
>>
>>> Fixed in the following patch. OK?
> 
> 
> Looks better!
> 
> You've done before-and-after runs of the test suite, right?  If there
> are no regressions there, please feel free to commit. 
> 
> Are there any tests currently in the test suite that catch this bug?
> 
> 

Re: Patch: Handle relative paths in .debug_line

[Bryce McKinlay]

There were some test suite regressions with the patch. I didn't get a 
chance to investigate them, yet.

Bryce

Andrew Cagney wrote:

> Did this get closed / committed?
>
>> Bryce McKinlay <mckinlay@redhat.com> writes:
>>
>>>> Jim Blandy wrote:
>>>
>>>
>>>>> >Almost --- dwarf2_start_subfile still passes comp_dir to concat
>>>>> >without checking whether it's NULL.
>>>>> >
>>>>
>>>
>>>> Fixed in the following patch. OK?
>>>
>>
>>
>> Looks better!
>>
>> You've done before-and-after runs of the test suite, right?  If there
>> are no regressions there, please feel free to commit.
>> Are there any tests currently in the test suite that catch this bug?
>>
>>
>

Re: Patch: Handle relative paths in .debug_line

[Andrew Cagney]

> There were some test suite regressions with the patch. I didn't get a chance to investigate them, yet.

So we don't loose this, would a simple testsuite addition be possible?

Andrew

> Andrew Cagney wrote:
> 
>> Did this get closed / committed?
>>
>>> Bryce McKinlay <mckinlay@redhat.com> writes:
>>>
>>>>> Jim Blandy wrote:
>>>>
>>>>
>>>>
>>>>>> >Almost --- dwarf2_start_subfile still passes comp_dir to concat
>>>>>> >without checking whether it's NULL.
>>>>>> >
>>>>>
>>>>>
>>>>
>>>>> Fixed in the following patch. OK?
>>>>
>>>>
>>>
>>>
>>> Looks better!
>>>
>>> You've done before-and-after runs of the test suite, right?  If there
>>> are no regressions there, please feel free to commit.
>>> Are there any tests currently in the test suite that catch this bug?
>>>
>>>
>>
> 
> 

Re: Patch: Handle relative paths in .debug_line

[Garrison]

Hi!

I've taken a look at dwarf standard, and I have a question (possibly
already answered elsewhere): it is specified that relative paths in list
are related to compilation directory, so why not make the substitution
while parsing the list? If it's OK please take a look at attached patch
(sorry if it wraps this time). It's against current CVS HEAD.

I'm not sure I got those make_cleanup() calls right, if so please correct
me.

--- dwarf2read.c.original       2004-09-27 23:28:12.000000000 +0400
+++ dwarf2read.c        2004-09-30 04:05:52.999436680 +0400
@@ -822,7 +822,7 @@

  static struct line_header *(dwarf_decode_line_header
                              (unsigned int offset,
-                             bfd *abfd, struct dwarf2_cu *cu));
+                             bfd *abfd, struct dwarf2_cu *cu, const char* comp_dir));

  static void dwarf_decode_lines (struct line_header *, char *, bfd *,
                                 struct dwarf2_cu *, struct partial_symtab *);
@@ -1359,7 +1359,7 @@
    bfd *abfd = objfile->obfd;
    struct line_header *lh;

-  lh = dwarf_decode_line_header (pdi->line_offset, abfd, cu);
+  lh = dwarf_decode_line_header (pdi->line_offset, abfd, cu, pst->dirname);
    if (lh == NULL)
      return;  /* No linetable, so no includes.  */

@@ -2652,7 +2652,7 @@
    if (attr)
      {
        unsigned int line_offset = DW_UNSND (attr);
-      line_header = dwarf_decode_line_header (line_offset, abfd, cu);
+      line_header = dwarf_decode_line_header (line_offset, abfd, cu, comp_dir);
        if (line_header)
          {
            make_cleanup ((make_cleanup_ftype *) free_line_header,
@@ -6013,6 +6013,23 @@
      return follow_die_ref (dwarf2_get_ref_die_offset (spec_attr, cu));
  }

+/* Create full path name from absolute and relative parts.
+   The result string should be freed by caller when necessary. */
+static char*
+make_full_path (const char *full_part, const char *rel_part)
+{
+  char *result;
+
+  int dir_len = strlen(full_part);
+
+  result = xmalloc(dir_len + 1 + strlen(rel_part) + 1);
+  strcpy (result, full_part);
+  result[dir_len] = '/';
+  strcpy (result + dir_len + 1, rel_part);
+
+  return result;
+}
+
  /* Free the line_header structure *LH, and any arrays and strings it
     refers to.  */
  static void
@@ -6026,18 +6043,19 @@
    if (lh->file_names)
      xfree (lh->file_names);

-  /* Similarly for the include directory names.  */
+  /* Similarly for the include directory names - they will be freed independently.  */
    if (lh->include_dirs)
      xfree (lh->include_dirs);

    xfree (lh);
  }

-
  /* Add an entry to LH's include directory table.  */
  static void
-add_include_dir (struct line_header *lh, char *include_dir)
+add_include_dir (struct line_header *lh, char *include_dir, const char *comp_dir)
  {
+  char* real_dir;
+
    /* Grow the array if necessary.  */
    if (lh->include_dirs_size == 0)
      {
@@ -6053,9 +6071,19 @@
                                      * sizeof (*lh->include_dirs)));
      }

-  lh->include_dirs[lh->num_include_dirs++] = include_dir;
+  if (!IS_ABSOLUTE_PATH(include_dir) && comp_dir != NULL)
+    {
+      real_dir = make_full_path(comp_dir, include_dir);
+      make_cleanup(xfree, real_dir);
+    }
+  else
+    {
+      real_dir = include_dir;
+    }
+
+  lh->include_dirs[lh->num_include_dirs++] = real_dir;
  }
-
+

  /* Add an entry to LH's file name table.  */
  static void
@@ -6100,7 +6128,7 @@
     freed.  */
  static struct line_header *
  dwarf_decode_line_header (unsigned int offset, bfd *abfd,
-                         struct dwarf2_cu *cu)
+                         struct dwarf2_cu *cu, const char* comp_dir)
  {
    struct cleanup *back_to;
    struct line_header *lh;
@@ -6168,7 +6196,7 @@
    while ((cur_dir = read_string (abfd, line_ptr, &bytes_read)) != NULL)
      {
        line_ptr += bytes_read;
-      add_include_dir (lh, cur_dir);
+      add_include_dir (lh, cur_dir, comp_dir);
      }
    line_ptr += bytes_read;

@@ -8669,12 +8697,7 @@

        if (dir)
          {
-          dir_len = strlen (dir);
-          full_name = xmalloc (dir_len + 1 + strlen (fe->name) + 1);
-          strcpy (full_name, dir);
-          full_name[dir_len] = '/';
-          strcpy (full_name + dir_len + 1, fe->name);
-          return full_name;
+          return make_full_path(dir, fe->name);
          }
        else
          return xstrdup (fe->name);
-----------
Kind regards,
Igor V. Kovalenko

Re: Patch: Handle relative paths in .debug_line

[Garrison]

Garrison wrote:
> Hi!
> 
> I've taken a look at dwarf standard, and I have a question (possibly
> already answered elsewhere): it is specified that relative paths in list
> are related to compilation directory, so why not make the substitution
> while parsing the list? If it's OK please take a look at attached patch
> (sorry if it wraps this time). It's against current CVS HEAD.
> 
> I'm not sure I got those make_cleanup() calls right, if so please correct
> me.
> 
Also I looked at regressions - this patch makes approximately the same number
of failures as original relative paths patch. Test case gdb.base/unload.exp
might show the reason - there seems to be a problem in handling relative paths
within regression testing scripts:

Running ../../../gdb-dejagnu/gdb/testsuite/gdb.base/unload.exp ...
ERROR: couldn't open "../../../gdb-dejagnu/gdb/testsuite/gdb.base/../../../gdb-dejagnu/gdb/testsuite/gdb.base/unloadshr.c":
no such file or directory
FAIL: gdb.base/unload.exp: rerun to shared library breakpoint

--
Kind regards,
Igor V. Kovalenko

Re: Patch: Handle relative paths in .debug_line

[Igor Kovalenko]

I'm willing to devote some time to this problem, as it prevents me from
doing some necessary c++ debugging. So the question is what is currently
more necessary to have this problem fixed:

- Beat the testsuite into accepting one of available patches (e.g. by teaching
   the suite about dwarf2 case or teaching patches to pass the testsuite)

- Have a test case for handling relative paths with dwarf2

Me wrote:
> 
> Also I looked at regressions - this patch makes approximately the same 
> number
> of failures as original relative paths patch. Test case gdb.base/unload.exp
> might show the reason - there seems to be a problem in handling relative 
> paths
> within regression testing scripts:
> 
> Running ../../../gdb-dejagnu/gdb/testsuite/gdb.base/unload.exp ...
> ERROR: couldn't open 
> "../../../gdb-dejagnu/gdb/testsuite/gdb.base/../../../gdb-dejagnu/gdb/testsuite/gdb.base/unloadshr.c": 
> 
> no such file or directory
> FAIL: gdb.base/unload.exp: rerun to shared library breakpoint
> 

The same holds for disassemble failures, I've not checked others yet.
My failure list have:

+FAIL: gdb.base/commands.exp: breakpoint in bp_deleted_in_command_test
+FAIL: gdb.base/commands.exp: breakpoint in temporary_breakpoint_commands
+FAIL: gdb.mi/mi-disassemble.exp: data-disassemble file, line assembly mixed
+FAIL: gdb.mi/mi-disassemble.exp: data-disassemble range assembly mixed
+FAIL: gdb.mi/mi-disassemble.exp: data-disassemble file, line, number assembly mixed
+FAIL: gdb.mi/mi-disassemble.exp: data-disassemble file, line, number (zero lines) assembly mixed
+FAIL: gdb.mi/mi-disassemble.exp: data-disassemble file, line, number (more than main lines) assembly mixed
+FAIL: gdb.mi/mi-file.exp: request path info of current source file (basics.c)
+FAIL: gdb.mi/mi2-disassemble.exp: data-disassemble file, line assembly mixed
+FAIL: gdb.mi/mi2-disassemble.exp: data-disassemble range assembly mixed
+FAIL: gdb.mi/mi2-disassemble.exp: data-disassemble file, line, number assembly mixed
+FAIL: gdb.mi/mi2-disassemble.exp: data-disassemble file, line, number (zero lines) assembly mixed
+FAIL: gdb.mi/mi2-disassemble.exp: data-disassemble file, line, number (more than main lines) assembly mixed
+FAIL: gdb.mi/mi2-file.exp: request path info of current source file (basics.c)

-- 
Kind regards,
Igor V. Kovalenko