[patch] Allow dummy CUs created by incremental linker

[patch] Allow dummy CUs created by incremental linker

[Cary Coutant]

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

With the new incremental linking support in gold, we leave patch space
in the file at link time, and use that patch space for incremental
updates. If the file has debug info, that means that the debug info
might have "holes" in it. Because the debugger can't deal with
arbitrary holes in the debug info, gold fills the holes with dummy
compilation units, where each dummy CU consists of a compilation unit
header whose length field cover the hole, and the rest of the space
filled with zeroes. This patch updated gdb to tolerate these dummy
CUs, by checking for an empty CU before attempting to read the DIEs.

Tested on x86_64 with the latest gold.

-cary


	* dwarf2read.c (peek_abbrev_code): New function.
	(dw2_get_file_names): Check for dummy compilation units.
	(create_debug_types_hash_table): Likewise.
	(process_psymtab_comp_unit): Likewise.
	(load_partial_comp_unit): Likewise.
	(load_full_comp_unit): Likewise.

[-- Attachment #2: gdb-empty-cu-patch-2.txt --]
[-- Type: text/plain, Size: 3879 bytes --]

2011-10-18  Cary Coutant  <ccoutant@google.com>

	* dwarf2read.c (peek_abbrev_code): New function.
	(dw2_get_file_names): Check for dummy compilation units.
	(create_debug_types_hash_table): Likewise.
	(process_psymtab_comp_unit): Likewise.
	(load_partial_comp_unit): Likewise.
	(load_full_comp_unit): Likewise.


Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.574
diff -u -p -r1.574 dwarf2read.c
--- dwarf2read.c	17 Oct 2011 12:57:14 -0000	1.574
+++ dwarf2read.c	19 Oct 2011 00:00:04 -0000
@@ -936,6 +936,8 @@ static void dwarf2_read_abbrevs (bfd *ab
 
 static void dwarf2_free_abbrev_table (void *);
 
+static unsigned int peek_abbrev_code (bfd *, gdb_byte *);
+
 static struct abbrev_info *peek_die_abbrev (gdb_byte *, unsigned int *,
 					    struct dwarf2_cu *);
 
@@ -2307,6 +2309,14 @@ dw2_get_file_names (struct objfile *objf
 					  buffer, buffer_size,
 					  abfd);
 
+  /* Skip dummy compilation units.  */
+  if (info_ptr >= buffer + buffer_size
+      || peek_abbrev_code (abfd, info_ptr) == 0)
+    {
+      do_cleanups (cleanups);
+      return NULL;
+    }
+
   this_cu->cu = &cu;
   cu.per_cu = this_cu;
 
@@ -3204,6 +3214,14 @@ create_debug_types_hash_table (struct ob
 	  signature = bfd_get_64 (objfile->obfd, ptr);
 	  ptr += 8;
 	  type_offset = read_offset_1 (objfile->obfd, ptr, offset_size);
+	  ptr += 1;
+
+	  /* Skip dummy type units.  */
+	  if (ptr >= end_ptr || peek_abbrev_code (objfile->obfd, ptr) == 0)
+	    {
+	      info_ptr = info_ptr + initial_length_size + length;
+	      continue;
+	    }
 
 	  type_sig = obstack_alloc (&objfile->objfile_obstack, sizeof (*type_sig));
 	  memset (type_sig, 0, sizeof (*type_sig));
@@ -3356,6 +3374,16 @@ process_psymtab_comp_unit (struct objfil
 					  buffer, buffer_size,
 					  abfd);
 
+  /* Skip dummy compilation units.  */
+  if (info_ptr >= buffer + buffer_size
+      || peek_abbrev_code (abfd, info_ptr) == 0)
+    {
+      info_ptr = (beg_of_comp_unit + cu.header.length
+		  + cu.header.initial_length_size);
+      do_cleanups (back_to_inner);
+      return info_ptr;
+    }
+
   cu.list_in_scope = &file_symbols;
 
   /* If this compilation unit was already read in, free the
@@ -3644,6 +3672,15 @@ load_partial_comp_unit (struct dwarf2_pe
 					      dwarf2_per_objfile->info.size,
 					      abfd);
 
+      /* Skip dummy compilation units.  */
+      if (info_ptr >= (dwarf2_per_objfile->info.buffer
+		       + dwarf2_per_objfile->info.size)
+	  || peek_abbrev_code (abfd, info_ptr) == 0)
+	{
+	  do_cleanups (free_cu_cleanup);
+	  return;
+	}
+
       /* Link this compilation unit into the compilation unit tree.  */
       this_cu->cu = cu;
       cu->per_cu = this_cu;
@@ -4256,6 +4293,15 @@ add_partial_enumeration (struct partial_
     }
 }
 
+/* Return the initial uleb128 in the die at INFO_PTR.  */
+
+static unsigned int
+peek_abbrev_code (bfd *abfd, gdb_byte *info_ptr)
+{
+  unsigned int bytes_read;
+  return read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
+}
+
 /* Read the initial uleb128 in the die at INFO_PTR in compilation unit CU.
    Return the corresponding abbrev, or NULL if the number is zero (indicating
    an empty DIE).  In either case *BYTES_READ will be set to the length of
@@ -4640,6 +4686,15 @@ load_full_comp_unit (struct dwarf2_per_c
       /* Read in the comp_unit header.  */
       info_ptr = read_comp_unit_head (&cu->header, info_ptr, abfd);
 
+      /* Skip dummy compilation units.  */
+      if (info_ptr >= (dwarf2_per_objfile->info.buffer
+		       + dwarf2_per_objfile->info.size)
+	  || peek_abbrev_code (abfd, info_ptr) == 0)
+	{
+	  do_cleanups (free_cu_cleanup);
+	  return;
+	}
+
       /* Complete the cu_header.  */
       cu->header.offset = offset;
       cu->header.first_die_offset = info_ptr - beg_of_comp_unit;

Re: [patch] Allow dummy CUs created by incremental linker

[Tom Tromey]

>>>>> "Cary" == Cary Coutant <ccoutant@google.com> writes:

Cary> With the new incremental linking support in gold, we leave patch space
Cary> in the file at link time, and use that patch space for incremental
Cary> updates. If the file has debug info, that means that the debug info
Cary> might have "holes" in it. Because the debugger can't deal with
Cary> arbitrary holes in the debug info, gold fills the holes with dummy
Cary> compilation units, where each dummy CU consists of a compilation unit
Cary> header whose length field cover the hole, and the rest of the space
Cary> filled with zeroes. This patch updated gdb to tolerate these dummy
Cary> CUs, by checking for an empty CU before attempting to read the DIEs.

Thanks.

Cary> +static unsigned int
Cary> +peek_abbrev_code (bfd *abfd, gdb_byte *info_ptr)
Cary> +{
Cary> +  unsigned int bytes_read;
Cary> +  return read_unsigned_leb128 (abfd, info_ptr, &bytes_read);

Our coding style requires a newline after the declarations.

Ok with that change.

Tom

Re: [patch] Allow dummy CUs created by incremental linker

[Cary Coutant]

> Cary> +static unsigned int
> Cary> +peek_abbrev_code (bfd *abfd, gdb_byte *info_ptr)
> Cary> +{
> Cary> +  unsigned int bytes_read;
> Cary> +  return read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
>
> Our coding style requires a newline after the declarations.
>
> Ok with that change.

Done, and committed. Thanks!

-cary

Regression for .debug_types [Re: [patch] Allow dummy CUs created by incremental linker]

[Jan Kratochvil]

On Wed, 19 Oct 2011 17:27:18 +0200, Cary Coutant wrote:
> Done, and committed. Thanks!

99374fd95e942049103b51b4d125c6a4adc38c50 is the first bad commit
commit 99374fd95e942049103b51b4d125c6a4adc38c50
Author: Cary Coutant <ccoutant@google.com>
Date:   Wed Oct 19 15:26:27 2011 +0000

	* dwarf2read.c (peek_abbrev_code): New function.
	(dw2_get_file_names): Check for dummy compilation units.
	(create_debug_types_hash_table): Likewise.
	(process_psymtab_comp_unit): Likewise.
	(load_partial_comp_unit): Likewise.
	(load_full_comp_unit): Likewise.

I did not debug it yet but it is a regression for many testcases iff
.debug_types are used.  Such as:

FAIL: gdb.base/call-ar-st.exp: print print_small_structs (pattern 6)
FAIL: gdb.base/call-ar-st.exp: step into print_long_arg_list
FAIL: gdb.base/call-ar-st.exp: print print_small_structs from print_long_arg_list (pattern 6)
FAIL: gdb.base/call-ar-st.exp: print print_bit_flags_combo from init_bit_flags_combo


FAIL: gcc (GCC) 4.6.2 20111018 (prerelease)
FAIL: PATH="$HOME/redhat/gcc46-root/bin:$PATH" runtest CC_FOR_TARGET="gcc -gdwarf-4 -g0" CXX_FOR _TARGET="g++ -gdwarf-4 -g0" gdb.base/call-ar-st.exp

FAIL: gcc (GCC) 4.7.0 20111018 (experimental)
FAIL: PATH="$HOME/redhat/gcchead-root/bin:$PATH" runtest CC_FOR_TARGET="gcc -gdwarf-4 -fdebug-types-section -g0" CXX_FOR_TARGET="g++ -gdwarf-4 -fdebug-types-section -g0" gdb.base/call-ar-st.exp
PASS: PATH="$HOME/redhat/gcchead-root/bin:$PATH" runtest CC_FOR_TARGET="gcc -gdwarf-4 -fno-debug-types-section -g0" CXX_FOR_TARGET="g++ -gdwarf-4 -fno-debug-types-section -g0" gdb.base/call-ar-st.exp


Thanks,
Jan

Re: Regression for .debug_types [Re: [patch] Allow dummy CUs created by incremental linker]

[Cary Coutant]

> I did not debug it yet but it is a regression for many testcases iff
> .debug_types are used.

Sorry, this patch fixes the problem. In process_psymtab_comp_unit, if
it was scanning a type unit, it was adjusting the info_ptr after my
check for a 0 abbrev code. It turns out that first_die_offset is also
set too early, so I moved the adjustment into
partial_read_comp_unit_head. Running the tests now, with -gdwarf4
explicitly turned on.

-cary


2011-10-20  Cary Coutant  <ccoutant@google.com>

	* dwarf2read.c (dw2_get_file_names): Move adjustment for type
	section to...
	(partial_read_comp_unit_head): ...here.  Add is_debug_type_section
	flag.  Adjust all callers.
	(process_psymtab_comp_unit): Remove adjustment for type section.


Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.576
diff -u -p -r1.576 dwarf2read.c
--- dwarf2read.c	20 Oct 2011 01:11:34 -0000	1.576
+++ dwarf2read.c	20 Oct 2011 18:40:54 -0000
@@ -1336,7 +1336,8 @@ static gdb_byte *partial_read_comp_unit_
 					      gdb_byte *info_ptr,
 					      gdb_byte *buffer,
 					      unsigned int buffer_size,
-					      bfd *abfd);
+					      bfd *abfd,
+					      int is_debug_type_section);

 static void init_cu_die_reader (struct die_reader_specs *reader,
 				struct dwarf2_cu *cu);
@@ -2307,7 +2308,8 @@ dw2_get_file_names (struct objfile *objf

   info_ptr = partial_read_comp_unit_head (&cu.header, info_ptr,
 					  buffer, buffer_size,
-					  abfd);
+					  abfd,
+					  this_cu->debug_type_section != NULL);

   /* Skip dummy compilation units.  */
   if (info_ptr >= buffer + buffer_size
@@ -2323,8 +2325,6 @@ dw2_get_file_names (struct objfile *objf
   dwarf2_read_abbrevs (abfd, &cu);
   make_cleanup (dwarf2_free_abbrev_table, &cu);

-  if (this_cu->debug_type_section)
-    info_ptr += 8 /*signature*/ + cu.header.offset_size;
   init_cu_die_reader (&reader_specs, &cu);
   read_full_die (&reader_specs, &comp_unit_die, info_ptr,
 		 &has_children);
@@ -2971,7 +2971,7 @@ read_comp_unit_head (struct comp_unit_he
 static gdb_byte *
 partial_read_comp_unit_head (struct comp_unit_head *header, gdb_byte *info_ptr,
 			     gdb_byte *buffer, unsigned int buffer_size,
-			     bfd *abfd)
+			     bfd *abfd, int is_debug_type_section)
 {
   gdb_byte *beg_of_comp_unit = info_ptr;

@@ -2979,6 +2979,11 @@ partial_read_comp_unit_head (struct comp

   info_ptr = read_comp_unit_head (header, info_ptr, abfd);

+  /* If we're reading a type unit, skip over the signature and
+     type_offset fields.  */
+  if (is_debug_type_section)
+    info_ptr += 8 /*signature*/ + header->offset_size;
+
   header->first_die_offset = info_ptr - beg_of_comp_unit;

   if (header->version != 2 && header->version != 3 && header->version != 4)
@@ -3372,7 +3377,8 @@ process_psymtab_comp_unit (struct objfil

   info_ptr = partial_read_comp_unit_head (&cu.header, info_ptr,
 					  buffer, buffer_size,
-					  abfd);
+					  abfd,
+					  this_cu->debug_type_section != NULL);

   /* Skip dummy compilation units.  */
   if (info_ptr >= buffer + buffer_size
@@ -3407,8 +3413,6 @@ process_psymtab_comp_unit (struct objfil
   make_cleanup (dwarf2_free_abbrev_table, &cu);

   /* Read the compilation unit die.  */
-  if (this_cu->debug_type_section)
-    info_ptr += 8 /*signature*/ + cu.header.offset_size;
   init_cu_die_reader (&reader_specs, &cu);
   info_ptr = read_full_die (&reader_specs, &comp_unit_die, info_ptr,
 			    &has_children);
@@ -3670,7 +3674,7 @@ load_partial_comp_unit (struct dwarf2_pe
       info_ptr = partial_read_comp_unit_head (&cu->header, info_ptr,
 					      dwarf2_per_objfile->info.buffer,
 					      dwarf2_per_objfile->info.size,
-					      abfd);
+					      abfd, 0);

       /* Skip dummy compilation units.  */
       if (info_ptr >= (dwarf2_per_objfile->info.buffer

Re: Regression for .debug_types [Re: [patch] Allow dummy CUs created by incremental linker]

[Cary Coutant]

> 2011-10-20  Cary Coutant  <ccoutant@google.com>
>
>        * dwarf2read.c (dw2_get_file_names): Move adjustment for type
>        section to...
>        (partial_read_comp_unit_head): ...here.  Add is_debug_type_section
>        flag.  Adjust all callers.
>        (process_psymtab_comp_unit): Remove adjustment for type section.

> Running the tests now, with -gdwarf4 explicitly turned on.

Tests completed, showing no regressions relative to a build of gdb
from just before my first patch yesterday. OK to commit?

(Apologies for the screw-up. I thought I was running the test suite
with -gdwarf4 enabled, but I wasn't using the right wrapper script. I
fixed that, and verified that the tests are now using .debug_types
sections.)

-cary

Re: Regression for .debug_types [Re: [patch] Allow dummy CUs created by incremental linker]

[Jan Kratochvil]

On Fri, 21 Oct 2011 00:09:26 +0200, Cary Coutant wrote:
> > 2011-10-20  Cary Coutant  <ccoutant@google.com>
> >
> >        * dwarf2read.c (dw2_get_file_names): Move adjustment for type
> >        section to...
> >        (partial_read_comp_unit_head): ...here.  Add is_debug_type_section
> >        flag.  Adjust all callers.
> >        (process_psymtab_comp_unit): Remove adjustment for type section.
> 
> > Running the tests now, with -gdwarf4 explicitly turned on.
> 
> Tests completed, showing no regressions relative to a build of gdb
> from just before my first patch yesterday. OK to commit?

OK, thanks, it looks OK to me, I also did not see the bug before.


Regards,
Jan