[DTrace-devel] [PATCH 6/6] Add support for pid function "-" with absolute offset

[Kris Van Hees]

On Fri, Jan 10, 2025 at 01:30:50AM -0500, Eugene Loh wrote:
> On 1/10/25 00:07, Kris Van Hees wrote:
> 
> > On Thu, Jan 09, 2025 at 04:00:42PM -0500, Eugene Loh wrote:
> > > On 1/9/25 15:52, Eugene Loh wrote:
> > > > On 1/9/25 14:12, Kris Van Hees wrote:
> > > > > On Fri, Dec 20, 2024 at 05:27:16PM -0500, eugene.loh--- via DTrace-devel wrote:
> > > > > > diff --git a/include/dtrace/pid.h b/include/dtrace/pid.h
> > > > > > @@ -47,6 +47,7 @@ typedef struct pid_probespec {
> > > > > >        /*
> > > > > >         * Fields below this point do not apply to underlying probes.
> > > > > >         */
> > > > > > +    int pps_absoff;                /* use "-" for overlying probe function */
> > > > > Why use this?  All you do with it is record that func is "-", and then later
> > > > > in the pid provider, you actually use that to force the function name in the
> > > > > probe description to be "-".  But that function name in the probe desc is (in
> > > > > the pid provider) already coming from pps_fun, and the code was already
> > > > > assigning func to pps_fun, so it would be "-" anyway.  So the pps_absoff seems
> > > > > to serve no purpose, and you could just rely on pps_fun being "-" when needed.
> > > > Yeah, good point.  Let me see if I can remember/understand/explain
> > > > what's going on here.
> > > > 
> > > > In essence, the problem is we have to pass two function names: "-" (for
> > > > the overlying probe) and the actual function name (for the underlying
> > > > probe);  up until now, they were both the same.  I tossed around
> > > > different ideas for this.  E.g., pass two function names in. Or, get the
> > > > actual function name inside dt_prov_uprobe. It seemed to make most sense
> > > > (given the alternatives) to pass in the actual function name and add
> > > > this flag to say when the overlying probe's function name should be
> > > > "-".  Does that make sense?
> > > > 
> > > > Note that pps_fun is not "-".  Although we set it to func ("-"), the
> > > > code that processes this case then fills pps_fun in with the actual
> > > > function name.  Again, one option is not to do this until
> > > > dt_prov_uprobe.c, but that introduces some other hassles.
> > Ok, I see what you mean...  because we rely on underlying probes, we need to
> > track the function name that the absolute offset falls in in order to be able
> > to determine the underlying uprobe that the PID probe maps onto (especially
> > since there might be another probe on it already).
> > 
> > However, as far as I can see there is not really any need to have a function
> > name on the underlying probe.  After all, it is actually a uprobe that is
> > identified by dev/inode/offset where offset is an offset into the mapping at
> > dev/inode, i.e. not relative to any particular function at all.  So, not
> > having the function name associated with the underlying probe would make more
> > sense than what we are doing now.
> > 
> > And if we drop the function name from the underlying probe, we suddenly no
> > longer need to carry it across for absolute offset probe, and this entire
> > two-function name thing goes away.
> 
> Right.  I considered that, but decided against it.  I think the reason was
> that, although we don't really need a function name for naming the
> underlying uprobe (as you point out), it's nice to have the function for the
> whole "USDT ignore clause" stuff.  Put another way, we *do* use the function
> name of the underlying probe... not so much for naming the uprobe but for an
> extra USDT ignore_clause() check.  If we wanted to, we could get rid of that
> check, but... well, that's the trade-off that we're facing here.

II would propose that we do the following:

- For absolute offset probes, use new type DTPPT_ABSOFF (or ABSOFFSET) as you
  suggest.  The uprobe provider will use that to know that to put "-" as the
  function name component of the overlying probe.

- For the underlying probe, store the function name (from pps_fun) in a member
  of dt_uprobe_t (fun or func, perhaps).  That can be used in ignore_clause().

- For the underlying probe, leave the function name empty (to be consistent
  with the fact that the underlying probe is a uprobe, identified by dev/inode
  and offset within the inode.)

This way we no longer display the function name for underlying probes in the
probe listing, which is correct, and yet we retain the ignore_clause()
optimization.

	Kris