gdb: Incorrect stack unwinding if debug info is compressed

[Vladimir Simonov <sv@sw.ru>]

[-- 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;