Re: [PATCH v2 4/4] probe: make it possible to destroy probes in more cases

[Kris Van Hees <kris.van.hees@oracle.com>]

This patch is not needed.  I do not see any failures related to the changes
in this patch and the test case passes even without these code changes as
far as I can see.

The test case has been added to the 3/4 patch, and has been converted to a 
simple .d file (and .r for test verification).  No need to use a shell script
in this case.

On Thu, Nov 14, 2024 at 10:01:08PM +0000, Nick Alcock wrote:
> In particular, when the parser has failed (so we have no yypcb)
> but we have successfully defined some interface-only probes but
> not yet cooked them (so we have no provimpl), we can find that
> *both* our sources for a dtp in dt_probe_destroy are unavailable.
> 
> Make one available in the dt_probe_t as well.
> 
> The test in this commit provides a .d like this:
> 
> provider prova { probe entrya(); };
> provider provb { probe entryb(); probe entryc(int, char *) : (char *, int); };
> 
> prova is defined but not cooked because parsing failed in provb, so we get
> this crash when trying to do final cleanup of prova, long after the parser is
> gone:
> 
> 1  0x00007ffff7cf408a in dt_iddtor_probe (idp=0x3cee000) at libdtrace/dt_ident.c:542
> 2  0x00007ffff7cf4d5a in dt_ident_destroy (idp=0x3cee000) at libdtrace/dt_ident.c:964
> 3  0x00007ffff7d0c226 in dt_node_free (dnp=0x21e05b0) at libdtrace/dt_parser.c:571
> 4  0x00007ffff7d17748 in dt_node_list_free (pnp=0x3cee1b8) at libdtrace/dt_parser.c:4851
> 5  0x00007ffff7d0c460 in dt_node_free (dnp=0x3cee180) at libdtrace/dt_parser.c:643
> 6  0x00007ffff7d177ab in dt_node_link_free (pnp=0x3cee278) at libdtrace/dt_parser.c:4865
> 7  0x00007ffff7d3df8b in dt_provider_del_prov (head=0x0, pvp=0x3cee1f0) at libdtrace/dt_provider.c:74
> 8  0x00007ffff7cf1d32 in dt_htab_destroy (dtp=0x53eb20, htab=0x187a8a0) at libdtrace/dt_htab.c:108
> 9  0x00007ffff7d0842e in dtrace_close (dtp=0x53eb20) at libdtrace/dt_open.c:1306
> 10 0x0000000000403f8e in dfatal (fmt=0x4d6a45 "failed to compile script %s") at cmd/dtrace.c:194
> 11 0x0000000000404b9b in compile_file (dcp=0x536990) at cmd/dtrace.c:480
> 12 0x00000000004073c9 in main (argc=8, argv=0x7fffffffe788) at cmd/dtrace.c:1351
> 
> Signed-off-by: Nick Alcock <nick.alcock@oracle.com>
> ---
>  libdtrace/dt_probe.c                          | 14 +++-----
>  libdtrace/dt_probe.h                          |  1 +
>  .../unittest/usdt/err.argmap-name-required.sh | 34 +++++++++++++++++++
>  3 files changed, 40 insertions(+), 9 deletions(-)
>  create mode 100755 test/unittest/usdt/err.argmap-name-required.sh
> 
> diff --git a/libdtrace/dt_probe.c b/libdtrace/dt_probe.c
> index ccaa3081c5b23..31c78d6d987e1 100644
> --- a/libdtrace/dt_probe.c
> +++ b/libdtrace/dt_probe.c
> @@ -140,8 +140,7 @@ alloc_arg_nodes(dtrace_hdl_t *dtp, dt_provider_t *pvp, int argc)
>  static void
>  dt_probe_alloc_args(dt_probe_t *prp, int nargc, int xargc)
>  {
> -	dt_provider_t	*pvp = prp->prov;
> -	dtrace_hdl_t	*dtp = pvp->pv_hdl;
> +	dtrace_hdl_t	*dtp = prp->dtp;
>  	dt_node_t	*nargs = NULL, *xargs = NULL;
>  	int		i;
>  
> @@ -252,6 +251,7 @@ dt_probe_create(dtrace_hdl_t *dtp, dt_ident_t *idp, int protoc,
>  	prp->pr_inst = NULL;
>  	prp->argv = dt_calloc(dtp, xargc, sizeof(dtrace_typeinfo_t));
>  	prp->argc = xargc;
> +	prp->dtp = dtp;
>  
>  	if ((prp->nargc != 0 && prp->nargv == NULL) ||
>  	    (prp->xargc != 0 && prp->xargv == NULL) ||
> @@ -323,12 +323,7 @@ dt_probe_destroy(dt_probe_t *prp)
>  	dt_probe_stmt_t		*psp, *psp_next;
>  	dt_probe_instance_t	*pip, *pip_next;
>  	dt_probe_dependent_t	*dep, *dep_next;
> -	dtrace_hdl_t		*dtp;
> -
> -	if (prp->prov != NULL)
> -		dtp = prp->prov->pv_hdl;
> -	else
> -		dtp = yypcb->pcb_hdl;
> +	dtrace_hdl_t		*dtp = prp->dtp;
>  
>  	if (prp->difo)
>  		dt_difo_free(dtp, prp->difo);
> @@ -473,7 +468,7 @@ dt_probe_define(dt_provider_t *pvp, dt_probe_t *prp, const char *fname,
>  dt_node_t *
>  dt_probe_tag(dt_probe_t *prp, uint_t argn, dt_node_t *dnp)
>  {
> -	dtrace_hdl_t *dtp = prp->prov->pv_hdl;
> +	dtrace_hdl_t *dtp = prp->dtp;
>  	dtrace_typeinfo_t dtt;
>  	size_t len;
>  	char *tag;
> @@ -557,6 +552,7 @@ dt_probe_insert(dtrace_hdl_t *dtp, dt_provider_t *prov, const char *prv,
>  	prp->prov = prov;
>  	prp->prv_data = datap;
>  	prp->argc = -1;
> +	prp->dtp = dtp;
>  
>  	dt_htab_insert(dtp->dt_byprv, prp);
>  	dt_htab_insert(dtp->dt_bymod, prp);
> diff --git a/libdtrace/dt_probe.h b/libdtrace/dt_probe.h
> index 2a78cb9ca4dae..da7a341354225 100644
> --- a/libdtrace/dt_probe.h
> +++ b/libdtrace/dt_probe.h
> @@ -55,6 +55,7 @@ typedef struct dt_probe {
>  	int argc;			/* output argument count */
>  	dt_probe_instance_t *pr_inst;	/* list of functions and offsets */
>  	dtrace_difo_t *difo;		/* BPF probe program */
> +	dtrace_hdl_t *dtp;		/* pointer to containing dtrace_hdl */
>  } dt_probe_t;
>  
>  extern dt_probe_t *dt_probe_lookup2(dt_provider_t *, const char *);
> diff --git a/test/unittest/usdt/err.argmap-name-required.sh b/test/unittest/usdt/err.argmap-name-required.sh
> new file mode 100755
> index 0000000000000..3fdefb8130ac7
> --- /dev/null
> +++ b/test/unittest/usdt/err.argmap-name-required.sh
> @@ -0,0 +1,34 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
> +# Licensed under the Universal Permissive License v 1.0 as shown at
> +# http://oss.oracle.com/licenses/upl.
> +#
> +# Make sure we get the right error when mappings are defined with no
> +# argument.
> +
> +if [ $# != 1 ]; then
> +	echo expected one argument: '<'dtrace-path'>'
> +	exit 2
> +fi
> +
> +dtrace=$1
> +CC=/usr/bin/gcc
> +CFLAGS="$test_cppflags"
> +LDFLAGS="$test_ldflags"
> +
> +DIRNAME="$tmpdir/argmap-name-required.$$.$RANDOM"
> +mkdir -p $DIRNAME
> +cd $DIRNAME
> +
> +# We define multiple probes here to verify the absence of a potential error
> +# where parser failures when previous probes have been successfully defined
> +# causes crashes due to problems freeing still-uncooked probes.
> +
> +cat > prov.d <<EOF
> +provider prova { probe entrya(); };
> +provider provb { probe entryb(); probe entryc(int, char *) : (char *, int); };
> +EOF
> +
> +exec $dtrace $dt_flags -h -s prov.d
> -- 
> 2.46.0.278.g36e3a12567
>