[RFA/dwarf2] Fix name of include psymtabs, avoid duplicates

[RFA/dwarf2] Fix name of include psymtabs, avoid duplicates

[Joel Brobecker]

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

Hello,

This patch fixes a problem reported by David Lecomber (lost the URL,
and couldn't find it in the archives). Here is how to reproduce it.
You'll need a file in a subdirectory:

        % cat foo/main.c
        int
        main (void)
        {
          return 0;
        }

Then build this file with the following command:

        % gcc -gdwarf-2 -o main foo/main.c

When GDB scans the dwarf2 data to create the psymtabs, it scans
the linetable for each CU and create "include psymtabs" for it.
These psymtabs correspond to the files included from that CU
(see call to dwarf2_build_include_psymtabs at the end of
dwarf2_build_psymtabs_hard).

Unfortunately, a little omission in my part lead to a bug:

        (gdb) file main
        (gdb) info sources
        [...]
        Source files for which symbols will be read in on demand:
        [...], main.c, /home/brobecke/tmp/dup/foo/main.c, [...]

As you see, main.c is duplicated... The problem comes from the
fact that the main.c CU is declared in the debug_info data as:
filename = foo/main.c, and comp_dir = /home/brobecke/tmp/dup.

        DW_AT_name        : foo/main.c
        DW_AT_comp_dir    : /home/brobecke/tmp/dup 

However, in the line table, we see the following declaration:

        The Directory Table:
         foo
         
        The File Name Table:
         Entry Dir     Time    Size    Name 
         1     1       0       0       main.c

So the name is main.c in the linetable, with a link to entry
number 1 in the directory table = foo. So the following check
in dwarf_decode_lines gets defeated:

            if (strcmp (include_name, pst->filename) != 0)
              dwarf2_create_include_psymtab (include_name, pst, objfile);

(include_name = "main.c" and pst->filename = "foo/main.c");

The fix is to concat the dir name and the file name to obtain
the proper name for the psymtab. As a consequence, the filename
check will also work, and avoid the duplication.

Normally, to be completely accurate, one would expect the filename

2004-10-16  Joel Brobecker  <brobecker@gnat.com>

        * dwarf2read.c (dwarf_decode_lines): Use the complete filename
        when creating include psymtabs.

Tested on x86-linux, no regression.
OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: dwarf2read.c.diff --]
[-- Type: text/plain, Size: 1209 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.167
diff -u -p -r1.167 dwarf2read.c
--- dwarf2read.c	16 Oct 2004 00:41:00 -0000	1.167
+++ dwarf2read.c	17 Oct 2004 00:01:23 -0000
@@ -6609,7 +6609,19 @@ dwarf_decode_lines (struct line_header *
       for (file_index = 0; file_index < lh->num_file_names; file_index++)
         if (lh->file_names[file_index].included_p == 1)
           {
-            char *include_name = lh->file_names [file_index].name;
+            const struct file_entry fe = lh->file_names [file_index];
+            char *include_name = fe.name;
+            char *dir_name = NULL;
+
+            if (fe.dir_index)
+              dir_name = lh->include_dirs[fe.dir_index - 1];
+
+            if (!IS_ABSOLUTE_PATH (include_name) && dir_name != NULL)
+              {
+                include_name =
+                  concat (dir_name, SLASH_STRING, include_name, NULL);
+                make_cleanup (xfree, include_name);
+              }
     
             if (strcmp (include_name, pst->filename) != 0)
               dwarf2_create_include_psymtab (include_name, pst, objfile);

Re: [RFA/dwarf2] Fix name of include psymtabs, avoid duplicates

[Joel Brobecker]

Ping?

On Sat, Oct 16, 2004 at 06:02:38PM -0700, Joel Brobecker wrote:
> Hello,
> 
> This patch fixes a problem reported by David Lecomber (lost the URL,
> and couldn't find it in the archives). Here is how to reproduce it.
> You'll need a file in a subdirectory:
> 
>         % cat foo/main.c
>         int
>         main (void)
>         {
>           return 0;
>         }
> 
> Then build this file with the following command:
> 
>         % gcc -gdwarf-2 -o main foo/main.c
> 
> When GDB scans the dwarf2 data to create the psymtabs, it scans
> the linetable for each CU and create "include psymtabs" for it.
> These psymtabs correspond to the files included from that CU
> (see call to dwarf2_build_include_psymtabs at the end of
> dwarf2_build_psymtabs_hard).
> 
> Unfortunately, a little omission in my part lead to a bug:
> 
>         (gdb) file main
>         (gdb) info sources
>         [...]
>         Source files for which symbols will be read in on demand:
>         [...], main.c, /home/brobecke/tmp/dup/foo/main.c, [...]
> 
> As you see, main.c is duplicated... The problem comes from the
> fact that the main.c CU is declared in the debug_info data as:
> filename = foo/main.c, and comp_dir = /home/brobecke/tmp/dup.
> 
>         DW_AT_name        : foo/main.c
>         DW_AT_comp_dir    : /home/brobecke/tmp/dup 
> 
> However, in the line table, we see the following declaration:
> 
>         The Directory Table:
>          foo
>          
>         The File Name Table:
>          Entry Dir     Time    Size    Name 
>          1     1       0       0       main.c
> 
> So the name is main.c in the linetable, with a link to entry
> number 1 in the directory table = foo. So the following check
> in dwarf_decode_lines gets defeated:
> 
>             if (strcmp (include_name, pst->filename) != 0)
>               dwarf2_create_include_psymtab (include_name, pst, objfile);
> 
> (include_name = "main.c" and pst->filename = "foo/main.c");
> 
> The fix is to concat the dir name and the file name to obtain
> the proper name for the psymtab. As a consequence, the filename
> check will also work, and avoid the duplication.
> 
> Normally, to be completely accurate, one would expect the filename
> 
> 2004-10-16  Joel Brobecker  <brobecker@gnat.com>
> 
>         * dwarf2read.c (dwarf_decode_lines): Use the complete filename
>         when creating include psymtabs.
> 
> Tested on x86-linux, no regression.
> OK to apply?
> 
> Thanks,
> -- 
> Joel

> Index: dwarf2read.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2read.c,v
> retrieving revision 1.167
> diff -u -p -r1.167 dwarf2read.c
> --- dwarf2read.c	16 Oct 2004 00:41:00 -0000	1.167
> +++ dwarf2read.c	17 Oct 2004 00:01:23 -0000
> @@ -6609,7 +6609,19 @@ dwarf_decode_lines (struct line_header *
>        for (file_index = 0; file_index < lh->num_file_names; file_index++)
>          if (lh->file_names[file_index].included_p == 1)
>            {
> -            char *include_name = lh->file_names [file_index].name;
> +            const struct file_entry fe = lh->file_names [file_index];
> +            char *include_name = fe.name;
> +            char *dir_name = NULL;
> +
> +            if (fe.dir_index)
> +              dir_name = lh->include_dirs[fe.dir_index - 1];
> +
> +            if (!IS_ABSOLUTE_PATH (include_name) && dir_name != NULL)
> +              {
> +                include_name =
> +                  concat (dir_name, SLASH_STRING, include_name, NULL);
> +                make_cleanup (xfree, include_name);
> +              }
>      
>              if (strcmp (include_name, pst->filename) != 0)
>                dwarf2_create_include_psymtab (include_name, pst, objfile);


-- 
Joel

Re: [RFA/dwarf2] Fix name of include psymtabs, avoid duplicates

[Elena Zannoni]

Joel Brobecker writes:

 > >         % gcc -gdwarf-2 -o main foo/main.c
 > > 
 > > When GDB scans the dwarf2 data to create the psymtabs, it scans
 > > the linetable for each CU and create "include psymtabs" for it.
 > > These psymtabs correspond to the files included from that CU
 > > (see call to dwarf2_build_include_psymtabs at the end of
 > > dwarf2_build_psymtabs_hard).
 > > 
 > > Unfortunately, a little omission in my part lead to a bug:
 > > 
 > >         (gdb) file main
 > >         (gdb) info sources
 > >         [...]
 > >         Source files for which symbols will be read in on demand:
 > >         [...], main.c, /home/brobecke/tmp/dup/foo/main.c, [...]

This seems easy to add to the testsuite. Just create a subdirectory in
gdb.dwarf2, and issue the above commands.
 

 > > 2004-10-16  Joel Brobecker  <brobecker@gnat.com>
 > > 
 > >         * dwarf2read.c (dwarf_decode_lines): Use the complete filename
 > >         when creating include psymtabs.
 > > 
 > > Tested on x86-linux, no regression.
 > > OK to apply?

Yes, after you add the test, which should fail before and pass after
your patch.

Re: [RFA/dwarf2] Fix name of include psymtabs, avoid duplicates

[Joel Brobecker]

>  > > 2004-10-16  Joel Brobecker  <brobecker@gnat.com>
>  > > 
>  > >         * dwarf2read.c (dwarf_decode_lines): Use the complete filename
>  > >         when creating include psymtabs.
>  > > 
>  > > Tested on x86-linux, no regression.
>  > > OK to apply?
> 
> Yes, after you add the test, which should fail before and pass after
> your patch.

I'll see what I can come up with, and commit the patch once a testcase
for it has been added.

Thanks for the review,
-- 
Joel

Re: [RFA/dwarf2] Fix name of include psymtabs, avoid duplicates

[Joel Brobecker]

> >  > > 2004-10-16  Joel Brobecker  <brobecker@gnat.com>
> >  > > 
> >  > >         * dwarf2read.c (dwarf_decode_lines): Use the complete filename
> >  > >         when creating include psymtabs.
> >  > > 
> >  > > Tested on x86-linux, no regression.
> >  > > OK to apply?
> > 
> > Yes, after you add the test, which should fail before and pass after
> > your patch.
> 
> I'll see what I can come up with, and commit the patch once a testcase
> for it has been added.

I have finally checked this patch in. The testcase I added can be found
at http://sources.redhat.com/ml/gdb-patches/2005-03/msg00139.html.
(the name of the testcase is gdb.dwarf2/dup-psym.exp).

-- 
Joel