Initial stap support for inode-based uprobes

Initial stap support for inode-based uprobes

[Josh Stone]

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

The attached patch implements initial support for SystemTap to use
Srikar's inode-based uprobes.  It is also published in the branch
jistone/inode-uprobes, in gitweb here:
<http://sourceware.org/git/gitweb.cgi?p=systemtap.git;a=shortlog;h=refs/heads/jistone/inode-uprobes>

The uprobes branch I worked from is here:
<http://git.kernel.org/?p=linux/kernel/git/srikar/linux-uprobes.git;a=shortlog;h=refs/heads/tip_inode_uprobes_010411>

The good news is that the basics appear to be working well.  I've tested
probing stap itself and libdw, and got the expected probe hits.  I'd
appreciate any review of my implementation so far.  Beyond these working
basics, there are a lot of details to hammer out, so here's the list of
what I know.

* EXPORT_SYMBOL_GPL, or uprobes' lack thereof.  Without kernel exports,
the whole API will be inaccessible to us.

* Return probes.  This hasn't yet been added to the new uprobes.

* Process filtering.  AFAICS, the current uprobes implementation sets
the breakpoint in all processes that map the particular inode.  There is
a filtering mechanism, but that seems only to decide whether to call the
handler each time.  You'll still take the bp/sstep overhead.  Also, on
stap's side, we previously had the ability to limit process probes to
the -x/-c target and children, which I haven't tried here yet.

* Runtime build-id verification.  Right now I'm just mapping the path to
inode*, without checking that the build-id is what we expected.  I'm not
sure we even could at the systemtap-init point.  Even if we did, the
file may still get modified without changing the inode, and I don't
think this uprobes gives us any way to notice or decide whether we like
the new form.

* SDT semaphore.  In the current form, we have no hook on individual
processes, so we can't modify the semaphores in applications that are
actively gating their markers.  We'll probably need something like
PR10994 to achieve this, which isn't really about uprobes per-se, but
rather about living without utrace.

* Argument access.  If you try $args, it will fail with a missing symbol
'task_user_regset_view'.  I haven't looked closely at this yet.

* Probe IP.  For many probe handlers, we try to set the pt_regs IP to
the actual breakpoint IP, but in this case we don't happen to even know
the virtualized address.  Uprobes itself uses uprobes_get_bkpt_addr() in
some instances, but that's not exposed for our use.


I think that's it.  So if you happen to build a kernel with the new
uprobes, please enjoy systemtap support too. :)

Josh

[-- Attachment #2: systemtap-inode-uprobes.patch --]
[-- Type: text/x-patch, Size: 14295 bytes --]

commit f8e4aa0bc62d79bfc7a2dcb1508215a675d9f83d
Author: Josh Stone <jistone@redhat.com>
Date:   Thu May 19 19:44:16 2011 -0700

    Add initial support for inode-based uprobes
    
    This adds support for placing regular userspace probes using the new
    inode+offset API being developed for the upstream kernel.  This includes
    probing functions, statements, and SDT markers, but return probes aren't
    yet supported in the new API.  A lot of the finer details of systemtap's
    userspace runtime still needs work too, but this is a functional start.
    
    * runtime/uprobes-inode.c: New, basic registration code to lookup
      filename inodes and connect uprobes using the new API.
    * tapsets.cxx (kernel_supports_inode_uprobes): New, guess whether this
      is an inode-uprobes kernel based on CONFIG values.
      (dwarf_builder::build): Disallow userspace return probes.
      (uprobe_derived_probe::join_group): Only trigger task_finder and the
      manual uprobes model for the old style of uprobes.
      (uprobe_builder::build): Disallow absolute-address userspace probes.
      (uprobe_derived_probe_group::emit*): Split into inode/utrace variants.

diff --git a/runtime/uprobes-inode.c b/runtime/uprobes-inode.c
new file mode 100644
index 0000000..b04ca6d
--- /dev/null
+++ b/runtime/uprobes-inode.c
@@ -0,0 +1,119 @@
+/* -*- linux-c -*-
+ * Common functions for using inode-based uprobes
+ * Copyright (C) 2011 Red Hat Inc.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
+#ifndef _UPROBES_INODE_C_
+#define _UPROBES_INODE_C_
+
+#include <linux/fs.h>
+#include <linux/namei.h>
+#include <linux/uprobes.h>
+
+struct stp_inode_uprobe_target {
+	const char * const filename;
+	struct inode *inode;
+};
+
+struct stp_inode_uprobe_consumer {
+	struct uprobe_consumer consumer;
+	struct stp_inode_uprobe_target * const target;
+	loff_t offset;
+	/* XXX sdt_sem_offset support? */
+
+	struct stap_probe * const probe;
+};
+
+
+static void
+stp_inode_uprobes_put(struct stp_inode_uprobe_target *targets,
+		      size_t ntargets)
+{
+	size_t i;
+	for (i = 0; i < ntargets; ++i) {
+		struct stp_inode_uprobe_target *ut = &targets[i];
+		iput(ut->inode);
+		ut->inode = NULL;
+	}
+}
+
+static int
+stp_inode_uprobes_get(struct stp_inode_uprobe_target *targets,
+		      size_t ntargets)
+{
+	int ret = 0;
+	size_t i;
+	for (i = 0; i < ntargets; ++i) {
+		struct path path;
+		struct stp_inode_uprobe_target *ut = &targets[i];
+		ret = kern_path(ut->filename, LOOKUP_FOLLOW, &path);
+		if (!ret) {
+			ut->inode = igrab(path.dentry->d_inode);
+			if (!ut->inode)
+				ret = -EINVAL;
+		}
+		if (ret)
+			break;
+	}
+	if (ret)
+		stp_inode_uprobes_put(targets, i);
+	return ret;
+}
+
+static void
+stp_inode_uprobes_unreg(struct stp_inode_uprobe_consumer *consumers,
+			size_t nconsumers)
+{
+	size_t i;
+	for (i = 0; i < nconsumers; ++i) {
+		struct stp_inode_uprobe_consumer *uc = &consumers[i];
+		unregister_uprobe(uc->target->inode, uc->offset,
+				  &uc->consumer);
+	}
+}
+
+static int
+stp_inode_uprobes_reg(struct stp_inode_uprobe_consumer *consumers,
+		      size_t nconsumers)
+{
+	int ret = 0;
+	size_t i;
+	for (i = 0; i < nconsumers; ++i) {
+		struct stp_inode_uprobe_consumer *uc = &consumers[i];
+		ret = register_uprobe(uc->target->inode, uc->offset,
+				      &uc->consumer);
+		if (ret)
+			break;
+	}
+	if (ret)
+		stp_inode_uprobes_unreg(consumers, i);
+	return ret;
+}
+
+static int
+stp_inode_uprobes_init(struct stp_inode_uprobe_target *targets, size_t ntargets,
+		       struct stp_inode_uprobe_consumer *consumers, size_t nconsumers)
+{
+	int ret = stp_inode_uprobes_get(targets, ntargets);
+	if (!ret) {
+		ret = stp_inode_uprobes_reg(consumers, nconsumers);
+		if (ret)
+			stp_inode_uprobes_put(targets, ntargets);
+	}
+	return ret;
+}
+
+static void
+stp_inode_uprobes_exit(struct stp_inode_uprobe_target *targets, size_t ntargets,
+		       struct stp_inode_uprobe_consumer *consumers, size_t nconsumers)
+{
+	stp_inode_uprobes_unreg(consumers, nconsumers);
+	stp_inode_uprobes_put(targets, ntargets);
+}
+
+#endif /* _UPROBES_INODE_C_ */
diff --git a/tapsets.cxx b/tapsets.cxx
index 8afe02e..25170dc 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -3795,6 +3795,16 @@ dwarf_derived_probe::join_group (systemtap_session& s)
 }
 
 
+static bool
+kernel_supports_inode_uprobes(systemtap_session& s)
+{
+  // The arch-supports is new to the builtin inode-uprobes, so it makes a
+  // reasonable indicator of the new API.  Else we'll need an autoconf...
+  return (s.kernel_config["CONFIG_ARCH_SUPPORTS_UPROBES"] == "y"
+          && s.kernel_config["CONFIG_UPROBES"] == "y");
+}
+
+
 dwarf_derived_probe::dwarf_derived_probe(const string& funcname,
                                          const string& filename,
                                          int line,
@@ -3835,6 +3845,12 @@ dwarf_derived_probe::dwarf_derived_probe(const string& funcname,
       // ET_DYN ones do (addr += run-time mmap base address).  We tell these apart
       // by the incoming section value (".absolute" vs. ".dynamic").
       // XXX Assert invariants here too?
+
+      // inode-uprobes needs an offset rather than an absolute VM address.
+      if (kernel_supports_inode_uprobes(q.dw.sess) &&
+          section == ".absolute" && addr == dwfl_addr &&
+          addr >= q.dw.module_start && addr < q.dw.module_end)
+        this->addr = addr - q.dw.module_start;
     }
   else
     {
@@ -6182,7 +6198,13 @@ dwarf_builder::build(systemtap_session & sess,
       else
 	module_name = user_path; // canonicalize it
 
-      if (sess.kernel_config["CONFIG_UTRACE"] != string("y"))
+      if (kernel_supports_inode_uprobes(sess))
+        {
+          if (has_null_param(parameters, TOK_RETURN))
+            throw semantic_error
+              (_("process return probes not available with inode-based uprobes"));
+        }
+      else if (sess.kernel_config["CONFIG_UTRACE"] != string("y"))
         throw semantic_error (_("process probes not available without kernel CONFIG_UTRACE"));
 
       // user-space target; we use one dwflpp instance per module name
@@ -6635,6 +6657,16 @@ private:
     return p->module + "|" + p->section + "|" + lex_cast(p->pid);
   }
 
+  // Using our own utrace-based uprobes
+  void emit_module_utrace_decls (systemtap_session& s);
+  void emit_module_utrace_init (systemtap_session& s);
+  void emit_module_utrace_exit (systemtap_session& s);
+
+  // Using the upstream inode-based uprobes
+  void emit_module_inode_decls (systemtap_session& s);
+  void emit_module_inode_init (systemtap_session& s);
+  void emit_module_inode_exit (systemtap_session& s);
+
 public:
   void emit_module_decls (systemtap_session& s);
   void emit_module_init (systemtap_session& s);
@@ -6648,11 +6680,15 @@ uprobe_derived_probe::join_group (systemtap_session& s)
   if (! s.uprobe_derived_probes)
     s.uprobe_derived_probes = new uprobe_derived_probe_group ();
   s.uprobe_derived_probes->enroll (this);
-  enable_task_finder(s);
 
-  // Ask buildrun.cxx to build extra module if needed, and
-  // signal staprun to load that module
-  s.need_uprobes = true;
+  if (!kernel_supports_inode_uprobes(s))
+    {
+      enable_task_finder(s);
+
+      // Ask buildrun.cxx to build extra module if needed, and
+      // signal staprun to load that module
+      s.need_uprobes = true;
+    }
 }
 
 
@@ -6684,7 +6720,7 @@ uprobe_derived_probe::emit_unprivileged_assertion (translator_output* o)
 struct uprobe_builder: public derived_probe_builder
 {
   uprobe_builder() {}
-  virtual void build(systemtap_session &,
+  virtual void build(systemtap_session & sess,
 		     probe * base,
 		     probe_point * location,
 		     literal_map_t const & parameters,
@@ -6692,6 +6728,9 @@ struct uprobe_builder: public derived_probe_builder
   {
     int64_t process, address;
 
+    if (kernel_supports_inode_uprobes(sess))
+      throw semantic_error (_("absolute process probes not available with inode-based uprobes"));
+
     bool b1 = get_param (parameters, TOK_PROCESS, process);
     (void) b1;
     bool b2 = get_param (parameters, TOK_STATEMENT, address);
@@ -6705,10 +6744,10 @@ struct uprobe_builder: public derived_probe_builder
 
 
 void
-uprobe_derived_probe_group::emit_module_decls (systemtap_session& s)
+uprobe_derived_probe_group::emit_module_utrace_decls (systemtap_session& s)
 {
   if (probes.empty()) return;
-  s.op->newline() << "/* ---- user probes ---- */";
+  s.op->newline() << "/* ---- utrace uprobes ---- */";
   // If uprobes isn't in the kernel, pull it in from the runtime.
 
   s.op->newline() << "#if defined(CONFIG_UPROBES) || defined(CONFIG_UPROBES_MODULE)";
@@ -6892,11 +6931,11 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s)
 
 
 void
-uprobe_derived_probe_group::emit_module_init (systemtap_session& s)
+uprobe_derived_probe_group::emit_module_utrace_init (systemtap_session& s)
 {
   if (probes.empty()) return;
 
-  s.op->newline() << "/* ---- user probes ---- */";
+  s.op->newline() << "/* ---- utrace uprobes ---- */";
 
   s.op->newline() << "for (j=0; j<MAXUPROBES; j++) {";
   s.op->newline(1) << "struct stap_uprobe *sup = & stap_uprobes[j];";
@@ -6925,10 +6964,10 @@ uprobe_derived_probe_group::emit_module_init (systemtap_session& s)
 
 
 void
-uprobe_derived_probe_group::emit_module_exit (systemtap_session& s)
+uprobe_derived_probe_group::emit_module_utrace_exit (systemtap_session& s)
 {
   if (probes.empty()) return;
-  s.op->newline() << "/* ---- user probes ---- */";
+  s.op->newline() << "/* ---- utrace uprobes ---- */";
 
   // NB: there is no stap_unregister_task_finder_target call;
   // important stuff like utrace cleanups are done by
@@ -6998,6 +7037,126 @@ uprobe_derived_probe_group::emit_module_exit (systemtap_session& s)
   s.op->newline() << "mutex_destroy (& stap_uprobes_lock);";
 }
 
+
+void
+uprobe_derived_probe_group::emit_module_inode_decls (systemtap_session& s)
+{
+  if (probes.empty()) return;
+  s.op->newline() << "/* ---- inode uprobes ---- */";
+  s.op->newline() << "#include \"uprobes-inode.c\"";
+
+  // Write the probe handler.
+  s.op->newline() << "static int enter_inode_uprobe "
+                  << "(struct uprobe_consumer *inst, struct pt_regs *regs) {";
+  s.op->newline(1) << "struct stp_inode_uprobe_consumer *sup = "
+                   << "container_of(inst, struct stp_inode_uprobe_consumer, consumer);";
+  common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING", "sup->probe");
+  s.op->newline() << "c->regs = regs;";
+  s.op->newline() << "c->regflags |= _STP_REGS_USER_FLAG;";
+  // XXX: Can't set SET_REG_IP; we don't actually know the relocated address.
+  // ...  In some error cases, uprobes itself calls uprobes_get_bkpt_addr().
+  s.op->newline() << "(*sup->probe->ph) (c);";
+  common_probe_entryfn_epilogue (s.op);
+  s.op->newline() << "return 0;";
+  s.op->newline(-1) << "}";
+  s.op->assert_0_indent();
+
+  // Index of all the modules for which we need inodes.
+  map<string, unsigned> module_index;
+  unsigned module_index_ctr = 0;
+
+  // Discover and declare targets for each unique path.
+  s.op->newline() << "static struct stp_inode_uprobe_target "
+                  << "stap_inode_uprobe_targets[] = {";
+  s.op->indent(1);
+  for (unsigned i=0; i<probes.size(); i++)
+    {
+      uprobe_derived_probe *p = probes[i];
+      if (module_index.find (p->module) == module_index.end())
+        {
+          module_index[p->module] = module_index_ctr++;
+          s.op->newline() << "{ .filename=" << lex_cast_qstring(p->module) << " },";
+        }
+    }
+  s.op->newline(-1) << "};";
+  s.op->assert_0_indent();
+
+  // Declare the actual probes.
+  s.op->newline() << "static struct stp_inode_uprobe_consumer "
+                  << "stap_inode_uprobe_consumers[] = {";
+  s.op->indent(1);
+  for (unsigned i=0; i<probes.size(); i++)
+    {
+      uprobe_derived_probe *p = probes[i];
+      unsigned index = module_index[p->module];
+      s.op->newline() << "{"
+                      << " .consumer={ .handler=enter_inode_uprobe },"
+                      << " .target=&stap_inode_uprobe_targets[" << index << "],"
+                      << " .offset=(loff_t)0x" << hex << p->addr << dec << "ULL,"
+                      << " .probe=" << common_probe_init (p) << ","
+                      << "},";
+    }
+  s.op->newline(-1) << "};";
+  s.op->assert_0_indent();
+}
+
+
+void
+uprobe_derived_probe_group::emit_module_inode_init (systemtap_session& s)
+{
+  if (probes.empty()) return;
+  s.op->newline() << "/* ---- inode uprobes ---- */";
+  s.op->newline() << "rc = stp_inode_uprobes_init ("
+                  << "stap_inode_uprobe_targets, "
+                  << "ARRAY_SIZE(stap_inode_uprobe_targets), "
+                  << "stap_inode_uprobe_consumers, "
+                  << "ARRAY_SIZE(stap_inode_uprobe_consumers));";
+}
+
+
+void
+uprobe_derived_probe_group::emit_module_inode_exit (systemtap_session& s)
+{
+  if (probes.empty()) return;
+  s.op->newline() << "/* ---- inode uprobes ---- */";
+  s.op->newline() << "stp_inode_uprobes_exit ("
+                  << "stap_inode_uprobe_targets, "
+                  << "ARRAY_SIZE(stap_inode_uprobe_targets), "
+                  << "stap_inode_uprobe_consumers, "
+                  << "ARRAY_SIZE(stap_inode_uprobe_consumers));";
+}
+
+
+void
+uprobe_derived_probe_group::emit_module_decls (systemtap_session& s)
+{
+  if (kernel_supports_inode_uprobes (s))
+    emit_module_inode_decls (s);
+  else
+    emit_module_utrace_decls (s);
+}
+
+
+void
+uprobe_derived_probe_group::emit_module_init (systemtap_session& s)
+{
+  if (kernel_supports_inode_uprobes (s))
+    emit_module_inode_init (s);
+  else
+    emit_module_utrace_init (s);
+}
+
+
+void
+uprobe_derived_probe_group::emit_module_exit (systemtap_session& s)
+{
+  if (kernel_supports_inode_uprobes (s))
+    emit_module_inode_exit (s);
+  else
+    emit_module_utrace_exit (s);
+}
+
+
 // ------------------------------------------------------------------------
 // Kprobe derived probes
 // ------------------------------------------------------------------------

Re: Initial stap support for inode-based uprobes

[Josh Stone]

On 11/16/2011 09:39 AM, David Smith wrote:
> I just got through running the systemtap testsuite on 2 different
> configurations:
[...]
> That isn't too bad for a first stab.

Thanks for the comparison, looks like we're in decent shape!  Hopefully
the last mile is not too painful...

>> * Argument access.  If you try $args, it will fail with a missing symbol
>> 'task_user_regset_view'.  I haven't looked closely at this yet.

This piece should actually be ok now -- task_user_regset_view just needs
to be exported along with the registration functions.

Josh

Re: Initial stap support for inode-based uprobes

[David Smith]

I just got through running the systemtap testsuite on 2 different
configurations:

1) stock f16 kernel and HEAD systemtap

# of expected passes		3096
# of unexpected failures	63
# of unexpected successes	8
# of expected failures		259
# of untested testcases		61
# of unsupported tests		4

2) f16 kernel with the new inode-based uprobes built-in and systemtap
with a merged dsmith/task_finder2 and jistone/inode-uprobes branches

# of expected passes		2638
# of unexpected failures	329
# of unexpected successes	9
# of expected failures		251
# of untested testcases		70
# of unsupported tests		4

That isn't too bad for a first stab.  Here's the link to the diff in
dejazilla:

<http://web.elastic.org/~dejazilla/viewrgdiff.php?rg1=402882&rg2=891228&_offset=0&_limit=40&_offset=0&testcase=&r1=&r2=>

I haven't had time to do a full analysis of the results, but the
problems Josh listed in his original email (included below) are still there.

> * Return probes.  This hasn't yet been added to the new uprobes.
> 
> * Process filtering.  AFAICS, the current uprobes implementation sets
> the breakpoint in all processes that map the particular inode.  There is
> a filtering mechanism, but that seems only to decide whether to call the
> handler each time.  You'll still take the bp/sstep overhead.  Also, on
> stap's side, we previously had the ability to limit process probes to
> the -x/-c target and children, which I haven't tried here yet.
> 
> * Runtime build-id verification.  Right now I'm just mapping the path to
> inode*, without checking that the build-id is what we expected.  I'm not
> sure we even could at the systemtap-init point.  Even if we did, the
> file may still get modified without changing the inode, and I don't
> think this uprobes gives us any way to notice or decide whether we like
> the new form.
> 
> * SDT semaphore.  In the current form, we have no hook on individual
> processes, so we can't modify the semaphores in applications that are
> actively gating their markers.  We'll probably need something like
> PR10994 to achieve this, which isn't really about uprobes per-se, but
> rather about living without utrace.
> 
> * Argument access.  If you try $args, it will fail with a missing symbol
> 'task_user_regset_view'.  I haven't looked closely at this yet.
> 
> * Probe IP.  For many probe handlers, we try to set the pt_regs IP to
> the actual breakpoint IP, but in this case we don't happen to even know
> the virtualized address.  Uprobes itself uses uprobes_get_bkpt_addr() in
> some instances, but that's not exposed for our use.


-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: Initial stap support for inode-based uprobes

[Mark Wielaard]

On Wed, 2011-11-16 at 11:39 -0600, David Smith wrote:
> I just got through running the systemtap testsuite on 2 different
> configurations:
> 
> 1) stock f16 kernel and HEAD systemtap
> 
> # of expected passes		3096
> # of unexpected failures	63
> # of unexpected successes	8
> # of expected failures		259
> # of untested testcases		61
> # of unsupported tests		4

63 FAILs seems a bit high. For comparison these are my results on the
same setup (assuming x86_64, but i686 on f15 is similar, and even on
x86_64/rhel5 I don't get more than 30 FAILs - admittedly I did clean up
the testsuite in the last week, so if your run is from a couple of days
ago then you might want to rerun):

Host: Linux toonder.wildebeest.org 3.1.1-1.fc16.x86_64 #1 SMP Fri Nov 11
21:47:56 UTC 2011 x86_64 x86_64 x86_64 GNU/Linux
Snapshot: version 1.7/0.152 commit release-1.6-419-g0f7b51d
GCC: 4.6.2 [gcc (GCC) 4.6.2 20111027 (Red Hat 4.6.2-1)]
Distro: Fedora release 16 (Verne)

# of expected passes            3142
# of unexpected failures        20
# of unexpected successes       8
# of expected failures          259
# of untested testcases         62
# of unsupported tests          2

Do you know where those extra 40 FAILs come from?
Below is my FAIL list. 13 come from the nd_syscall testcase, which
probably should be marked KFAIL (or is that XFAIL).

Cheers,

Mark

FAIL: cast-scope
FAIL: buildok/nfs_proc-detailed.stp
FAIL: buildok/nfsd-all-probes.stp
FAIL: buildok/nfsd-detailed.stp
FAIL: buildok/rpc-detailed.stp
FAIL: buildok/vfs-detailed.stp
FAIL: semok/thirtynine.stp
FAIL: 64-bit dup nd_syscall
FAIL: 64-bit eventfd nd_syscall
FAIL: 64-bit inotify nd_syscall
FAIL: 64-bit pipe nd_syscall
FAIL: 64-bit poll nd_syscall
FAIL: 64-bit signalfd nd_syscall
FAIL: 32-bit dup nd_syscall
FAIL: 32-bit eventfd nd_syscall
FAIL: 32-bit inotify nd_syscall
FAIL: 32-bit net1 nd_syscall
FAIL: 32-bit pipe nd_syscall
FAIL: 32-bit poll nd_syscall
FAIL: 32-bit signalfd nd_syscall

Re: Initial stap support for inode-based uprobes

[Mark Wielaard]

On Wed, 2011-11-16 at 11:39 -0600, David Smith wrote:
> > * Probe IP.  For many probe handlers, we try to set the pt_regs IP to
> > the actual breakpoint IP, but in this case we don't happen to even know
> > the virtualized address.  Uprobes itself uses uprobes_get_bkpt_addr() in
> > some instances, but that's not exposed for our use.

This is really architecture dependent, we just currently do it by
querying kprobes or uprobes where the breakpoint was set. On some
architectures (x86 at least) the PC gets munged during the breakpoint
processing (it is set after the breakpoint instruction on that
architecture), which causes some confusion for some of the runtime or
scripts which look at the REG_IP and see something different than they
expect (in the worst case the PC is just after the current function or
module). If you cannot get at the actual breakpoint address you will
need to resort to architecture specific code, that know the breakpoint
instruction used and how much the PC should be adjusted (normally the
width of the breakpoint instruction).

uprobe_stmt_num.exp and uprobe_uaddr.exp are the tests to look at. But
also tests that do unwinding rely on REG_IP being somewhat sane.

Cheers,

Mark

Re: Initial stap support for inode-based uprobes

[Mark Wielaard]

On Wed, 2011-11-16 at 11:39 -0600, David Smith wrote:
> > * Runtime build-id verification.  Right now I'm just mapping the path to
> > inode*, without checking that the build-id is what we expected.  I'm not
> > sure we even could at the systemtap-init point.  Even if we did, the
> > file may still get modified without changing the inode, and I don't
> > think this uprobes gives us any way to notice or decide whether we like
> > the new form.

I am not sure where this exactly is/should be done, or how/why the inode
pointed to can still change. But if the inode points to an ELF file,
then it shouldn't be too hard to get at the build-id.

You have to parse the elf header, get at the program headers (e_phoff),
iterate over them (e_phentsize) till you find the one that is marked
with PT_NOTE, and then parse all notes till you get the one with the
right type (NT_GNU_BUILD_ID) and name ("stapsdt")

OK, so it is somewhat involved. But assuming we are in user context when
we get at the inode and the inode represents an ELF file we can parse,
then that would actually be nicer than what we currently do by
extracting the offset to the note directly and peeling it out through
that offset.

Cheers,

Mark

Re: Initial stap support for inode-based uprobes

[David Smith]

On 11/17/2011 03:27 AM, Mark Wielaard wrote:

> On Wed, 2011-11-16 at 11:39 -0600, David Smith wrote:
>> I just got through running the systemtap testsuite on 2 different
>> configurations:
>>
>> 1) stock f16 kernel and HEAD systemtap
>>
>> # of expected passes		3096
>> # of unexpected failures	63
>> # of unexpected successes	8
>> # of expected failures		259
>> # of untested testcases		61
>> # of unsupported tests		4
> 
> 63 FAILs seems a bit high...


I looked at my failures, and it seems that since that machine didn't
have nss-devel, which caused a bunch of server failures.

-- 

David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: Initial stap support for inode-based uprobes

[David Smith]

On 11/16/2011 11:39 AM, David Smith wrote:

> I just got through running the systemtap testsuite on 2 different
> configurations:
> 
> 1) stock f16 kernel and HEAD systemtap
> 
> # of expected passes		3096
> # of unexpected failures	63
> # of unexpected successes	8
> # of expected failures		259
> # of untested testcases		61
> # of unsupported tests		4
> 
> 2) f16 kernel with the new inode-based uprobes built-in and systemtap
> with a merged dsmith/task_finder2 and jistone/inode-uprobes branches
> 
> # of expected passes		2638
> # of unexpected failures	329
> # of unexpected successes	9
> # of expected failures		251
> # of untested testcases		70
> # of unsupported tests		4
> 
> That isn't too bad for a first stab.  Here's the link to the diff in
> dejazilla:
> 
> <http://web.elastic.org/~dejazilla/viewrgdiff.php?rg1=402882&rg2=891228&_offset=0&_limit=40&_offset=0&testcase=&r1=&r2=>


I've looked into this a bit, and the lack of uprobes return probes
causes a good number of failures.  I've just checked in a change to
uprobes.exp, so that it tests .call and .return probes individually (as
well as together).  The inode-based uprobes code passes the .call probe
test.

-- 

David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: Initial stap support for inode-based uprobes

[Srikar Dronamraju]

> > 
> > 1) stock f16 kernel and HEAD systemtap
> > 
> > # of expected passes		3096
> > # of unexpected failures	63
> > # of unexpected successes	8
> > # of expected failures		259
> > # of untested testcases		61
> > # of unsupported tests		4
> > 
> > 2) f16 kernel with the new inode-based uprobes built-in and systemtap
> > with a merged dsmith/task_finder2 and jistone/inode-uprobes branches
> > 
> > # of expected passes		2638
> > # of unexpected failures	329
> > # of unexpected successes	9
> > # of expected failures		251
> > # of untested testcases		70
> > # of unsupported tests		4
> > 
> > That isn't too bad for a first stab.  Here's the link to the diff in
> > dejazilla:
> > 
> > <http://web.elastic.org/~dejazilla/viewrgdiff.php?rg1=402882&rg2=891228&_offset=0&_limit=40&_offset=0&testcase=&r1=&r2=>
> 
> 
> I've looked into this a bit, and the lack of uprobes return probes
> causes a good number of failures.  I've just checked in a change to
> uprobes.exp, so that it tests .call and .return probes individually (as
> well as together).  The inode-based uprobes code passes the .call probe
> test.

Nice.
Thanks for the confirmation.

-- 
Thanks and Regards
Srikar