Re: [RFC/dwarf-2] Add support for included files

Re: [RFC/dwarf-2] Add support for included files

[Andrew Pinski]

I try to test building gdb with the mainline of gcc and it fails
to compile with the following error:

/home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c: In 
function `dwarf2_create_include_psymtab':
/home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c:1244: 
error: invalid lvalue in assignment

Could you fix the problem, please?

Thanks,
Andrew Pinski


2004-04-30  J. Brobecker  <brobecker@gnat.com>

         * dwarf2read.c (line_header): Add new included_p field in
         field file_names.
         (partial_die_info): New field has_stmt_list. New field 
line_offset.
         (dwarf2_create_include_psymtab): New function.
         (dwarf2_build_include_psymtabs): New function.
         (add_file_name): Add forward declaration. Initialize new field.
         (dwarf_decode_lines): Add new parameter. Enhance this procedure
         to be able to determine the list of files included by the
         given unit, and build their associated psymtabs.
         (dwarf2_build_psymtabs_hard): Build the psymtabs for the 
included
         files as well.
         (psymtab_to_symtab_1): Build the symtabs of all dependencies as 
well.
         (read_file_scope): Update call to dwarf_decode_lines.
         (read_partial_die): Handle DW_AT_stmt_list attributes.

Re: [RFC/dwarf-2] Add support for included files

[Joel Brobecker]

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

> /home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c: In 
> function `dwarf2_create_include_psymtab':
> /home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c:1244: 
> error: invalid lvalue in assignment

Sorry about that, this error didn't show up with the compiler I used.
I just built one straight from mainline and could reproduce the error.
Apparently, expressions like this are not legal?

        (char *) something = NULL;

Anyway, I fixed the immediate problem by expanding the PST_PRIVATE
macro into its actual definition, but without the cast. That should
allow you to build again.

2004-05-03  Joel Brobecker  <brobecker@gnat.com>

        * dwarf2read.c (dwarf2_create_include_psymtab): Fix build
        failure detected by recent versions of GCC.
        (psymtab_to_symtab_1): No longer use the PST_PRIVATE macro
        to be consistent with the usage in dwarf2_create_include_psymtab.

Checked in mainline.

-- 
Joel

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

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.149
diff -u -p -r1.149 dwarf2read.c
--- dwarf2read.c	3 May 2004 16:21:50 -0000	1.149
+++ dwarf2read.c	4 May 2004 00:08:10 -0000
@@ -1239,9 +1239,9 @@ dwarf2_create_include_psymtab (char *nam
   /* No private part is necessary for include psymtabs.  This property
      can be used to differentiate between such include psymtabs and
      the regular ones.  If it ever happens that a regular psymtab can
-     legitimally have a NULL PST_PRIVATE part, then we'll have to add a
+     legitimally have a NULL private part, then we'll have to add a
      dedicated field for that in the dwarf2_pinfo structure.  */
-  PST_PRIVATE (subpst) = NULL;
+  subpst->read_symtab_private = NULL;
 }
 
 /* Read the Line Number Program data and extract the list of files
@@ -2109,7 +2109,7 @@ psymtab_to_symtab_1 (struct partial_symt
         psymtab_to_symtab_1 (pst->dependencies[i]);
       }
 
-  if (PST_PRIVATE (pst) == NULL)
+  if (pst->read_symtab_private == NULL)
     {
       /* It's an include file, no symbols to read for it.
          Everything is in the parent symtab.  */

Re: [RFC/dwarf-2] Add support for included files

[Andrew Pinski]

On May 3, 2004, at 20:15, Joel Brobecker wrote:

>> /home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c: In
>> function `dwarf2_create_include_psymtab':
>> /home/gates/pinskia/src/gnu/combinesources/src/gdb/dwarf2read.c:1244:
>> error: invalid lvalue in assignment
>
> Sorry about that, this error didn't show up with the compiler I used.
> I just built one straight from mainline and could reproduce the error.
> Apparently, expressions like this are not legal?
>
>         (char *) something = NULL;

Yes that is invalid C, it was deprecated in 3.4.0 (which warns about
the usage) and removed on the mainline.

Thanks for fast fixing the problem.

-Andrew Pinski

Re: [RFC/dwarf-2] Add support for included files

[Joel Brobecker]

> Let me make sure I've got it right.  When we're calling
> dwarf_decode_lines for the partial symbol tables, we never call
> dwarf2_start_subfile anyway --- that's for building full symbol tables
> --- so the use of 'dirname' in that function doesn't matter.  And
> since it's never used to compute the full name of the symtabs, it
> doesn't introduce inconsistencies to omit it.

That's right. I updated a bit the comment for that function to include
this information. I think it was worthwhile. Here is the new comment
describing the function. If you think any change needs to be made, let
me know, I'll gladly make them.

   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
      builds all the associated partial symbol tables.  In this case,
      the value of COMP_DIR is ignored, and can thus be NULL (the COMP_DIR
      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 -).  */

> If that's correct, then this patch is ready to commit.  Thanks very
> much!

Thanks a lot! This patch is now in.

-- 
Joel

Re: [RFC/dwarf-2] Add support for included files

[Jim Blandy]

Joel Brobecker <brobecker@gnat.com> writes:
> > - Is it really okay to pass NULL to dwarf_decode_lines for comp_dir?
> >   Won't the filenames of the partial symbol tables be different from
> >   those of the symbol tables?  To fix this, read_partial_die would
> >   have to check for DW_AT_comp_dir attributes, too.
> 
> I don't think we need to worry about the comp_dir when building
> the psymtabs for included files. The filename used for the psymtab
> is the same as the filename used to build the symtab (checked it
> by inspecting start_subfile() and end_symtab()). The only thing
> we could gain from computing the comp_dir as well during that phase
> is to compute the psymtab fullname. But then comp_dir is used
> to set the symtab dirname, not to compute the fullname. So it doesn't
> help improve the consistency between psymtab and symtab.

Let me make sure I've got it right.  When we're calling
dwarf_decode_lines for the partial symbol tables, we never call
dwarf2_start_subfile anyway --- that's for building full symbol tables
--- so the use of 'dirname' in that function doesn't matter.  And
since it's never used to compute the full name of the symtabs, it
doesn't introduce inconsistencies to omit it.

If that's correct, then this patch is ready to commit.  Thanks very
much!

Re: [RFC/dwarf-2] Add support for included files

[Joel Brobecker]

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

> I actually have still more comments:

Jim, you'll be happy, your comments make the patch neater :-).
Once you get to know the data structures a little bit more
(I really should have studied them a little harder before working
on this), everything seems to be falling into place almost naturally...

Attached is a revised version of the patch, up to date as of now,
and incorporating all your comments, except one which I wasn't sure
about.

> - Rather than passing 'line_offset' by reference to read_partial_die,
>   could you add a line_offset field to 'struct partial_die_info, and have
>   read_partial_die set that?  That would seem more consistent with the
>   rest of what read_partial_die does.  Then
>   'dwarf2_build_include_psymtabs' should take a pointer to the CU die.

This brings a nice cleanup.

>   That's assuming Daniel doesn't see any problem enlarging
>   partial_die_info.  If he does, maybe we could add the fields to
>   'struct dwarf2_cu' instead: it's a little odd to have
>   'read_partial_die' set the cu's fields directly, but not horribly
>   so, as long as the die's tag is DW_TAG_compile_unit.

partial_die_info seems better suited to hold that information.
Hopefully Daniel won't object.

> - Is it really okay to pass NULL to dwarf_decode_lines for comp_dir?
>   Won't the filenames of the partial symbol tables be different from
>   those of the symbol tables?  To fix this, read_partial_die would
>   have to check for DW_AT_comp_dir attributes, too.

I don't think we need to worry about the comp_dir when building
the psymtabs for included files. The filename used for the psymtab
is the same as the filename used to build the symtab (checked it
by inspecting start_subfile() and end_symtab()). The only thing
we could gain from computing the comp_dir as well during that phase
is to compute the psymtab fullname. But then comp_dir is used
to set the symtab dirname, not to compute the fullname. So it doesn't
help improve the consistency between psymtab and symtab.

I think that part is fine...

... At least for now :-).

> - It seems to me the file_is_included array should be in struct
>   line_header, with add_file_name managing its allocation /
>   initialization in parallel with file_names.  It's already the case
>   that that structure is mutated by the act of processing the line
>   number program, so I don't see that it's a serious change to the
>   usage patterns of that structure type.

Good idea. I included it, and the code becomes one notch cleaner.

> - I'd rather see the call to dwarf2_build_include_psymtabs before the
>   assignment to info_ptr, not after.  Not that it matters much.

Done.

> - Thanks for fixing the comments on dwarf_decode_lines --- especially
>   the references to arguments that had been missing for some time now.

My pleasure :-).

2004-04-30  J. Brobecker  <brobecker@gnat.com>

        * dwarf2read.c (line_header): Add new included_p field in
        field file_names.
        (partial_die_info): New field has_stmt_list. New field line_offset.
        (dwarf2_create_include_psymtab): New function.
        (dwarf2_build_include_psymtabs): New function.
        (add_file_name): Add forward declaration. Initialize new field.
        (dwarf_decode_lines): Add new parameter. Enhance this procedure
        to be able to determine the list of files included by the
        given unit, and build their associated psymtabs.
        (dwarf2_build_psymtabs_hard): Build the psymtabs for the included
        files as well.
        (psymtab_to_symtab_1): Build the symtabs of all dependencies as well.
        (read_file_scope): Update call to dwarf_decode_lines.
        (read_partial_die): Handle DW_AT_stmt_list attributes.

Tested on x86-linux, no regression. Fixes the two sep.exp KFAILS.
OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: separate2.diff --]
[-- Type: text/plain, Size: 12133 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.148
diff -u -p -r1.148 dwarf2read.c
--- dwarf2read.c	19 Apr 2004 18:20:50 -0000	1.148
+++ dwarf2read.c	1 May 2004 01:07:21 -0000
@@ -341,6 +341,7 @@ struct line_header
     unsigned int dir_index;
     unsigned int mod_time;
     unsigned int length;
+    int included_p; /* Non-zero if referenced by the Line Number Program.  */
   } *file_names;
 
   /* The start and end of the statement program following this
@@ -368,6 +369,7 @@ struct partial_die_info
     unsigned int is_declaration : 1;
     unsigned int has_type : 1;
     unsigned int has_specification : 1;
+    unsigned int has_stmt_list : 1;
     unsigned int has_pc_info : 1;
 
     /* Flag set if the SCOPE field of this structure has been
@@ -400,6 +402,9 @@ struct partial_die_info
        DW_AT_extension).  */
     unsigned int spec_offset;
 
+    /* If HAS_STMT_LIST, the offset of the Line Number Information data.  */
+    unsigned int line_offset;
+
     /* Pointers to this DIE's parent, first child, and next sibling,
        if any.  */
     struct partial_die_info *die_parent, *die_child, *die_sibling;
@@ -631,6 +636,13 @@ static void dwarf2_locate_sections (bfd 
 static void dwarf2_build_psymtabs_easy (struct objfile *, int);
 #endif
 
+static void dwarf2_create_include_psymtab (char *, struct partial_symtab *,
+                                           struct objfile *);
+
+static void dwarf2_build_include_psymtabs (struct dwarf2_cu *,
+                                           struct partial_die_info *,
+                                           struct partial_symtab *);
+
 static void dwarf2_build_psymtabs_hard (struct objfile *, int);
 
 static void scan_partial_symbols (struct partial_die_info *,
@@ -739,12 +751,15 @@ static struct die_info *die_specificatio
 
 static void free_line_header (struct line_header *lh);
 
+static void add_file_name (struct line_header *, char *, unsigned int,
+                           unsigned int, unsigned int);
+
 static struct line_header *(dwarf_decode_line_header
                             (unsigned int offset,
                              bfd *abfd, struct dwarf2_cu *cu));
 
 static void dwarf_decode_lines (struct line_header *, char *, bfd *,
-				struct dwarf2_cu *);
+				struct dwarf2_cu *, struct partial_symtab *);
 
 static void dwarf2_start_subfile (char *, char *);
 
@@ -1194,6 +1209,68 @@ partial_read_comp_unit_head (struct comp
   return info_ptr;
 }
 
+/* Allocate a new partial symtab for file named NAME and mark this new
+   partial symtab as being an include of PST.  */
+
+static void
+dwarf2_create_include_psymtab (char *name, struct partial_symtab *pst,
+                               struct objfile *objfile)
+{
+  struct partial_symtab *subpst = allocate_psymtab (name, objfile);
+
+  subpst->section_offsets = pst->section_offsets;
+  subpst->textlow = 0;
+  subpst->texthigh = 0;
+
+  subpst->dependencies = (struct partial_symtab **)
+    obstack_alloc (&objfile->objfile_obstack,
+                   sizeof (struct partial_symtab *));
+  subpst->dependencies[0] = pst;
+  subpst->number_of_dependencies = 1;
+
+  subpst->globals_offset = 0;
+  subpst->n_global_syms = 0;
+  subpst->statics_offset = 0;
+  subpst->n_static_syms = 0;
+  subpst->symtab = NULL;
+  subpst->read_symtab = pst->read_symtab;
+  subpst->readin = 0;
+
+  /* No private part is necessary for include psymtabs.  This property
+     can be used to differentiate between such include psymtabs and
+     the regular ones.  If it ever happens that a regular psymtab can
+     legitimally have a NULL PST_PRIVATE part, then we'll have to add a
+     dedicated field for that in the dwarf2_pinfo structure.  */
+  PST_PRIVATE (subpst) = NULL;
+}
+
+/* Read the Line Number Program data and extract the list of files
+   included by the source file represented by PST.  Build an include
+   partial symtab for each of these included files.
+   
+   This procedure assumes that there *is* a Line Number Program in
+   the given CU.  Callers should check that PDI->HAS_STMT_LIST is set
+   before calling this procedure.  */
+
+static void
+dwarf2_build_include_psymtabs (struct dwarf2_cu *cu,
+                               struct partial_die_info *pdi,
+                               struct partial_symtab *pst)
+{
+  struct objfile *objfile = cu->objfile;
+  bfd *abfd = objfile->obfd;
+  struct line_header *lh;
+
+  lh = dwarf_decode_line_header (pdi->line_offset, abfd, cu);
+  if (lh == NULL)
+    return;  /* No linetable, so no includes.  */
+
+  dwarf_decode_lines (lh, NULL, abfd, cu, pst);
+
+  free_line_header (lh);
+}
+
+
 /* Build the partial symbol table by doing a quick pass through the
    .debug_info and .debug_abbrev sections.  */
 
@@ -1321,6 +1398,13 @@ dwarf2_build_psymtabs_hard (struct objfi
          also happen.) This happens in VxWorks.  */
       free_named_symtabs (pst->filename);
 
+      if (comp_unit_die.has_stmt_list)
+        {
+          /* Get the list of files included in the current compilation unit,
+             and build a psymtab for each of them.  */
+          dwarf2_build_include_psymtabs (&cu, &comp_unit_die, pst);
+        }
+
       info_ptr = beg_of_comp_unit + cu.header.length
                                   + cu.header.initial_length_size;
 
@@ -2006,6 +2090,32 @@ psymtab_to_symtab_1 (struct partial_symt
   struct cleanup *back_to;
   struct attribute *attr;
   CORE_ADDR baseaddr;
+  int i;
+
+  for (i = 0; i < pst->number_of_dependencies; i++)
+    if (!pst->dependencies[i]->readin)
+      {
+        /* Inform about additional files that need to be read in.  */
+        if (info_verbose)
+          {
+            fputs_filtered (" ", gdb_stdout);
+            wrap_here ("");
+            fputs_filtered ("and ", gdb_stdout);
+            wrap_here ("");
+            printf_filtered ("%s...", pst->dependencies[i]->filename);
+            wrap_here ("");     /* Flush output */
+            gdb_flush (gdb_stdout);
+          }
+        psymtab_to_symtab_1 (pst->dependencies[i]);
+      }
+
+  if (PST_PRIVATE (pst) == NULL)
+    {
+      /* It's an include file, no symbols to read for it.
+         Everything is in the parent symtab.  */
+      pst->readin = 1;
+      return;
+    }
 
   dwarf2_per_objfile = objfile_data (pst->objfile, dwarf2_objfile_data_key);
 
@@ -2294,7 +2404,7 @@ read_file_scope (struct die_info *die, s
         {
           make_cleanup ((make_cleanup_ftype *) free_line_header,
                         (void *) line_header);
-          dwarf_decode_lines (line_header, comp_dir, abfd, cu);
+          dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL);
         }
     }
 
@@ -4826,6 +4936,10 @@ read_partial_die (struct partial_die_inf
 	    part_die->sibling = dwarf2_per_objfile->info_buffer
 	      + dwarf2_get_ref_die_offset (&attr, cu);
 	  break;
+        case DW_AT_stmt_list:
+          part_die->has_stmt_list = 1;
+          part_die->line_offset = DW_UNSND (&attr);
+          break;
 	default:
 	  break;
 	}
@@ -5628,6 +5742,7 @@ add_file_name (struct line_header *lh,
   fe->dir_index = dir_index;
   fe->mod_time = mod_time;
   fe->length = length;
+  fe->included_p = 0;
 }
  
 
@@ -5785,13 +5900,23 @@ check_cu_functions (CORE_ADDR address, s
   return fn->lowpc;
 }
 
-/* Decode the line number information for the compilation unit whose
-   line number info is at OFFSET in the .debug_line section.
-   The compilation directory of the file is passed in COMP_DIR.  */
+/* 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.
+   
+   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
+      builds all the associated partial symbol tables.  In this case,
+      the value of COMP_DIR is ignored, and can thus be NULL.  */
 
 static void
 dwarf_decode_lines (struct line_header *lh, char *comp_dir, bfd *abfd,
-		    struct dwarf2_cu *cu)
+		    struct dwarf2_cu *cu, struct partial_symtab *pst)
 {
   char *line_ptr;
   char *line_end;
@@ -5799,6 +5924,7 @@ dwarf_decode_lines (struct line_header *
   unsigned char op_code, extended_op, adj_opcode;
   CORE_ADDR baseaddr;
   struct objfile *objfile = cu->objfile;
+  const int decode_for_pst_p = (pst != NULL);
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
@@ -5817,9 +5943,9 @@ dwarf_decode_lines (struct line_header *
       int basic_block = 0;
       int end_sequence = 0;
 
-      /* Start a subfile for the current file of the state machine.  */
-      if (lh->num_file_names >= file)
+      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.  */
@@ -5844,9 +5970,12 @@ dwarf_decode_lines (struct line_header *
 	      address += (adj_opcode / lh->line_range)
 		* lh->minimum_instruction_length;
 	      line += lh->line_base + (adj_opcode % lh->line_range);
-	      /* append row to matrix using current values */
-	      record_line (current_subfile, line, 
-	                   check_cu_functions (address, cu));
+              if (!decode_for_pst_p)
+                {
+	          /* append row to matrix using current values */
+	          record_line (current_subfile, line, 
+	                       check_cu_functions (address, cu));
+                }
 	      basic_block = 1;
 	    }
 	  else switch (op_code)
@@ -5859,7 +5988,8 @@ dwarf_decode_lines (struct line_header *
 		{
 		case DW_LNE_end_sequence:
 		  end_sequence = 1;
-		  record_line (current_subfile, 0, address);
+                  if (!decode_for_pst_p)
+		    record_line (current_subfile, 0, address);
 		  break;
 		case DW_LNE_set_address:
 		  address = read_address (abfd, line_ptr, cu, &bytes_read);
@@ -5892,8 +6022,9 @@ dwarf_decode_lines (struct line_header *
 		}
 	      break;
 	    case DW_LNS_copy:
-	      record_line (current_subfile, line, 
-	                   check_cu_functions (address, cu));
+              if (!decode_for_pst_p)
+	        record_line (current_subfile, line, 
+	                     check_cu_functions (address, cu));
 	      basic_block = 0;
 	      break;
 	    case DW_LNS_advance_pc:
@@ -5915,11 +6046,13 @@ dwarf_decode_lines (struct line_header *
                 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;
-                dwarf2_start_subfile (fe->name, dir);
+                if (!decode_for_pst_p)
+                  dwarf2_start_subfile (fe->name, dir);
               }
 	      break;
 	    case DW_LNS_set_column:
@@ -5956,6 +6089,22 @@ dwarf_decode_lines (struct line_header *
 	      }
 	    }
 	}
+    }
+
+  if (decode_for_pst_p)
+    {
+      int file_index;
+
+      /* Now that we're done scanning the Line Header Program, we can
+         create the psymtab of each included file.  */
+      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;
+    
+            if (strcmp (include_name, pst->filename) != 0)
+              dwarf2_create_include_psymtab (include_name, pst, objfile);
+          }
     }
 }
 

Re: [RFC/dwarf-2] Add support for included files

[Jim Blandy]

Joel Brobecker <brobecker@gnat.com> writes:
> Here is an updated version of the previous patch, adapted to fit in
> the current dwarf2read.c after Daniel's changes, and with comments
> added. It's actually slightly smaller thanks to Daniel's changes
> :-).

Great!

I actually have still more comments:

- Rather than passing 'line_offset' by reference to read_partial_die,
  could you add a line_offset field to 'struct partial_die_info, and have
  read_partial_die set that?  That would seem more consistent with the
  rest of what read_partial_die does.  Then
  'dwarf2_build_include_psymtabs' should take a pointer to the CU die.

  That's assuming Daniel doesn't see any problem enlarging
  partial_die_info.  If he does, maybe we could add the fields to
  'struct dwarf2_cu' instead: it's a little odd to have
  'read_partial_die' set the cu's fields directly, but not horribly
  so, as long as the die's tag is DW_TAG_compile_unit.

- Is it really okay to pass NULL to dwarf_decode_lines for comp_dir?
  Won't the filenames of the partial symbol tables be different from
  those of the symbol tables?  To fix this, read_partial_die would
  have to check for DW_AT_comp_dir attributes, too.

- It seems to me the file_is_included array should be in struct
  line_header, with add_file_name managing its allocation /
  initialization in parallel with file_names.  It's already the case
  that that structure is mutated by the act of processing the line
  number program, so I don't see that it's a serious change to the
  usage patterns of that structure type.

- I'd rather see the call to dwarf2_build_include_psymtabs before the
  assignment to info_ptr, not after.  Not that it matters much.

- Thanks for fixing the comments on dwarf_decode_lines --- especially
  the references to arguments that had been missing for some time now.

Re: [RFC/dwarf-2] Add support for included files

[Joel Brobecker]

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

Hello Jim,

> If you add a comment above dwarf_decode_lines explaining how it's
> called, and that PST determines whether we're building psymtabs or
> symtabs, we'll call it even.
> 
> I just approved Daniel's mondo psymtab construction patch, so one of
> you is going to get in the others' way.  Have fun.

Here is an updated version of the previous patch, adapted to fit in
the current dwarf2read.c after Daniel's changes, and with comments
added. It's actually slightly smaller thanks to Daniel's changes :-).

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

        * dwarf2read.c (dwarf2_create_include_psymtab): New function.
        (dwarf2_build_include_psymtabs): New function.
        (read_partial_die): Add new parameter.
        (add_file_name): Add forward declaration.
        (dwarf_decode_lines): Add new parameter. Enhance this procedure
        to be able to determine the list of files included by the
        given unit, and build their associated psymtabs.
        (dwarf2_build_psymtabs_hard): Build the psymtabs for the
        included files as well.
        (psymtab_to_symtab_1): Build the symtabs of all dependencies
        as well.
        (read_file_scope): Update call to read_partial_die.
        (load_partial_dies): Likewise.

Tested on x86-linux. No regression, and it fixes the following two
KFAILs:

        KFAIL: gdb.base/sep.exp: list using location inside included file
               (PRMS: symtab/1607)
        KFAIL: gdb.base/sep.exp: breakpoint inside included file
               (PRMS: symtab/1607)

If this patch is approved, I'll send another one removing the setup_kfails.

Thanks,
-- 
Joel

[-- Attachment #2: separate.diff --]
[-- Type: text/plain, Size: 15326 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.146
diff -u -p -r1.146 dwarf2read.c
--- dwarf2read.c	16 Apr 2004 16:12:52 -0000	1.146
+++ dwarf2read.c	16 Apr 2004 23:03:07 -0000
@@ -634,6 +634,13 @@ static void dwarf2_locate_sections (bfd 
 static void dwarf2_build_psymtabs_easy (struct objfile *, int);
 #endif
 
+static void dwarf2_create_include_psymtab (char *, struct partial_symtab *,
+                                           struct objfile *);
+
+static void dwarf2_build_include_psymtabs (struct dwarf2_cu *,
+                                           const unsigned int,
+                                           struct partial_symtab *);
+
 static void dwarf2_build_psymtabs_hard (struct objfile *, int);
 
 static void scan_partial_symbols (struct partial_die_info *,
@@ -677,7 +684,8 @@ static struct partial_die_info *load_par
 
 static char *read_partial_die (struct partial_die_info *,
 			       struct abbrev_info *abbrev, unsigned int,
-			       bfd *, char *, struct dwarf2_cu *);
+			       bfd *, char *, struct dwarf2_cu *,
+                               unsigned int *);
 
 static struct partial_die_info *find_partial_die (unsigned long,
 						  struct dwarf2_cu *,
@@ -739,12 +747,15 @@ static struct die_info *die_specificatio
 
 static void free_line_header (struct line_header *lh);
 
+static void add_file_name (struct line_header *, char *, unsigned int,
+                           unsigned int, unsigned int);
+
 static struct line_header *(dwarf_decode_line_header
                             (unsigned int offset,
                              bfd *abfd, struct dwarf2_cu *cu));
 
 static void dwarf_decode_lines (struct line_header *, char *, bfd *,
-				struct dwarf2_cu *);
+				struct dwarf2_cu *, struct partial_symtab *);
 
 static void dwarf2_start_subfile (char *, char *);
 
@@ -1196,6 +1207,65 @@ partial_read_comp_unit_head (struct comp
   return info_ptr;
 }
 
+/* Allocate a new partial symtab for file named NAME and mark this new
+   partial symtab as being an include of PST.  */
+
+static void
+dwarf2_create_include_psymtab (char *name, struct partial_symtab *pst,
+                               struct objfile *objfile)
+{
+  struct partial_symtab *subpst = allocate_psymtab (name, objfile);
+
+  subpst->section_offsets = pst->section_offsets;
+  subpst->textlow = 0;
+  subpst->texthigh = 0;
+
+  subpst->dependencies = (struct partial_symtab **)
+    obstack_alloc (&objfile->objfile_obstack,
+                   sizeof (struct partial_symtab *));
+  subpst->dependencies[0] = pst;
+  subpst->number_of_dependencies = 1;
+
+  subpst->globals_offset = 0;
+  subpst->n_global_syms = 0;
+  subpst->statics_offset = 0;
+  subpst->n_static_syms = 0;
+  subpst->symtab = NULL;
+  subpst->read_symtab = pst->read_symtab;
+  subpst->readin = 0;
+
+  /* No private part is necessary for include psymtabs.  This property
+     can be used to differentiate between such include psymtabs and
+     the regular ones.  If it ever happens that a regular psymtab can
+     legitimally have a NULL PST_PRIVATE part, then we'll have to add a
+     dedicated field for that in the dwarf2_pinfo structure.  */
+  PST_PRIVATE (subpst) = NULL;
+}
+
+/* Read the line number information located at LINE_OFFSET,
+   and extract the list of sources files included by the
+   source file represented by PST.  Build an include partial
+   symtab for each of these included files.  */
+
+static void
+dwarf2_build_include_psymtabs (struct dwarf2_cu *cu,
+                               const unsigned int line_offset,
+                               struct partial_symtab *pst)
+{
+  struct objfile *objfile = cu->objfile;
+  bfd *abfd = objfile->obfd;
+  struct line_header *lh;
+
+  lh = dwarf_decode_line_header (line_offset, abfd, cu);
+  if (lh == NULL)
+    return;  /* No linetable, so no includes.  */
+
+  dwarf_decode_lines (lh, NULL, abfd, cu, pst);
+
+  free_line_header (lh);
+}
+
+
 /* Build the partial symbol table by doing a quick pass through the
    .debug_info and .debug_abbrev sections.  */
 
@@ -1266,6 +1336,7 @@ dwarf2_build_psymtabs_hard (struct objfi
       struct abbrev_info *abbrev;
       unsigned int bytes_read;
       struct dwarf2_per_cu_data *this_cu;
+      unsigned int line_offset = 0;
 
       beg_of_comp_unit = info_ptr;
 
@@ -1294,7 +1365,7 @@ dwarf2_build_psymtabs_hard (struct objfi
       /* Read the compilation unit die */
       abbrev = peek_die_abbrev (info_ptr, &bytes_read, &cu);
       info_ptr = read_partial_die (&comp_unit_die, abbrev, bytes_read,
-				   abfd, info_ptr, &cu);
+				   abfd, info_ptr, &cu, &line_offset);
 
       /* Set the language we're debugging */
       set_cu_language (comp_unit_die.language, &cu);
@@ -1358,6 +1429,10 @@ dwarf2_build_psymtabs_hard (struct objfi
       info_ptr = beg_of_comp_unit + cu.header.length
                                   + cu.header.initial_length_size;
 
+      /* Get the list of files included in the current compilation unit,
+         and build a psymtab for each of them.  */
+      dwarf2_build_include_psymtabs (&cu, line_offset, pst);
+
       do_cleanups (back_to_inner);
     }
   do_cleanups (back_to);
@@ -2041,6 +2116,32 @@ psymtab_to_symtab_1 (struct partial_symt
   struct cleanup *back_to;
   struct attribute *attr;
   CORE_ADDR baseaddr;
+  int i;
+
+  for (i = 0; i < pst->number_of_dependencies; i++)
+    if (!pst->dependencies[i]->readin)
+      {
+        /* Inform about additional files that need to be read in.  */
+        if (info_verbose)
+          {
+            fputs_filtered (" ", gdb_stdout);
+            wrap_here ("");
+            fputs_filtered ("and ", gdb_stdout);
+            wrap_here ("");
+            printf_filtered ("%s...", pst->dependencies[i]->filename);
+            wrap_here ("");     /* Flush output */
+            gdb_flush (gdb_stdout);
+          }
+        psymtab_to_symtab_1 (pst->dependencies[i]);
+      }
+
+  if (PST_PRIVATE (pst) == NULL)
+    {
+      /* It's an include file, no symbols to read for it.
+         Everything is in the parent symtab.  */
+      pst->readin = 1;
+      return;
+    }
 
   dwarf2_per_objfile = objfile_data (pst->objfile, dwarf2_objfile_data_key);
 
@@ -2329,7 +2430,7 @@ read_file_scope (struct die_info *die, s
         {
           make_cleanup ((make_cleanup_ftype *) free_line_header,
                         (void *) line_header);
-          dwarf_decode_lines (line_header, comp_dir, abfd, cu);
+          dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL);
         }
     }
 
@@ -4636,7 +4737,7 @@ load_partial_dies (bfd *abfd, char *info
 	}
 
       info_ptr = read_partial_die (part_die, abbrev, bytes_read,
-				   abfd, info_ptr, cu);
+				   abfd, info_ptr, cu, NULL);
 
       /* This two-pass algorithm for processing partial symbols has a
 	 high cost in cache pressure.  Thus, handle some simple cases
@@ -4768,13 +4869,16 @@ load_partial_dies (bfd *abfd, char *info
     }
 }
 
-/* Read a minimal amount of information into the minimal die structure.  */
+/* Read a minimal amount of information into the minimal die structure.
+   If not NULL, the offset where the Line Number Information data is
+   stored will be saved in LINE_OFFSET.  */
 
 static char *
 read_partial_die (struct partial_die_info *part_die,
 		  struct abbrev_info *abbrev,
 		  unsigned int abbrev_len, bfd *abfd,
-		  char *info_ptr, struct dwarf2_cu *cu)
+		  char *info_ptr, struct dwarf2_cu *cu,
+                  unsigned int *line_offset)
 {
   unsigned int bytes_read, i;
   struct attribute attr;
@@ -4861,6 +4965,9 @@ read_partial_die (struct partial_die_inf
 	    part_die->sibling = dwarf2_per_objfile->info_buffer
 	      + dwarf2_get_ref_die_offset (&attr, cu);
 	  break;
+        case DW_AT_stmt_list:
+          if (line_offset != NULL)
+            *line_offset = DW_UNSND (&attr);
 	default:
 	  break;
 	}
@@ -5801,13 +5908,23 @@ check_cu_functions (CORE_ADDR address, s
   return fn->lowpc;
 }
 
-/* Decode the line number information for the compilation unit whose
-   line number info is at OFFSET in the .debug_line section.
-   The compilation directory of the file is passed in COMP_DIR.  */
+/* 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.
+   
+   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
+      builds all the associated partial symbol tables.  In this case,
+      the value of COMP_DIR is ignored, and can thus be NULL.  */
 
 static void
 dwarf_decode_lines (struct line_header *lh, char *comp_dir, bfd *abfd,
-		    struct dwarf2_cu *cu)
+		    struct dwarf2_cu *cu, struct partial_symtab *pst)
 {
   char *line_ptr;
   char *line_end;
@@ -5816,6 +5933,40 @@ dwarf_decode_lines (struct line_header *
   CORE_ADDR baseaddr;
   struct objfile *objfile = cu->objfile;
 
+  /* When decoding the Line Number Program for the purpose of building
+     the partial symtabs included by the current CU, we need to do
+     the following:
+
+     We first scan the Line Header.  It contains a list of files referenced
+     by the Line Number Program.  We then scan the Line Number Program
+     for opcodes changing the source file.  For each file selected in
+     the program, we mark it as included using the FILE_IS_INCLUDED
+     array. Once we're finished scanning the Line Number Program, we can
+     then iterate over FILE_IS_INCLUDED and create a corresponding
+     include partial symtab for each file that was marked as included.  */
+
+  /* FILE_IS_INCLUDED is only used when we're creating the psymtabs
+     for the files included by the current unit.  
+
+     It is an array allocated on the heap, which size is stored in
+     SIZE_OF_FILE_IS_INCLUDED.  Each element of this array corresponds
+     to the file of the same index in the Line Header, as stored in
+     the line_header struct we built for the current unit.  Each element
+     is initially set to zero, and then to nonzero if the corresponding
+     file is included.  The size of this array may be larger than
+     necessary, and the number of meaningful entries is stored in
+     lh->num_file_names.  */
+  char *file_is_included = NULL;
+  int size_of_file_is_included = 0;
+  const int decode_for_pst_p = (pst != NULL);
+
+  if (decode_for_pst_p)
+    {
+      file_is_included = xmalloc (lh->file_names_size * sizeof (char));
+      memset (file_is_included, 0, lh->file_names_size * sizeof (char));
+      size_of_file_is_included = lh->file_names_size;
+    }
+
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
   line_ptr = lh->statement_program_start;
@@ -5833,9 +5984,9 @@ dwarf_decode_lines (struct line_header *
       int basic_block = 0;
       int end_sequence = 0;
 
-      /* Start a subfile for the current file of the state machine.  */
-      if (lh->num_file_names >= file)
+      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.  */
@@ -5860,9 +6011,12 @@ dwarf_decode_lines (struct line_header *
 	      address += (adj_opcode / lh->line_range)
 		* lh->minimum_instruction_length;
 	      line += lh->line_base + (adj_opcode % lh->line_range);
-	      /* append row to matrix using current values */
-	      record_line (current_subfile, line, 
-	                   check_cu_functions (address, cu));
+              if (!decode_for_pst_p)
+                {
+	          /* append row to matrix using current values */
+	          record_line (current_subfile, line, 
+	                       check_cu_functions (address, cu));
+                }
 	      basic_block = 1;
 	    }
 	  else switch (op_code)
@@ -5875,7 +6029,8 @@ dwarf_decode_lines (struct line_header *
 		{
 		case DW_LNE_end_sequence:
 		  end_sequence = 1;
-		  record_line (current_subfile, 0, address);
+                  if (!decode_for_pst_p)
+		    record_line (current_subfile, 0, address);
 		  break;
 		case DW_LNE_set_address:
 		  address = read_address (abfd, line_ptr, cu, &bytes_read);
@@ -5899,6 +6054,20 @@ dwarf_decode_lines (struct line_header *
                       read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
                     line_ptr += bytes_read;
                     add_file_name (lh, cur_file, dir_index, mod_time, length);
+
+                    if (decode_for_pst_p)
+                      {
+                        if (size_of_file_is_included != lh->file_names_size)
+                          {
+                            /* The add_file_name() operation above caused
+                               the file_names array size in the line_header
+                               struct to be increased.  Increase our
+                               file_is_included array size accordingly.  */
+                            file_is_included = xrealloc (file_is_included,
+                                                         lh->file_names_size);
+                          }
+                        file_is_included [lh->num_file_names - 1] = 0;
+                      }
                   }
 		  break;
 		default:
@@ -5908,8 +6077,9 @@ dwarf_decode_lines (struct line_header *
 		}
 	      break;
 	    case DW_LNS_copy:
-	      record_line (current_subfile, line, 
-	                   check_cu_functions (address, cu));
+              if (!decode_for_pst_p)
+	        record_line (current_subfile, line, 
+	                     check_cu_functions (address, cu));
 	      basic_block = 0;
 	      break;
 	    case DW_LNS_advance_pc:
@@ -5935,7 +6105,10 @@ dwarf_decode_lines (struct line_header *
                   dir = lh->include_dirs[fe->dir_index - 1];
                 else
                   dir = comp_dir;
-                dwarf2_start_subfile (fe->name, dir);
+                if (decode_for_pst_p)
+                  file_is_included[file - 1] = 1;
+                else
+                  dwarf2_start_subfile (fe->name, dir);
               }
 	      break;
 	    case DW_LNS_set_column:
@@ -5972,6 +6145,22 @@ dwarf_decode_lines (struct line_header *
 	      }
 	    }
 	}
+    }
+
+  if (decode_for_pst_p)
+    {
+      int file_index;
+
+      /* Now that we're done scanning the Line Header Program, we can
+         create the psymtab of each included file.  */
+      for (file_index = 0; file_index < lh->num_file_names; file_index++)
+        if (file_is_included[file_index] == 1)
+          {
+            char *include_name = lh->file_names [file_index].name;
+    
+            if (strcmp (include_name, pst->filename) != 0)
+              dwarf2_create_include_psymtab (include_name, pst, objfile);
+          }
     }
 }
 

Re: [RFC/dwarf-2] Add support for included files

[Joel Brobecker]

> If you add a comment above dwarf_decode_lines explaining how it's
> called, and that PST determines whether we're building psymtabs or
> symtabs, we'll call it even.

Understood.

> I just approved Daniel's mondo psymtab construction patch, so one of
> you is going to get in the others' way.  Have fun.

My changes will be quite easy to reproduce, I believe, so I'll let
him go first.

Revised patch coming up...

Thanks!
-- 
Joel

Re: [RFC/dwarf-2] Add support for included files

[Jim Blandy]

Joel Brobecker <brobecker@gnat.com> writes:
> Using the approach you suggest (which I like): We need to define a
> couple of wrapper functions around start_subfile and record_line
> because their profile will not match the profile of our callbacks.
> We'll need 3 callbacks per case (3 for psymtabs, 3 for symtabs).
> We'll also have to use void* pointers if we want to make the context
> generic, which I would prefer to avoid. It's great for heavily reused
> code, but is this worth it for this function.

Maybe not.

If you add a comment above dwarf_decode_lines explaining how it's
called, and that PST determines whether we're building psymtabs or
symtabs, we'll call it even.

I just approved Daniel's mondo psymtab construction patch, so one of
you is going to get in the others' way.  Have fun.

Re: [RFC/dwarf-2] Add support for included files

[Joel Brobecker]

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

> Could you try one more thing for me?

Sure!

> As you noticed, the only two functions that dwarf_decode_lines calls
> to actually record the information it gets are
> buildsym.c:start_subfile (via dwarf2_start_subfile) and
> buildsym.c:record_line.  Could you change dwarf_decode_lines to take
> two function pointers (with an accompanying closure pointer) for those
> two functions?  I guess this is an instance of the 'builder' pattern,
> so maybe name it something appropriately suggestive.  Let
> dwarf_decode_lines continue to call dwarf2_start_subfile directly,
> just passing the builder func and closure along with filename and
> dirname.
> 
> Then, instead of duplicating dwarf_decode_lines, have the existing
> call in read_file_scope and the new call you've added just pass
> different function/closures to it.

I was just wondering, before trying that, if we couldn't maybe do
it in a little simpler way, but non-generic way.

Using the approach you suggest (which I like): We need to define a
couple of wrapper functions around start_subfile and record_line
because their profile will not match the profile of our callbacks.
We'll need 3 callbacks per case (3 for psymtabs, 3 for symtabs).
We'll also have to use void* pointers if we want to make the context
generic, which I would prefer to avoid. It's great for heavily reused
code, but is this worth it for this function.

What we could do instead, is:
  . Add a new parameter: struct partial_symtab *pst
  . If this pst is NULL, then scan for symtabs as before
  . If pst is non NULL, then scan for partial symtabs, and do not
    call the 2 buildsym.c functions.

Just for illustration purposes, I tweaked my previous patch. Attached
is a new patch that shows what it would become. I did this because
even if we end up not choosing this solution, it's still a good
intermediate version on which to implement your suggestion.

I verified that the performance impact on GDB was still pretty small.

If you like it, then I'll review, clean it up and properly submit it
for review. Otherwise, I'll send a new version including your
suggestion.

Thanks,
-- 
Joel

[-- Attachment #2: dw2-includes.diff --]
[-- Type: text/plain, Size: 15090 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.142
diff -u -p -r1.142 dwarf2read.c
--- dwarf2read.c	2 Apr 2004 04:35:46 -0000	1.142
+++ dwarf2read.c	15 Apr 2004 22:02:36 -0000
@@ -583,6 +583,13 @@ static void dwarf2_locate_sections (bfd 
 static void dwarf2_build_psymtabs_easy (struct objfile *, int);
 #endif
 
+static void dwarf2_create_include_psymtab (char *, struct partial_symtab *,
+                                           struct objfile *);
+
+static void dwarf2_build_include_psymtabs (struct dwarf2_cu *,
+                                           const unsigned int,
+                                           struct partial_symtab *);
+
 static void dwarf2_build_psymtabs_hard (struct objfile *, int);
 
 static char *scan_partial_symbols (char *, CORE_ADDR *, CORE_ADDR *,
@@ -629,7 +636,8 @@ static struct abbrev_info *dwarf2_lookup
 						 struct dwarf2_cu *);
 
 static char *read_partial_die (struct partial_die_info *,
-			       bfd *, char *, struct dwarf2_cu *);
+			       bfd *, char *, struct dwarf2_cu *,
+                               unsigned int *);
 
 static char *read_full_die (struct die_info **, bfd *, char *,
 			    struct dwarf2_cu *, int *);
@@ -684,12 +692,15 @@ static struct die_info *die_specificatio
 
 static void free_line_header (struct line_header *lh);
 
+static void add_file_name (struct line_header *, char *, unsigned int,
+                           unsigned int, unsigned int);
+
 static struct line_header *(dwarf_decode_line_header
                             (unsigned int offset,
                              bfd *abfd, struct dwarf2_cu *cu));
 
 static void dwarf_decode_lines (struct line_header *, char *, bfd *,
-				struct dwarf2_cu *);
+				struct dwarf2_cu *, struct partial_symtab *);
 
 static void dwarf2_start_subfile (char *, char *);
 
@@ -1100,6 +1111,65 @@ read_comp_unit_head (struct comp_unit_he
   return info_ptr;
 }
 
+/* Allocate a new partial symtab for file named NAME and mark this new
+   partial symtab as being an include of PST.  */
+
+static void
+dwarf2_create_include_psymtab (char *name, struct partial_symtab *pst,
+                               struct objfile *objfile)
+{
+  struct partial_symtab *subpst = allocate_psymtab (name, objfile);
+
+  subpst->section_offsets = pst->section_offsets;
+  subpst->textlow = 0;
+  subpst->texthigh = 0;
+
+  subpst->dependencies = (struct partial_symtab **)
+    obstack_alloc (&objfile->objfile_obstack,
+                   sizeof (struct partial_symtab *));
+  subpst->dependencies[0] = pst;
+  subpst->number_of_dependencies = 1;
+
+  subpst->globals_offset = 0;
+  subpst->n_global_syms = 0;
+  subpst->statics_offset = 0;
+  subpst->n_static_syms = 0;
+  subpst->symtab = NULL;
+  subpst->read_symtab = pst->read_symtab;
+  subpst->readin = 0;
+
+  /* No private part is necessary for include psymtabs.  This property
+     can be used to differentiate between such include psymtabs and
+     the regular ones.  If it ever happens that a regular psymtab can
+     legitimally have a NULL PST_PRIVATE part, then we'll have to add a
+     dedicated field for that in the dwarf2_pinfo structure.  */
+  PST_PRIVATE (subpst) = NULL;
+}
+
+/* Read the line number information located at LINE_OFFSET,
+   and extract the list of sources files included by the
+   source file represented by PST.  Build an include partial
+   symtab for each of these included files.  */
+
+static void
+dwarf2_build_include_psymtabs (struct dwarf2_cu *cu,
+                               const unsigned int line_offset,
+                               struct partial_symtab *pst)
+{
+  struct objfile *objfile = cu->objfile;
+  bfd *abfd = objfile->obfd;
+  struct line_header *lh;
+
+  lh = dwarf_decode_line_header (line_offset, abfd, cu);
+  if (lh == NULL)
+    return;  /* No linetable, so no includes.  */
+
+  dwarf_decode_lines (lh, NULL, abfd, cu, pst);
+
+  free_line_header (lh);
+}
+
+
 /* Build the partial symbol table by doing a quick pass through the
    .debug_info and .debug_abbrev sections.  */
 
@@ -1168,6 +1238,8 @@ dwarf2_build_psymtabs_hard (struct objfi
     {
       struct cleanup *back_to_inner;
       struct dwarf2_cu cu;
+      unsigned int line_offset = 0;
+
       beg_of_comp_unit = info_ptr;
 
       cu.objfile = objfile;
@@ -1208,7 +1280,7 @@ dwarf2_build_psymtabs_hard (struct objfi
 
       /* Read the compilation unit die */
       info_ptr = read_partial_die (&comp_unit_die, abfd, info_ptr,
-				   &cu);
+				   &cu, &line_offset);
 
       /* Set the language we're debugging */
       set_cu_language (comp_unit_die.language, &cu);
@@ -1269,6 +1341,10 @@ dwarf2_build_psymtabs_hard (struct objfi
       info_ptr = beg_of_comp_unit + cu.header.length 
                                   + cu.header.initial_length_size;
 
+      /* Get the list of files included in the current compilation unit,
+         and build a psymtab for each of them.  */
+      dwarf2_build_include_psymtabs (&cu, line_offset, pst);
+
       do_cleanups (back_to_inner);
     }
   do_cleanups (back_to);
@@ -1300,7 +1376,7 @@ scan_partial_symbols (char *info_ptr, CO
 	 inside the loop.  */
       int info_ptr_updated = 0;
 
-      info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu);
+      info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu, NULL);
 
       /* Anonymous namespaces have no name but have interesting
 	 children, so we need to look at them.  Ditto for anonymous
@@ -1652,7 +1728,7 @@ add_partial_structure (struct partial_di
 	  struct partial_die_info child_pdi;
 
 	  next_child = read_partial_die (&child_pdi, abfd, next_child,
-					 cu);
+					 cu, NULL);
 	  if (!child_pdi.tag)
 	    break;
 	  if (child_pdi.tag == DW_TAG_subprogram)
@@ -1691,7 +1767,7 @@ add_partial_enumeration (struct partial_
   
   while (1)
     {
-      info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu);
+      info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu, NULL);
       if (pdi.tag == 0)
 	break;
       if (pdi.tag != DW_TAG_enumerator || pdi.name == NULL)
@@ -1916,6 +1992,32 @@ psymtab_to_symtab_1 (struct partial_symt
   struct cleanup *back_to;
   struct attribute *attr;
   CORE_ADDR baseaddr;
+  int i;
+
+  for (i = 0; i < pst->number_of_dependencies; i++)
+    if (!pst->dependencies[i]->readin)
+      {
+        /* Inform about additional files that need to be read in.  */
+        if (info_verbose)
+          {
+            fputs_filtered (" ", gdb_stdout);
+            wrap_here ("");
+            fputs_filtered ("and ", gdb_stdout);
+            wrap_here ("");
+            printf_filtered ("%s...", pst->dependencies[i]->filename);
+            wrap_here ("");     /* Flush output */
+            gdb_flush (gdb_stdout);
+          }
+        psymtab_to_symtab_1 (pst->dependencies[i]);
+      }
+
+  if (PST_PRIVATE (pst) == NULL)
+    {
+      /* It's an include file, no symbols to read for it.
+         Everything is in the parent symtab.  */
+      pst->readin = 1;
+      return;
+    }
 
   dwarf2_per_objfile = objfile_data (pst->objfile, dwarf2_objfile_data_key);
 
@@ -2204,7 +2306,7 @@ read_file_scope (struct die_info *die, s
         {
           make_cleanup ((make_cleanup_ftype *) free_line_header,
                         (void *) line_header);
-          dwarf_decode_lines (line_header, comp_dir, abfd, cu);
+          dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL);
         }
     }
 
@@ -4414,11 +4516,14 @@ dwarf2_lookup_abbrev (unsigned int numbe
   return NULL;
 }
 
-/* Read a minimal amount of information into the minimal die structure.  */
+/* Read a minimal amount of information into the minimal die structure.
+   If not NULL, the offset where the Line Number Information data is
+   stored will be saved in LINE_OFFSET.  */
 
 static char *
 read_partial_die (struct partial_die_info *part_die, bfd *abfd,
-		  char *info_ptr, struct dwarf2_cu *cu)
+		  char *info_ptr, struct dwarf2_cu *cu,
+                  unsigned int *line_offset)
 {
   unsigned int abbrev_number, bytes_read, i;
   struct abbrev_info *abbrev;
@@ -4512,6 +4617,9 @@ read_partial_die (struct partial_die_inf
 	    part_die->sibling = dwarf2_per_objfile->info_buffer
 	      + dwarf2_get_ref_die_offset (&attr, cu);
 	  break;
+        case DW_AT_stmt_list:
+          if (line_offset != NULL)
+            *line_offset = DW_UNSND (&attr);
 	default:
 	  break;
 	}
@@ -4527,7 +4635,7 @@ read_partial_die (struct partial_die_inf
 
       spec_ptr = dwarf2_per_objfile->info_buffer
 	+ dwarf2_get_ref_die_offset (&spec_attr, cu);
-      read_partial_die (&spec_die, abfd, spec_ptr, cu);
+      read_partial_die (&spec_die, abfd, spec_ptr, cu, NULL);
       if (spec_die.name)
 	{
 	  part_die->name = spec_die.name;
@@ -5399,7 +5507,7 @@ check_cu_functions (CORE_ADDR address, s
 
 static void
 dwarf_decode_lines (struct line_header *lh, char *comp_dir, bfd *abfd,
-		    struct dwarf2_cu *cu)
+		    struct dwarf2_cu *cu, struct partial_symtab *pst)
 {
   char *line_ptr;
   char *line_end;
@@ -5408,6 +5516,38 @@ dwarf_decode_lines (struct line_header *
   CORE_ADDR baseaddr;
   struct objfile *objfile = cu->objfile;
 
+  /* When decoding the Line Number Program for the purpose of building
+     the partial symtabs included by the current CU, we need to do
+     the following:
+
+     We first scan the Line Header.  It contains a list of files referenced
+     by the Line Number Program.  We then scan the Line Number Program
+     for opcodes changing the source file.  For each file selected in
+     the program, we mark it as included using the FILE_IS_INCLUDED
+     array. Once we're finished scanning the Line Number Program, we can
+     then iterate over FILE_IS_INCLUDED and create a corresponding
+     include partial symtab for each file that was marked as included.
+     */
+
+  /* FILE_IS_INCLUDED is an array allocated on the heap, which size
+     is stored in FILE_IS_INCLUDED_SIZE.  Each element of this array
+     corresponds to the file of the same index in the Line Header,
+     as stored in the line_header struct we built for the current unit.
+     Each element is initially set to zero, and then to nonzero if the
+     corresponding file is included.  The size of this array may be
+     larger than necessary, and the number of meaningful entries is
+     stored in lh->num_file_names.  */
+  char *file_is_included = NULL;
+  int file_is_included_size = 0;
+  const int decode_for_pst_p = (pst != NULL);
+
+  if (decode_for_pst_p)
+    {
+      file_is_included = xmalloc (lh->file_names_size * sizeof (char));
+      memset (file_is_included, 0, lh->file_names_size * sizeof (char));
+      file_is_included_size = lh->file_names_size;
+    }
+
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
   line_ptr = lh->statement_program_start;
@@ -5425,9 +5565,9 @@ dwarf_decode_lines (struct line_header *
       int basic_block = 0;
       int end_sequence = 0;
 
-      /* Start a subfile for the current file of the state machine.  */
-      if (lh->num_file_names >= file)
+      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.  */
@@ -5452,9 +5592,12 @@ dwarf_decode_lines (struct line_header *
 	      address += (adj_opcode / lh->line_range)
 		* lh->minimum_instruction_length;
 	      line += lh->line_base + (adj_opcode % lh->line_range);
-	      /* append row to matrix using current values */
-	      record_line (current_subfile, line, 
-	                   check_cu_functions (address, cu));
+              if (!decode_for_pst_p)
+                {
+	          /* append row to matrix using current values */
+	          record_line (current_subfile, line, 
+	                       check_cu_functions (address, cu));
+                }
 	      basic_block = 1;
 	    }
 	  else switch (op_code)
@@ -5467,7 +5610,8 @@ dwarf_decode_lines (struct line_header *
 		{
 		case DW_LNE_end_sequence:
 		  end_sequence = 1;
-		  record_line (current_subfile, 0, address);
+                  if (!decode_for_pst_p)
+		    record_line (current_subfile, 0, address);
 		  break;
 		case DW_LNE_set_address:
 		  address = read_address (abfd, line_ptr, cu, &bytes_read);
@@ -5491,6 +5635,20 @@ dwarf_decode_lines (struct line_header *
                       read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
                     line_ptr += bytes_read;
                     add_file_name (lh, cur_file, dir_index, mod_time, length);
+
+                    if (decode_for_pst_p)
+                      {
+                        if (file_is_included_size != lh->file_names_size)
+                          {
+                            /* The add_file_name() operation above caused
+                               the file_names array size in the line_header
+                               struct to be increased.  Increase our
+                               file_is_included array size accordingly.  */
+                            file_is_included = xrealloc (file_is_included,
+                                                         lh->file_names_size);
+                          }
+                        file_is_included [lh->num_file_names - 1] = 0;
+                      }
                   }
 		  break;
 		default:
@@ -5500,8 +5658,9 @@ dwarf_decode_lines (struct line_header *
 		}
 	      break;
 	    case DW_LNS_copy:
-	      record_line (current_subfile, line, 
-	                   check_cu_functions (address, cu));
+              if (!decode_for_pst_p)
+	        record_line (current_subfile, line, 
+	                     check_cu_functions (address, cu));
 	      basic_block = 0;
 	      break;
 	    case DW_LNS_advance_pc:
@@ -5527,7 +5686,10 @@ dwarf_decode_lines (struct line_header *
                   dir = lh->include_dirs[fe->dir_index - 1];
                 else
                   dir = comp_dir;
-                dwarf2_start_subfile (fe->name, dir);
+                if (decode_for_pst_p)
+                  file_is_included[file - 1] = 1;
+                else
+                  dwarf2_start_subfile (fe->name, dir);
               }
 	      break;
 	    case DW_LNS_set_column:
@@ -5564,6 +5726,22 @@ dwarf_decode_lines (struct line_header *
 	      }
 	    }
 	}
+    }
+
+  if (decode_for_pst_p)
+    {
+      int file_index;
+
+      /* Now that we're done scanning the Line Header Program, we can
+         create the psymtab of each included file.  */
+      for (file_index = 0; file_index < lh->num_file_names; file_index++)
+        if (file_is_included[file_index] == 1)
+          {
+            char *include_name = lh->file_names [file_index].name;
+    
+            if (strcmp (include_name, pst->filename) != 0)
+              dwarf2_create_include_psymtab (include_name, pst, objfile);
+          }
     }
 }
 

Re: [RFC/dwarf-2] Add support for included files

[Jim Blandy]

Okay, let's take it as established that the speed impact is not too
bad.  Given the improved accuracy, I think it's worth taking the
chance that we'll have to revisit it.

Could you try one more thing for me?

As you noticed, the only two functions that dwarf_decode_lines calls
to actually record the information it gets are
buildsym.c:start_subfile (via dwarf2_start_subfile) and
buildsym.c:record_line.  Could you change dwarf_decode_lines to take
two function pointers (with an accompanying closure pointer) for those
two functions?  I guess this is an instance of the 'builder' pattern,
so maybe name it something appropriately suggestive.  Let
dwarf_decode_lines continue to call dwarf2_start_subfile directly,
just passing the builder func and closure along with filename and
dirname.

Then, instead of duplicating dwarf_decode_lines, have the existing
call in read_file_scope and the new call you've added just pass
different function/closures to it.

If it's easy, re-run your speed tests to make sure this doesn't make
some mysterious difference.

Re: [RFC/dwarf-2] Add support for included files

[Joel Brobecker]

> Picking up a dropped thread from January:
> 
> http://sources.redhat.com/ml/gdb-patches/2004-01/msg00015.html

Thank you Jim.

> Some comments:
> 
> It's fine for GDB core code to assume that the psymtabs mention every
> source file in the program.  They're supposed to.  The whole purpose
> of the psymtabs is to act as an index for the full symbol information,
> so that without taking the time/memory to read full symbols, we can at
> least find the compilation unit we do need to read.  If that index
> doesn't mention a source file, then it's incomplete.

Cool.

> I'm not too enthusiastic about Eli's suggestion that we put off
> scanning for source file names until we get a filename we can't find.
> It would help: we can find source files most of the time already, and
> when the second scan was needed, it wouldn't be any more work than
> what Joel's proposing we do every time.  But it's adding a third phase
> to debug reading; I'd prefer to fix this problem without architectural
> changes like that, if it's practical.

That's my feeling too.

> The original motivation for introducing psymtabs at all was speed:
> GDB's startup time was, believe it or not, CPU-bound.  By constructing
> only partial symtabs at startup time, we avoided parsing types
> (essentially).  How does this patch affect the time GDB takes to start
> up on itself?  (If you had some monster program lying around, that
> would be better, but if all we've got is a brute...)

Unfortunately, I don't have a monster program. I did some experimenting
with GDB, though. Here is how I did it: Measuring the total elapsed time
it takes GDB in batch mode to load the symbols and then exit, and did
it ten times in a row inside a for loop. I measured the time difference
between an unmodified GDB and a modified one. Without the change, I
get a total elapsed time for the 10 consecutive runs of 9.453 secs.
After my change, I get a total time of 9.604 secs. That gives us a
rough increase of 1.6%, or said differently, an increase of startup
time of 1 sec for each minute. That's all supposing that GDB is your
fairly typical program.

If somebody could send me a large binary, something like mozilla
all built with dwarf-2 for instance, I'd be happy to rerun my
experiment.

> If it makes a big speed difference, then let's try ignoring
> DW_LNE_define_file, not scanning the line number program, and just
> entering the files given in the file_names table as psymtabs.  GCC
> doesn't generate DW_LNE_define_file.  If that is much faster, then I
> think it'll be okay.

I tried that, but unfortunately, I've seen cases where the compiler
emits an entry for an included file, but then never mentions it in
the line table. For instance with an Ada program, I see that the
compiler includes an entry for file "system.ads" (not sure why,
is it because it's in the direct closure of my program?) in the Line
Number Program Header, but then never uses it to provide line numbers.
The latter is expected, since this file is a runtime file containing
only declarations, so no statements causing code to be generated.

If we were to skip parsing the line table, we would end up creating
a psymtab for that system.ads file, and I think that it could cause
an internal error later down the road when we try to promote that
psymtab to a full symtab. The current code, I think, is not prepared
for that situation. I can give it a try, but that's what I remember from
my analysis of the current code back when I started working on this.

I think it would be reasonable to argue that mentioning a file in the
header without using it in the line table is a compiler bug. I am not
sure about this as the spec is vague, but I wouldn't certainly object.
However, if we decide that yes it is a compiler bug, and yes we should
create the psymtab only based on the line table header, then we also
have to accept the fact that we'll still have the problem should the
compiler decide to rely on DW_LNE_define_file rather than the header to
build the list of included files.  Given the small startup time increase
that I have measured so far, I think it's fine to scan the line table.

But I am happy any way. I hope I have provided sufficient information
for us to make a decision.

> (It's interesting to note that, since DW_LNE_define_file exists, the
> mechanisms Dwarf 3 provides for "accelerated access" are insufficient:
> there's no way to build a complete index of source file names without
> reading the line tables.)

I really wondered about the usefulness of this opcode. Now, I am
wondering even more...

-- 
Joel

Re: [RFC/dwarf-2] Add support for included files

[Jim Blandy]

Picking up a dropped thread from January:

http://sources.redhat.com/ml/gdb-patches/2004-01/msg00015.html

Some comments:

It's fine for GDB core code to assume that the psymtabs mention every
source file in the program.  They're supposed to.  The whole purpose
of the psymtabs is to act as an index for the full symbol information,
so that without taking the time/memory to read full symbols, we can at
least find the compilation unit we do need to read.  If that index
doesn't mention a source file, then it's incomplete.

I'm not too enthusiastic about Eli's suggestion that we put off
scanning for source file names until we get a filename we can't find.
It would help: we can find source files most of the time already, and
when the second scan was needed, it wouldn't be any more work than
what Joel's proposing we do every time.  But it's adding a third phase
to debug reading; I'd prefer to fix this problem without architectural
changes like that, if it's practical.

The original motivation for introducing psymtabs at all was speed:
GDB's startup time was, believe it or not, CPU-bound.  By constructing
only partial symtabs at startup time, we avoided parsing types
(essentially).  How does this patch affect the time GDB takes to start
up on itself?  (If you had some monster program lying around, that
would be better, but if all we've got is a brute...)

If it makes a big speed difference, then let's try ignoring
DW_LNE_define_file, not scanning the line number program, and just
entering the files given in the file_names table as psymtabs.  GCC
doesn't generate DW_LNE_define_file.  If that is much faster, then I
think it'll be okay.

(It's interesting to note that, since DW_LNE_define_file exists, the
mechanisms Dwarf 3 provides for "accelerated access" are insufficient:
there's no way to build a complete index of source file names without
reading the line tables.)

Re: [RFC/dwarf-2] Add support for included files

[Joel Brobecker]

> Want to create a "testsuite/gdb.ada/" directory with this as the first 
> test case?  I think gdb.objc would serve as a good model as that code 
> knows how to skip things when there isn't an ada compiler.

That's a good idea. I will do that too (sending a "patch" first, of
course).

-- 
Joel

Re: [RFC/dwarf-2] Add support for included files

[Andrew Cagney]

Joel,

Want to create a "testsuite/gdb.ada/" directory with this as the first 
test case?  I think gdb.objc would serve as a good model as that code 
knows how to skip things when there isn't an ada compiler.

Andrew

Re: [RFC/dwarf-2] Add support for included files

[Joel Brobecker]

> I made that suggestion because it sounded like the addition you made
> caused some percepted slow-down of the psymtab scan.  If that is not
> true, consider my reservations to be withdrawn.

My original message probably wasn't clear. My concern was not exactly
about performance, but rather code duplication.  Currently, the task
of scanning the symbols in the .debug_info data to build the psymtabs
is simplified by following the DW_AT_sibling attribute. This attribute
is not guaranteed to be always generated when convenient for the debugger,
but it helps when it is there.

In the .debug_line data, there are no such links. You really have
to read byte after byte to interpret the data, and the number of
bytes to read depends on the instruction. If you screw up at some
point, you will end up slightly off and will therefore necessarily
screw up the rest of the program (or you even fail to interpret it).

My prototype patch almost duplicates a piece of code that's not really
that complex, but the real problem is that it doesn't know how to
recover from an error. I was trying to describe this issue for which
I didn't see any simple solution, except doing some surgery in the
function we currently have that reads that data and stores it into
the full symtab.

Hmmm, maybe the best approach to mitigate my concerns would be
to actually make dwarf_decode_lines() a bit more general. Right
now, its purpose is limited to storing the data it read directly into
the appropriate symtabs. Modifying it to allow the us to build both
symtabs but also psymtabs would make it possible to share this code.

Hmmm, could I even keep it as it is?  The only parts that tie this
function to symtab building seem to only be from using start_subfile()
and record_line() from buildsym.c. So maybe we can reuse this function
directly as is: it would build subfiles, and then we could dig out the
necessary information from there to build the psymtabs?  I even believe
we could save these subfiles in the private part of the "main" psymtab
so we don't have to rescan these tables again during the psymtab to
symtab conversion?

Sounds like a good idea, doesn't it? Before I start experimenting, 
does anybody know buildsym.c enough to tell me if it would be ok to
use the functions above outside of the context of building symtabs,
and then to save the subfiles chain for later use during the psymtab
to symtab conversion?

Thanks,
-- 
Joel

Re: [RFC/dwarf-2] Add support for included files

[Eli Zaretskii]

> Date: Sat, 3 Jan 2004 15:42:50 +0100
> From: Joel Brobecker <brobecker@gnat.com>
> 
> Eli Zaretskii wrote:
> > How about if we only do that scan when the file name is not found in
> > the partial symbols, i.e. just before GDB is about to give up and
> > report the file as nonexistent?  Assuming that the cases you have in
> > mind are rare, this would mean faster operation in most cases.
> 
> I am a bit relunctant to go that way, because I think the current
> approach of using a half-baked psymtabs to hold include files is
> a bit too adhoc for my taste. Adding an extra step after having scanned
> the psymtab list to iterate over all objfiles, and re-partially scan the
> debugging information seems to be going one step further in
> ``legitimizing'' this adhoc approach.

I made that suggestion because it sounded like the addition you made
caused some percepted slow-down of the psymtab scan.  If that is not
true, consider my reservations to be withdrawn.

In other words, I would also like to see some measurements, as you
say:

> It seems a bit of effort for a gain that we haven't measured.
> I would suggest going the simple way first and re-think it if
> the performance becomes noticeably worse. If somebody has a large
> app like mozilla that he can use to do some measurement, I would
> sure appreciate how much slower it is to startup with the patch
> I posted.

Perhaps even a not-so-behemoth example would do fine.

Re: [RFC/dwarf-2] Add support for included files

[Joel Brobecker]

On Fri, Jan 02, 2004 at 09:18:02AM -0500, Daniel Jacobowitz wrote:
> Well, the question is whether this works well enough.  Things _are_ in
> these files - DW_AT_decl_file points to them.  Do we need to also put
> things into the proper psymtabs?

At this point, I don't see anything else that we need.

> Otherwise the approach seems reasonable.

Thanks. I can submit a proper patch soon, but there is Eli's suggestion,
though:

Eli Zaretskii wrote:
> How about if we only do that scan when the file name is not found in
> the partial symbols, i.e. just before GDB is about to give up and
> report the file as nonexistent?  Assuming that the cases you have in
> mind are rare, this would mean faster operation in most cases.

I am a bit relunctant to go that way, because I think the current
approach of using a half-baked psymtabs to hold include files is
a bit too adhoc for my taste. Adding an extra step after having scanned
the psymtab list to iterate over all objfiles, and re-partially scan the
debugging information seems to be going one step further in
``legitimizing'' this adhoc approach. If I were to add this extra step,
I would rather introduce a separate data structure for include files,
and then we could build the list of include files on-the-fly as you
suggest when scanning this list.

Trouble is, we may have also made some assumptions in several
places of GDB's code that the psymtab list contains all source
files. I suspect that the extra step you are suggesting will not
affect most of the instances of the code scaning the psymtab list,
so only a few of them will in fact need to be updated. But I'd rather
fix the build_psymtab phase.

It seems a bit of effort for a gain that we haven't measured.
I would suggest going the simple way first and re-think it if
the performance becomes noticeably worse. If somebody has a large
app like mozilla that he can use to do some measurement, I would
sure appreciate how much slower it is to startup with the patch
I posted.

(Eli, I don't know about C++, but you have a good point about lex & yacc
code) 

Let me know what you all think.

Thanks,
-- 
Joel

Re: [RFC/dwarf-2] Add support for included files

[Eli Zaretskii]

> Date: Fri, 2 Jan 2004 08:25:00 +0100
> From: Joel Brobecker <brobecker@gnat.com>
> 
> This capability can be reproduced in C as well, using #include.

Doesn't this happen with C++ programs (which have code fragments on
header files) as well?  What about Yacc- and Lex-processed code?

> If we want to support the case above, I don't see any other way but to
> scan the line table as well.

How about if we only do that scan when the file name is not found in
the partial symbols, i.e. just before GDB is about to give up and
report the file as nonexistent?  Assuming that the cases you have in
mind are rare, this would mean faster operation in most cases.

Re: [RFC/dwarf-2] Add support for included files

[Daniel Jacobowitz]

On Fri, Jan 02, 2004 at 08:25:00AM +0100, Joel Brobecker wrote:
> We have noticed that, if compiled with dwarf-2, GDB is unable to
> set a breakpoint inside hello.c using the file:lineno syntax:
> 
>         % gcc -gdwarf-2 foo.c -o foo
>         % gdb foo
>         (gdb) b hello.c:4
>         No source file named hello.c.
>         (gdb) list hello.c:4
>         No source file named hello.c.
> 
> However, we can set the breakpoint using the function name:
> 
>         (gdb) b say_hello
>         Breakpoint 1 at 0x804833e: file hello.c, line 4.
> 
> And then once this breakpoint is set, we can now insert the
> breakpoint using the source location:
> 
>         (gdb) b hello.c:4
>         Note: breakpoint 1 also set at pc 0x804833e.
>         Breakpoint 2 at 0x804833e: file hello.c, line 4.
>         (gdb) list hello.c:4
>         1       void
>         2       say_hello (void)
>         3       {
>         4         printf ("Hello world.\n");
>         5       }

Yes... I've noticed this several times recently.

> In Dwarf-2, the list of included files is linked to the line table.
> First, we have "Line Number Program Header" which contains the "file
> names" table. And then the program itself which uses the DW_LNS_set_file
> opcode to change from file to file. There is also the DW_LNE_defile_file
> opcode that can be used to define new files instead of using the "file
> names" table.

You keep saying that.  I don't think it means what you think it means
:)  (Hint: DW_LNE_define_file, not DW_LNE_defile_file.)

> Basically, I just copy/pasted the code from dwarf_decode_lines(),
> simplified it for partial symtab processing, and then called it
> right after the compilation unit psymtab has been built. Currently,
> the function scans the line number program, and records which files
> of the "file names" table have really been included in the line program
> (that's the "file_included" array).
> 
> Here is a patch against GDB 6.0 that I used as a proof of concept.
> It does not handle the case where the compiler uses DW_LNE_defile_file
> instead of the the "file names" table yet, but that will be taken care
> of.  Would that be an acceptable approach to suggest for inclusion?

Well, the question is whether this works well enough.  Things _are_ in
these files - DW_AT_decl_file points to them.  Do we need to also put
things into the proper psymtabs?

Otherwise the approach seems reasonable.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

[RFC/dwarf-2] Add support for included files

[Joel Brobecker]

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

Hello,

Ada95 defines the notion of "separates", which allows a developper
to write the implementation of either a function or a package in
a separate file. Here is a small example:

foo.ads:

        package Foo is
           procedure Proc;
        end Foo;

foo.adb:

        Package body Foo is
           procedure Proc is separate;
                          ^^^^^^^^^^^
        End Foo;

And then the body for procedure "Proc", as a separate in foo-proc.adb:

        separate (Foo)
        ^^^^^^^^^^^^^^
        
        procedure Proc is
        begin
           null;
        end Proc;

This capability can be reproduced in C as well, using #include.
The following example is somewhat ugly with respect to the usual
coding practices, but does reproduce well the capability above:

hello.c:

        void
        say_hello (void)
        {
          printf ("Hello world.\n");
        }

foo.c:

        #include <stdio.h>
        
        #include "hello.c"    <--- Reproduces the "is separate"...
        
        int
        main (void)
        {
          say_hello ();
          return 0;
        }

We have noticed that, if compiled with dwarf-2, GDB is unable to
set a breakpoint inside hello.c using the file:lineno syntax:

        % gcc -gdwarf-2 foo.c -o foo
        % gdb foo
        (gdb) b hello.c:4
        No source file named hello.c.
        (gdb) list hello.c:4
        No source file named hello.c.

However, we can set the breakpoint using the function name:

        (gdb) b say_hello
        Breakpoint 1 at 0x804833e: file hello.c, line 4.

And then once this breakpoint is set, we can now insert the
breakpoint using the source location:

        (gdb) b hello.c:4
        Note: breakpoint 1 also set at pc 0x804833e.
        Breakpoint 2 at 0x804833e: file hello.c, line 4.
        (gdb) list hello.c:4
        1       void
        2       say_hello (void)
        3       {
        4         printf ("Hello world.\n");
        5       }

In Dwarf-2, the list of included files is linked to the line table.
First, we have "Line Number Program Header" which contains the "file
names" table. And then the program itself which uses the DW_LNS_set_file
opcode to change from file to file. There is also the DW_LNE_defile_file
opcode that can be used to define new files instead of using the "file
names" table.

Our problem here is that we don't process this line table when building
the partial symbols. Hence, we miss the list of included files, and
therefore do not create any partial symtab for them. The problem does
not exist with stabs because the N_SOL and line number information
are embedded inside the rest of the stabs data.

If we want to support the case above, I don't see any other way but to
scan the line table as well. I suggest a fast scan, although I don't
see a simple way to skip the opcodes and their data that are ignored
during our scan without having a good knowledge of the opcode we are
skipping. This makes the function that does the quick scan of the line
table almost as fat as the one that is used to build the GDB line
table, almost a copy except that we ignore most of what we read.

Basically, I just copy/pasted the code from dwarf_decode_lines(),
simplified it for partial symtab processing, and then called it
right after the compilation unit psymtab has been built. Currently,
the function scans the line number program, and records which files
of the "file names" table have really been included in the line program
(that's the "file_included" array).

Here is a patch against GDB 6.0 that I used as a proof of concept.
It does not handle the case where the compiler uses DW_LNE_defile_file
instead of the the "file names" table yet, but that will be taken care
of.  Would that be an acceptable approach to suggest for inclusion?

2004-01-02  Joel Brobecker  <brobecker@gnat.com>

        * dwarf2read.c (read_partial_die): Add new parameter.
        (scan_partial_symbols): Update call to read_partial_die.
        (dwarf2_add_include_psymtab): New function.
        (dwarf2_build_include_psymtabs): New function.
        (dwarf2_build_psymtabs_hard): Build the psymtabs for the
        included files as well.
        (psymtab_to_symtab_1): Build the symtab of all dependencies
        as well.

Thank you.
-- 
Joel

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

Index: dwarf2read.c
===================================================================
RCS file: /nile.c/cvs/Dev/gdb/gdb-6.0/gdb/dwarf2read.c,v
retrieving revision 1.5
diff -u -p -r1.5 dwarf2read.c
--- dwarf2read.c	19 Nov 2003 18:02:23 -0000	1.5
+++ dwarf2read.c	2 Jan 2004 07:10:08 -0000
@@ -693,7 +693,8 @@ static struct abbrev_info *dwarf2_lookup
 
 static char *read_partial_die (struct partial_die_info *,
 			       bfd *, char *,
-			       const struct comp_unit_head *);
+			       const struct comp_unit_head *,
+                               unsigned int *);
 
 static char *read_full_die (struct die_info **, bfd *, char *,
 			    const struct comp_unit_head *);
@@ -1196,6 +1197,195 @@ read_comp_unit_head (struct comp_unit_he
   return info_ptr;
 }
 
+static void
+dwarf2_add_include_psymtab (char *name,
+                            struct partial_symtab *pst,
+                            struct objfile *objfile)
+{
+  struct partial_symtab *subpst = allocate_psymtab (name, objfile);
+
+  /* Copy the private data from the main psymtab. */
+  subpst->section_offsets = pst->section_offsets;
+  PST_PRIVATE (subpst) =
+    (char *) obstack_alloc (&objfile->psymbol_obstack,
+                            sizeof (struct dwarf2_pinfo));
+  *PST_PRIVATE (subpst) = *PST_PRIVATE (pst);
+
+  subpst->textlow = 0;
+  subpst->texthigh = 0;
+
+  /* We could save slight bits of space by only making one of these,
+     shared by the entire set of include files.  FIXME-someday.  */
+  subpst->dependencies = (struct partial_symtab **)
+    obstack_alloc (&objfile->psymbol_obstack,
+                   sizeof (struct partial_symtab *));
+  subpst->dependencies[0] = pst;
+  subpst->number_of_dependencies = 1;
+
+  subpst->globals_offset = 0;
+  subpst->n_global_syms = 0;
+  subpst->statics_offset = 0;
+  subpst->n_static_syms = 0;
+
+  subpst->readin = 0;
+  subpst->symtab = 0;
+  subpst->read_symtab = pst->read_symtab;
+}
+
+static void
+dwarf2_build_include_psymtabs (struct comp_unit_head *cu_header,
+                               const unsigned int line_offset,
+                               struct partial_symtab *pst,
+                               struct objfile *objfile)
+{
+  bfd *abfd = objfile->obfd;
+  struct line_header *lh;
+  int file_index;
+  char *line_ptr;
+  char *line_end;
+  unsigned int bytes_read;
+  unsigned char op_code, extended_op, adj_opcode; // All used??? FIXME:JOEL
+  char *file_included;
+
+  lh = dwarf_decode_line_header (line_offset, abfd, cu_header);
+
+  if (lh == NULL)
+    return;  /* No includes...  */
+
+  line_ptr = lh->statement_program_start;
+  line_end = lh->statement_program_end;
+
+  /* FIXME: brobecker: Add comments.  */
+  file_included = alloca (lh->num_file_names * sizeof (char));
+  memset (file_included, 0, lh->num_file_names * sizeof (char));
+
+  /* Read the statement sequences until there's nothing left.  */
+  while (line_ptr < line_end)
+    {
+      /* state machine registers  */
+      int end_sequence = 0;
+
+      /* Decode the table. */
+      while (!end_sequence)
+	{
+	  op_code = read_1_byte (abfd, line_ptr);
+	  line_ptr += 1;
+
+	  if (op_code >= lh->opcode_base)
+	    {		/* Special operand.  Nothing else to read.  */
+	    }
+	  else switch (op_code)
+	    {
+	    case DW_LNS_extended_op:
+	      line_ptr += 1;	/* ignore length */
+	      extended_op = read_1_byte (abfd, line_ptr);
+	      line_ptr += 1;
+	      switch (extended_op)
+		{
+		case DW_LNE_end_sequence:
+		  end_sequence = 1;
+		  break;
+		case DW_LNE_set_address:
+		  read_address (abfd, line_ptr, cu_header, &bytes_read);
+		  line_ptr += bytes_read;
+		  break;
+		case DW_LNE_define_file:
+                  {
+                    // char *cur_file;
+                    // unsigned int dir_index, mod_time, length;
+                    
+                    // cur_file = read_string (abfd, line_ptr, &bytes_read);
+                    // line_ptr += bytes_read;
+                    // dir_index =
+                    //   read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+                    // line_ptr += bytes_read;
+                    // mod_time =
+                    //   read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+                    // line_ptr += bytes_read;
+                    // length =
+                    //   read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+                    // line_ptr += bytes_read;
+                    // add_file_name (lh, cur_file, dir_index, mod_time, length);
+                    /* Ignore for now, but the compiler is allowed to
+                       create the list of files via DW_LNE_define_file
+                       entries too.  We will likely need to take this
+                       into account someday.  */
+                    read_string (abfd, line_ptr, &bytes_read);
+                    line_ptr += bytes_read;
+                    read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+                    line_ptr += bytes_read;
+                    read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+                    line_ptr += bytes_read;
+                    read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+                    line_ptr += bytes_read;
+                  }
+		  break;
+		default:
+		  complaint (&symfile_complaints,
+			     "mangled .debug_line section");
+		  return;
+		}
+	      break;
+	    case DW_LNS_copy:
+	      break;
+	    case DW_LNS_advance_pc:
+	      read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+	      line_ptr += bytes_read;
+	      break;
+	    case DW_LNS_advance_line:
+	      read_signed_leb128 (abfd, line_ptr, &bytes_read);
+	      line_ptr += bytes_read;
+	      break;
+	    case DW_LNS_set_file:
+              {
+                /* lh->include_dirs and lh->file_names are 0-based,
+                   but the directory and file name numbers in the
+                   statement program are 1-based.  */
+                unsigned int file;
+
+                file = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+                line_ptr += bytes_read;
+                file_included[file - 1] = 1;
+              }
+	      break;
+	    case DW_LNS_set_column:
+	      read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+	      line_ptr += bytes_read;
+	      break;
+	    case DW_LNS_negate_stmt:
+	      break;
+	    case DW_LNS_set_basic_block:
+	      break;
+	    case DW_LNS_const_add_pc:
+	      break;
+	    case DW_LNS_fixed_advance_pc:
+	      line_ptr += 2;
+	      break;
+	    default:
+	      {  /* Unknown standard opcode, ignore it.  */
+		int i;
+		for (i = 0; i < lh->standard_opcode_lengths[op_code]; i++)
+		  {
+		    (void) read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
+		    line_ptr += bytes_read;
+		  }
+	      }
+	    }
+	}
+    }
+
+  for (file_index = 0; file_index < lh->num_file_names; file_index++)
+    if (file_included[file_index] == 1)
+      {
+        char *include_name = lh->file_names [file_index].name;
+
+        if (strcmp (include_name, pst->filename) != 0)
+          dwarf2_add_include_psymtab (include_name, pst, objfile);
+      }
+
+  free_line_header (lh);
+}
+
 /* Build the partial symbol table by doing a quick pass through the
    .debug_info and .debug_abbrev sections.  */
 
@@ -1262,6 +1452,8 @@ dwarf2_build_psymtabs_hard (struct objfi
   while (info_ptr < dwarf_info_buffer + dwarf_info_size)
     {
       struct comp_unit_head cu_header;
+      unsigned int line_offset = 0;
+
       beg_of_comp_unit = info_ptr;
       info_ptr = read_comp_unit_head (&cu_header, info_ptr, abfd);
 
@@ -1307,7 +1499,7 @@ dwarf2_build_psymtabs_hard (struct objfi
 
       /* Read the compilation unit die */
       info_ptr = read_partial_die (&comp_unit_die, abfd, info_ptr,
-				   &cu_header);
+				   &cu_header, &line_offset);
 
       /* Set the language we're debugging */
       set_cu_language (comp_unit_die.language);
@@ -1373,6 +1565,10 @@ dwarf2_build_psymtabs_hard (struct objfi
 
       info_ptr = beg_of_comp_unit + cu_header.length 
                                   + cu_header.initial_length_size;
+
+      /* Get the list of files included in the current compilation unit,
+         and build a psymtab for each of them.  */
+      dwarf2_build_include_psymtabs (&cu_header, line_offset, pst, objfile);
     }
   do_cleanups (back_to);
 }
@@ -1421,7 +1617,7 @@ scan_partial_symbols (char *info_ptr, st
 
   while (nesting_level)
     {
-      info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu_header);
+      info_ptr = read_partial_die (&pdi, abfd, info_ptr, cu_header, NULL);
 
       /* Anonymous namespaces have no name but are interesting.  */
 
@@ -1677,6 +1873,32 @@ psymtab_to_symtab_1 (struct partial_symt
   struct symtab *symtab;
   struct cleanup *back_to;
   struct attribute *attr;
+  int i;
+
+  for (i = 0; i < pst->number_of_dependencies; i++)
+    if (!pst->dependencies[i]->readin)
+      {
+        /* Inform about additional files that need to be read in.  */
+        if (info_verbose)
+          {
+            fputs_filtered (" ", gdb_stdout);
+            wrap_here ("");
+            fputs_filtered ("and ", gdb_stdout);
+            wrap_here ("");
+            printf_filtered ("%s...", pst->dependencies[i]->filename);
+            wrap_here ("");     /* Flush output */
+            gdb_flush (gdb_stdout);
+          }
+        psymtab_to_symtab_1 (pst->dependencies[i]);
+      }
+
+  if (pst->textlow == 0 && pst->texthigh == 0)
+    {
+      /* It's an include file, no symbols to read for it.
+         Everything is in the parent symtab.  */
+      pst->readin = 1;
+      return;
+    }
 
   /* Set local variables from the partial symbol table info.  */
   offset = DWARF_INFO_OFFSET (pst);
@@ -4001,7 +4223,8 @@ dwarf2_lookup_abbrev (unsigned int numbe
 
 static char *
 read_partial_die (struct partial_die_info *part_die, bfd *abfd,
-		  char *info_ptr, const struct comp_unit_head *cu_header)
+		  char *info_ptr, const struct comp_unit_head *cu_header,
+                  unsigned int *line_offset)
 {
   unsigned int abbrev_number, bytes_read, i;
   struct abbrev_info *abbrev;
@@ -4096,6 +4319,9 @@ read_partial_die (struct partial_die_inf
 	    part_die->sibling =
 	      dwarf_info_buffer + dwarf2_get_ref_die_offset (&attr);
 	  break;
+        case DW_AT_stmt_list:
+          if (line_offset != NULL)
+            *line_offset = DW_UNSND (&attr);
 	default:
 	  break;
 	}
@@ -4111,7 +4337,7 @@ read_partial_die (struct partial_die_inf
       int dummy;
 
       spec_ptr = dwarf_info_buffer + dwarf2_get_ref_die_offset (&spec_attr);
-      read_partial_die (&spec_die, abfd, spec_ptr, cu_header);
+      read_partial_die (&spec_die, abfd, spec_ptr, cu_header, NULL);
       if (spec_die.name)
 	{
 	  part_die->name = spec_die.name;