[PATCH 2/3] libdw: optimize dwarf_hasattr to just look@abbrev

[PATCH 2/3] libdw: optimize dwarf_hasattr to just look@abbrev

[Josh Stone]

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

To just check the presence of an attribute, we only need to look at the
abbreviation data.  This optimization avoids reading over die data at all,
except possibly just to get the abbrev code.

Signed-off-by: Josh Stone <jistone@redhat.com>
---
 libdw/ChangeLog       |  3 +++
 libdw/dwarf_hasattr.c | 42 ++++++++++++++++++++++++++++++++++++++----
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index a2a4e1d49747..2a69bea371fc 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,4 +1,7 @@
 2014-12-10  Josh Stone  <jistone@redhat.com>
+	* dwarf_hasattr.c (dwarf_hasattr): Just walk abbrev for presence.
+
+2014-12-10  Josh Stone  <jistone@redhat.com>
 
 	* libdwP.h (__libdw_dieabbrev): New die->abbrev lookup function.
 	* dwarf_child.c (__libdw_find_attr, dwarf_child): Use it.
diff --git a/libdw/dwarf_hasattr.c b/libdw/dwarf_hasattr.c
index fb7e1d5ed495..6ed1a98cbb49 100644
--- a/libdw/dwarf_hasattr.c
+++ b/libdw/dwarf_hasattr.c
@@ -43,10 +43,44 @@ dwarf_hasattr (die, search_name)
   if (die == NULL)
     return 0;
 
-  /* Search for the attribute with the given name.  */
-  unsigned int code;
-  unsigned char *addr = __libdw_find_attr (die, search_name, &code, NULL);
+  /* Find the abbreviation entry.  */
+  Dwarf_Abbrev *abbrevp = __libdw_dieabbrev (die, NULL);
+  if (unlikely (abbrevp == DWARF_END_ABBREV))
+    {
+    invalid_dwarf:
+      __libdw_seterrno (DWARF_E_INVALID_DWARF);
+      return 0;
+    }
 
-  return addr != NULL && code == search_name;
+  Dwarf *dbg = die->cu->dbg;
+
+  /* Search the name attribute.  */
+  unsigned char *const endp
+    = ((unsigned char *) dbg->sectiondata[IDX_debug_abbrev]->d_buf
+       + dbg->sectiondata[IDX_debug_abbrev]->d_size);
+
+  const unsigned char *attrp = abbrevp->attrp;
+  while (1)
+    {
+      /* Are we still in bounds?  This test needs to be refined.  */
+      if (unlikely (attrp + 1 >= endp))
+	goto invalid_dwarf;
+
+      /* Get attribute name and form.
+
+	 XXX We don't check whether this reads beyond the end of the
+	 section.  */
+      unsigned int attr_name;
+      get_uleb128 (attr_name, attrp);
+      unsigned int attr_form;
+      get_uleb128 (attr_form, attrp);
+
+      /* We can stop if we found the attribute with value zero.  */
+      if (attr_name == 0 || attr_form == 0)
+	return 0;
+
+      if (attr_name == search_name)
+	return 1;
+    }
 }
 INTDEF (dwarf_hasattr)
-- 
2.1.0

Re: [PATCH 2/3] libdw: optimize dwarf_hasattr to just look@abbrev

[Mark Wielaard]

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

On Wed, 2014-12-10 at 18:48 -0800, Josh Stone wrote:
> To just check the presence of an attribute, we only need to look at the
> abbreviation data.  This optimization avoids reading over die data at all,
> except possibly just to get the abbrev code.
>
>  2014-12-10  Josh Stone  <jistone@redhat.com>
> +	* dwarf_hasattr.c (dwarf_hasattr): Just walk abbrev for presence.
> +

Sorry for the irritating nitpick, but there is a missing blank line
after the date line in the ChangeLog entry.

Looks nice. Good find!

Thanks,

Mark