Re: [PATCH] libdw: Add new function dwarf_getlocations.

[Mark Wielaard <mjw at redhat dot com>]

On Wed, 2013-09-04 at 14:14 +0200, Jan Kratochvil wrote:
> Currently you call getlocation->__libdw_intern_expression for every range in
> the location list, even if that range does not contain our PC.  That is both
> CPU expensive and it also "leaks" memory (until cu->dbg gets destroyed) even
> more than formerly (formerly only one range got "leaked" on each
> dwarf_getlocation call).
> 
> My idea was more to create dwarf_getlocations_internal without its last
> getlocation call, use that from refactored dwarf_getlocation and create
> current public dwarf_getlocations as a wrapper around
> dwarf_getlocations_internal with that getlocation call.

Good point, I missed that. It is always good to do less work if at all
possible. I implemented it slightly differently from your suggestion,
but I believe it addresses your point. See new patch attached.

Thanks,

Mark

>From dbe07f66ea3d6123c21c1bb86e74a6fb2e6eba8d Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Fri, 23 Aug 2013 16:12:37 +0200
Subject: [PATCH] libdw: Add new function dwarf_getlocations.

Using dwarf_getlocation it is possible to get single location
descriptions and with dwarf_getlocation_addr it is possible to
get a location list covering a specific address.  But sometimes
it is more convenient to get all ranges covered by a location
list.  For example when a specific address isn't covered and
you want to find alternative addresses where a location
description is defined.

dwarf_getlocations is modelled after dwarf_ranges. It enumerates
the location ranges and descriptions covered by the given
attribute.  In the first call OFFSET should be zero and *BASEP
need not be initialized.  Returns -1 for errors, zero when
there are no more locations to report, or a nonzero OFFSET
value to pass to the next call.  Each subsequent call must
preserve *BASEP from the prior call.  Successful calls fill in
*STARTP and *ENDP with a contiguous address range and *EXPR with
a pointer to an array of operations with length *EXPRLEN.  If
the attribute describes a single location description and not a
location list the first call (with OFFSET zero) will return the
location description in *EXPR with *STARTP set to zero and *ENDP
set to minus one.

ptrdiff_t dwarf_getlocations (Dwarf_Attribute *attr, ptrdiff_t offset,
                              Dwarf_Addr *basep, Dwarf_Addr *startp,
                              Dwarf_Addr *endp, Dwarf_Op **expr,
                              size_t *exprlen);

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libdw/ChangeLog           |   12 ++
 libdw/dwarf_getlocation.c |  273 ++++++++++++++++++++++++++++++++++-----------
 libdw/libdw.h             |   18 +++-
 libdw/libdw.map           |    5 +
 4 files changed, 240 insertions(+), 68 deletions(-)

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index aa1e2cc..153ea74 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,15 @@
+2013-08-23  Mark Wielaard  <mjw@redhat.com>
+
+	* dwarf_getlocation.c (attr_ok): Also accept DW_AT_segment.
+	(attr_base_address): New function.
+	(initial_offset_base): New function.
+	(getlocations_addr): New function. Taken from...
+	(dwarf_getlocation_addr): here. Use new initial_offset_base and
+	getlocations_addr.
+	(dwarf_getlocations): New function.
+	* libdw.h (dwarf_getlocations): New function definition.
+	* libdw.map (ELFUTILS_0.157): New.
+
 2013-07-02  Mark Wielaard  <mjw@redhat.com>
 
 	* dwarf_getsrclines.c (dwarf_getsrclines): Add new stack allocation
diff --git a/libdw/dwarf_getlocation.c b/libdw/dwarf_getlocation.c
index c3f3f47..9ab5ac4 100644
--- a/libdw/dwarf_getlocation.c
+++ b/libdw/dwarf_getlocation.c
@@ -1,5 +1,5 @@
 /* Return location expression list.
-   Copyright (C) 2000-2010 Red Hat, Inc.
+   Copyright (C) 2000-2010, 2013 Red Hat, Inc.
    This file is part of elfutils.
    Written by Ulrich Drepper <drepper@redhat.com>, 2000.
 
@@ -56,6 +56,7 @@ attr_ok (Dwarf_Attribute *attr)
     case DW_AT_frame_base:
     case DW_AT_return_addr:
     case DW_AT_static_link:
+    case DW_AT_segment:
       break;
 
     default:
@@ -567,6 +568,121 @@ dwarf_getlocation (attr, llbuf, listlen)
   return getlocation (attr->cu, &block, llbuf, listlen, cu_sec_idx (attr->cu));
 }
 
+static int
+attr_base_address (attr, basep)
+     Dwarf_Attribute *attr;
+     Dwarf_Addr *basep;
+{
+  /* Fetch the CU's base address.  */
+  Dwarf_Die cudie = CUDIE (attr->cu);
+
+  /* Find the base address of the compilation unit.  It will
+     normally be specified by DW_AT_low_pc.  In DWARF-3 draft 4,
+     the base address could be overridden by DW_AT_entry_pc.  It's
+     been removed, but GCC emits DW_AT_entry_pc and not DW_AT_lowpc
+     for compilation units with discontinuous ranges.  */
+  Dwarf_Attribute attr_mem;
+  if (unlikely (INTUSE(dwarf_lowpc) (&cudie, basep) != 0)
+      && INTUSE(dwarf_formaddr) (INTUSE(dwarf_attr) (&cudie,
+						     DW_AT_entry_pc,
+						     &attr_mem),
+				 basep) != 0)
+    {
+      if (INTUSE(dwarf_errno) () != 0)
+	return -1;
+
+      /* The compiler provided no base address when it should
+	 have.  Buggy GCC does this when it used absolute
+	 addresses in the location list and no DW_AT_ranges.  */
+      *basep = 0;
+    }
+  return 0;
+}
+
+static int
+initial_offset_base (attr, offset, basep)
+     Dwarf_Attribute *attr;
+     ptrdiff_t *offset;
+     Dwarf_Addr *basep;
+{
+  if (attr_base_address (attr, basep) != 0)
+    return -1;
+
+  Dwarf_Word start_offset;
+  if (__libdw_formptr (attr, IDX_debug_loc,
+		       DWARF_E_NO_LOCLIST,
+		       NULL, &start_offset) == NULL)
+    return -1;
+
+  *offset = start_offset;
+  return 0;
+}
+
+static ptrdiff_t
+getlocations_addr (attr, offset, basep, startp, endp, address,
+		   locs, expr, exprlen)
+     Dwarf_Attribute *attr;
+     ptrdiff_t offset;
+     Dwarf_Addr *basep;
+     Dwarf_Addr *startp;
+     Dwarf_Addr *endp;
+     Dwarf_Addr address;
+     Elf_Data *locs;
+     Dwarf_Op **expr;
+     size_t *exprlen;
+{
+  unsigned char *readp = locs->d_buf + offset;
+  unsigned char *readendp = locs->d_buf + locs->d_size;
+
+ next:
+  if (readendp - readp < attr->cu->address_size * 2)
+    {
+    invalid:
+      __libdw_seterrno (DWARF_E_INVALID_DWARF);
+      return -1;
+    }
+
+  Dwarf_Addr begin;
+  Dwarf_Addr end;
+
+  switch (__libdw_read_begin_end_pair_inc (attr->cu->dbg, IDX_debug_loc,
+					   &readp, attr->cu->address_size,
+					   &begin, &end, basep))
+    {
+    case 0: /* got location range. */
+      break;
+    case 1: /* base address setup. */
+      goto next;
+    case 2: /* end of loclist */
+      return 0;
+    default: /* error */
+      return -1;
+    }
+
+  if (readendp - readp < 2)
+    goto invalid;
+
+  /* We have a location expression.  */
+  Dwarf_Block block;
+  block.length = read_2ubyte_unaligned_inc (attr->cu->dbg, readp);
+  block.data = readp;
+  if (readendp - readp < (ptrdiff_t) block.length)
+    goto invalid;
+  readp += block.length;
+
+  *startp = *basep + begin;
+  *endp = *basep + end;
+
+  /* If address is zero we want them all, otherwise only those that match.  */
+  if (address != 0 && (address < *endp || address >= *startp))
+    goto next;
+
+  if (getlocation (attr->cu, &block, expr, exprlen, IDX_debug_loc) != 0)
+    return -1;
+
+  return readp - (unsigned char *) locs->d_buf;
+}
+
 int
 dwarf_getlocation_addr (attr, address, llbufs, listlens, maxlocs)
      Dwarf_Attribute *attr;
@@ -605,85 +721,108 @@ dwarf_getlocation_addr (attr, address, llbufs, listlens, maxlocs)
   if (result != 1)
     return result ?: 1;
 
-  unsigned char *endp;
-  unsigned char *readp = __libdw_formptr (attr, IDX_debug_loc,
-					  DWARF_E_NO_LOCLIST, &endp, NULL);
-  if (readp == NULL)
+  Dwarf_Addr base, start, end;
+  Dwarf_Op *expr;
+  size_t expr_len;
+  ptrdiff_t off = 0;
+  size_t got = 0;
+
+  /* This is a true loclistptr, fetch the initial base address and offset.  */
+  if (initial_offset_base (attr, &off, &base) != 0)
     return -1;
 
-  Dwarf_Addr base = (Dwarf_Addr) -1;
-  size_t got = 0;
-  while (got < maxlocs)
+  const Elf_Data *d = attr->cu->dbg->sectiondata[IDX_debug_loc];
+  if (d == NULL)
     {
-      if (endp - readp < attr->cu->address_size * 2)
+      __libdw_seterrno (DWARF_E_NO_LOCLIST);
+      return -1;
+    }
+
+  while (got < maxlocs
+         && (off = getlocations_addr (attr, off, &base, &start, &end,
+				   address, d, &expr, &expr_len)) > 0)
+    {
+      /* This one matches the address.  */
+      if (llbufs != NULL)
 	{
-	invalid:
-	  __libdw_seterrno (DWARF_E_INVALID_DWARF);
-	  return -1;
+	  llbufs[got] = expr;
+	  listlens[got] = expr_len;
 	}
+      ++got;
+    }
 
-      Dwarf_Addr begin;
-      Dwarf_Addr end;
+  if (off != 0)
+    return -1;
 
-      int status
-	= __libdw_read_begin_end_pair_inc (attr->cu->dbg, IDX_debug_loc,
-					   &readp, attr->cu->address_size,
-					   &begin, &end, &base);
-      if (status == 2) /* End of list entry.  */
-	break;
-      else if (status == 1) /* Base address selected.  */
-	continue;
-      else if (status < 0)
-	return status;
-
-      if (endp - readp < 2)
-	goto invalid;
-
-      /* We have a location expression.  */
-      block.length = read_2ubyte_unaligned_inc (attr->cu->dbg, readp);
-      block.data = readp;
-      if (endp - readp < (ptrdiff_t) block.length)
-	goto invalid;
-      readp += block.length;
-
-      if (base == (Dwarf_Addr) -1)
+  return got;
+}
+
+ptrdiff_t
+dwarf_getlocations (attr, offset, basep, startp, endp, expr, exprlen)
+     Dwarf_Attribute *attr;
+     ptrdiff_t offset;
+     Dwarf_Addr *basep;
+     Dwarf_Addr *startp;
+     Dwarf_Addr *endp;
+     Dwarf_Op **expr;
+     size_t *exprlen;
+{
+  if (! attr_ok (attr))
+    return -1;
+
+  /* 1 is an invalid offset, meaning no more locations. */
+  if (offset == 1)
+    return 0;
+
+  if (offset == 0)
+    {
+      /* If it has a block form, it's a single location expression.  */
+      Dwarf_Block block;
+      if (INTUSE(dwarf_formblock) (attr, &block) == 0)
 	{
-	  /* Fetch the CU's base address.  */
-	  Dwarf_Die cudie = CUDIE (attr->cu);
-
-	  /* Find the base address of the compilation unit.  It will
-	     normally be specified by DW_AT_low_pc.  In DWARF-3 draft 4,
-	     the base address could be overridden by DW_AT_entry_pc.  It's
-	     been removed, but GCC emits DW_AT_entry_pc and not DW_AT_lowpc
-	     for compilation units with discontinuous ranges.  */
-	  Dwarf_Attribute attr_mem;
-	  if (unlikely (INTUSE(dwarf_lowpc) (&cudie, &base) != 0)
-	      && INTUSE(dwarf_formaddr) (INTUSE(dwarf_attr) (&cudie,
-							     DW_AT_entry_pc,
-							     &attr_mem),
-					 &base) != 0)
-	    {
-	      if (INTUSE(dwarf_errno) () != 0)
-		return -1;
+	  if (getlocation (attr->cu, &block, expr, exprlen,
+			   cu_sec_idx (attr->cu)) != 0)
+	    return -1;
 
-	      /* The compiler provided no base address when it should
-		 have.  Buggy GCC does this when it used absolute
-		 addresses in the location list and no DW_AT_ranges.  */
-	      base = 0;
-	    }
+	  /* This is the one and only location covering everything. */
+	  *startp = 0;
+	  *endp = -1;
+	  return 1;
 	}
 
-      if (address >= base + begin && address < base + end)
+      int error = INTUSE(dwarf_errno) ();
+      if (unlikely (error != DWARF_E_NO_BLOCK))
 	{
-	  /* This one matches the address.  */
-	  if (llbufs != NULL
-	      && unlikely (getlocation (attr->cu, &block,
-					&llbufs[got], &listlens[got],
-					IDX_debug_loc) != 0))
-	    return -1;
-	  ++got;
+	  __libdw_seterrno (error);
+	  return -1;
 	}
+
+      int result = check_constant_offset (attr, expr, exprlen);
+      if (result != 1)
+	{
+	  if (result == 0)
+	    {
+	      /* This is the one and only location covering everything. */
+	      *startp = 0;
+	      *endp = -1;
+	      return 1;
+	    }
+	  return result;
+	}
+
+      /* We must be looking at a true loclistptr, fetch the initial
+	 base address and offset.  */
+      if (initial_offset_base (attr, &offset, basep) != 0)
+	return -1;
     }
 
-  return got;
+  const Elf_Data *d = attr->cu->dbg->sectiondata[IDX_debug_loc];
+  if (d == NULL)
+    {
+      __libdw_seterrno (DWARF_E_NO_LOCLIST);
+      return -1;
+    }
+
+  return getlocations_addr (attr, offset, basep, startp, endp, 0, d,
+			    expr, exprlen);
 }
diff --git a/libdw/libdw.h b/libdw/libdw.h
index f5fc4e2..898aa74 100644
--- a/libdw/libdw.h
+++ b/libdw/libdw.h
@@ -1,5 +1,5 @@
 /* Interfaces for libdw.
-   Copyright (C) 2002-2010 Red Hat, Inc.
+   Copyright (C) 2002-2010, 2013 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -630,6 +630,22 @@ extern int dwarf_getlocation_addr (Dwarf_Attribute *attr, Dwarf_Addr address,
 				   Dwarf_Op **exprs, size_t *exprlens,
 				   size_t nlocs);
 
+/* Enumerate the locations ranges and descriptions covered by the
+   given attribute.  In the first call OFFSET should be zero and
+   *BASEP need not be initialized.  Returns -1 for errors, zero when
+   there are no more locations to report, or a nonzero OFFSET
+   value to pass to the next call.  Each subsequent call must preserve
+   *BASEP from the prior call.  Successful calls fill in *STARTP and
+   *ENDP with a contiguous address range and *EXPR with a pointer to
+   an array of operations with length *EXPRLEN.  If the attribute
+   describes a single location description and not a location list the
+   first call (with OFFSET zero) will return the location description
+   in *EXPR with *STARTP set to zero and *ENDP set to minus one.  */
+extern ptrdiff_t dwarf_getlocations (Dwarf_Attribute *attr,
+				     ptrdiff_t offset, Dwarf_Addr *basep,
+				     Dwarf_Addr *startp, Dwarf_Addr *endp,
+				     Dwarf_Op **expr, size_t *exprlen);
+
 /* Return the block associated with a DW_OP_implicit_value operation.
    The OP pointer must point into an expression that dwarf_getlocation
    or dwarf_getlocation_addr has returned given the same ATTR.  */
diff --git a/libdw/libdw.map b/libdw/libdw.map
index d38a8ef..2d2d37c 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -260,3 +260,8 @@ ELFUTILS_0.156 {
     # Replaced ELFUTILS_0.122 version, which has a wrapper without add_p_vaddr.
     dwfl_report_elf;
 } ELFUTILS_0.149;
+
+ELFUTILS_0.157 {
+  global:
+    dwarf_getlocations;
+} ELFUTILS_0.156;
-- 
1.7.1