[patch] dwarf2read.c: Don't read pc/line-number mapping for type units

[patch] dwarf2read.c: Don't read pc/line-number mapping for type units

[Doug Evans]

Hi.

fyi, I'm checking this in.
Type units don't need the pc/line-number mapping,
and this can save a lot of space.
[There's still a fair bit of wasted space, but that's for another day.
This brings the amount down to a tolerable level.]

Joel: I'd also like to check this into the 7.4 branch.
IMO it's safe enough.

[btw, the cleanup handling of the line header here feels clumsy.
It's accomplished as a side-effect of handle_DW_AT_stmt_list.
It feels cleaner to let the caller decide how it wants to clean things up.
That is also left for another day.]

2012-01-09  Doug Evans  <dje@google.com>

	* dwarf2read.c (handle_DW_AT_stmt_list): Add function comment.
	New arg "want_line_info".  All callers updated.
	(read_file_scope,read_type_unit_scope): Move comment from
	handle_DW_AT_stmt_list to here.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.605
diff -u -p -r1.605 dwarf2read.c
--- dwarf2read.c	9 Jan 2012 17:40:05 -0000	1.605
+++ dwarf2read.c	10 Jan 2012 01:30:13 -0000
@@ -5521,19 +5521,19 @@ find_file_and_directory (struct die_info
     *name = "<unknown>";
 }
 
-/* Handle DW_AT_stmt_list for a compilation unit.  */
+/* Handle DW_AT_stmt_list for a compilation unit or type unit.
+   DIE is the DW_TAG_compile_unit or DW_TAG_type_unit die for CU.
+   COMP_DIR is the compilation directory.
+   WANT_LINE_INFO is non-zero if the pc/line-number mapping is needed.  */
 
 static void
 handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
-			const char *comp_dir)
+			const char *comp_dir, int want_line_info)
 {
   struct attribute *attr;
   struct objfile *objfile = cu->objfile;
   bfd *abfd = objfile->obfd;
 
-  /* Decode line number information if present.  We do this before
-     processing child DIEs, so that the line header table is available
-     for DW_AT_decl_file.  */
   attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
   if (attr)
     {
@@ -5545,7 +5545,8 @@ handle_DW_AT_stmt_list (struct die_info 
         {
           cu->line_header = line_header;
           make_cleanup (free_cu_line_header, cu);
-          dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL);
+	  if (want_line_info)
+	    dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL);
         }
     }
 }
@@ -5604,7 +5605,10 @@ read_file_scope (struct die_info *die, s
   record_debugformat ("DWARF 2");
   record_producer (cu->producer);
 
-  handle_DW_AT_stmt_list (die, cu, comp_dir);
+  /* Decode line number information if present.  We do this before
+     processing child DIEs, so that the line header table is available
+     for DW_AT_decl_file.  */
+  handle_DW_AT_stmt_list (die, cu, comp_dir, 1);
 
   /* Process all dies in compilation unit.  */
   if (die->child != NULL)
@@ -5707,7 +5711,11 @@ read_type_unit_scope (struct die_info *d
   record_debugformat ("DWARF 2");
   record_producer (cu->producer);
 
-  handle_DW_AT_stmt_list (die, cu, comp_dir);
+  /* Decode line number information if present.  We do this before
+     processing child DIEs, so that the line header table is available
+     for DW_AT_decl_file.
+     We don't need the pc/line-number mapping for type units.  */
+  handle_DW_AT_stmt_list (die, cu, comp_dir, 0);
 
   /* Process the dies in the type unit.  */
   if (die->child == NULL)

[patch] dwarf2read.c: Don't read pc/line-number mapping for type units

[Doug Evans]

Hi.

fyi, I'm checking this in.
Type units don't need the pc/line-number mapping,
and this can save a lot of space.
[There's still a fair bit of wasted space, but that's for another day.
This brings the amount down to a tolerable level.]

Joel: I'd also like to check this into the 7.4 branch.
IMO it's safe enough.

[btw, the cleanup handling of the line header here feels clumsy.
It's accomplished as a side-effect of handle_DW_AT_stmt_list.
It feels cleaner to let the caller decide how it wants to clean things up.
That is also left for another day.]

2012-01-09  Doug Evans  <dje@google.com>

	* dwarf2read.c (handle_DW_AT_stmt_list): Add function comment.
	New arg "want_line_info".  All callers updated.
	(read_file_scope,read_type_unit_scope): Move comment from
	handle_DW_AT_stmt_list to here.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.605
diff -u -p -r1.605 dwarf2read.c
--- dwarf2read.c	9 Jan 2012 17:40:05 -0000	1.605
+++ dwarf2read.c	10 Jan 2012 01:30:13 -0000
@@ -5521,19 +5521,19 @@ find_file_and_directory (struct die_info
     *name = "<unknown>";
 }
 
-/* Handle DW_AT_stmt_list for a compilation unit.  */
+/* Handle DW_AT_stmt_list for a compilation unit or type unit.
+   DIE is the DW_TAG_compile_unit or DW_TAG_type_unit die for CU.
+   COMP_DIR is the compilation directory.
+   WANT_LINE_INFO is non-zero if the pc/line-number mapping is needed.  */
 
 static void
 handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
-			const char *comp_dir)
+			const char *comp_dir, int want_line_info)
 {
   struct attribute *attr;
   struct objfile *objfile = cu->objfile;
   bfd *abfd = objfile->obfd;
 
-  /* Decode line number information if present.  We do this before
-     processing child DIEs, so that the line header table is available
-     for DW_AT_decl_file.  */
   attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
   if (attr)
     {
@@ -5545,7 +5545,8 @@ handle_DW_AT_stmt_list (struct die_info 
         {
           cu->line_header = line_header;
           make_cleanup (free_cu_line_header, cu);
-          dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL);
+	  if (want_line_info)
+	    dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL);
         }
     }
 }
@@ -5604,7 +5605,10 @@ read_file_scope (struct die_info *die, s
   record_debugformat ("DWARF 2");
   record_producer (cu->producer);
 
-  handle_DW_AT_stmt_list (die, cu, comp_dir);
+  /* Decode line number information if present.  We do this before
+     processing child DIEs, so that the line header table is available
+     for DW_AT_decl_file.  */
+  handle_DW_AT_stmt_list (die, cu, comp_dir, 1);
 
   /* Process all dies in compilation unit.  */
   if (die->child != NULL)
@@ -5707,7 +5711,11 @@ read_type_unit_scope (struct die_info *d
   record_debugformat ("DWARF 2");
   record_producer (cu->producer);
 
-  handle_DW_AT_stmt_list (die, cu, comp_dir);
+  /* Decode line number information if present.  We do this before
+     processing child DIEs, so that the line header table is available
+     for DW_AT_decl_file.
+     We don't need the pc/line-number mapping for type units.  */
+  handle_DW_AT_stmt_list (die, cu, comp_dir, 0);
 
   /* Process the dies in the type unit.  */
   if (die->child == NULL)

Re: [patch] dwarf2read.c: Don't read pc/line-number mapping for type units

[Joel Brobecker]

> Type units don't need the pc/line-number mapping,
> and this can save a lot of space.

Types in general have a sloc, and we sometimes use them. For instance,
trying the following:

        (gdb) ptype ambiguous_type
        Multiple matches for ambiguous_type
        [0] cancel
        [1] pck.ambiguous_type at pck.adb:8
        [2] bar.ambiguous_type at bar.adb:9
        >

Is your patch going to affect the above?

> Joel: I'd also like to check this into the 7.4 branch.
> IMO it's safe enough.

No problem on my end if the concerns above are unfounded.
Although I know I do not know this file as well as you do.
-- 
Joel

Re: [patch] dwarf2read.c: Don't read pc/line-number mapping for type units

[Doug Evans]

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

On Mon, Jan 9, 2012 at 7:51 PM, Joel Brobecker <brobecker@adacore.com> wrote:
>> Type units don't need the pc/line-number mapping,
>> and this can save a lot of space.
>
> Types in general have a sloc, and we sometimes use them. For instance,
> trying the following:
>
>        (gdb) ptype ambiguous_type
>        Multiple matches for ambiguous_type
>        [0] cancel
>        [1] pck.ambiguous_type at pck.adb:8
>        [2] bar.ambiguous_type at bar.adb:9
>        >
>
> Is your patch going to affect the above?

Appended is what I committed to trunk.
I think it's fine.

>> Joel: I'd also like to check this into the 7.4 branch.
>> IMO it's safe enough.
>
> No problem on my end if the concerns above are unfounded.
> Although I know I do not know this file as well as you do.

It's rarely how much I know, it's how much I remember. 1/2 :-)

The memory space reduction is substantial so I'd like to get this in 7.4.
[the reduction only kicks in for .debug_types, btw]

2012-01-10  Doug Evans  <dje@google.com>

        * dwarf2read.c (dwarf_decode_lines): Remove arg "abfd".  New arg
        "want_line_info".  All callers updated.
        (dwarf_decode_lines_1): New function.
        (handle_DW_AT_stmt_list): Add function comment.
        New arg "want_line_info".  All callers updated.
        (read_file_scope,read_type_unit_scope): Move comment from
        handle_DW_AT_stmt_list to here.

[-- Attachment #2: gdb-120110-type-unit-line-info-2.patch.txt --]
[-- Type: text/plain, Size: 7978 bytes --]

2012-01-10  Doug Evans  <dje@google.com>

	* dwarf2read.c (dwarf_decode_lines): Remove arg "abfd".  New arg
	"want_line_info".  All callers updated.
	(dwarf_decode_lines_1): New function.
	(handle_DW_AT_stmt_list): Add function comment.
	New arg "want_line_info".  All callers updated.
	(read_file_scope,read_type_unit_scope): Move comment from
	handle_DW_AT_stmt_list to here.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.605
diff -u -p -r1.605 dwarf2read.c
--- dwarf2read.c	9 Jan 2012 17:40:05 -0000	1.605
+++ dwarf2read.c	10 Jan 2012 21:09:39 -0000
@@ -986,8 +986,9 @@ static struct line_header *(dwarf_decode
                             (unsigned int offset,
                              bfd *abfd, struct dwarf2_cu *cu));
 
-static void dwarf_decode_lines (struct line_header *, const char *, bfd *,
-				struct dwarf2_cu *, struct partial_symtab *);
+static void dwarf_decode_lines (struct line_header *, const char *,
+				struct dwarf2_cu *, struct partial_symtab *,
+				int);
 
 static void dwarf2_start_subfile (char *, const char *, const char *);
 
@@ -3102,7 +3103,7 @@ dwarf2_build_include_psymtabs (struct dw
     return;  /* No linetable, so no includes.  */
 
   /* NOTE: pst->dirname is DW_AT_comp_dir (if present).  */
-  dwarf_decode_lines (lh, pst->dirname, abfd, cu, pst);
+  dwarf_decode_lines (lh, pst->dirname, cu, pst, 1);
 
   free_line_header (lh);
 }
@@ -5521,19 +5522,19 @@ find_file_and_directory (struct die_info
     *name = "<unknown>";
 }
 
-/* Handle DW_AT_stmt_list for a compilation unit.  */
+/* Handle DW_AT_stmt_list for a compilation unit or type unit.
+   DIE is the DW_TAG_compile_unit or DW_TAG_type_unit die for CU.
+   COMP_DIR is the compilation directory.
+   WANT_LINE_INFO is non-zero if the pc/line-number mapping is needed.  */
 
 static void
 handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
-			const char *comp_dir)
+			const char *comp_dir, int want_line_info)
 {
   struct attribute *attr;
   struct objfile *objfile = cu->objfile;
   bfd *abfd = objfile->obfd;
 
-  /* Decode line number information if present.  We do this before
-     processing child DIEs, so that the line header table is available
-     for DW_AT_decl_file.  */
   attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
   if (attr)
     {
@@ -5545,7 +5546,7 @@ handle_DW_AT_stmt_list (struct die_info 
         {
           cu->line_header = line_header;
           make_cleanup (free_cu_line_header, cu);
-          dwarf_decode_lines (line_header, comp_dir, abfd, cu, NULL);
+	  dwarf_decode_lines (line_header, comp_dir, cu, NULL, want_line_info);
         }
     }
 }
@@ -5604,7 +5605,10 @@ read_file_scope (struct die_info *die, s
   record_debugformat ("DWARF 2");
   record_producer (cu->producer);
 
-  handle_DW_AT_stmt_list (die, cu, comp_dir);
+  /* Decode line number information if present.  We do this before
+     processing child DIEs, so that the line header table is available
+     for DW_AT_decl_file.  */
+  handle_DW_AT_stmt_list (die, cu, comp_dir, 1);
 
   /* Process all dies in compilation unit.  */
   if (die->child != NULL)
@@ -5707,7 +5711,11 @@ read_type_unit_scope (struct die_info *d
   record_debugformat ("DWARF 2");
   record_producer (cu->producer);
 
-  handle_DW_AT_stmt_list (die, cu, comp_dir);
+  /* Decode line number information if present.  We do this before
+     processing child DIEs, so that the line header table is available
+     for DW_AT_decl_file.
+     We don't need the pc/line-number mapping for type units.  */
+  handle_DW_AT_stmt_list (die, cu, comp_dir, 0);
 
   /* Process the dies in the type unit.  */
   if (die->child == NULL)
@@ -11046,31 +11054,12 @@ noop_record_line (struct subfile *subfil
   return;
 }
 
-/* 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.
-
-   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.
-
-   COMP_DIR is the compilation directory (DW_AT_comp_dir) or NULL if unknown.
-   It is used for relative paths in the line table.
-   NOTE: When processing partial symtabs (pst != NULL),
-   comp_dir == pst->dirname.
-
-   NOTE: It is important that psymtabs have the same file name (via strcmp)
-   as the corresponding symtab.  Since COMP_DIR is not used in the name of the
-   symtab we don't use it in the name of the psymtabs we create.
-   E.g. expand_line_sal requires this when finding psymtabs to expand.
-   A good testcase for this is mb-inline.exp.  */
+/* Subroutine of dwarf_decode_lines to simplify it.
+   Process the line number information in LH.  */
 
 static void
-dwarf_decode_lines (struct line_header *lh, const char *comp_dir, bfd *abfd,
-		    struct dwarf2_cu *cu, struct partial_symtab *pst)
+dwarf_decode_lines_1 (struct line_header *lh, const char *comp_dir,
+		      struct dwarf2_cu *cu, struct partial_symtab *pst)
 {
   gdb_byte *line_ptr, *extended_end;
   gdb_byte *line_end;
@@ -11078,9 +11067,10 @@ dwarf_decode_lines (struct line_header *
   unsigned char op_code, extended_op, adj_opcode;
   CORE_ADDR baseaddr;
   struct objfile *objfile = cu->objfile;
+  bfd *abfd = objfile->obfd;
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   const int decode_for_pst_p = (pst != NULL);
-  struct subfile *last_subfile = NULL, *first_subfile = current_subfile;
+  struct subfile *last_subfile = NULL;
   void (*p_record_line) (struct subfile *subfile, int line, CORE_ADDR pc)
     = record_line;
 
@@ -11358,6 +11348,41 @@ dwarf_decode_lines (struct line_header *
 	    }
         }
     }
+}
+
+/* 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.
+
+   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.
+
+   COMP_DIR is the compilation directory (DW_AT_comp_dir) or NULL if unknown.
+   It is used for relative paths in the line table.
+   NOTE: When processing partial symtabs (pst != NULL),
+   comp_dir == pst->dirname.
+
+   NOTE: It is important that psymtabs have the same file name (via strcmp)
+   as the corresponding symtab.  Since COMP_DIR is not used in the name of the
+   symtab we don't use it in the name of the psymtabs we create.
+   E.g. expand_line_sal requires this when finding psymtabs to expand.
+   A good testcase for this is mb-inline.exp.  */
+
+static void
+dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
+		    struct dwarf2_cu *cu, struct partial_symtab *pst,
+		    int want_line_info)
+{
+  struct objfile *objfile = cu->objfile;
+  const int decode_for_pst_p = (pst != NULL);
+  struct subfile *first_subfile = current_subfile;
+
+  if (want_line_info)
+    dwarf_decode_lines_1 (lh, comp_dir, cu, pst);
 
   if (decode_for_pst_p)
     {
@@ -11379,13 +11404,12 @@ dwarf_decode_lines (struct line_header *
       /* Make sure a symtab is created for every file, even files
 	 which contain only variables (i.e. no code with associated
 	 line numbers).  */
-
       int i;
-      struct file_entry *fe;
 
       for (i = 0; i < lh->num_file_names; i++)
 	{
 	  char *dir = NULL;
+	  struct file_entry *fe;
 
 	  fe = &lh->file_names[i];
 	  if (fe->dir_index)

Re: [patch] dwarf2read.c: Don't read pc/line-number mapping for type units

[Joel Brobecker]

> Appended is what I committed to trunk.
> I think it's fine.
[...]
> The memory space reduction is substantial so I'd like to get this in 7.4.
> [the reduction only kicks in for .debug_types, btw]
> 
> 2012-01-10  Doug Evans  <dje@google.com>
> 
>         * dwarf2read.c (dwarf_decode_lines): Remove arg "abfd".  New arg
>         "want_line_info".  All callers updated.
>         (dwarf_decode_lines_1): New function.
>         (handle_DW_AT_stmt_list): Add function comment.
>         New arg "want_line_info".  All callers updated.
>         (read_file_scope,read_type_unit_scope): Move comment from
>         handle_DW_AT_stmt_list to here.

If the gain is substantial, and you think it's fine, then no objection
on my end.

-- 
Joel

Re: [patch] dwarf2read.c: Don't read pc/line-number mapping for type units

[Doug Evans]

On Wed, Jan 11, 2012 at 1:04 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> Appended is what I committed to trunk.
>> I think it's fine.
> [...]
>> The memory space reduction is substantial so I'd like to get this in 7.4.
>> [the reduction only kicks in for .debug_types, btw]
>>
>> 2012-01-10  Doug Evans  <dje@google.com>
>>
>>         * dwarf2read.c (dwarf_decode_lines): Remove arg "abfd".  New arg
>>         "want_line_info".  All callers updated.
>>         (dwarf_decode_lines_1): New function.
>>         (handle_DW_AT_stmt_list): Add function comment.
>>         New arg "want_line_info".  All callers updated.
>>         (read_file_scope,read_type_unit_scope): Move comment from
>>         handle_DW_AT_stmt_list to here.
>
> If the gain is substantial, and you think it's fine, then no objection
> on my end.

Thanks.  Committed to the 7.4 branch.