Re: DWARF Information not Used with Wildcards

[Mark Wielaard <mark at klomp dot org>]

On Sun, 2018-04-22 at 22:44 +0200, Mark Wielaard wrote:
> On Sun, 2018-04-22 at 09:27 +0200, Torsten Polle wrote:
> > > I digged into this issue further.
> > > 
> > > The failure is caused by xpc_dictionary_create_compat() [1]. This
> > > function causes an exception at the following place:
> > > 
> > > dwflpp::resolve_prologue_endings()
> > > {
> > >  ...
> > >  for(auto it = funcs.begin(); it != funcs.end(); it++)
> > >    {
> > >      Dwarf_Addr entrypc = it->entrypc;
> > >      Dwarf_Addr highpc; // NB: highpc is exclusive: [entrypc,highpc)
> > >      DWFL_ASSERT ("dwarf_highpc", dwarf_highpc (& it->die,
> > >                                                 & highpc));
> > >   ...
> > > }
> > > 
> > > As a result of the exception query_cu() returns DWARF_CB_ABORT,
> > > which in turn makes dwflpp::iterate_over_cus<void>() bail out
> > > prematurely. Not all CUs are traversed, which includes the one that
> > > holds RTPJitterBufferRead().
> > > 
> > > Without the commit, dwflpp::function_entrypc() simply returns
> > > „bad“. With the commit, dwflpp::function_entrypc() returns „true“.
> 
> aha. good observation.
> 
> So, the DWFL_ASSERT apparently triggers because the function is
> accepted by dwflpp::function_entrypc(), but wasn't before, since it
> doesn't have a simple low_pc/high_pc range.
> 
> I think this is a latent bug because the function assumes the code
> range for a function is just one block, but functions could be split in
> multiple ranges (for example a compiler might put the "cold" part of a
> function somewhere else than the "hot" part of a function).
> 
> The assert seem bogus. The function really shouldn't get and compare
> against highpc. And everywhere this function checks "addr >= highpc" it
> really should call dwarf_haspc (&it->die, addr).

Could you try the attached patch or the prologue-hasp branch at
https://code.wildebeest.org/git/user/mjw/systemtap/
(untested, but does compile)

Thanks,

Mark

From 8828e7bd2f1c02588dec60702845e21b7f85979a Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Sun, 22 Apr 2018 22:50:55 +0200
Subject: [PATCH] dwflpp::resolve_prologue_endings should use dwarf_haspc, not
 high_pc.

See https://sourceware.org/ml/systemtap/2018-q2/msg00029.html
---
 dwflpp.cxx | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/dwflpp.cxx b/dwflpp.cxx
index 70c6b4b..bfbb6b0 100644
--- a/dwflpp.cxx
+++ b/dwflpp.cxx
@@ -2337,9 +2337,6 @@ dwflpp::resolve_prologue_endings (func_info_map_t & funcs)
   for(auto it = funcs.begin(); it != funcs.end(); it++)
     {
       Dwarf_Addr entrypc = it->entrypc;
-      Dwarf_Addr highpc; // NB: highpc is exclusive: [entrypc,highpc)
-      DWFL_ASSERT ("dwarf_highpc", dwarf_highpc (& it->die,
-                                                 & highpc));
 
       unsigned entrypc_srcline_idx = 0;
       Dwarf_Line *entrypc_srcline = NULL;
@@ -2373,8 +2370,8 @@ dwflpp::resolve_prologue_endings (func_info_map_t & funcs)
         }
 
       if (sess.verbose>2)
-        clog << _F("searching for prologue of function '%s' %#" PRIx64 "-%#" PRIx64 
-                   "@%s:%d\n", it->name.to_string().c_str(), entrypc, highpc,
+        clog << _F("searching for prologue of function '%s' %#" PRIx64
+                   "@%s:%d\n", it->name.to_string().c_str(), entrypc,
                    it->decl_file.to_string().c_str(), it->decl_line);
 
       // For each function, we look for the prologue-end marker (e.g. clang
@@ -2384,7 +2381,7 @@ dwflpp::resolve_prologue_endings (func_info_map_t & funcs)
 
       // We may have to skip a few because some old compilers plop
       // in dummy line records for longer prologues.  If we go too
-      // far (addr >= highpc), we take the previous one.  Or, it may
+      // far (outside function), we take the previous one.  Or, it may
       // be the first one, if the function had no prologue, and thus
       // the entrypc maps to a statement in the body rather than the
       // declaration.
@@ -2407,7 +2404,7 @@ dwflpp::resolve_prologue_endings (func_info_map_t & funcs)
                        linesrc, lineno, lineprologue_end ? " (marked)" : "");
 
           // have we passed the function?
-          if (lineaddr >= highpc)
+          if (dwarf_haspc (& it->die, lineaddr) != 1)
             break;
           // is there an explicit prologue_end marker?
           if (lineprologue_end)
@@ -2433,7 +2430,7 @@ dwflpp::resolve_prologue_endings (func_info_map_t & funcs)
 
 
       Dwarf_Addr postprologue_addr = DWARF_LINEADDR(postprologue_srcline);
-      if (postprologue_addr >= highpc)
+      if (dwarf_haspc (& it->die, postprologue_addr) != 1)
         {
           // pick addr of previous line record
           Dwarf_Line *lr = dwarf_onesrcline(lines, postprologue_srcline_idx-1);
@@ -2450,7 +2447,8 @@ dwflpp::resolve_prologue_endings (func_info_map_t & funcs)
           if (postprologue_addr == entrypc)
             clog << _(" (naked)");
           //TRANSLATORS: Here we're adding some classification datum (ie we went over)
-          if (DWARF_LINEADDR(postprologue_srcline) >= highpc)
+          if (dwarf_haspc (& it->die,
+			   DWARF_LINEADDR(postprologue_srcline)) != 1)
             clog << _(" (tail-call?)");
           //TRANSLATORS: Here we're adding some classification datum (ie it was marked)
           if (DWARF_LINEPROLOGUEEND(postprologue_srcline))
-- 
1.8.3.1