[DTrace-devel] [PATCH] test: Skip err.Z_no-w for now

[Kris Van Hees]

On Thu, Jul 10, 2025 at 04:03:01PM -0400, Eugene Loh wrote:
> Okay.  I'm withdrawing this patch and will post a different one in its
> place.
> 
> Your first proposal sounds great, but I'll go with the third one (which you
> recommend) in order to mimic Solaris more closely.

Actually, looking at the code a little bit, I think the first option may not
be bad at all.  We can introduce a new error tag:

	D_DESTRUCT	destructive actions not allowed

and report that one from dt_cg_clsflags() when a destructive action is being
reported and destructive is not set (in pcb->pcb_hdl->dt_options[]).  That
seems entirely reasonable, and while it is an enhancement over what Solaris
was doing, it is also something that is simply possible now where before it
was not really considered the right thing.

Even if we eventually support pre-compilation, I think it would be reasonable
to expect compilation of script swith destructive actions to require -w, as an
extra safety (requiring it both at pre-compilation *and* execution).

> On 7/9/25 19:45, Kris Van Hees wrote:
> > Yes, this is a bug.  Solaris (and DTrace 1.x) would load all the DOF into the
> > kernel, even for probes that are not (yet) available (when -Z is used) because
> > the discovery of probes happened at the kernel level.
> > 
> > Since we do it at the userspace level, we do not load BPF programs for any
> > probes that we cannot attach to (yet), and therefore the destructive action
> > condition is not detected in this case.
> > 
> > That is obviously wrong.
> > 
> > There are a few ways to handle this:
> > 
> > - Report the error at compilation time, i.e. if we encounter a destructive
> >    action during compilation and -w was not set, we report an error.  That was
> >    not really a valid way to do it in the Solaris days because you could compile
> >    D code and save it, so you wouldn't want compilation to abort due to a
> >    destructive action because you didn't know yet if you were going to use it.
> >    We don't have anything to save compiled code right now, so this could be an
> >    option.  But it would change the error from a runtime error to a compile
> >    error, which is a change of behaviour.
> > 
> > - When we are ready to load programs and start tracing, we can loop through
> >    through the compiled clauses, and if any use destructive actions, abort if
> >    -w is not enabled.
> > 
> > - We could have destructive actions trigger setting a global flag, and check
> >    that when we start loading programs for tracing.
> > 
> > I think that the 3rd option is likely to be the nicest, because that flag can
> > be set during compilation, and if we ever implement something like saving and
> > loading compiled programs, this would still work fine (we would process BPF
> > programs that we load and set that flag upon load if an instruction is found
> > that amounts to a destructive action).
> > 
> > Thoughts?
> > 
> > On Wed, Jul 09, 2025 at 06:39:31PM -0400, Eugene Loh wrote:
> > > On 7/8/25 08:15, Nick Alcock wrote:
> > > 
> > > > From: Eugene Loh <eugene.loh at oracle.com>
> > > > > It is unclear what behavior is desired.  For example, consider:
> > > > > 
> > > > >        dtrace -Z -n 'BEGIN { exit(0) } foo:bar:baz:bop { raise(SIGUSR1) }'
> > > > ... isn't the lack of semicolons here a syntax error? (Or have I been
> > > > inserting them pointlessly all these years just because awk needs them?)
> > > I routinely omit semicolons.  Okay since even Solaris.  E.g.,
> > > 
> > > # uname -s
> > > SunOS
> > > # /usr/sbin/dtrace -n 'BEGIN { printf("hello") } BEGIN { printf("world") }
> > > BEGIN { exit(0) }'
> > > dtrace: description 'BEGIN ' matched 3 probes
> > >   CPU     ID                    FUNCTION:NAME
> > >    51      1                           :BEGIN hello
> > >    51      1                           :BEGIN world
> > >    51      1                           :BEGIN
> > > 
> > > > > The first probe exists.  The second one will be ignored.  Solaris will
> > > > > reject the script with:
> > > > > 
> > > > >        dtrace: description 'BEGIN ' matched 1 probe
> > > > >        dtrace: could not enable tracing: Destructive actions not allowed
> > > > > 
> > > > > On Linux, we have:
> > > > > 
> > > > >        dtrace: description 'BEGIN ' matched 1 probe
> > > > >        CPU     ID                    FUNCTION:NAME
> > > > >          0      1                           :BEGIN
> > > > > 
> > > > > Perhaps both behaviors have merit.  For now, just skip the test to
> > > > > avoid test failures we are not ready to arbitrate.
> > > > Does execution fail if the probe exists but there's a BEGIN that exits?
> > > > I guess so.
> > > Right.
> > > 
> > > > So the interesting question is: if you start with -Z and
> > > > specify a destructive action in a nonexistent probe, and then a
> > > > USDT-containing program starts up that provides that probe, what do we
> > > > do? If we don't terminate, that would be surprising. If we terminate in
> > > > the middle of execution, would that be more surprising than checking all
> > > > bodies at startup and failing early?
> > > I think I finally understand what's going on.  The point of this patch was
> > > to move on to more urgent matters, but there is maybe a bug here I should
> > > fix.
> > > 
> > > Solaris rejects a clause even if it's not going to be used.  E.g.,
> > > 
> > >      # dtrace -Z -n 'BEGIN { exit(0) }
> > >          bogus:bogus:bogus:bogus { raise(SIGUSR1) }'
> > > 
> > > Solaris says:
> > > 
> > >      dtrace: description 'BEGIN ' matched 1 probe
> > >      dtrace: could not enable tracing: Destructive actions not allowed
> > > 
> > > Nevermind we do not use the second probe description.  It's a baddie.  Our
> > > Linux port has no such complaint.
> > > 
> > > What about the case you propose (which is, after all, the question being
> > > addressed by this test)?  Well, "it depends."  Put another way, we won't run
> > > a destructive clause without -w.  The problem is that during discovery, we
> > > might first encounter an is-enabled probe (which we "enable" since it's
> > > safe), then later encounter a real probe and reject it due to its
> > > destructive clause.  If you squint just right, you could see this as being
> > > not incorrect, but it's kind of goofy.
> > > 
> > > Ideally, enabling probes and their is-enabled probes atomically would be the
> > > right thing to do.  That's a tall order.
> > > 
> > > But falling back to the Solaris behavior of rejecting destructive clauses
> > > without -w is the right thing to do, even if we are not using them (yet).