[DTrace-devel] [PATCH v2 3/4] probe: do not try to reify probes from uncooked providers

[Kris Van Hees]

On Thu, Nov 14, 2024 at 10:01:07PM +0000, Nick Alcock wrote:
> If there is a syntax error in the middle of a .d script processed
> by -h, we will be left with a provider in question in an uncooked
> state (where dt_provider_create() has been called on it with a NULL
> provider by dt_node_provider(), but it has not had its provider
> set yet by dt_cook_provider()).
> 
> In this case, the attempt to pop the parser stack on error will
> fail because probe iteration is attempting to reify probes out
> of providers without making sure they're cooked first:
> 
>     at libdtrace/dt_probe.c:1006
>     argv=0x535830, fp=0x21e4630, s=0x0) at libdtrace/dt_cc.c:789
>     fp=0x21e4630, s=0x0) at libdtrace/dt_cc.c:1419
>     at libdtrace/dt_cc.c:1441
> 
> The fix is simple: treat providers with no provimpl just like we do
> providers with no provide() method in their provimpl: they cannot provide
> probes, so don't try. (Test coming in a later commit in this series.)

This entire commit message (and the one-liner subject) are actually not
quite correct in terms of what the problem is.  It has nothing to do with
cooked vs uncooked providers or probes.  The problem is simply that the
dt_probe_iter() functions calls the provide() hook in provider prior to
trying to match probe descriptions, and that cannot be done for a provider
without implementation.  That is all.

This also needs a test.

I'm posting an alternative patch (with my reviewed-by) with updated commit
message and 2 tests (signed-off-by for both of us since you wrote the code).

> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> ---
>  libdtrace/dt_probe.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libdtrace/dt_probe.c b/libdtrace/dt_probe.c
> index 189bf7928e145..ccaa3081c5b23 100644
> --- a/libdtrace/dt_probe.c
> +++ b/libdtrace/dt_probe.c
> @@ -1007,7 +1007,8 @@ dt_probe_iter(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp,
>  	 * Loop over providers, allowing them to provide these probes.
>  	 */
>  	while ((pvp = dt_htab_next(dtp->dt_provs, &it)) != NULL) {
> -		if (pvp->impl->provide == NULL ||
> +		if (pvp->impl == NULL ||
> +		    pvp->impl->provide == NULL ||
>  		    !dt_gmatch(pvp->desc.dtvd_name, pdp->prv))
>  			continue;
>  		memcpy(&desc, pdp, sizeof(desc));
> -- 
> 2.46.0.278.g36e3a12567
>