[DTrace-devel] [PATCH 21/26] usdt: DTrace userspace side

[Nick Alcock]

On 21 Oct 2022, Kris Van Hees via DTrace-devel stated:

> This is implemented almost entirely in the pid provider, which is
> reassuringly similar to how it was done in the in-kernel days. They're
> really very closely-related beasts, and the same code can handle both
> easily enough.

OK so this commit message is not true any more :) do you want to fix it
or shall I?

>  - adjusts provide_pid so it can be called at USDT uprobe discovery time
>    to create underlying probes that user-requested USDT probes will then
>    utilize.  The changes here look rather extreme but are actually quite
>    small: the underlying probe stuff is split into a new function,
>    provide_pid_underlying(), and provide_pid can be asked to do nothingg
>    but call this; the probe name follows the new dtprobed rules, looking
>    like pid:$inum::$offset, and thus is identical for probes created for
>    USDT and for pids at the same offset.
>
>    The struct pid_probe attached to the underlying probe gains a device
>    number (which it should always have had) and keeps track of the
>    underlying uprobe name from create_uprobe or USDT probe discovery
>    and remembers whether or not DTrace created it (if dtprobed created
>    it, dtrace must not delete it).
>
>    provide_pid itself is adjusted to call provide_pid_underlying as
>    needed, but also to do more work if the probe already exists: USDT
>    probes can be associated with more than one underlying probe (if the
>    probe appears repeatedly in a program), so if it is repeatedly
>    provide_pid'ed with different offsets, we spot this and chain it into
>    all the necessary underlying probes, and also keep track of all the
>    underlying probes associated with an overlying probe so we can enable
>    them all (see next item).

This bit, in particular, is not exactly true any more: the same things
are done but in a much easier-to-understand way.

>  - stack arg handling needs a bit of a tweak.  usdt probes (but not any
>    other sorts of probes) are placed after the setup of args for a
>    function call but obviously before the actual call, since the
>    "function" doesn't exist.  On platforms on which
>    PT_REGS_ARGSTKBASE > 0, we have to remember to *not* apply the
>    ARGSTKBASE adjustment when a usdt probe is being called, since
>    that applies to the actual function call, which hasn't happened.
>    (In theory, pid offset probes might need this, but figuring out when
>    this would be needed would be tricky: we'd need to do it only when
>    the probe was placed at the actual site of a function call -- and
>    even that would leave us in trouble if the probe was placed in the
>    middle of arg setup!)

(and this should mention PP_IS_FUNCALL.)

> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>

FWIW, for the bits I wrote and you rewrote,

Reviewed-by: Nick Alcock <nick.alcock at oracle.com>

-- even if they don't pass their tests this is so much easier to
   understand than what I did. I had the division of labour between
   provide_pid and provide_pid_underlying all wrong...

> +		/* FIXME: DTPPT_IS_ENABLED needs to be supported also.  */
> +		psp.pps_type = DTPPT_OFFSETS;

Indeed. Not in this phase though :)

> +		/*
> +		 * Does this match the overlying probe we are meant to be
> +		 * creating?  If so, create an overlying probe and let
> +		 * provide_probe() create the underlying probe for us.  Among
> +		 * other things, this ensures that the PID part of the provider
> +		 * name gets stuck into the overlying probe properly.
> +		 *
> +		 * TODO: wildcard probes get handled here.
> +		 */

... this, less useful though it is, is quite likely to happen first
(because the stuff it needs is a prerequisite for is-enabled probes
too).

> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> new file mode 100644
[...]
> +/*
> + * Look up or create an underlying (real) probe, corresponding directly to a
> + * uprobe.  Since multiple pid and USDT probes may all map onto the same
> + * underlying probe, we may already have one in the system.
> + *
> + * If not found, we create a new probe.
> + */
> +static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
> +				     const pid_probespec_t *psp)
> +{
[...]
> +static int provide_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
> +			 const char *prb, const dt_provimpl_t *pvops, int flags)
> +{
> +	char			prv[DTRACE_PROVNAMELEN];
> +	dt_provider_t		*pvp;
> +	dtrace_probedesc_t	pd;
> +	dt_uprobe_t		*upp;
> +	dt_probe_t		*prp, *uprp;
> +	list_probe_t		*pop, *pup;
[...]

This is so much nicer than what I did... ack ack ack.

-- 
NULL && (void)