[DTrace-devel] [PATCH 2/2] usdt: introduce DOF_VERSION_3 and use it when is-enabled probes are present

[Nick Alcock]

On 13 May 2023, Kris Van Hees said:

> On Wed, Apr 19, 2023 at 04:44:13PM +0100, Nick Alcock via DTrace-devel wrote:
>> This guardrail prevents DTrace v2 from picking up v1-style is-enabled
>> probes, by suppressing is-enabled probes when DOF_VERSION <= 3, setting
>> the DOF_VERSION to 3 for programs containing is-enabled probes, and
>> refusing to -G-link object files containing DOF_VERSION_2 and is-enabled
>> probes. This is all necessary because v1-style probes have no function
>> arguments, so it would be dangerous to try to pick up the first
>> argument, treat it as a pointer, and write a 1 down it.
>> 
>> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
>
> Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
>
> ... but this should be coming before the USDT implementation patch.

True! Will reshuffle, unless it's impractically difficult (I doubt it,
If I remember right, it hardly touches any of the same code at all).

>> diff --git a/libdtrace/dt_link.c b/libdtrace/dt_link.c
>> index e6059e9840cde..3346720fcf62e 100644
>> --- a/libdtrace/dt_link.c
>> +++ b/libdtrace/dt_link.c
>> @@ -1539,16 +1539,29 @@ dtrace_program_link(dtrace_hdl_t *dtp, dtrace_prog_t *pgp, uint_t dflags,
>>  		return 0;
>>  	}
>>  
>> -	for (i = 0; i < objc; i++)
>> +	for (i = 0; i < objc; i++) {
>>  		if (process_obj(dtp, objv[i], &eprobes) != 0)
>>  			return -1; /* errno is set for us */
>>  
>> +		/*
>> +		 * If there are is-enabled probes then we need to error out if
>> +		 * DOF version 2 was used, preventing linkage of pre-existing
>> +		 * objects that contain return-value-driven is-enabled probes
>> +		 * with a DTrace that implements argument-driven is-enabled
>> +		 * probes.  (New objects will at this stage have DOF version 1.)
>> +		 */
>
> How do new object end up with DOF version 1?  Shouldn't new objects be using
> DOF version 3 after this patch?

Nope! This is a very strange handling of versioning but actually saves
us here -- things only get DOF_VERSION_2 (before this patch) or
DOF_VERSION_3 (after it) if they use is-enabled probes (see the hunk
below). So most old object files have DOF_VERSION_1 and will still work
after this commit: only old object files that actually *use* is-enabled
probes will be in trouble and need re-G-ing. (We retain this behaviour,
only switching the version we flip on when is-enabled probes are seen.)

>> +		if (eprobes && pgp->dp_dofversion == DOF_VERSION_2)
>
> Shouldn't this be pgp->dp_dofversion <` DOF_VERSION_3 ?

Nope! We'll see lots of v1 around in future.

>> +			return dt_link_error(dtp, NULL, -1, NULL,
>> +			    "DOF in %s is too old: regenerate with dtrace -G",
>> +			    objv[i]);
>> +	}
>> +
>>  	/*
>>  	 * If there are is-enabled probes then we need to force use of DOF
>> -	 * version 2.
>> +	 * version 3.
>>  	 */
>> -	if (eprobes && pgp->dp_dofversion < DOF_VERSION_2)
>> -		pgp->dp_dofversion = DOF_VERSION_2;
>> +	if (eprobes && pgp->dp_dofversion < DOF_VERSION_3)
>> +		pgp->dp_dofversion = DOF_VERSION_3;

This forces DOF_VERSION_3 for ie probes -- otherwise we get the default
DOF_VERSION_1, set by dt_program_create (where there is a comment
explaining this).

-- 
NULL && (void)