gdb: Incorrect stack unwinding if compressed debug info is used

gdb: Incorrect stack unwinding if compressed debug info is used

[Vladimir Simonov]

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

Hi all,

Environment: gdb-weekly-CVS-7.2.50.20110125, binutils 2.21, gcc 4.4.3, Linux, i686,
  32 bit kernel 2.6.32.

If I create -Od -g3 executable with -Wl,compressed-debug-sections=zlib
using gold linker or compress debug-info via objcopy I have problems with
local variables and bacttraces in gdb.
Something like this:
gdb: bt
....
#11 0xb2356a74 in Core::WorkerImpl::WorkerThread (this=Could not find
the frame base for "Core::WorkerImpl::WorkerThread()".
)
....

I've spend some time and, looks like, found the problem. It is in
dwarf2_symbol_mark_computed function (dwarf2read.c). Check
"DW_UNSND (attr) < dwarf2_per_objfile->loc.size"
is incorrect if compressed section is used. In this case
loc.size contains compressed section size, not decompressed one.
It happens if the section has not been read via dwarf2_read_section yet.
But dwarf2_locate_sections has been done.
As result symbols not passed above verification are left with
size==0 and data==NULL after dwarf2_symbol_mark_computed function.

The patch idea is to introduce uncompressed_size field in
struct dwarf2_section_info. And fill it in dwarf2_locate_sections.
Check in dwarf2_symbol_mark_computed function takes into
account uncompressed_size. The patch is quite large cause I
try to avoid code duplication with zlib_decompress section.

Any comments are welcome

Best regards
Vladimir Simonov

[-- Attachment #2: gdb-7.2-compressed-section.diff --]
[-- Type: text/plain, Size: 9432 bytes --]

diff -ruN gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c gdb-7.2/gdb/dwarf2read.c
--- gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c	2011-01-12 19:16:20.000000000 +0300
+++ ./gdb/dwarf2read.c	2011-01-31 18:03:37.072688100 +0300
@@ -136,6 +136,7 @@
   int was_mmapped;
   /* True if we have tried to read this section.  */
   int readin;
+  bfd_size_type uncompressed_size;
 };
 
 /* All offsets in the index are of this type.  It must be
@@ -1357,6 +1358,59 @@
 		  && strcmp (section_name + 2, name) == 0)));
 }
 
+static int
+is_compressed_section_name (const char *section_name)
+{
+  return (section_name[0] == '.' && section_name[1] == 'z');
+}
+
+static int
+parse_zlib_section_header (bfd_byte *compressed_buffer, bfd_size_type *size)
+{
+  bfd_size_type uncompressed_size;
+
+ /* Read the zlib header. In this case, it should be "ZLIB" followed
+     by the uncompressed section size, 8 bytes in big-endian order.  */
+  if (strncmp (compressed_buffer, "ZLIB", 4) != 0)
+    return FALSE;
+  uncompressed_size = compressed_buffer[4]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[5]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[6]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[7]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[8]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[9]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[10]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[11];
+  *size = uncompressed_size;
+  return TRUE;
+}
+
+static int
+read_uncompressed_size (bfd *abfd, asection *sec, bfd_size_type *size)
+{
+  bfd_byte compressed_buffer [12];
+  bfd_size_type uncompressed_size;
+
+  if (sec->size < 12 || !bfd_get_section_contents (abfd, sec,
+					compressed_buffer, 0, 12))
+    return FALSE;
+  return parse_zlib_section_header (compressed_buffer, size);
+}
+
+static void
+fill_dwarf2_section_info (struct dwarf2_section_info* info,
+			  bfd *abfd, asection *sectp)
+{
+  bfd_size_type size;
+
+  info->asection = sectp;
+  info->size = bfd_get_section_size (sectp);
+  info->uncompressed_size = 0;
+  if (!is_compressed_section_name (sectp->name))
+    return;
+  read_uncompressed_size (abfd, sectp, &info->uncompressed_size);
+}
+
 /* This function is mapped across the sections and remembers the
    offset and size of each of the debugging sections we are interested
    in.  */
@@ -1366,38 +1420,31 @@
 {
   if (section_is_p (sectp->name, INFO_SECTION))
     {
-      dwarf2_per_objfile->info.asection = sectp;
-      dwarf2_per_objfile->info.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->info, abfd, sectp);
     }
   else if (section_is_p (sectp->name, ABBREV_SECTION))
     {
-      dwarf2_per_objfile->abbrev.asection = sectp;
-      dwarf2_per_objfile->abbrev.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->abbrev, abfd, sectp);
     }
   else if (section_is_p (sectp->name, LINE_SECTION))
     {
-      dwarf2_per_objfile->line.asection = sectp;
-      dwarf2_per_objfile->line.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->line, abfd, sectp);
     }
   else if (section_is_p (sectp->name, LOC_SECTION))
     {
-      dwarf2_per_objfile->loc.asection = sectp;
-      dwarf2_per_objfile->loc.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->loc, abfd, sectp);
     }
   else if (section_is_p (sectp->name, MACINFO_SECTION))
     {
-      dwarf2_per_objfile->macinfo.asection = sectp;
-      dwarf2_per_objfile->macinfo.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->macinfo, abfd, sectp);
     }
   else if (section_is_p (sectp->name, STR_SECTION))
     {
-      dwarf2_per_objfile->str.asection = sectp;
-      dwarf2_per_objfile->str.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->str, abfd, sectp);
     }
   else if (section_is_p (sectp->name, FRAME_SECTION))
     {
-      dwarf2_per_objfile->frame.asection = sectp;
-      dwarf2_per_objfile->frame.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->frame, abfd, sectp);
     }
   else if (section_is_p (sectp->name, EH_FRAME_SECTION))
     {
@@ -1405,24 +1452,20 @@
 
       if (aflag & SEC_HAS_CONTENTS)
         {
-	  dwarf2_per_objfile->eh_frame.asection = sectp;
-          dwarf2_per_objfile->eh_frame.size = bfd_get_section_size (sectp);
+          fill_dwarf2_section_info(&dwarf2_per_objfile->eh_frame, abfd, sectp);
         }
     }
   else if (section_is_p (sectp->name, RANGES_SECTION))
     {
-      dwarf2_per_objfile->ranges.asection = sectp;
-      dwarf2_per_objfile->ranges.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->ranges, abfd, sectp);
     }
   else if (section_is_p (sectp->name, TYPES_SECTION))
     {
-      dwarf2_per_objfile->types.asection = sectp;
-      dwarf2_per_objfile->types.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->types, abfd, sectp);
     }
   else if (section_is_p (sectp->name, GDB_INDEX_SECTION))
     {
-      dwarf2_per_objfile->gdb_index.asection = sectp;
-      dwarf2_per_objfile->gdb_index.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->gdb_index, abfd, sectp);
     }
 
   if ((bfd_get_section_flags (abfd, sectp) & SEC_LOAD)
@@ -1435,7 +1478,7 @@
 
 static void
 zlib_decompress_section (struct objfile *objfile, asection *sectp,
-                         gdb_byte **outbuf, bfd_size_type *outsize)
+                         gdb_byte **outbuf)
 {
   bfd *abfd = objfile->obfd;
 #ifndef HAVE_ZLIB_H
@@ -1452,26 +1495,19 @@
   int rc;
   int header_size = 12;
 
+  if (compressed_size < header_size)
+    error (_("Dwarf Error: Too small DWARF ZLIB header from '%s'"),
+           bfd_get_filename (abfd));
+
   if (bfd_seek (abfd, sectp->filepos, SEEK_SET) != 0
       || bfd_bread (compressed_buffer,
 		    compressed_size, abfd) != compressed_size)
     error (_("Dwarf Error: Can't read DWARF data from '%s'"),
            bfd_get_filename (abfd));
 
-  /* Read the zlib header.  In this case, it should be "ZLIB" followed
-     by the uncompressed section size, 8 bytes in big-endian order.  */
-  if (compressed_size < header_size
-      || strncmp (compressed_buffer, "ZLIB", 4) != 0)
+  if (!parse_zlib_section_header (compressed_buffer, &uncompressed_size))
     error (_("Dwarf Error: Corrupt DWARF ZLIB header from '%s'"),
            bfd_get_filename (abfd));
-  uncompressed_size = compressed_buffer[4]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[5]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[6]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[7]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[8]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[9]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[10]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[11];
 
   /* It is possible the section consists of several compressed
      buffers concatenated together, so we uncompress in a loop.  */
@@ -1505,7 +1541,6 @@
 
   do_cleanups (cleanup);
   *outbuf = uncompressed_buffer;
-  *outsize = uncompressed_size;
 #endif
 }
 
@@ -1519,29 +1554,23 @@
   bfd *abfd = objfile->obfd;
   asection *sectp = info->asection;
   gdb_byte *buf, *retbuf;
-  unsigned char header[4];
 
   if (info->readin)
     return;
   info->buffer = NULL;
   info->was_mmapped = 0;
   info->readin = 1;
+  info->uncompressed_size = 0;
 
   if (info->asection == NULL || info->size == 0)
     return;
 
-  /* Check if the file has a 4-byte header indicating compression.  */
-  if (info->size > sizeof (header)
-      && bfd_seek (abfd, sectp->filepos, SEEK_SET) == 0
-      && bfd_bread (header, sizeof (header), abfd) == sizeof (header))
+  /* Check if the file has a 12-byte header indicating compression.  */
+  if (read_uncompressed_size (abfd, sectp, &info->uncompressed_size))
     {
-      /* Upon decompression, update the buffer and its size.  */
-      if (strncmp (header, "ZLIB", sizeof (header)) == 0)
-        {
-          zlib_decompress_section (objfile, sectp, &info->buffer,
-				   &info->size);
-          return;
-        }
+      zlib_decompress_section (objfile, sectp, &info->buffer);
+      info->size = info->uncompressed_size;
+      return;
     }
 
 #ifdef HAVE_MMAP
@@ -14356,6 +14385,14 @@
   baton->base_address = cu->base_address;
 }
 
+static int
+check_attr_location (struct dwarf2_section_info *info, struct attribute *attr)
+{
+  if (info->uncompressed_size)
+    return DW_UNSND (attr) < info->uncompressed_size;
+  return DW_UNSND (attr) < info->size;
+}
+
 static void
 dwarf2_symbol_mark_computed (struct attribute *attr, struct symbol *sym,
 			     struct dwarf2_cu *cu)
@@ -14364,7 +14401,7 @@
       /* ".debug_loc" may not exist at all, or the offset may be outside
 	 the section.  If so, fall through to the complaint in the
 	 other branch.  */
-      && DW_UNSND (attr) < dwarf2_per_objfile->loc.size)
+      && check_attr_location (&dwarf2_per_objfile->loc, attr))
     {
       struct dwarf2_loclist_baton *baton;
 

gdb: Incorrect stack unwinding if debug info is compressed

[Vladimir Simonov]

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

Ian and Paul,

Could you please comment below.

Executable created with -Wl,compressed-debug-sections=zlib
using gold linker or compress debug-info via objcopy. I see problems with
local variables and backtraces in gdb.
Something like this:
gdb: bt
....
#11 0xb2356a74 in Core::WorkerImpl::WorkerThread (this=Could not find
the frame base for "Core::WorkerImpl::WorkerThread()".
)
....
Debug info is kept in separate file linked to executable
via --add-gnu-debuglink objcopy option.

Environment: gdb-weekly-CVS-7.2.50.20110125, binutils 2.21, gcc 4.4.3,
Linux, i686, 32 bit kernel 2.6.32.

IMO the problem is in
dwarf2_symbol_mark_computed function (dwarf2read.c). Check
"DW_UNSND (attr) < dwarf2_per_objfile->loc.size"
is incorrect if compressed section is used. In this case
loc.size contains compressed section size, not decompressed one.
As result large part of symbols are not passed above verification
and are left with size==0 and data==NULL after dwarf2_symbol_mark_computed
function.
The problem observed if the section has not been read via dwarf2_read_section.
But dwarf2_locate_sections has been done for the section.

The patch introduces uncompressed_size field in
struct dwarf2_section_info. And fill it in dwarf2_locate_sections.
Check in dwarf2_symbol_mark_computed function takes into
account uncompressed_size. Fields "size' and "uncompressed_size" meaning:
- "size" - section/compressed section size between dwarf2_locate_sections
and dwarf2_read_section calls. After dwarf2_read_section it is
uncompressed section size.
- "uncompressed_size" - always zero for uncompressed sections. For
compressed sections it is uncompressed section size always.

The patch is quite large cause I try to avoid code duplication
with zlib_decompress section and have to do new field initialization
properly.

Any comments are welcome

Best regards
Vladimir Simonov


[-- Attachment #2: gdb-7.2-compressed-section.diff --]
[-- Type: text/plain, Size: 9433 bytes --]

diff -ruN gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c gdb-7.2/gdb/dwarf2read.c
--- gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c	2011-01-12 19:16:20.000000000 +0300
+++ ./gdb/dwarf2read.c	2011-01-31 18:03:37.072688100 +0300
@@ -136,6 +136,7 @@
   int was_mmapped;
   /* True if we have tried to read this section.  */
   int readin;
+  bfd_size_type uncompressed_size;
 };
 
 /* All offsets in the index are of this type.  It must be
@@ -1357,6 +1358,59 @@
 		  && strcmp (section_name + 2, name) == 0)));
 }
 
+static int
+is_compressed_section_name (const char *section_name)
+{
+  return (section_name[0] == '.' && section_name[1] == 'z');
+}
+
+static int
+parse_zlib_section_header (bfd_byte *compressed_buffer, bfd_size_type *size)
+{
+  bfd_size_type uncompressed_size;
+
+ /* Read the zlib header. In this case, it should be "ZLIB" followed
+     by the uncompressed section size, 8 bytes in big-endian order.  */
+  if (strncmp (compressed_buffer, "ZLIB", 4) != 0)
+    return FALSE;
+  uncompressed_size = compressed_buffer[4]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[5]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[6]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[7]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[8]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[9]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[10]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[11];
+  *size = uncompressed_size;
+  return TRUE;
+}
+
+static int
+read_uncompressed_size (bfd *abfd, asection *sec, bfd_size_type *size)
+{
+  bfd_byte compressed_buffer [12];
+  bfd_size_type uncompressed_size;
+
+  if (sec->size < 12 || !bfd_get_section_contents (abfd, sec,
+					compressed_buffer, 0, 12))
+    return FALSE;
+  return parse_zlib_section_header (compressed_buffer, size);
+}
+
+static void
+fill_dwarf2_section_info (struct dwarf2_section_info* info,
+			  bfd *abfd, asection *sectp)
+{
+  bfd_size_type size;
+
+  info->asection = sectp;
+  info->size = bfd_get_section_size (sectp);
+  info->uncompressed_size = 0;
+  if (!is_compressed_section_name (sectp->name))
+    return;
+  read_uncompressed_size (abfd, sectp, &info->uncompressed_size);
+}
+
 /* This function is mapped across the sections and remembers the
    offset and size of each of the debugging sections we are interested
    in.  */
@@ -1366,38 +1420,31 @@
 {
   if (section_is_p (sectp->name, INFO_SECTION))
     {
-      dwarf2_per_objfile->info.asection = sectp;
-      dwarf2_per_objfile->info.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->info, abfd, sectp);
     }
   else if (section_is_p (sectp->name, ABBREV_SECTION))
     {
-      dwarf2_per_objfile->abbrev.asection = sectp;
-      dwarf2_per_objfile->abbrev.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->abbrev, abfd, sectp);
     }
   else if (section_is_p (sectp->name, LINE_SECTION))
     {
-      dwarf2_per_objfile->line.asection = sectp;
-      dwarf2_per_objfile->line.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->line, abfd, sectp);
     }
   else if (section_is_p (sectp->name, LOC_SECTION))
     {
-      dwarf2_per_objfile->loc.asection = sectp;
-      dwarf2_per_objfile->loc.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->loc, abfd, sectp);
     }
   else if (section_is_p (sectp->name, MACINFO_SECTION))
     {
-      dwarf2_per_objfile->macinfo.asection = sectp;
-      dwarf2_per_objfile->macinfo.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->macinfo, abfd, sectp);
     }
   else if (section_is_p (sectp->name, STR_SECTION))
     {
-      dwarf2_per_objfile->str.asection = sectp;
-      dwarf2_per_objfile->str.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->str, abfd, sectp);
     }
   else if (section_is_p (sectp->name, FRAME_SECTION))
     {
-      dwarf2_per_objfile->frame.asection = sectp;
-      dwarf2_per_objfile->frame.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->frame, abfd, sectp);
     }
   else if (section_is_p (sectp->name, EH_FRAME_SECTION))
     {
@@ -1405,24 +1452,20 @@
 
       if (aflag & SEC_HAS_CONTENTS)
         {
-	  dwarf2_per_objfile->eh_frame.asection = sectp;
-          dwarf2_per_objfile->eh_frame.size = bfd_get_section_size (sectp);
+          fill_dwarf2_section_info(&dwarf2_per_objfile->eh_frame, abfd, sectp);
         }
     }
   else if (section_is_p (sectp->name, RANGES_SECTION))
     {
-      dwarf2_per_objfile->ranges.asection = sectp;
-      dwarf2_per_objfile->ranges.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->ranges, abfd, sectp);
     }
   else if (section_is_p (sectp->name, TYPES_SECTION))
     {
-      dwarf2_per_objfile->types.asection = sectp;
-      dwarf2_per_objfile->types.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->types, abfd, sectp);
     }
   else if (section_is_p (sectp->name, GDB_INDEX_SECTION))
     {
-      dwarf2_per_objfile->gdb_index.asection = sectp;
-      dwarf2_per_objfile->gdb_index.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info(&dwarf2_per_objfile->gdb_index, abfd, sectp);
     }
 
   if ((bfd_get_section_flags (abfd, sectp) & SEC_LOAD)
@@ -1435,7 +1478,7 @@
 
 static void
 zlib_decompress_section (struct objfile *objfile, asection *sectp,
-                         gdb_byte **outbuf, bfd_size_type *outsize)
+                         gdb_byte **outbuf)
 {
   bfd *abfd = objfile->obfd;
 #ifndef HAVE_ZLIB_H
@@ -1452,26 +1495,19 @@
   int rc;
   int header_size = 12;
 
+  if (compressed_size < header_size)
+    error (_("Dwarf Error: Too small DWARF ZLIB header from '%s'"),
+           bfd_get_filename (abfd));
+
   if (bfd_seek (abfd, sectp->filepos, SEEK_SET) != 0
       || bfd_bread (compressed_buffer,
 		    compressed_size, abfd) != compressed_size)
     error (_("Dwarf Error: Can't read DWARF data from '%s'"),
            bfd_get_filename (abfd));
 
-  /* Read the zlib header.  In this case, it should be "ZLIB" followed
-     by the uncompressed section size, 8 bytes in big-endian order.  */
-  if (compressed_size < header_size
-      || strncmp (compressed_buffer, "ZLIB", 4) != 0)
+  if (!parse_zlib_section_header (compressed_buffer, &uncompressed_size))
     error (_("Dwarf Error: Corrupt DWARF ZLIB header from '%s'"),
            bfd_get_filename (abfd));
-  uncompressed_size = compressed_buffer[4]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[5]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[6]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[7]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[8]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[9]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[10]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[11];
 
   /* It is possible the section consists of several compressed
      buffers concatenated together, so we uncompress in a loop.  */
@@ -1505,7 +1541,6 @@
 
   do_cleanups (cleanup);
   *outbuf = uncompressed_buffer;
-  *outsize = uncompressed_size;
 #endif
 }
 
@@ -1519,29 +1554,23 @@
   bfd *abfd = objfile->obfd;
   asection *sectp = info->asection;
   gdb_byte *buf, *retbuf;
-  unsigned char header[4];
 
   if (info->readin)
     return;
   info->buffer = NULL;
   info->was_mmapped = 0;
   info->readin = 1;
+  info->uncompressed_size = 0;
 
   if (info->asection == NULL || info->size == 0)
     return;
 
-  /* Check if the file has a 4-byte header indicating compression.  */
-  if (info->size > sizeof (header)
-      && bfd_seek (abfd, sectp->filepos, SEEK_SET) == 0
-      && bfd_bread (header, sizeof (header), abfd) == sizeof (header))
+  /* Check if the file has a 12-byte header indicating compression.  */
+  if (read_uncompressed_size (abfd, sectp, &info->uncompressed_size))
     {
-      /* Upon decompression, update the buffer and its size.  */
-      if (strncmp (header, "ZLIB", sizeof (header)) == 0)
-        {
-          zlib_decompress_section (objfile, sectp, &info->buffer,
-				   &info->size);
-          return;
-        }
+      zlib_decompress_section (objfile, sectp, &info->buffer);
+      info->size = info->uncompressed_size;
+      return;
     }
 
 #ifdef HAVE_MMAP
@@ -14356,6 +14385,14 @@
   baton->base_address = cu->base_address;
 }
 
+static int
+check_attr_location (struct dwarf2_section_info *info, struct attribute *attr)
+{
+  if (info->uncompressed_size)
+    return DW_UNSND (attr) < info->uncompressed_size;
+  return DW_UNSND (attr) < info->size;
+}
+
 static void
 dwarf2_symbol_mark_computed (struct attribute *attr, struct symbol *sym,
 			     struct dwarf2_cu *cu)
@@ -14364,7 +14401,7 @@
       /* ".debug_loc" may not exist at all, or the offset may be outside
 	 the section.  If so, fall through to the complaint in the
 	 other branch.  */
-      && DW_UNSND (attr) < dwarf2_per_objfile->loc.size)
+      && check_attr_location (&dwarf2_per_objfile->loc, attr))
     {
       struct dwarf2_loclist_baton *baton;
 

Re: gdb: Incorrect stack unwinding if compressed debug info is used

[Paul Pluzhnikov]

On Mon, Jan 31, 2011 at 7:42 AM, Vladimir Simonov <sv@sw.ru> wrote:

> If I create -Od -g3 executable with -Wl,compressed-debug-sections=zlib

What is '-Od' ?

And you mean '-Wl,--compress-debug-sections=zlib', not
'-Wl,compressed-debug-sections=zlib'

> using gold linker or compress debug-info via objcopy I have problems with
> local variables and bacttraces in gdb.

Can you construct a small example showing the problem? I haven't been able
to reproduce it.

> Something like this:
> gdb: bt
> ....
> #11 0xb2356a74 in Core::WorkerImpl::WorkerThread (this=Could not find
> the frame base for "Core::WorkerImpl::WorkerThread()".
> )
> ....
>
> I've spend some time and, looks like, found the problem. It is in
> dwarf2_symbol_mark_computed function (dwarf2read.c). Check
> "DW_UNSND (attr) < dwarf2_per_objfile->loc.size"
> is incorrect if compressed section is used. In this case
> loc.size contains compressed section size, not decompressed one.
> It happens if the section has not been read via dwarf2_read_section yet.
> But dwarf2_locate_sections has been done.

I am curious how your GDB avoids dwarf2_read_section(). As far as I can
tell, it should always be called (indirectly) by dwarf2_initialize_objfile().

> As result symbols not passed above verification are left with
> size==0 and data==NULL after dwarf2_symbol_mark_computed function.
>
> The patch idea is to introduce uncompressed_size field in
> struct dwarf2_section_info. And fill it in dwarf2_locate_sections.
> Check in dwarf2_symbol_mark_computed function takes into
> account uncompressed_size. The patch is quite large cause I
> try to avoid code duplication with zlib_decompress section.


Assuming the patch makes sense (which I am not yet convinced) ...

+static void
+fill_dwarf2_section_info (struct dwarf2_section_info* info,
+			  bfd *abfd, asection *sectp)
+{
+  bfd_size_type size;
+
+  info->asection = sectp;
+  info->size = bfd_get_section_size (sectp);
+  info->uncompressed_size = 0;
+  if (!is_compressed_section_name (sectp->name))
+    return;
+  read_uncompressed_size (abfd, sectp, &info->uncompressed_size);
+}

Would it make sense to just set uncompressed_size to size if the section
is not compressed? I think that would simplify the patch a bit.

+      fill_dwarf2_section_info(&dwarf2_per_objfile->info, abfd, sectp);

Missing space before '('.

Your patch is also missing ChangeLog entry.

Cheers,
-- 
Paul Pluzhnikov

Re: gdb: Incorrect stack unwinding if compressed debug info is used

[Vladimir Simonov]

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

On 02/02/2011 10:55 PM, Paul Pluzhnikov wrote:
> On Mon, Jan 31, 2011 at 7:42 AM, Vladimir Simonov<sv@sw.ru>  wrote:
>
>> If I create -Od -g3 executable with -Wl,compressed-debug-sections=zlib
>
> What is '-Od' ?
>
> And you mean '-Wl,--compress-debug-sections=zlib', not
> '-Wl,compressed-debug-sections=zlib'
Sorry, to the end of day cl/gcc options are messed in my head.
"-g -O0" are used as gcc options, "objcopy --compress-debug-sections --only-keep-debug"
is used to compress debug info.

You can see build procedure in attached b.sh file.

>
>> using gold linker or compress debug-info via objcopy I have problems with
>> local variables and bacttraces in gdb.
>
> Can you construct a small example showing the problem? I haven't been able
> to reproduce it.
>
>> Something like this:
>> gdb: bt
>> ....
>> #11 0xb2356a74 in Core::WorkerImpl::WorkerThread (this=Could not find
>> the frame base for "Core::WorkerImpl::WorkerThread()".
>> )
>> ....
>>
>> I've spend some time and, looks like, found the problem. It is in
>> dwarf2_symbol_mark_computed function (dwarf2read.c). Check
>> "DW_UNSND (attr)<  dwarf2_per_objfile->loc.size"
>> is incorrect if compressed section is used. In this case
>> loc.size contains compressed section size, not decompressed one.
>> It happens if the section has not been read via dwarf2_read_section yet.
>> But dwarf2_locate_sections has been done.
>
> I am curious how your GDB avoids dwarf2_read_section(). As far as I can
> tell, it should always be called (indirectly) by dwarf2_initialize_objfile().
Please see gdb_old_log - session of debug original gdb using real
program. "b dwarf2read.c:14367" means breakpoint in the head of
dwarf2_symbol_mark_computed.
When break is reached
(gdb) p dwarf2_per_objfile->loc
$1 = {asection = 0x91effc0, buffer = 0x0, size = 56165, was_mmapped = 0,
   readin = 0}
(gdb) p (attr)->u.unsnd
$2 = 83336
You can see:
1. dwarf2_per_objfile->loc is not read (readin = 0)
2. (attr)->u.unsnd > dwarf2_per_objfile->loc.size
As result the code goes into lowest part of dwarf2_symbol_mark_computed,
complaint and set baton->size = 0, baton->data = 0.

gdb_debug_log - session of debug patched gdb using dbg_info_test.
In it we see that _real_ section read will be done
if check "DW_UNSND (attr)<  dwarf2_per_objfile->loc.size" is _passed_.

#0  dwarf2_read_section (objfile=0x849a2c8, info=0x849eed0)
#1  0x081ac376 in fill_in_loclist_baton (cu=0x8491130, baton=0x84a7354,
#2  0x081ac5a7 in dwarf2_symbol_mark_computed (attr=0x84a3328, sym=0x84a7328, cu=0x8491130)
...
This answers your curiosity -  GDB avoids dwarf2_read_section
because of incorrect (for compressed sections)
"DW_UNSND (attr)<  dwarf2_per_objfile->loc.size" check.

Unfortunately I was not able to force  (attr)->u.unsnd
to be greater than compressed size in simple example.
But hope above analysis unveils the problem.

In real life it leads to situation when if you are unlucky
and set breakpoint on function which attr outside of loc.size
you'll never see its local variables in current debug session.

>
>> As result symbols not passed above verification are left with
>> size==0 and data==NULL after dwarf2_symbol_mark_computed function.
>>
>> The patch idea is to introduce uncompressed_size field in
>> struct dwarf2_section_info. And fill it in dwarf2_locate_sections.
>> Check in dwarf2_symbol_mark_computed function takes into
>> account uncompressed_size. The patch is quite large cause I
>> try to avoid code duplication with zlib_decompress section.
>
>
> Assuming the patch makes sense (which I am not yet convinced) ...
>
> +static void
> +fill_dwarf2_section_info (struct dwarf2_section_info* info,
> +			  bfd *abfd, asection *sectp)
> +{
> +  bfd_size_type size;
> +
> +  info->asection = sectp;
> +  info->size = bfd_get_section_size (sectp);
> +  info->uncompressed_size = 0;
> +  if (!is_compressed_section_name (sectp->name))
> +    return;
> +  read_uncompressed_size (abfd, sectp,&info->uncompressed_size);
> +}
>
> Would it make sense to just set uncompressed_size to size if the section
> is not compressed? I think that would simplify the patch a bit.
>
> +      fill_dwarf2_section_info(&dwarf2_per_objfile->info, abfd, sectp);
>
> Missing space before '('.
>
> Your patch is also missing ChangeLog entry.
Thank you for comments.
Attaching new version of the patch.

Regards
Vladimir Simonov

[-- Attachment #2: b.sh --]
[-- Type: application/x-sh, Size: 468 bytes --]

[-- Attachment #3: dbg_info_test.c --]
[-- Type: text/plain, Size: 826 bytes --]

#include <stdio.h>

#define INSTANTIATE_FUNC(callee, name) int name(int argc, char **argv) \
	{ \
		printf("%s: argc=%d, call %s\n", #name, argc, #callee); \
		return callee(argc, argv); \
	}

int func1(int argc, char **argv)
{
	printf("%s: argc=%d\n", "func1", argc);
	return 0;
}

INSTANTIATE_FUNC(func1, func2)
INSTANTIATE_FUNC(func2, func3)
INSTANTIATE_FUNC(func3, func4)
INSTANTIATE_FUNC(func4, func5)
INSTANTIATE_FUNC(func5, func6)
INSTANTIATE_FUNC(func6, func7)
INSTANTIATE_FUNC(func7, func8)
INSTANTIATE_FUNC(func8, func9)
INSTANTIATE_FUNC(func9, func10)
INSTANTIATE_FUNC(func10, func11)
INSTANTIATE_FUNC(func11, func12)
INSTANTIATE_FUNC(func12, func13)
INSTANTIATE_FUNC(func13, func14)
INSTANTIATE_FUNC(func14, func15)
INSTANTIATE_FUNC(func15, func16)
INSTANTIATE_FUNC(func16, func17)
INSTANTIATE_FUNC(func17, main)


[-- Attachment #4: gdb_old_log --]
[-- Type: text/plain, Size: 4219 bytes --]

# ./test_gdb/gdb_old ./test_gdb/gdb_old
GNU gdb (GDB) 7.2.50.20110124-cvs
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-unknown-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /tmp/root/test_gdb/gdb_old...done.
(gdb)  b dwarf2read.c:14367
Breakpoint 1 at 0x81ac2f7: file /build_root64_new/i686-gcc-4.4.3-gli
bc-2.11.1-0.43/build/gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c, line 14367
.
(gdb) set args /bin/product
(gdb) r
Starting program: /tmp/root/test_gdb/gdb_old /bin/product
GNU gdb (GDB) 7.2.50.20110124-cvs
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-unknown-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /bin/product...(no debugging symbols found)...done.
(gdb) r
Starting program: /bin/product
[Thread debugging using libthread_db enabled]
[New Thread 0xb725db70 (LWP 3092)]
[New Thread 0xb6a5cb70 (LWP 3093)]
[New Thread 0xb5c5bb70 (LWP 3094)]
[New Thread 0xb522fb70 (LWP 3095)]
[New Thread 0xb4973b70 (LWP 3096)]
[New Thread 0xb4172b70 (LWP 3097)]
[New Thread 0xb17ffb70 (LWP 3110)]
[New Thread 0xb0ffeb70 (LWP 3111)]
[New Thread 0xb07fdb70 (LWP 3114)]
[New Thread 0xafffcb70 (LWP 3115)]
[New Thread 0xa9db9b70 (LWP 3117)]
[Thread 0xa9db9b70 (LWP 3117) exited]
[New Thread 0xa9db9b70 (LWP 3120)]
[New Thread 0xa95b8b70 (LWP 3121)]
[New Thread 0xa8dabb70 (LWP 3122)]
[New Thread 0xa7177b70 (LWP 3123)]
[New Thread 0xa6976b70 (LWP 3124)]
[New Thread 0xa6175b70 (LWP 3125)]
[New Thread 0xa5974b70 (LWP 3126)]
[New Thread 0xa5055b70 (LWP 3127)]
[New Thread 0xa4854b70 (LWP 3128)]
[New Thread 0xa3852b70 (LWP 3130)]
[New Thread 0xa4053b70 (LWP 3129)]
[Thread 0xa4053b70 (LWP 3129) exited]
[New Thread 0xa3051b70 (LWP 3131)]
[New Thread 0xa2850b70 (LWP 3132)]
[New Thread 0xa204fb70 (LWP 3133)]
^C
Program received signal SIGINT, Interrupt.
0xffffe424 in __kernel_vsyscall ()
(gdb) b GrGetEvent

Breakpoint 1, dwarf2_symbol_mark_computed (attr=0xfe338ac, sym=0xfe4676c,
    cu=0xd214ca8)
    at /build_root64_new/i686-gcc-4.4.3-glibc-2.11.1-0.43/build/gdb-
weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c:14367
14367   /build_root64_new/i686-gcc-4.4.3-glibc-2.11.1-0.43/build/gdb
-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c: No such file or directory.
        in /build_root64_new/i686-gcc-4.4.3-glibc-2.11.1-0.43/build/
gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c
(gdb) p dwarf2_per_objfile->loc
$1 = {asection = 0x91effc0, buffer = 0x0, size = 56165, was_mmapped = 0,
  readin = 0}
(gdb) p (attr)->u.unsnd
$2 = 83336
(gdb) n
14363   in /build_root64_new/i686-gcc-4.4.3-glibc-2.11.1-0.43/build/
gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c
(gdb)
14367   in /build_root64_new/i686-gcc-4.4.3-glibc-2.11.1-0.43/build/
gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c
(gdb)
14363   in /build_root64_new/i686-gcc-4.4.3-glibc-2.11.1-0.43/build/
gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c
(gdb)
14388   in /build_root64_new/i686-gcc-4.4.3-glibc-2.11.1-0.43/build/
gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c
(gdb) disa 1
(gdb) c
Continuing.
Breakpoint 1 at 0xb783ae33: file /w/current/graphapi/event_l.cpp, line 54.
(gdb) set complaints 10
(gdb) c
Continuing.
[New Thread 0xa184eb70 (LWP 3140)]
[New Thread 0xa084cb70 (LWP 3142)]
[New Thread 0xa004bb70 (LWP 3143)]
[New Thread 0x9f84ab70 (LWP 3144)]
[New Thread 0x9f049b70 (LWP 3145)]
[New Thread 0x9e848b70 (LWP 3146)]

Breakpoint 1, GrGetEvent (event=Could not find the frame base for "GrGetEvent(Gr
Event&, timeval*)".
) at /w/current/graphapi/event_l.cpp:54
54        GrKeyboard* Default = GrKeyboard::GetDefault();
(gdb)

[-- Attachment #5: gdb_debug_log --]
[-- Type: text/plain, Size: 7734 bytes --]

# ./gdb ./gdb
GNU gdb (GDB) 7.2.50.20110124-cvs
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-unknown-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /tmp/root/test_gdb/gdb...done.
(gdb) b dwarf2read.c:1435
Breakpoint 1 at 0x8193fd3: file dwarf2read.c, line 1435.

(gdb) b check_attr_location
Breakpoint 2 at 0x81ac40a: file dwarf2read.c, line 14391
(gdb) set args ./dbg_info_test
(gdb) r
Starting program: /tmp/root/test_gdb/gdb ./dbg_info_test
GNU gdb (GDB) 7.2.50.20110124-cvs
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i686-unknown-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /tmp/root/test_gdb/dbg_info_test...Reading symbols from /tmp/root/test_gdb/dbg_info_test.dbg...
Breakpoint 1, dwarf2_locate_sections (abfd=0x8479b60, sectp=0x8498960,    ignore_ptr=0x0)
    at dwarf2read.c:1435
(gdb) p sectp->name
$1 = 0x84907ab ".zdebug_loc"
(gdb) n
1471    in dwarf2read.c
(gdb) p dwarf2_per_objfile->loc
$2 = {asection = 0x8498960, buffer = 0x0, size = 245, was_mmapped = 0,
  readin = 0, uncompressed_size = 792}
(gdb) c
Continuing.
done.
done.
(gdb) b func1

Breakpoint 2, check_attr_location (info=0x849eed0, attr=0x84a3328)
    at dwarf2read.c:14391
14391   in dwarf2read.c
(gdb) p *info
$3 = {asection = 0x8498960, buffer = 0x0, size = 245, was_mmapped = 0,
  readin = 0, uncompressed_size = 792}
(gdb) b dwarf2_read_section
Breakpoint 3 at dwarf2read.c, line 1554.

(gdb) disa 2
(gdb) c
Continuing.

Breakpoint 3, dwarf2_read_section (objfile=0x849a2c8, info=0x849eed0)
    at dwarf2read.c:1554
1554    dwarf2read.c
(gdb) p *info
$4 = {asection = 0x8498960, buffer = 0x0, size = 245, was_mmapped = 0,
  readin = 0, uncompressed_size = 792}
(gdb) bt
#0  dwarf2_read_section (objfile=0x849a2c8, info=0x849eed0)
    at dwarf2read.c:1554
#1  0x081ac376 in fill_in_loclist_baton (cu=0x8491130, baton=0x84a7354,
    attr=0x84a3328)
    at dwarf2read.c:14376
#2  0x081ac5a7 in dwarf2_symbol_mark_computed (attr=0x84a3328, sym=0x84a7328,
    cu=0x8491130)
    at dwarf2read.c:14411
#3  0x0819bfde in read_func_scope (die=0x84a32b0, cu=0x8491130)
    at dwarf2read.c:5652
#4  0x0819a3ce in process_die (die=0x84a32b0, cu=0x8491130)
    at dwarf2read.c:4669
#5  0x0819b38f in read_file_scope (die=0x84a2f50, cu=0x8491130)
    at dwarf2read.c:5303
#6  0x0819a3a0 in process_die (die=0x84a2f50, cu=0x8491130)
    at dwarf2read.c:4662
#7  0x0819a26d in process_full_comp_unit (per_cu=0x84a11a4)
    at dwarf2read.c:4616
#8  0x08199be9 in process_queue (objfile=0x849a2c8)
    at dwarf2read.c:4381
#9  0x081949b2 in dw2_do_instantiate_symtab (objfile=0x849a2c8,
    per_cu=0x84a11a4)
    at dwarf2read.c:1777
#10 0x08199dbe in psymtab_to_symtab_1 (pst=0x84a1330)
    at dwarf2read.c:4457
#11 0x08199afc in dwarf2_psymtab_to_symtab (pst=0x84a1330)
    at dwarf2read.c:4338
#12 0x0812eca9 in psymtab_to_symtab (pst=0x84a1330)
    at psymtab.c:623
#13 0x0812e784 in lookup_symbol_aux_psymtabs (objfile=0x849a2c8,
    block_index=0, name=0xbffff480 "func1", domain=VAR_DOMAIN)
    at psymtab.c:429
#14 0x08128dac in lookup_symbol_aux_quick (objfile=0x849a2c8, kind=0,
    name=0xbffff480 "func1", domain=VAR_DOMAIN)
    at symtab.c:1383
#15 0x08128fb8 in lookup_symbol_global (name=0xbffff480 "func1", block=0x0,
    domain=VAR_DOMAIN)
    at symtab.c:1504
#16 0x08128ed5 in basic_lookup_symbol_nonlocal (name=0xbffff480 "func1",
    block=0x0, domain=VAR_DOMAIN)
    at symtab.c:1461
#17 0x0812896b in lookup_symbol_aux (name=0xbffff480 "func1", block=0x0,
    domain=VAR_DOMAIN, language=language_c, is_a_field_of_this=0x0)
    at symtab.c:1170
#18 0x08128789 in lookup_symbol_in_language (name=0xbffff480 "func1",
    block=0x0, domain=VAR_DOMAIN, lang=language_c, is_a_field_of_this=0x0)
    at symtab.c:1077
#19 0x081287d1 in lookup_symbol (name=0xbffff480 "func1", block=0x0,
    domain=VAR_DOMAIN, is_a_field_of_this=0x0)
    at symtab.c:1091
#20 0x081de795 in find_imps (symtab=0x0, block=0x0, method=0x8434192 "func1",
    syms=0x0, nsym=0xbffff550, ndebug=0xbffff54c)
    at objc-lang.c:1339
#21 0x0813a80d in decode_objc (argptr=0xbffff7d4, funfirstline=1,
    file_symtab=0x0, canonical=0xbffff748, saved_arg=0x8434192 "func1")
    at linespec.c:1146
#22 0x08139f2c in decode_line_1 (argptr=0xbffff7d4, funfirstline=1,
    default_symtab=0x0, default_line=0, canonical=0xbffff748,
    not_found_ptr=0xbffff734)
    at linespec.c:778
#23 0x080f621f in parse_breakpoint_sals (address=0xbffff7d4, sals=0xbffff770,
    addr_string=0xbffff748, not_found_ptr=0xbffff734)
    at breakpoint.c:7493
#24 0x080f6408 in do_captured_parse_breakpoint (ui=0x8471b10, data=0xbffff738)
    at breakpoint.c:7563
#25 0x081574af in catch_exception (uiout=0x8471b10,
    func=0x80f63d0 <do_captured_parse_breakpoint>, func_args=0xbffff738,
    mask=6)
    at exceptions.c:471
#26 0x080f6a15 in create_breakpoint (gdbarch=0x846ae18,
    arg=0x8434192 "func1", cond_string=0x0, thread=0,
    parse_condition_and_thread=1, tempflag=0, type_wanted=bp_breakpoint,
    ignore_count=0, pending_break_support=AUTO_BOOLEAN_AUTO, ops=0x0,
    from_tty=1, enabled=1, internal=0)
    at breakpoint.c:7746
#27 0x080f70d9 in break_command_1 (arg=0x8434192 "func1", flag=0, from_tty=1)
    at breakpoint.c:7977
#28 0x080f7257 in break_command (arg=0x8434192 "func1", from_tty=1)
    at breakpoint.c:8050
#29 0x080c3b67 in do_cfunc (c=0x844a318, args=0x8434192 "func1", from_tty=1)
    at cli-decode.c:67
#30 0x080c6215 in cmd_func (cmd=0x844a318, args=0x8434192 "func1", from_tty=1)
    at cli-decode.c:1777
#31 0x08055e6d in execute_command (p=0x8434196 "1", from_tty=1)
    at top.c:428
#32 0x0815e241 in command_handler (command=0x8434190 "b func1")
    at event-top.c:499
#33 0x0815e7a5 in command_line_handler (rl=0x8478f40 "\330\371I\b\350\371I\b")
    at event-top.c:704
#34 0x08259fc2 in rl_callback_read_char ()
#35 0x0815da03 in rl_callback_read_char_wrapper (client_data=0x0)
    at event-top.c:177
#36 0x0815e139 in stdin_event_handler (error=0, client_data=0x0)
    at event-top.c:434
#37 0x0815cecc in handle_file_event (data=...)
    at event-loop.c:831
#38 0x0815c710 in process_event ()
    at event-loop.c:402
#39 0x0815c7d4 in gdb_do_one_event (data=0x0)
    at event-loop.c:467
#40 0x0815766d in catch_errors (func=0x815c71e <gdb_do_one_event>,
    func_args=0x0, errstring=0x832e051 "", mask=6)
    at exceptions.c:521
#41 0x080d9950 in tui_command_loop (data=0x0)
    at tui-interp.c:172
#42 0x08157d38 in current_interp_command_loop ()
    at interps.c:291
#43 0x0804d0ee in captured_command_loop (data=0x0)
    at main.c:228
#44 0x0815766d in catch_errors (func=0x804d0e3 <captured_command_loop>,
    func_args=0x0, errstring=0x830e1ff "", mask=6)
    at exceptions.c:521
#45 0x0804dfad in captured_main (data=0xbffffca0)
    at main.c:933
#46 0x0815766d in catch_errors (func=0x804d124 <captured_main>,
    func_args=0xbffffca0, errstring=0x830e1ff "", mask=6)
    at exceptions.c:521
#47 0x0804dfe3 in gdb_main (args=0xbffffca0)
    at main.c:942
#48 0x0804ce73 in main (argc=2, argv=0xbffffd64)
    at gdb.c:35
(gdb)

[-- Attachment #6: gdb-7.2-compressed-section-4.patch --]
[-- Type: text/plain, Size: 10092 bytes --]

diff -ruN gdb-weekly-CVS-7.2.50.20110125.orig/gdb/ChangeLog gdb-weekly-CVS-7.2.50.20110125/gdb/ChangeLog
--- gdb-weekly-CVS-7.2.50.20110125.orig/gdb/ChangeLog	2011-01-25 00:34:18.000000000 +0300
+++ gdb-weekly-CVS-7.2.50.20110125/gdb/ChangeLog	2011-02-03 19:26:39.231295100 +0300
@@ -1,3 +1,19 @@
+2011-02-03  Vladimir Simonov  <sv@sw.ru>
+
+	Compressed debug info sections handling improved.
+
+	* dwarf2read.c (struct dwarf2_section_info) (uncompressed_size): New
+	field.
+	(is_compressed_section_name, parse_zlib_section_header)
+	(parse_zlib_section_header, read_uncompressed_size)
+	(fill_dwarf2_section_info): New function.
+	(dwarf2_locate_sections): Use fill_dwarf2_section_info instead of
+	inline assignment.
+	(zlib_decompress_section): Prototype changed. Cleanup with reuse
+	parse_zlib_section_header.
+	(dwarf2_read_section): Cleanup with reuse read_uncompressed_size.
+	(dwarf2_symbol_mark_computed): Use uncompressed_size instead of size.
+
 2011-01-24  Kevin Buettner  <kevinb@redhat.com>
 
 	* configure.tgt (mips*-*-elf): New; just like mips*-*-*, but
diff -ruN gdb-weekly-CVS-7.2.50.20110125.orig/gdb/dwarf2read.c gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c
--- gdb-weekly-CVS-7.2.50.20110125.orig/gdb/dwarf2read.c	2011-01-12 19:16:20.000000000 +0300
+++ gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c	2011-02-03 18:29:50.999859400 +0300
@@ -136,6 +136,7 @@
   int was_mmapped;
   /* True if we have tried to read this section.  */
   int readin;
+  bfd_size_type uncompressed_size;
 };
 
 /* All offsets in the index are of this type.  It must be
@@ -1357,6 +1358,59 @@
 		  && strcmp (section_name + 2, name) == 0)));
 }
 
+static int
+is_compressed_section_name (const char *section_name)
+{
+  return (section_name[0] == '.' && section_name[1] == 'z');
+}
+
+static int
+parse_zlib_section_header (bfd_byte *compressed_buffer, bfd_size_type *size)
+{
+  bfd_size_type uncompressed_size;
+
+ /* Read the zlib header. In this case, it should be "ZLIB" followed
+     by the uncompressed section size, 8 bytes in big-endian order.  */
+  if (strncmp (compressed_buffer, "ZLIB", 4) != 0)
+    return FALSE;
+  uncompressed_size = compressed_buffer[4]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[5]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[6]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[7]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[8]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[9]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[10]; uncompressed_size <<= 8;
+  uncompressed_size += compressed_buffer[11];
+  *size = uncompressed_size;
+  return TRUE;
+}
+
+static int
+read_uncompressed_size (bfd *abfd, asection *sec, bfd_size_type *size)
+{
+  bfd_byte compressed_buffer [12];
+  bfd_size_type uncompressed_size;
+
+  if (sec->size < 12 || !bfd_get_section_contents (abfd, sec,
+					compressed_buffer, 0, 12))
+    return FALSE;
+  return parse_zlib_section_header (compressed_buffer, size);
+}
+
+static void
+fill_dwarf2_section_info (struct dwarf2_section_info* info,
+			  bfd *abfd, asection *sectp)
+{
+  bfd_size_type size;
+
+  info->asection = sectp;
+  info->size = bfd_get_section_size (sectp);
+  info->uncompressed_size = info->size;
+  if (!is_compressed_section_name (sectp->name))
+    return;
+  read_uncompressed_size (abfd, sectp, &info->uncompressed_size);
+}
+
 /* This function is mapped across the sections and remembers the
    offset and size of each of the debugging sections we are interested
    in.  */
@@ -1366,38 +1420,31 @@
 {
   if (section_is_p (sectp->name, INFO_SECTION))
     {
-      dwarf2_per_objfile->info.asection = sectp;
-      dwarf2_per_objfile->info.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info (&dwarf2_per_objfile->info, abfd, sectp);
     }
   else if (section_is_p (sectp->name, ABBREV_SECTION))
     {
-      dwarf2_per_objfile->abbrev.asection = sectp;
-      dwarf2_per_objfile->abbrev.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info (&dwarf2_per_objfile->abbrev, abfd, sectp);
     }
   else if (section_is_p (sectp->name, LINE_SECTION))
     {
-      dwarf2_per_objfile->line.asection = sectp;
-      dwarf2_per_objfile->line.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info (&dwarf2_per_objfile->line, abfd, sectp);
     }
   else if (section_is_p (sectp->name, LOC_SECTION))
     {
-      dwarf2_per_objfile->loc.asection = sectp;
-      dwarf2_per_objfile->loc.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info (&dwarf2_per_objfile->loc, abfd, sectp);
     }
   else if (section_is_p (sectp->name, MACINFO_SECTION))
     {
-      dwarf2_per_objfile->macinfo.asection = sectp;
-      dwarf2_per_objfile->macinfo.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info (&dwarf2_per_objfile->macinfo, abfd, sectp);
     }
   else if (section_is_p (sectp->name, STR_SECTION))
     {
-      dwarf2_per_objfile->str.asection = sectp;
-      dwarf2_per_objfile->str.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info (&dwarf2_per_objfile->str, abfd, sectp);
     }
   else if (section_is_p (sectp->name, FRAME_SECTION))
     {
-      dwarf2_per_objfile->frame.asection = sectp;
-      dwarf2_per_objfile->frame.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info (&dwarf2_per_objfile->frame, abfd, sectp);
     }
   else if (section_is_p (sectp->name, EH_FRAME_SECTION))
     {
@@ -1405,24 +1452,20 @@
 
       if (aflag & SEC_HAS_CONTENTS)
         {
-	  dwarf2_per_objfile->eh_frame.asection = sectp;
-          dwarf2_per_objfile->eh_frame.size = bfd_get_section_size (sectp);
+          fill_dwarf2_section_info (&dwarf2_per_objfile->eh_frame, abfd, sectp);
         }
     }
   else if (section_is_p (sectp->name, RANGES_SECTION))
     {
-      dwarf2_per_objfile->ranges.asection = sectp;
-      dwarf2_per_objfile->ranges.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info (&dwarf2_per_objfile->ranges, abfd, sectp);
     }
   else if (section_is_p (sectp->name, TYPES_SECTION))
     {
-      dwarf2_per_objfile->types.asection = sectp;
-      dwarf2_per_objfile->types.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info (&dwarf2_per_objfile->types, abfd, sectp);
     }
   else if (section_is_p (sectp->name, GDB_INDEX_SECTION))
     {
-      dwarf2_per_objfile->gdb_index.asection = sectp;
-      dwarf2_per_objfile->gdb_index.size = bfd_get_section_size (sectp);
+      fill_dwarf2_section_info (&dwarf2_per_objfile->gdb_index, abfd, sectp);
     }
 
   if ((bfd_get_section_flags (abfd, sectp) & SEC_LOAD)
@@ -1435,7 +1478,7 @@
 
 static void
 zlib_decompress_section (struct objfile *objfile, asection *sectp,
-                         gdb_byte **outbuf, bfd_size_type *outsize)
+                         gdb_byte **outbuf)
 {
   bfd *abfd = objfile->obfd;
 #ifndef HAVE_ZLIB_H
@@ -1452,26 +1495,19 @@
   int rc;
   int header_size = 12;
 
+  if (compressed_size < header_size)
+    error (_("Dwarf Error: Too small DWARF ZLIB header from '%s'"),
+           bfd_get_filename (abfd));
+
   if (bfd_seek (abfd, sectp->filepos, SEEK_SET) != 0
       || bfd_bread (compressed_buffer,
 		    compressed_size, abfd) != compressed_size)
     error (_("Dwarf Error: Can't read DWARF data from '%s'"),
            bfd_get_filename (abfd));
 
-  /* Read the zlib header.  In this case, it should be "ZLIB" followed
-     by the uncompressed section size, 8 bytes in big-endian order.  */
-  if (compressed_size < header_size
-      || strncmp (compressed_buffer, "ZLIB", 4) != 0)
+  if (!parse_zlib_section_header (compressed_buffer, &uncompressed_size))
     error (_("Dwarf Error: Corrupt DWARF ZLIB header from '%s'"),
            bfd_get_filename (abfd));
-  uncompressed_size = compressed_buffer[4]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[5]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[6]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[7]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[8]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[9]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[10]; uncompressed_size <<= 8;
-  uncompressed_size += compressed_buffer[11];
 
   /* It is possible the section consists of several compressed
      buffers concatenated together, so we uncompress in a loop.  */
@@ -1505,7 +1541,6 @@
 
   do_cleanups (cleanup);
   *outbuf = uncompressed_buffer;
-  *outsize = uncompressed_size;
 #endif
 }
 
@@ -1519,7 +1554,6 @@
   bfd *abfd = objfile->obfd;
   asection *sectp = info->asection;
   gdb_byte *buf, *retbuf;
-  unsigned char header[4];
 
   if (info->readin)
     return;
@@ -1530,18 +1564,12 @@
   if (info->asection == NULL || info->size == 0)
     return;
 
-  /* Check if the file has a 4-byte header indicating compression.  */
-  if (info->size > sizeof (header)
-      && bfd_seek (abfd, sectp->filepos, SEEK_SET) == 0
-      && bfd_bread (header, sizeof (header), abfd) == sizeof (header))
+  /* Check if the file has a 12-byte header indicating compression.  */
+  if (read_uncompressed_size (abfd, sectp, &info->uncompressed_size))
     {
-      /* Upon decompression, update the buffer and its size.  */
-      if (strncmp (header, "ZLIB", sizeof (header)) == 0)
-        {
-          zlib_decompress_section (objfile, sectp, &info->buffer,
-				   &info->size);
-          return;
-        }
+      zlib_decompress_section (objfile, sectp, &info->buffer);
+      info->size = info->uncompressed_size;
+      return;
     }
 
 #ifdef HAVE_MMAP
@@ -14364,7 +14392,7 @@
       /* ".debug_loc" may not exist at all, or the offset may be outside
 	 the section.  If so, fall through to the complaint in the
 	 other branch.  */
-      && DW_UNSND (attr) < dwarf2_per_objfile->loc.size)
+      && DW_UNSND (attr) < dwarf2_per_objfile->loc.uncompressed_size)
     {
       struct dwarf2_loclist_baton *baton;
 

Re: gdb: Incorrect stack unwinding if compressed debug info is used

[Tom Tromey]

>>>>> "Vladimir" == Vladimir Simonov <sv@sw.ru> writes:

Vladimir> I've spend some time and, looks like, found the problem. It is in
Vladimir> dwarf2_symbol_mark_computed function (dwarf2read.c). Check
Vladimir> "DW_UNSND (attr) < dwarf2_per_objfile->loc.size"
Vladimir> is incorrect if compressed section is used.

Thanks for finding this.

Since fill_in_loclist_baton is going to read the loc section right away
anyhow, it seems to me that it is simpler to just have
dwarf2_symbol_mark_computed do it.

Could you try the appended?

I didn't audit the other uses of .size (yet) to see if this problem
occurs elsewhere.

Tom

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 6a98d57..e1657b6 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -14361,6 +14361,13 @@ static void
 dwarf2_symbol_mark_computed (struct attribute *attr, struct symbol *sym,
 			     struct dwarf2_cu *cu)
 {
+  if (attr_form_is_section_offset (attr))
+    /* We need to read the section before we can check its size,
+       because the size is only valid once the section is
+       uncompressed.  */
+    dwarf2_read_section (dwarf2_per_objfile->objfile,
+			 &dwarf2_per_objfile->loc);
+
   if (attr_form_is_section_offset (attr)
       /* ".debug_loc" may not exist at all, or the offset may be outside
 	 the section.  If so, fall through to the complaint in the

Re: gdb: Incorrect stack unwinding if compressed debug info is used

[Tom Tromey]

>>>>> "Paul" == Paul Pluzhnikov <ppluzhnikov@google.com> writes:

Paul> I am curious how your GDB avoids dwarf2_read_section(). As far as
Paul> I can tell, it should always be called (indirectly) by
Paul> dwarf2_initialize_objfile().

I am going to guess that you are looking at a gdb from before the lazy
section reading patch went in.

Tom

Re: gdb: Incorrect stack unwinding if compressed debug info is used

[Vladimir Simonov]

On 02/04/2011 07:34 PM, Tom Tromey wrote:
>>>>>> "Vladimir" == Vladimir Simonov<sv@sw.ru>  writes:
>
> Vladimir>  I've spend some time and, looks like, found the problem. It is in
> Vladimir>  dwarf2_symbol_mark_computed function (dwarf2read.c). Check
> Vladimir>  "DW_UNSND (attr)<  dwarf2_per_objfile->loc.size"
> Vladimir>  is incorrect if compressed section is used.
>
> Thanks for finding this.
>
> Since fill_in_loclist_baton is going to read the loc section right away
> anyhow, it seems to me that it is simpler to just have
> dwarf2_symbol_mark_computed do it.
>
> Could you try the appended?
>
All is OK with it.
I'd also suggest to remove dwarf2_read_section from
fill_in_loclist_baton. Without it all works ok also.

But resulted code looks a bit hackish:
dwarf2_symbol_mark_computed (...)
{
   if (attr_form_is_section_offset (attr))
      /* We need to read the section before we can check its size,
         because the size is only valid once the section is
         uncompressed.  */
      dwarf2_read_section (dwarf2_per_objfile->objfile,
   		 &dwarf2_per_objfile->loc);
   if (attr_form_is_section_offset (attr)
       ........
        && DW_UNSND (attr)<  dwarf2_per_objfile->loc.size)
   )
   {
      ....
      fill_in_loclist_baton /* which uses results of dwarf2_read_section */
      ....
   }
...

Regards
Vladimir

Re: gdb: Incorrect stack unwinding if compressed debug info is used

[Tom Tromey]

>>>>> "Vladimir" == Vladimir Simonov <sv@sw.ru> writes:

Vladimir> I'd also suggest to remove dwarf2_read_section from
Vladimir> fill_in_loclist_baton. Without it all works ok also.

There is another caller of fill_in_loclist_baton, and it wasn't obvious
to me that this change would be safe.

Vladimir> But resulted code looks a bit hackish:

I agree.  Also, I audited all other uses of the 'size' field.
I found one more where it was not obvious whether the use was safe.
And, I found another area that could use a little cleanup.

So, I came up with the appended.  Let me know what you think.  If you
could try it, that would be helpful.

I am running it through the test suite.  I plan to commit it if it all
passes.

Tom

2011-02-04  Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (dwarf2_section_empty_p): New function.
	(dwarf2_read_section): Use dwarf2_section_empty_p.
	(dwarf2_section_size): New function.
	(dwarf2_get_section_info): Unconditionally read section.
	(dwarf2_read_index): Use dwarf2_section_empty_p.
	(partial_read_comp_unit_head): Use dwarf2_section_size.
	(dwarf2_symbol_mark_computed): Likewise.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 6a98d57..d7dd3d5 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1509,6 +1509,14 @@ zlib_decompress_section (struct objfile *objfile, asection *sectp,
 #endif
 }
 
+/* A helper function that decides whether a section is empty.  */
+
+static int
+dwarf2_section_empty_p (struct dwarf2_section_info *info)
+{
+  return info->asection == NULL || info->size == 0;
+}
+
 /* Read the contents of the section SECTP from object file specified by
    OBJFILE, store info about the section into INFO.
    If the section is compressed, uncompress it before returning.  */
@@ -1527,7 +1535,7 @@ dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)
   info->was_mmapped = 0;
   info->readin = 1;
 
-  if (info->asection == NULL || info->size == 0)
+  if (dwarf2_section_empty_p (info))
     return;
 
   /* Check if the file has a 4-byte header indicating compression.  */
@@ -1592,6 +1600,18 @@ dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)
 	   bfd_get_filename (abfd));
 }
 
+/* A helper function that returns the size of a section in a safe
+   way.  */
+
+static bfd_size_type
+dwarf2_section_size (struct objfile *objfile,
+		     struct dwarf2_section_info *info)
+{
+  if (!info->readin)
+    dwarf2_read_section (objfile, info);
+  return info->size;
+}
+
 /* Fill in SECTP, BUFP and SIZEP with section info, given OBJFILE and
    SECTION_NAME.  */
 
@@ -1620,9 +1640,7 @@ dwarf2_get_section_info (struct objfile *objfile, const char *section_name,
   else
     gdb_assert_not_reached ("unexpected section");
 
-  if (info->asection != NULL && info->size != 0 && info->buffer == NULL)
-    /* We haven't read this section in yet.  Do it now.  */
-    dwarf2_read_section (objfile, info);
+  dwarf2_read_section (objfile, info);
 
   *sectp = info->asection;
   *bufp = info->buffer;
@@ -2008,8 +2026,7 @@ dwarf2_read_index (struct objfile *objfile)
   offset_type types_list_elements = 0;
   int i;
 
-  if (dwarf2_per_objfile->gdb_index.asection == NULL
-      || dwarf2_per_objfile->gdb_index.size == 0)
+  if (dwarf2_section_empty_p (&dwarf2_per_objfile->gdb_index))
     return 0;
 
   /* Older elfutils strip versions could keep the section in the main
@@ -2823,7 +2840,9 @@ partial_read_comp_unit_head (struct comp_unit_head *header, gdb_byte *info_ptr,
 	   "(is %d, should be 2, 3, or 4) [in module %s]"), header->version,
 	   bfd_get_filename (abfd));
 
-  if (header->abbrev_offset >= dwarf2_per_objfile->abbrev.size)
+  if (header->abbrev_offset
+      >= dwarf2_section_size (dwarf2_per_objfile->objfile,
+			      &dwarf2_per_objfile->abbrev))
     error (_("Dwarf Error: bad offset (0x%lx) in compilation unit header "
 	   "(offset 0x%lx + 6) [in module %s]"),
 	   (long) header->abbrev_offset,
@@ -14365,7 +14384,8 @@ dwarf2_symbol_mark_computed (struct attribute *attr, struct symbol *sym,
       /* ".debug_loc" may not exist at all, or the offset may be outside
 	 the section.  If so, fall through to the complaint in the
 	 other branch.  */
-      && DW_UNSND (attr) < dwarf2_per_objfile->loc.size)
+      && DW_UNSND (attr) < dwarf2_section_size (dwarf2_per_objfile->objfile,
+						&dwarf2_per_objfile->loc))
     {
       struct dwarf2_loclist_baton *baton;
 

Re: gdb: Incorrect stack unwinding if compressed debug info is used

[Vladimir Simonov]

On 02/04/2011 08:56 PM, Tom Tromey wrote:

>
> So, I came up with the appended.  Let me know what you think.  If you
> could try it, that would be helpful.
>
This one works fine on my test also.

The only I can mention - the name
dwarf2_section_size a bit misleading, it hides
the fact that it will read section if it is not done.
May be just change dwarf2_read_section to return
size?

Also I'm not sure that call functions inside "if"
are encouraged in gdb coding style. I mean
+  if (header->abbrev_offset
+      >= dwarf2_section_size (dwarf2_per_objfile->objfile,
+			      &dwarf2_per_objfile->abbrev))
etc.


Regards
Vladimir

Re: gdb: Incorrect stack unwinding if compressed debug info is used

[Tom Tromey]

>>>>> "Vladimir" == Vladimir Simonov <sv@sw.ru> writes:

Vladimir> The only I can mention - the name
Vladimir> dwarf2_section_size a bit misleading, it hides
Vladimir> the fact that it will read section if it is not done.
Vladimir> May be just change dwarf2_read_section to return
Vladimir> size?

I think the name is ok, but I extended the comment to try to clear up
the semantics.

I am going to check in the appended patch.  If you still think the name
is not good, pick a better one and I will rename it.

Vladimir> Also I'm not sure that call functions inside "if"
Vladimir> are encouraged in gdb coding style. I mean
Vladimir> +  if (header->abbrev_offset
Vladimir> +      >= dwarf2_section_size (dwarf2_per_objfile->objfile,
Vladimir> +			      &dwarf2_per_objfile->abbrev))

This is ok, what is prohibited is assignments in a condition.

Tom

2011-02-04  Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (dwarf2_section_empty_p): New function.
	(dwarf2_read_section): Use dwarf2_section_empty_p.
	(dwarf2_section_size): New function.
	(dwarf2_get_section_info): Unconditionally read section.
	(dwarf2_read_index): Use dwarf2_section_empty_p.
	(partial_read_comp_unit_head): Use dwarf2_section_size.
	(dwarf2_symbol_mark_computed): Likewise.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 6a98d57..f443ba3 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1509,6 +1509,14 @@ zlib_decompress_section (struct objfile *objfile, asection *sectp,
 #endif
 }
 
+/* A helper function that decides whether a section is empty.  */
+
+static int
+dwarf2_section_empty_p (struct dwarf2_section_info *info)
+{
+  return info->asection == NULL || info->size == 0;
+}
+
 /* Read the contents of the section SECTP from object file specified by
    OBJFILE, store info about the section into INFO.
    If the section is compressed, uncompress it before returning.  */
@@ -1527,7 +1535,7 @@ dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)
   info->was_mmapped = 0;
   info->readin = 1;
 
-  if (info->asection == NULL || info->size == 0)
+  if (dwarf2_section_empty_p (info))
     return;
 
   /* Check if the file has a 4-byte header indicating compression.  */
@@ -1592,6 +1600,22 @@ dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)
 	   bfd_get_filename (abfd));
 }
 
+/* A helper function that returns the size of a section in a safe way.
+   If you are positive that the section has been read before using the
+   size, then it is safe to refer to the dwarf2_section_info object's
+   "size" field directly.  In other cases, you must call this
+   function, because for compressed sections the size field is not set
+   correctly until the section has been read.  */
+
+static bfd_size_type
+dwarf2_section_size (struct objfile *objfile,
+		     struct dwarf2_section_info *info)
+{
+  if (!info->readin)
+    dwarf2_read_section (objfile, info);
+  return info->size;
+}
+
 /* Fill in SECTP, BUFP and SIZEP with section info, given OBJFILE and
    SECTION_NAME.  */
 
@@ -1620,9 +1644,7 @@ dwarf2_get_section_info (struct objfile *objfile, const char *section_name,
   else
     gdb_assert_not_reached ("unexpected section");
 
-  if (info->asection != NULL && info->size != 0 && info->buffer == NULL)
-    /* We haven't read this section in yet.  Do it now.  */
-    dwarf2_read_section (objfile, info);
+  dwarf2_read_section (objfile, info);
 
   *sectp = info->asection;
   *bufp = info->buffer;
@@ -2008,8 +2030,7 @@ dwarf2_read_index (struct objfile *objfile)
   offset_type types_list_elements = 0;
   int i;
 
-  if (dwarf2_per_objfile->gdb_index.asection == NULL
-      || dwarf2_per_objfile->gdb_index.size == 0)
+  if (dwarf2_section_empty_p (&dwarf2_per_objfile->gdb_index))
     return 0;
 
   /* Older elfutils strip versions could keep the section in the main
@@ -2823,7 +2844,9 @@ partial_read_comp_unit_head (struct comp_unit_head *header, gdb_byte *info_ptr,
 	   "(is %d, should be 2, 3, or 4) [in module %s]"), header->version,
 	   bfd_get_filename (abfd));
 
-  if (header->abbrev_offset >= dwarf2_per_objfile->abbrev.size)
+  if (header->abbrev_offset
+      >= dwarf2_section_size (dwarf2_per_objfile->objfile,
+			      &dwarf2_per_objfile->abbrev))
     error (_("Dwarf Error: bad offset (0x%lx) in compilation unit header "
 	   "(offset 0x%lx + 6) [in module %s]"),
 	   (long) header->abbrev_offset,
@@ -14365,7 +14388,8 @@ dwarf2_symbol_mark_computed (struct attribute *attr, struct symbol *sym,
       /* ".debug_loc" may not exist at all, or the offset may be outside
 	 the section.  If so, fall through to the complaint in the
 	 other branch.  */
-      && DW_UNSND (attr) < dwarf2_per_objfile->loc.size)
+      && DW_UNSND (attr) < dwarf2_section_size (dwarf2_per_objfile->objfile,
+						&dwarf2_per_objfile->loc))
     {
       struct dwarf2_loclist_baton *baton;
 

Re: gdb: Incorrect stack unwinding if compressed debug info is used

[Vladimir Simonov]

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

On 02/04/2011 11:31 PM, Tom Tromey wrote:
> Vladimir>  May be just change dwarf2_read_section to return
> Vladimir>  size?
>
> I think the name is ok, but I extended the comment to try to clear up
> the semantics.
>
> I am going to check in the appended patch.  If you still think the name
> is not good, pick a better one and I will rename it.

Please see attached patch. Hope it explains what I meant under
"change dwarf2_read_section".
If such changes are not acceptable by some reason(performance?,
readability?, something else) then I'm OK with current naming.

I definitely don't insist on such changes, I just interested
in understanding gdb "coding policy". If it is difficult
to explain please ignore this mail.

Best regards
Vladimir Simonov

[-- Attachment #2: gdb-7.2-compressed-section-cleanup.patch --]
[-- Type: text/plain, Size: 3732 bytes --]

--- gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c.tom	2011-02-05 14:45:40.204371900 +0300
+++ gdb-weekly-CVS-7.2.50.20110125/gdb/dwarf2read.c	2011-02-05 15:41:51.501930300 +0300
@@ -1519,9 +1519,10 @@
 
 /* Read the contents of the section SECTP from object file specified by
    OBJFILE, store info about the section into INFO.
-   If the section is compressed, uncompress it before returning.  */
+   If the section is compressed, uncompress it before returning.
+   Returns the size of data accessable via info->buffer. */
 
-static void
+static bfd_size_type
 dwarf2_read_section (struct objfile *objfile, struct dwarf2_section_info *info)
 {
   bfd *abfd = objfile->obfd;
@@ -1530,13 +1531,13 @@
   unsigned char header[4];
 
   if (info->readin)
-    return;
+    return info->size;
   info->buffer = NULL;
   info->was_mmapped = 0;
   info->readin = 1;
 
   if (dwarf2_section_empty_p (info))
-    return;
+    return info->size;
 
   /* Check if the file has a 4-byte header indicating compression.  */
   if (info->size > sizeof (header)
@@ -1548,7 +1549,7 @@
         {
           zlib_decompress_section (objfile, sectp, &info->buffer,
 				   &info->size);
-          return;
+          return info->size;
         }
     }
 
@@ -1574,7 +1575,7 @@
 #if HAVE_POSIX_MADVISE
 	  posix_madvise (retbuf, map_length, POSIX_MADV_WILLNEED);
 #endif
-	  return;
+	  return info->size;
 	}
     }
 #endif
@@ -1591,28 +1592,13 @@
   if (retbuf != NULL)
     {
       info->buffer = retbuf;
-      return;
+      return info->size;
     }
 
   if (bfd_seek (abfd, sectp->filepos, SEEK_SET) != 0
       || bfd_bread (buf, info->size, abfd) != info->size)
     error (_("Dwarf Error: Can't read DWARF data from '%s'"),
 	   bfd_get_filename (abfd));
-}
-
-/* A helper function that returns the size of a section in a safe way.
-   If you are positive that the section has been read before using the
-   size, then it is safe to refer to the dwarf2_section_info object's
-   "size" field directly.  In other cases, you must call this
-   function, because for compressed sections the size field is not set
-   correctly until the section has been read.  */
-
-static bfd_size_type
-dwarf2_section_size (struct objfile *objfile,
-		     struct dwarf2_section_info *info)
-{
-  if (!info->readin)
-    dwarf2_read_section (objfile, info);
   return info->size;
 }
 
@@ -1644,11 +1630,10 @@
   else
     gdb_assert_not_reached ("unexpected section");
 
-  dwarf2_read_section (objfile, info);
+  *sizep = dwarf2_read_section (objfile, info);
 
   *sectp = info->asection;
   *bufp = info->buffer;
-  *sizep = info->size;
 }
 
 \f
@@ -2149,8 +2134,7 @@
     sec = &dwarf2_per_objfile->types;
   else
     sec = &dwarf2_per_objfile->info;
-  dwarf2_read_section (objfile, sec);
-  buffer_size = sec->size;
+  buffer_size = dwarf2_read_section (objfile, sec);
   buffer = sec->buffer;
   info_ptr = buffer + this_cu->offset;
   beg_of_comp_unit = info_ptr;
@@ -2845,7 +2829,7 @@
 	   bfd_get_filename (abfd));
 
   if (header->abbrev_offset
-      >= dwarf2_section_size (dwarf2_per_objfile->objfile,
+      >= dwarf2_read_section (dwarf2_per_objfile->objfile,
 			      &dwarf2_per_objfile->abbrev))
     error (_("Dwarf Error: bad offset (0x%lx) in compilation unit header "
 	   "(offset 0x%lx + 6) [in module %s]"),
@@ -14413,7 +14397,7 @@
       /* ".debug_loc" may not exist at all, or the offset may be outside
 	 the section.  If so, fall through to the complaint in the
 	 other branch.  */
-      && DW_UNSND (attr) < dwarf2_section_size (dwarf2_per_objfile->objfile,
+      && DW_UNSND (attr) < dwarf2_read_section (dwarf2_per_objfile->objfile,
 						&dwarf2_per_objfile->loc))
     {
       struct dwarf2_loclist_baton *baton;

Re: gdb: Incorrect stack unwinding if compressed debug info is used

[Tom Tromey]

>>>>> "Vladimir" == Vladimir Simonov <sv@sw.ru> writes:

Vladimir> Please see attached patch. Hope it explains what I meant under
Vladimir> "change dwarf2_read_section".
Vladimir> If such changes are not acceptable by some reason(performance?,
Vladimir> readability?, something else) then I'm OK with current naming.

I'm sorry I didn't reply to that part of your message the first time.

I think the current code is a bit clearer, because seeing
"dwarf2_read_section" in a conditional seems strange, but seeing
"dwarf2_section_size" does not.

However, it is a minor point.  I would probably approve either patch :-)

Vladimir> I definitely don't insist on such changes, I just interested
Vladimir> in understanding gdb "coding policy". If it is difficult
Vladimir> to explain please ignore this mail.

For something like this it is just a matter of personal taste, unless
some argument emerges to prefer one approach over the other.  I think I
try to defer to other maintainers or to the original patch submitter on
matters of naming, but it is hard to say if that is what I actually do
;-)

Tom