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

[Eugene Loh <eugene.loh@oracle.com>]

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.

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@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).