Re: [RFC][PATCH 3/4] kprobe-based symbol resolution for stap-translator

[Prerna Saxena <prerna at linux dot vnet dot ibm dot com>]

Prerna Saxena wrote:
> Hi,
> Here's a set of patches which enable the stap-translator to utilize 
> kprobes for resolving function addresses.
> ( Similar to James Bottomley's patch sent out last july )
> In place of resolving probe points in semantic pass (Pass 2 ) by 
> consulting vmlinux/debuginfo, this approach defers symbol resolution 
> to the generated kprobes module. The kprobes module is passed the name 
> of the function to be probed, which gets resolved against the kernel 
> symbol tables to insert probes.
>
> This construct can be invoked using a new switch "--ksym" .
>
> In its present form, it is capable of probing function entry & returns 
> (non-inlines). It does /*not*/ support :
>    *  a wildcard - based search for module/function names
>    *  probing  select/all functions in a given source file
>    *  probing inline functions.
>    *  statement probes
>
> Known issues :
> 1. Apparently systemtap modules pick up build-id from the debuginfo 
> files. Since debuginfo lookup is completely bypassed here, the 
> generated stap modules fail a consistency check later owing to 
> incorrect build id. For now, patch 4 comments out this check and the 
> stap modules run fine, but I'd appreciate some pointers on how to set 
> it right. :-)
>
> 2. An incorrect indentation parameter passed to the translated output 
> in pass 3 causes stap to abort due to assert failure. Patch 4 corrects 
> this as well.
>
> Patches :
> 1. kallsym_patch_1 , kallsym_patch_2 : introduce changes to session.h 
> / main.cxx for the new switch "--ksym"
> 2. kallsym_patch_3 : changes to tapsets.cxx.
> 3. kallsym_patch_4 : workarounds for known problems.
>
> I'm working on fine-tuning its capabilities.....looking fwd to 
> suggestions... :-)

> --
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India 

Index: git-02-mar/tapsets.cxx
===================================================================
--- git-02-mar.orig/tapsets.cxx
+++ git-02-mar/tapsets.cxx
@@ -931,7 +931,7 @@ struct dwflpp
                                                elfutils_kernel_path.c_str(),
                                                NULL);
 
-    if (debuginfo_needed)
+    if (debuginfo_needed && !sess.consult_kallsym)
       dwfl_assert (string("missing ") + sess.architecture +
                    string(" kernel/module debuginfo under '") +
                    sess.kernel_build_tree + string("'"),
@@ -2572,6 +2572,7 @@ struct dwarf_derived_probe: public deriv
                        int line,
                        const string& module,
                        const string& section,
+                       const string& symbol_str,
 		       Dwarf_Addr dwfl_addr,
 		       Dwarf_Addr addr,
 		       dwarf_query & q,
@@ -2579,6 +2580,7 @@ struct dwarf_derived_probe: public deriv
 
   string module;
   string section;
+  string symbol_string;
   Dwarf_Addr addr;
   bool has_return;
   bool has_maxactive;
@@ -3633,7 +3635,9 @@ dwarf_query::add_probe_point(const strin
         {
           assert (has_kernel || has_module);
           results.push_back (new dwarf_derived_probe(funcname, filename, line,
-                                                     module, reloc_section, addr, reloc_addr,
+                                                     module, reloc_section,
+						     funcname,
+						     addr, reloc_addr,
                                                      *this, scope_die));
         }
     }
@@ -5120,7 +5124,7 @@ dwarf_derived_probe::printsig (ostream& 
   // function instances.  This is distinct from the verbose/clog
   // output, since this part goes into the cache hash calculations.
   sole_location()->print (o);
-  o << " /* pc=" << section << "+0x" << hex << addr << dec << " */";
+  o << " /* symbol: "<< symbol_string <<" pc=" << section << "+0x" << hex << addr << dec << " */";
   printsig_nested (o);
 }
 
@@ -5143,6 +5147,7 @@ dwarf_derived_probe::dwarf_derived_probe
                                          // (equivalently module=="kernel")
                                          const string& module,
                                          const string& section,
+					 const string& symbol_str,
                                          // NB: dwfl_addr is the virtualized
                                          // address for this symbol.
                                          Dwarf_Addr dwfl_addr,
@@ -5152,7 +5157,7 @@ dwarf_derived_probe::dwarf_derived_probe
                                          dwarf_query& q,
                                          Dwarf_Die* scope_die /* may be null */)
   : derived_probe (q.base_probe, new probe_point(*q.base_loc) /* .components soon rewritten */ ),
-    module (module), section (section), addr (addr),
+    module (module), section (section), symbol_string (symbol_str), addr (addr),
     has_return (q.has_return),
     has_maxactive (q.has_maxactive),
     maxactive_val (q.maxactive_val)
@@ -5353,6 +5358,8 @@ dwarf_derived_probe_group::emit_module_d
   s.op->newline() << "#error \"Need CONFIG_KPROBES!\"";
   s.op->newline() << "#endif";
   s.op->newline();
+  s.op->newline() << "#define KALLSYMS_USED " << s.consult_kallsym;
+  s.op->newline();
 
   // Forward declare the master entry functions
   s.op->newline() << "static int enter_kprobe_probe (struct kprobe *inst,";
@@ -5387,8 +5394,8 @@ dwarf_derived_probe_group::emit_module_d
   // Let's find some stats for the three embedded strings.  Maybe they
   // are small and uniform enough to justify putting char[MAX]'s into
   // the array instead of relocated char*'s.
-  size_t module_name_max = 0, section_name_max = 0, pp_name_max = 0;
-  size_t module_name_tot = 0, section_name_tot = 0, pp_name_tot = 0;
+  size_t module_name_max = 0, section_name_max = 0, pp_name_max = 0, symbol_string_name_max = 0;
+  size_t module_name_tot = 0, section_name_tot = 0, pp_name_tot = 0, symbol_string_name_tot = 0;
   size_t all_name_cnt = probes_by_module.size(); // for average
   for (p_b_m_iterator it = probes_by_module.begin(); it != probes_by_module.end(); it++)
     {
@@ -5400,6 +5407,7 @@ dwarf_derived_probe_group::emit_module_d
       DOIT(module_name, p->module.size());
       DOIT(section_name, p->section.size());
       DOIT(pp_name, lex_cast_qstring(*p->sole_location()).size());
+      DOIT(symbol_string_name,p->symbol_string.size());
 #undef DOIT
     }
 
@@ -5422,6 +5430,7 @@ dwarf_derived_probe_group::emit_module_d
   CALCIT(module);
   CALCIT(section);
   CALCIT(pp);
+  CALCIT(symbol_string);
 
   s.op->newline() << "const unsigned long address;";
   s.op->newline() << "void (* const ph) (struct context*);";
@@ -5444,7 +5453,8 @@ dwarf_derived_probe_group::emit_module_d
       s.op->line() << " .module=\"" << p->module << "\",";
       s.op->line() << " .section=\"" << p->section << "\",";
       s.op->line() << " .pp=" << lex_cast_qstring (*p->sole_location()) << ",";
-      s.op->line() << " .ph=&" << p->name;
+      s.op->line() << " .ph=&" << p->name << ",";
+      s.op->line() << " .symbol_string=\""<< p->symbol_string <<"\"";
       s.op->line() << " },";
     }
 
@@ -5502,11 +5512,19 @@ dwarf_derived_probe_group::emit_module_i
   s.op->newline() << "for (i=0; i<" << probes_by_module.size() << "; i++) {";
   s.op->newline(1) << "struct stap_dwarf_probe *sdp = & stap_dwarf_probes[i];";
   s.op->newline() << "struct stap_dwarf_kprobe *kp = & stap_dwarf_kprobes[i];";
-  s.op->newline() << "unsigned long relocated_addr = _stp_module_relocate (sdp->module, sdp->section, sdp->address);";
-  s.op->newline() << "if (relocated_addr == 0) continue;"; // quietly; assume module is absent
+  s.op->newline() << "unsigned long relocated_addr = 0;";
+  s.op->newline() << "if(!KALLSYMS_USED)";
+  s.op->newline() << "   {";
+  s.op->newline() << "     relocated_addr = _stp_module_relocate (sdp->module, sdp->section, sdp->address);";
+  s.op->newline() << "     if (relocated_addr == 0) continue;"; // quietly; assume module is absent
+  s.op->newline() << "   } ";
   s.op->newline() << "probe_point = sdp->pp;"; // for error messages
   s.op->newline() << "if (sdp->return_p) {";
-  s.op->newline(1) << "kp->u.krp.kp.addr = (void *) relocated_addr;";
+  s.op->newline() << "kp->u.krp.kp.addr = (void *) relocated_addr;";
+  s.op->newline() << "if(KALLSYMS_USED)";
+  s.op->newline() << "   {";
+  s.op->newline() << "      kp->u.krp.kp.symbol_name = sdp->symbol_string;";
+  s.op->newline() << "   }";
   s.op->newline() << "if (sdp->maxactive_p) {";
   s.op->newline(1) << "kp->u.krp.maxactive = sdp->maxactive_val;";
   s.op->newline(-1) << "} else {";
@@ -5517,6 +5535,10 @@ dwarf_derived_probe_group::emit_module_i
   s.op->newline() << "#ifdef __ia64__";
   s.op->newline() << "kp->dummy.addr = kp->u.krp.kp.addr;";
   s.op->newline() << "kp->dummy.pre_handler = NULL;";
+  s.op->newline() << "if(KALLSYMS_USED)";
+  s.op->newline() << "   {";
+  s.op->newline() << "      kp->dummy.symbol_name = sdp->symbol_string;";
+  s.op->newline() << "   }";
   s.op->newline() << "rc = register_kprobe (& kp->dummy);";
   s.op->newline() << "if (rc == 0) {";
   s.op->newline(1) << "rc = register_kretprobe (& kp->u.krp);";
@@ -5529,10 +5551,18 @@ dwarf_derived_probe_group::emit_module_i
   s.op->newline(-1) << "} else {";
   // to ensure safeness of bspcache, always use aggr_kprobe on ia64
   s.op->newline(1) << "kp->u.kp.addr = (void *) relocated_addr;";
+  s.op->newline() << "if(KALLSYMS_USED)";
+  s.op->newline() << "   {";
+  s.op->newline() << "      kp->u.kp.symbol_name = sdp->symbol_string;";
+  s.op->newline() << "   }";
   s.op->newline() << "kp->u.kp.pre_handler = &enter_kprobe_probe;";
   s.op->newline() << "#ifdef __ia64__";
   s.op->newline() << "kp->dummy.addr = kp->u.kp.addr;";
   s.op->newline() << "kp->dummy.pre_handler = NULL;";
+  s.op->newline() << "if(KALLSYMS_USED)";
+  s.op->newline() << "   {";
+  s.op->newline() << "      kp->dummy.symbol_name = sdp->symbol_string;";
+  s.op->newline() << "   }";
   s.op->newline() << "rc = register_kprobe (& kp->dummy);";
   s.op->newline() << "if (rc == 0) {";
   s.op->newline(1) << "rc = register_kprobe (& kp->u.kp);";
@@ -5545,7 +5575,16 @@ dwarf_derived_probe_group::emit_module_i
   s.op->newline(-1) << "}";
   s.op->newline() << "if (rc) {"; // PR6749: tolerate a failed register_*probe.
   s.op->newline(1) << "sdp->registered_p = 0;";
-  s.op->newline() << "_stp_warn (\"probe %s registration error (rc %d)\", probe_point, rc);";
+  s.op->newline() << " if(KALLSYMS_USED && rc == -EINVAL) ";
+  s.op->newline() << " {";
+  s.op->newline() << "   _stp_error(\"Error registering kprobe,possibly an incorrect name : %s\", sdp->symbol_string); ";
+  s.op->newline() << "   atomic_set (&session_state, STAP_SESSION_ERROR);";
+  s.op->newline() << "   goto out;";
+
+  s.op->newline() << " }";
+  s.op->newline() << " else";
+
+  s.op->newline() << "_stp_warn (\"probe %s for %s registration error (rc %d)\", probe_point, sdp->pp, rc);";
   s.op->newline() << "rc = 0;"; // continue with other probes
   // XXX: shall we increment numskipped?
   s.op->newline(-1) << "}";
@@ -5782,10 +5821,6 @@ dwarf_builder::build(systemtap_session &
   
   dwarf_query q(sess, base, location, *dw, parameters, finished_results);
 
-
-  // XXX: kernel.statement.absolute is a special case that requires no
-  // dwfl processing.  This code should be in a separate builder.
-
   if (q.has_kernel && q.has_absolute)
     {
       // assert guru mode for absolute probes
@@ -5799,13 +5834,52 @@ dwarf_builder::build(systemtap_session &
       // dwarf_derived_probe right here and now.
       dwarf_derived_probe* p =
         new dwarf_derived_probe ("", "", 0, "kernel", "",
-                                 q.statement_num_val, q.statement_num_val,
+                                 q.function_str_val,
+				 q.statement_num_val, q.statement_num_val,
                                  q, 0);
       finished_results.push_back (p);
       sess.unwindsym_modules.insert ("kernel");
       return;
     }
 
+  //consult_kallsym case : leave the name resolution to kprobes
+  if (sess.consult_kallsym )
+     {
+	   if (q.has_statement_str)
+		throw semantic_error("statement probes not supported with kprobes based symbol resolution");
+	   if (q.function_str_val == "")
+		throw semantic_error("empty function name string");
+	   string sym_funcname = q.function_str_val;
+	   unsigned int sym_length = sym_funcname.size();
+           if ( (sym_funcname.find_first_of('@',0) < sym_length ) || (sym_funcname.find_first_of(':',0) < sym_length ) || (sym_funcname.find_first_of('*',0)< sym_length) )
+		throw semantic_error("kprobe-based symbol resolution doesn not support wildcards or source-file based probing : " + sym_funcname + "\n");
+
+	    if (q.has_inline)
+		throw semantic_error("kprobe-based resolution does not support inlines \n");
+       	    if (q.has_kernel)
+	  	{
+ 	    	  dwarf_derived_probe* p =
+	          new dwarf_derived_probe (q.function_str_val, "", 0, "kernel", "",
+	               			   q.function_str_val, 0, 0, q, NULL );
+                  finished_results.push_back (p);
+                  sess.unwindsym_modules.insert ("kernel");
+                  return;
+                }
+            else if (q.has_module)
+	        {
+ 	          dwarf_derived_probe* p =
+	          new dwarf_derived_probe (q.function_str_val, "", 0, module_name, "",
+	               			   q.function_str_val, 0, 0, q, NULL );
+                  finished_results.push_back (p);
+                  sess.unwindsym_modules.insert (module_name);
+                  return;
+                 }
+      }
+  // XXX: kernel.statement.absolute is a special case that requires no
+  // dwfl processing.  This code should be in a separate builder.
+
+
+
   dw->query_modules(&q);
 }