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

[Mark Wielaard <mjw at redhat dot com>]

On Wed, 2013-09-04 at 17:42 +0200, Jan Kratochvil wrote:
> On Wed, 04 Sep 2013 17:03:55 +0200, Mark Wielaard wrote:
> > -  /* If address is zero we want them all, otherwise only those that match.  */
> > -  if (address != 0 && (address < *endp || address >= *startp))
> > +  /* If address is minus one we want them all, otherwise only matching.  */
> > +  if (address != (Dwarf_Word) -1 && (address < *endp || address >= *startp))
> 
> I did not test it and I am curious it can work but I think here should be:
>      if (address != (Dwarf_Word) -1 && (address < *startp || address >= *endp))

It couldn't work! :) I had seen the same and was indeed scratching my
head why my testing didn't catch that. But in all tests when
dwarf_location_addr was used, it was hitting an expression block, and
not a loc list. So this part of the code was never used... duh.

So I extended the varlocs test to explicitly test all loc lists found:

diff --git a/tests/varlocs.c b/tests/varlocs.c
index cda18b6..6f4d490 100644
--- a/tests/varlocs.c
+++ b/tests/varlocs.c
@@ -687,8 +687,23 @@ print_varlocs (Dwarf_Die *funcdie)
                      printf ("      (%" PRIx64 ",%" PRIx64
                              ") <empty range>\n", begin, end); // XXX report?
                    else
-                     print_expr_block_addrs (&attr, begin, end,
-                                             expr, exprlen);
+                     {
+                       print_expr_block_addrs (&attr, begin, end,
+                                               expr, exprlen);
+
+                       // Extra sanity check for dwarf_getlocation_addr
+                       // Must at least find one range for begin and end-1.
+                       Dwarf_Op *expraddr;
+                       size_t expraddr_len;
+                       int locs = dwarf_getlocation_addr (&attr, begin,
+                                                          &expraddr,
+                                                          &expraddr_len, 1);
+                       assert (locs == 1);
+                       locs = dwarf_getlocation_addr (&attr, end - 1,
+                                                      &expraddr,
+                                                      &expraddr_len, 1);
+                       assert (locs == 1);
+                     }
 
                  if (offset < 0)
                    error (EXIT_FAILURE, 0, "dwarf_getlocations: %s",

That showed the bug, and another slightly more subtle one:

@@ -756,7 +756,8 @@ dwarf_getlocation_addr (attr, address, llbufs, listlens, maxlocs)
       ++got;
     }
 
-  if (off != 0)
+  /* We might stop early, so off can be zero or positive on success.  */
+  if (off < 0)
     return -1;
 
   return got;

I pushed those fixlets to the mjw/dwarf_location git branch, which
contains these three patches combined (the first two are independent,
but the test case is for both):

  libdw: Add new function dwarf_getlocations.
  libdw: Add new functions dwarf_getlocation_attr and dwarf_getlocation_die.
  tests: Add new varlocs test for dwarf_getlocation* functions.

Cheers,

Mark