Re: [PATCH] Use libbacktrace as libsanitizer's symbolizer

[Ian Lance Taylor <iant at google dot com>]

On Wed, Nov 27, 2013 at 10:05 AM, Alexey Samsonov <samsonov@google.com> wrote:
>
> Sorry, I wasn't clear enough. GCC emits DW_AT_low_pc and DW_AT_high_pc for
> *compile unit* DIE, so the consumer doesn't have to iterate over all
> the functions DIEs.
> LLVM doesn't emit DW_AT_low_pc/DW_AT_high_pc for compile unit,
> but emits them for function DIEs.
>
> I've added some debugging output to libbacktrace and noticed it was
> able to build
> [address range]->[compile unit] mapping only for sources compiled with GCC, so I
> suggested that it doesn't examine all the descendants of compile unit
> DIEs to build the ranges.

Thanks.

This patch fixes up libbacktrace enough so that the libbacktrace
testsuite works using clang 3.0.

The main change is that the compilation unit data structure is created
sooner.  For GCC, where the range will be in the DW_TAG_compile_unit,
it won't read anything else.  If it fails to find the PC range in the
compile_unit, it will it read all the child DIEs and add PC ranges as
it finds them.

OK to commit in stage 3?

Ian


2013-11-27  Ian Lance Taylor  <iant@google.com>

	* dwarf.c (find_address_ranges): New static function, broken out
	of build_address_map.
	(build_address_map): Call it.
	* btest.c (check): Check for missing filename or function, rather
	than crashing.
	(f3): Check that enough frames were returned.

Index: dwarf.c
===================================================================
--- dwarf.c	(revision 205274)
+++ dwarf.c	(working copy)
@@ -1235,54 +1235,24 @@ add_unit_ranges (struct backtrace_state
   return 1;
 }
 
-/* Build a mapping from address ranges to the compilation units where
-   the line number information for that range can be found.  Returns 1
-   on success, 0 on failure.  */
+/* Find the address range covered by a compilation unit, reading from
+   UNIT_BUF and adding values to U.  Returns 1 if all data could be
+   read, 0 if there is some error.  */
 
 static int
-build_address_map (struct backtrace_state *state, uintptr_t base_address,
-		   const unsigned char *dwarf_info, size_t dwarf_info_size,
-		   const unsigned char *dwarf_abbrev, size_t dwarf_abbrev_size,
-		   const unsigned char *dwarf_ranges, size_t dwarf_ranges_size,
-		   const unsigned char *dwarf_str, size_t dwarf_str_size,
-		   int is_bigendian, backtrace_error_callback error_callback,
-		   void *data, struct unit_addrs_vector *addrs)
+find_address_ranges (struct backtrace_state *state, uintptr_t base_address,
+		     struct dwarf_buf *unit_buf, 
+		     const unsigned char *dwarf_str, size_t dwarf_str_size,
+		     const unsigned char *dwarf_ranges,
+		     size_t dwarf_ranges_size,
+		     int is_bigendian, backtrace_error_callback error_callback,
+		     void *data, struct unit *u,
+		     struct unit_addrs_vector *addrs)
 {
-  struct dwarf_buf info;
-  struct abbrevs abbrevs;
-
-  memset (&addrs->vec, 0, sizeof addrs->vec);
-  addrs->count = 0;
-
-  /* Read through the .debug_info section.  FIXME: Should we use the
-     .debug_aranges section?  gdb and addr2line don't use it, but I'm
-     not sure why.  */
-
-  info.name = ".debug_info";
-  info.start = dwarf_info;
-  info.buf = dwarf_info;
-  info.left = dwarf_info_size;
-  info.is_bigendian = is_bigendian;
-  info.error_callback = error_callback;
-  info.data = data;
-  info.reported_underflow = 0;
-
-  memset (&abbrevs, 0, sizeof abbrevs);
-  while (info.left > 0)
+  while (unit_buf->left > 0)
     {
-      const unsigned char *unit_data_start;
-      uint64_t len;
-      int is_dwarf64;
-      struct dwarf_buf unit_buf;
-      int version;
-      uint64_t abbrev_offset;
-      const struct abbrev *abbrev;
-      int addrsize;
-      const unsigned char *unit_data;
-      size_t unit_data_len;
-      size_t unit_data_offset;
       uint64_t code;
-      size_t i;
+      const struct abbrev *abbrev;
       uint64_t lowpc;
       int have_lowpc;
       uint64_t highpc;
@@ -1290,57 +1260,15 @@ build_address_map (struct backtrace_stat
       int highpc_is_relative;
       uint64_t ranges;
       int have_ranges;
-      uint64_t lineoff;
-      int have_lineoff;
-      const char *filename;
-      const char *comp_dir;
-
-      if (info.reported_underflow)
-	goto fail;
-
-      unit_data_start = info.buf;
-
-      is_dwarf64 = 0;
-      len = read_uint32 (&info);
-      if (len == 0xffffffff)
-	{
-	  len = read_uint64 (&info);
-	  is_dwarf64 = 1;
-	}
-
-      unit_buf = info;
-      unit_buf.left = len;
-
-      if (!advance (&info, len))
-	goto fail;
-
-      version = read_uint16 (&unit_buf);
-      if (version < 2 || version > 4)
-	{
-	  dwarf_buf_error (&unit_buf, "unrecognized DWARF version");
-	  goto fail;
-	}
-
-      abbrev_offset = read_offset (&unit_buf, is_dwarf64);
-      if (!read_abbrevs (state, abbrev_offset, dwarf_abbrev, dwarf_abbrev_size,
-			 is_bigendian, error_callback, data, &abbrevs))
-	goto fail;
-
-      addrsize = read_byte (&unit_buf);
+      size_t i;
 
-      unit_data = unit_buf.buf;
-      unit_data_len = unit_buf.left;
-      unit_data_offset = unit_buf.buf - unit_data_start;
-
-      /* We only look at the first attribute in the compilation unit.
-	 In practice this will be a DW_TAG_compile_unit which will
-	 tell us the PC range and where to find the line number
-	 information.  */
+      code = read_uleb128 (unit_buf);
+      if (code == 0)
+	return 1;
 
-      code = read_uleb128 (&unit_buf);
-      abbrev = lookup_abbrev (&abbrevs, code, error_callback, data);
+      abbrev = lookup_abbrev (&u->abbrevs, code, error_callback, data);
       if (abbrev == NULL)
-	goto fail;
+	return 0;
 
       lowpc = 0;
       have_lowpc = 0;
@@ -1349,18 +1277,14 @@ build_address_map (struct backtrace_stat
       highpc_is_relative = 0;
       ranges = 0;
       have_ranges = 0;
-      lineoff = 0;
-      have_lineoff = 0;
-      filename = NULL;
-      comp_dir = NULL;
       for (i = 0; i < abbrev->num_attrs; ++i)
 	{
 	  struct attr_val val;
 
-	  if (!read_attribute (abbrev->attrs[i].form, &unit_buf, is_dwarf64,
-			       version, addrsize, dwarf_str, dwarf_str_size,
-			       &val))
-	    goto fail;
+	  if (!read_attribute (abbrev->attrs[i].form, unit_buf,
+			       u->is_dwarf64, u->version, u->addrsize,
+			       dwarf_str, dwarf_str_size, &val))
+	    return 0;
 
 	  switch (abbrev->attrs[i].name)
 	    {
@@ -1371,6 +1295,7 @@ build_address_map (struct backtrace_stat
 		  have_lowpc = 1;
 		}
 	      break;
+
 	    case DW_AT_high_pc:
 	      if (val.encoding == ATTR_VAL_ADDRESS)
 		{
@@ -1384,6 +1309,7 @@ build_address_map (struct backtrace_stat
 		  highpc_is_relative = 1;
 		}
 	      break;
+
 	    case DW_AT_ranges:
 	      if (val.encoding == ATTR_VAL_UINT
 		  || val.encoding == ATTR_VAL_REF_SECTION)
@@ -1392,73 +1318,46 @@ build_address_map (struct backtrace_stat
 		  have_ranges = 1;
 		}
 	      break;
+
 	    case DW_AT_stmt_list:
-	      if (val.encoding == ATTR_VAL_UINT
-		  || val.encoding == ATTR_VAL_REF_SECTION)
-		{
-		  lineoff = val.u.uint;
-		  have_lineoff = 1;
-		}
+	      if (abbrev->tag == DW_TAG_compile_unit
+		  && (val.encoding == ATTR_VAL_UINT
+		      || val.encoding == ATTR_VAL_REF_SECTION))
+		u->lineoff = val.u.uint;
 	      break;
+
 	    case DW_AT_name:
-	      if (val.encoding == ATTR_VAL_STRING)
-		filename = val.u.string;
+	      if (abbrev->tag == DW_TAG_compile_unit
+		  && val.encoding == ATTR_VAL_STRING)
+		u->filename = val.u.string;
 	      break;
+
 	    case DW_AT_comp_dir:
-	      if (val.encoding == ATTR_VAL_STRING)
-		comp_dir = val.u.string;
+	      if (abbrev->tag == DW_TAG_compile_unit
+		  && val.encoding == ATTR_VAL_STRING)
+		u->comp_dir = val.u.string;
 	      break;
+
 	    default:
 	      break;
 	    }
 	}
 
-      if (unit_buf.reported_underflow)
-	goto fail;
-
-      if (((have_lowpc && have_highpc) || have_ranges) && have_lineoff)
+      if (abbrev->tag == DW_TAG_compile_unit
+	  || abbrev->tag == DW_TAG_subprogram)
 	{
-	  struct unit *u;
-	  struct unit_addrs a;
-
-	  u = ((struct unit *)
-	       backtrace_alloc (state, sizeof *u, error_callback, data));
-	  if (u == NULL)
-	    goto fail;
-	  u->unit_data = unit_data;
-	  u->unit_data_len = unit_data_len;
-	  u->unit_data_offset = unit_data_offset;
-	  u->version = version;
-	  u->is_dwarf64 = is_dwarf64;
-	  u->addrsize = addrsize;
-	  u->filename = filename;
-	  u->comp_dir = comp_dir;
-	  u->abs_filename = NULL;
-	  u->lineoff = lineoff;
-	  u->abbrevs = abbrevs;
-	  memset (&abbrevs, 0, sizeof abbrevs);
-
-	  /* The actual line number mappings will be read as
-	     needed.  */
-	  u->lines = NULL;
-	  u->lines_count = 0;
-	  u->function_addrs = NULL;
-	  u->function_addrs_count = 0;
-
 	  if (have_ranges)
 	    {
 	      if (!add_unit_ranges (state, base_address, u, ranges, lowpc,
 				    is_bigendian, dwarf_ranges,
-				    dwarf_ranges_size, error_callback, data,
-				    addrs))
-		{
-		  free_abbrevs (state, &u->abbrevs, error_callback, data);
-		  backtrace_free (state, u, sizeof *u, error_callback, data);
-		  goto fail;
-		}
+				    dwarf_ranges_size, error_callback,
+				    data, addrs))
+		return 0;
 	    }
-	  else
+	  else if (have_lowpc && have_highpc)
 	    {
+	      struct unit_addrs a;
+
 	      if (highpc_is_relative)
 		highpc += lowpc;
 	      a.low = lowpc;
@@ -1467,17 +1366,146 @@ build_address_map (struct backtrace_stat
 
 	      if (!add_unit_addr (state, base_address, a, error_callback, data,
 				  addrs))
-		{
-		  free_abbrevs (state, &u->abbrevs, error_callback, data);
-		  backtrace_free (state, u, sizeof *u, error_callback, data);
-		  goto fail;
-		}
+		return 0;
 	    }
+
+	  /* If we found the PC range in the DW_TAG_compile_unit, we
+	     can stop now.  */
+	  if (abbrev->tag == DW_TAG_compile_unit
+	      && (have_ranges || (have_lowpc && have_highpc)))
+	    return 1;
+	}
+
+      if (abbrev->has_children)
+	{
+	  if (!find_address_ranges (state, base_address, unit_buf,
+				    dwarf_str, dwarf_str_size,
+				    dwarf_ranges, dwarf_ranges_size,
+				    is_bigendian, error_callback, data,
+				    u, addrs))
+	    return 0;
+	}
+    }
+
+  return 1;
+}
+
+/* Build a mapping from address ranges to the compilation units where
+   the line number information for that range can be found.  Returns 1
+   on success, 0 on failure.  */
+
+static int
+build_address_map (struct backtrace_state *state, uintptr_t base_address,
+		   const unsigned char *dwarf_info, size_t dwarf_info_size,
+		   const unsigned char *dwarf_abbrev, size_t dwarf_abbrev_size,
+		   const unsigned char *dwarf_ranges, size_t dwarf_ranges_size,
+		   const unsigned char *dwarf_str, size_t dwarf_str_size,
+		   int is_bigendian, backtrace_error_callback error_callback,
+		   void *data, struct unit_addrs_vector *addrs)
+{
+  struct dwarf_buf info;
+  struct abbrevs abbrevs;
+
+  memset (&addrs->vec, 0, sizeof addrs->vec);
+  addrs->count = 0;
+
+  /* Read through the .debug_info section.  FIXME: Should we use the
+     .debug_aranges section?  gdb and addr2line don't use it, but I'm
+     not sure why.  */
+
+  info.name = ".debug_info";
+  info.start = dwarf_info;
+  info.buf = dwarf_info;
+  info.left = dwarf_info_size;
+  info.is_bigendian = is_bigendian;
+  info.error_callback = error_callback;
+  info.data = data;
+  info.reported_underflow = 0;
+
+  memset (&abbrevs, 0, sizeof abbrevs);
+  while (info.left > 0)
+    {
+      const unsigned char *unit_data_start;
+      uint64_t len;
+      int is_dwarf64;
+      struct dwarf_buf unit_buf;
+      int version;
+      uint64_t abbrev_offset;
+      int addrsize;
+      struct unit *u;
+
+      if (info.reported_underflow)
+	goto fail;
+
+      unit_data_start = info.buf;
+
+      is_dwarf64 = 0;
+      len = read_uint32 (&info);
+      if (len == 0xffffffff)
+	{
+	  len = read_uint64 (&info);
+	  is_dwarf64 = 1;
 	}
-      else
+
+      unit_buf = info;
+      unit_buf.left = len;
+
+      if (!advance (&info, len))
+	goto fail;
+
+      version = read_uint16 (&unit_buf);
+      if (version < 2 || version > 4)
 	{
-	  free_abbrevs (state, &abbrevs, error_callback, data);
-	  memset (&abbrevs, 0, sizeof abbrevs);
+	  dwarf_buf_error (&unit_buf, "unrecognized DWARF version");
+	  goto fail;
+	}
+
+      abbrev_offset = read_offset (&unit_buf, is_dwarf64);
+      if (!read_abbrevs (state, abbrev_offset, dwarf_abbrev, dwarf_abbrev_size,
+			 is_bigendian, error_callback, data, &abbrevs))
+	goto fail;
+
+      addrsize = read_byte (&unit_buf);
+
+      u = ((struct unit *)
+	   backtrace_alloc (state, sizeof *u, error_callback, data));
+      if (u == NULL)
+	goto fail;
+      u->unit_data = unit_buf.buf;
+      u->unit_data_len = unit_buf.left;
+      u->unit_data_offset = unit_buf.buf - unit_data_start;
+      u->version = version;
+      u->is_dwarf64 = is_dwarf64;
+      u->addrsize = addrsize;
+      u->filename = NULL;
+      u->comp_dir = NULL;
+      u->abs_filename = NULL;
+      u->lineoff = 0;
+      u->abbrevs = abbrevs;
+      memset (&abbrevs, 0, sizeof abbrevs);
+
+      /* The actual line number mappings will be read as needed.  */
+      u->lines = NULL;
+      u->lines_count = 0;
+      u->function_addrs = NULL;
+      u->function_addrs_count = 0;
+
+      if (!find_address_ranges (state, base_address, &unit_buf,
+				dwarf_str, dwarf_str_size,
+				dwarf_ranges, dwarf_ranges_size,
+				is_bigendian, error_callback, data,
+				u, addrs))
+	{
+	  free_abbrevs (state, &u->abbrevs, error_callback, data);
+	  backtrace_free (state, u, sizeof *u, error_callback, data);
+	  goto fail;
+	}
+
+      if (unit_buf.reported_underflow)
+	{
+	  free_abbrevs (state, &u->abbrevs, error_callback, data);
+	  backtrace_free (state, u, sizeof *u, error_callback, data);
+	  goto fail;
 	}
     }
   if (info.reported_underflow)
Index: btest.c
===================================================================
--- btest.c	(revision 205274)
+++ btest.c	(working copy)
@@ -129,6 +129,13 @@ check (const char *name, int index, cons
 {
   if (*failed)
     return;
+  if (all[index].filename == NULL || all[index].function == NULL)
+    {
+      fprintf (stderr, "%s: [%d]: missing file name or function name\n",
+	       name, index);
+      *failed = 1;
+      return;
+    }
   if (strcmp (base (all[index].filename), "btest.c") != 0)
     {
       fprintf (stderr, "%s: [%d]: got %s expected test.c\n", name, index,
@@ -310,6 +317,14 @@ f3 (int f1line, int f2line)
       data.failed = 1;
     }
 
+  if (data.index < 3)
+    {
+      fprintf (stderr,
+	       "test1: not enough frames; got %zu, expected at least 3\n",
+	       data.index);
+      data.failed = 1;
+    }
+
   check ("test1", 0, all, f3line, "f3", &data.failed);
   check ("test1", 1, all, f2line, "f2", &data.failed);
   check ("test1", 2, all, f1line, "test1", &data.failed);