[DTrace-devel] [PATCH] Allow tstrings to persist across clause boundaries within a program

[Kris Van Hees]

On Tue, Nov 02, 2021 at 04:22:50PM -0400, Eugene Loh wrote:
> I'd like to hear more about how this fits into strtok.  For example, 
> does this bump DT_TSTRING_SLOTS up from 4 to 5?  Or, would the number of 
> slots be bumped up if and only if any clause for any probe uses strtok?

Well, that is not for me to decide.  You are implementing strtok() so I would
think you manage what the needs are.  I simply propose this patch to make the
use of tstrings available across clause boundaries.

> Also, we currently assume that stack actions and string subroutines are 
> never co-active.  That is, the space we allocate for stacks and for 
> tstrings overlap one another and we allocate only enough space for the 
> MAX() of the two.  But if two strtok() calls are separated by a stack 
> call, then the strtok internal string would seem to get corrupted with 
> such a space-saving scheme.

Well, yes, because this patch is not actually doing implementation work for
strtok() since I do not want to conflict with your work.  It is e.g. quite
possible for you to decide to ensure that the strtok string is kept in a
specific tstring slot, increase the number of slots, and move the stack scratch
space to be starting at tstring slot 1.  Or wait for alloca() and use that for
the strtok() source string, allocating the space from the general scratch area.
Those are all design decisions that are directly related to strtok() so they
are things that would be part of your patch(es).

> I started looking at strtok and I simply allocated another string at the 
> end of the MAX(tstring, stack) area.  That is potentially wasteful but 
> it's easy.
> 
> On 11/2/21 1:35 AM, Kris Van Hees wrote:
> > The upcoming strtok() implementation has a need to store the string to
> > be tokenized in a temporary string (tstring).  The strtok() semantics
> > require that source string to persist across clause boundaries within
> > a given probe firing.  That means the tstrings needs to persist for
> > the duration of the probe program.
> >
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> >   libdtrace/dt_cc.c   | 2 ++
> >   libdtrace/dt_cg.c   | 3 +--
> >   libdtrace/dt_impl.h | 2 ++
> >   3 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
> > index a25f3548..08bc9438 100644
> > --- a/libdtrace/dt_cc.c
> > +++ b/libdtrace/dt_cc.c
> > @@ -1996,6 +1996,8 @@ dt_compile(dtrace_hdl_t *dtp, int context, dtrace_probespec_t pspec, void *arg,
> >   		    "not referenced)\n", yypcb->pcb_sargv[argc - 1], argc - 1);
> >   	}
> >   
> > +	dt_cg_tstring_reset(dtp);
> > +
> >   	/*
> >   	 * If we have successfully created a parse tree for a D program, loop
> >   	 * over the clauses and actions and instantiate the corresponding
> > diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> > index 0196b114..6a4500a5 100644
> > --- a/libdtrace/dt_cg.c
> > +++ b/libdtrace/dt_cg.c
> > @@ -801,7 +801,7 @@ dt_cg_spill_load(int reg)
> >   /*
> >    * Initialize the temporary string offsets and mark all not in use.
> >    */
> > -static void
> > +void
> >   dt_cg_tstring_reset(dtrace_hdl_t *dtp)
> >   {
> >   	int		i;
> > @@ -5654,7 +5654,6 @@ dt_cg(dt_pcb_t *pcb, dt_node_t *dnp)
> >   	}
> >   
> >   	dt_regset_reset(pcb->pcb_regs);
> > -	dt_cg_tstring_reset(pcb->pcb_hdl);
> >   
> >   	dt_irlist_destroy(&pcb->pcb_ir);
> >   	dt_irlist_create(&pcb->pcb_ir);
> > diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
> > index b461d73d..3be62657 100644
> > --- a/libdtrace/dt_impl.h
> > +++ b/libdtrace/dt_impl.h
> > @@ -760,6 +760,8 @@ extern dtrace_difo_t *dt_dlib_get_func_difo(dtrace_hdl_t *, const dt_ident_t *);
> >   extern void dt_dlib_reset(dtrace_hdl_t *dtp, boolean_t);
> >   extern int dt_load_libs(dtrace_hdl_t *dtp);
> >   
> > +extern void dt_cg_tstring_reset(dtrace_hdl_t *dtp);
> > +
> >   extern void *dt_compile(dtrace_hdl_t *dtp, int context,
> >   			dtrace_probespec_t pspec, void *arg, uint_t cflags,
> >   			int argc, char *const argv[], FILE *fp, const char *s);
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel