[RFC] [PATCH] Add statement ranges to kernel.statement

[RFC] [PATCH] Add statement ranges to kernel.statement

[Stan Cox]

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

This adds two types of statement ranges to kernel.statement:
 kernel.statement("bio_put@fs/bio.c:*")
 kernel.statement("bio_put@fs/bio.c:212-219"

e.g. Given: probe kernel.statement("bio_put@fs/bio.c:212-219") {printf
("%s\n", pp())}
Results:
kernel.statement("bio_put@fs/bio.c:213")
kernel.statement("bio_put@fs/bio.c:212")
kernel.statement("bio_put@fs/bio.c:218")
kernel.statement("bio_put@fs/bio.c:219")
kernel.statement("bio_put@fs/bio.c:213")
kernel.statement("bio_put@fs/bio.c:212")
kernel.statement("bio_put@fs/bio.c:218")
kernel.statement("bio_put@fs/bio.c:219")

This patch converts tapsets.cxx#iterate_over_srcfile_lines into a loop
where every kernel.statement is assumed to have a line range, with :212
assumed to be the range 212-212.  The :* case explicitly sets the start
to the beginning of the function and checks when the end of the function
is reached.



[-- Attachment #2: stap-4904.patch --]
[-- Type: text/x-patch, Size: 4935 bytes --]

/work/scox/systemtap/src /work/scox/systemtap/bld/x86_64-redhat-linux ~
diff --git a/tapsets.cxx b/tapsets.cxx
index 7bfe8b4..bcb448a 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -608,6 +608,8 @@ dwarf_diename_integrate (Dwarf_Die *die)
   return dwarf_formstring (dwarf_attr_integrate (die, DW_AT_name, &attr_mem));
 }
 
+enum line_t { ABSOLUTE, RELATIVE, RANGE, WILDCARD };
+
 struct dwflpp
 {
   systemtap_session & sess;
@@ -1135,9 +1137,9 @@ struct dwflpp
   bool has_single_line_record (dwarf_query * q, char const * srcfile, int lineno);
 
   void iterate_over_srcfile_lines (char const * srcfile,
-				   int lineno,
+				   int lines[2],
 				   bool need_single_match,
-				   bool line_type_relative,
+				   enum line_t line_type,
 				   void (* callback) (const dwarf_line_t& line,
                                                       void * arg),
 				   void *data)
@@ -1145,10 +1147,11 @@ struct dwflpp
     Dwarf_Line **srcsp = NULL;
     size_t nsrcs = 0;
     dwarf_query * q = static_cast<dwarf_query *>(data);
+    int lineno = lines[0];
 
     get_module_dwarf();
 
-    if (line_type_relative) 
+    if (line_type == RELATIVE) 
       {
 	Dwarf_Addr addr;
 	Dwarf_Line *line;
@@ -1160,12 +1163,27 @@ struct dwflpp
 	dwarf_assert ("dwarf_lineno", dwarf_lineno (line, &line_number));
 	lineno += line_number;
       }
+    else if (line_type == WILDCARD)
+      function_line (&lineno);
 
+    for (int l = lineno; ; l = l + 1)
+      {
     dwarf_assert ("dwarf_getsrc_file",
 		  dwarf_getsrc_file (module_dwarf,
-				     srcfile, lineno, 0,
+					 srcfile, l, 0,
 				     &srcsp, &nsrcs));
 
+	if (line_type == WILDCARD || line_type == RANGE)
+	  {
+	    Dwarf_Addr line_addr;
+	    dwarf_lineno (srcsp [0], &lineno);
+	    if (lineno != l)
+	      continue;
+	    dwarf_lineaddr (srcsp [0], &line_addr);
+	    if (dwarf_haspc (function, line_addr) != 1)
+	      break;
+	  }
+
     // NB: Formerly, we used to filter, because:
 
     // dwarf_getsrc_file gets one *near hits* for line numbers, not
@@ -1181,24 +1199,6 @@ struct dwflpp
     // CU name.
 
     size_t remaining_nsrcs = nsrcs;
-#if 0
-    for (size_t i = 0; i < nsrcs; ++i)
-      {
-        int l_no;
-        Dwarf_Line* l = srcsp[i];
-        dwarf_assert ("dwarf_lineno", dwarf_lineno (l, & l_no));
-        if (l_no != lineno)
-          {
-            if (sess.verbose > 3)
-	      clog << "skipping line number mismatch "
-                   << "(" << l_no << " vs " << lineno << ")"
-                   << " in file '" << srcfile << "'"
-                   << "\n";
-            srcsp[i] = 0;
-            remaining_nsrcs --;
-          }
-      }
-#endif
 
     if (need_single_match && remaining_nsrcs > 1)
       {
@@ -1250,6 +1250,9 @@ struct dwflpp
 	free (srcsp);
 	throw;
       }
+	if (line_type != WILDCARD && l == lines[1])
+	  break;
+      }
     free (srcsp);
   }
 
@@ -2338,8 +2341,6 @@ base_query::get_number_param(literal_map_t const & params,
 typedef map<Dwarf_Addr, inline_instance_info> inline_instance_map_t;
 typedef map<Dwarf_Addr, func_info> func_info_map_t;
 
-enum line_t { ABSOLUTE, RELATIVE };
-
 struct dwarf_query : public base_query
 {
   dwarf_query(systemtap_session & sess,
@@ -2407,7 +2408,7 @@ struct dwarf_query : public base_query
   string function;
   string file;
   line_t line_type;
-  int line;
+  int line[2];
   bool query_done;	// Found exact match
 
   set<char const *> filtered_srcfiles;
@@ -2873,7 +2874,8 @@ dwarf_query::parse_function_spec(string & spec)
 
   function.clear();
   file.clear();
-  line = 0;
+  line[0] = 0;
+  line[1] = 0;
 
   while (i != e && *i != '@')
     {
@@ -2897,7 +2899,12 @@ dwarf_query::parse_function_spec(string & spec)
   while (i != e && *i != ':' && *i != '+')
     file += *i++;
   if (*i == ':') 
+    {
+      if (*(i + 1) == '*')
+	line_type = WILDCARD;
+      else
     line_type = ABSOLUTE;
+    }
   else if (*i == '+')
     line_type = RELATIVE;
 
@@ -2916,7 +2923,22 @@ dwarf_query::parse_function_spec(string & spec)
 
   try
     {
-      line = lex_cast<int>(string(i, e));
+      if (line_type != WILDCARD)
+	{
+	  string::const_iterator dash = i;
+	  
+	  while (dash != e && *dash != '-')
+	    dash++;
+	  if (dash == e)
+	    line[0] = line[1] = lex_cast<int>(string(i, e));
+	  else
+	    {
+	      line_type = RANGE;
+	      line[0] = lex_cast<int>(string(i, dash));
+	      line[1] = lex_cast<int>(string(dash + 1, e));
+	    }
+	}
+      
       if (sess.verbose>2)
 	clog << "parsed '" << spec
 	     << "' -> func '"<< function
@@ -3308,7 +3330,7 @@ query_srcfile_line (const dwarf_line_t& line, void * arg)
 	    clog << "inline instance DIE lands on srcfile\n";
 	  if (q->has_statement_str)
 	    query_statement (i->second.name, i->second.decl_file,
-			     q->line, &(i->second.die), addr, q);
+			     q->line[0], &(i->second.die), addr, q);
 	  else
 	    query_inline_instance_info (i->first, i->second, q);
 	}

Re: [RFC] [PATCH] Add statement ranges to kernel.statement

[Stone, Joshua I]

Hi Stan,

This looks good -- I have just a few comments:

Stan Cox wrote:
> This adds two types of statement ranges to kernel.statement:
>  kernel.statement("bio_put@fs/bio.c:*")
>  kernel.statement("bio_put@fs/bio.c:212-219"

Can you make this work for the relative syntax?  I would imagine that
"+*" should work identically to ":*", and "+1-42" seems reasonable too.

> +	if (line_type == WILDCARD || line_type == RANGE)
> +	  {
> +	    Dwarf_Addr line_addr;
> +	    dwarf_lineno (srcsp [0], &lineno);
> +	    if (lineno != l)
> +	      continue;
> +	    dwarf_lineaddr (srcsp [0], &line_addr);
> +	    if (dwarf_haspc (function, line_addr) != 1)
> +	      break;
> +	  }

It wasn't obvious to me what this is trying to check, so I would
appreciate a brief explanation in comments.

Also, when the specified range is out of bounds, it should probably
throw an error.

> +	if (line_type != WILDCARD && l == lines[1])
> +	  break;
> +      }

I think this will break for relative probes, because the incoming
lines[2] are relative, but 'l' is absolute.


Hope that's helpful...

Josh

Re: [RFC] [PATCH] Add statement ranges to kernel.statement

[Stone, Joshua I]

Stone, Joshua I wrote:
> > +     if (line_type != WILDCARD && l == lines[1])
> > +       break;
> > +      }
> 
> I think this will break for relative probes, because the incoming
> lines[2] are relative, but 'l' is absolute.

Ouch - I just confused myself with this ambiguous wording.  By "this
will break" I mean that the condition won't work as intended, so in fact
this will not break (out of the loop).

Josh