[DTrace-devel] [PATCH 03/14] Let USDT module names contain dots

[Eugene Loh]

On 6/4/24 16:42, Kris Van Hees wrote:

> I do not know what to do with this patch.  Either it is correct and then the
> code should not be #if 0 .. #endif, or it is not, and then the patch should
> not exist.

I was looking for feedback.  If we're good with the patch, the "#if 0 / 
#endif" stuff will be removed in the final version of the patch.

> Consider that the tst.dlclose1.sh test has output like this:
> test/unittest/usdt/tst.dlclose1.sh: Running timeout --signal=TERM 41 test/unittest/usdt/tst.dlclose1.sh /home/kvanhees/dtrace-bpf-user/build/dtrace
> PASS.
> started pid 68990
>     ID   PROVIDER            MODULE                          FUNCTION NAME
> 97026 test_prov68990        livelib.so                                go go
>
> So that shows that the module component certain can contain a period (.),
> so the module component can certainly contain a period (.)

Right.

>   whereas we know
> that the function component cannot.  Likewise, provider name cannot contain
> period as far as I know, nor can the probe name.  So, only the module name
> seems to be an issue.

There are basically two distinct cases here.

In the dof_stash.c case, I note that we are checking a string of the 
form "%s:%s:%s:%s" to see if it is "." or "..".  That simply does not 
make sense.  I think the indicated code should be removed.

In the dt_pid.c case, we check whether any of the probe components has a 
".".  As you point out, probemod may certainly have one.  But why are we 
enforcing this constraint for the other elements?  E.g., if the function 
component cannot have a ".", why are we enforcing that in dt_pid.c?  I 
assume the issue is broader than pid/usdt.

In short, the eventual point of the patch is to eliminate the two 
indicated sections of code.  In retrospect, the "#if 0 / #endif" stuff 
was a dumb and confusing way of indicating that.  Sorry.

> On Tue, Jun 04, 2024 at 02:11:02PM -0400, eugene.loh--- via DTrace-devel wrote:
>> From: Eugene Loh <eugene.loh at oracle.com>
>>
>> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
>> ---
>>   dtprobed/dof_stash.c | 5 +++++
>>   libdtrace/dt_pid.c   | 2 ++
>>   2 files changed, 7 insertions(+)
>>
>> diff --git a/dtprobed/dof_stash.c b/dtprobed/dof_stash.c
>> index 62418b66..44c67462 100644
>> --- a/dtprobed/dof_stash.c
>> +++ b/dtprobed/dof_stash.c
>> @@ -589,6 +589,10 @@ dof_stash_write_parsed(pid_t pid, dev_t dev, ino_t ino, dt_list_t *accum)
>>   							    mod, fun, prb)) == NULL)
>>   				goto err_provider;
>>   
>> +#if 0
>> +This does not make any sense.  We are checking parsedfn against "." and "..",
>> +but parsedfn comes from make_probespec_name(), whose output is of the form
>> +"%s:%s:%s:%s" and therefore can never be "." or "..".
>>   			/*
>>   			 * Ban "." and ".." as name components.  Obviously names
>>   			 * containing dots are commonplace (shared libraries,
>> @@ -604,6 +608,7 @@ dof_stash_write_parsed(pid_t pid, dev_t dev, ino_t ino, dt_list_t *accum)
>>   			if (strcmp(parsedfn, ".") == 0 ||
>>   			    strcmp(parsedfn, "..") == 0)
>>   				goto err_provider;
>> +#endif
>>   
>>   			op = "probe module";
>>   
>> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
>> index 7c7d7e30..93a7ce76 100644
>> --- a/libdtrace/dt_pid.c
>> +++ b/libdtrace/dt_pid.c
>> @@ -833,6 +833,7 @@ dt_pid_create_usdt_probes(dtrace_hdl_t *dtp, dt_proc_t *dpr, dtrace_probedesc_t
>>   
>>   	assert(pvp->impl != NULL && pvp->impl->provide_probe != NULL);
>>   
>> +#if 0
>>   	if (strchr(pdp->prv, '.') != NULL ||
>>   	    strchr(pdp->mod, '.') != NULL ||
>>   	    strchr(pdp->fun, '.') != NULL ||
>> @@ -840,6 +841,7 @@ dt_pid_create_usdt_probes(dtrace_hdl_t *dtp, dt_proc_t *dpr, dtrace_probedesc_t
>>   		dt_dprintf("Probe component contains dots: cannot be a USDT probe.\n");
>>   		return 0;
>>   	}
>> +#endif
>>   
>>   	if (asprintf(&probepath, "%s/probes/%i/%s/%s/%s/%s", dtp->dt_dofstash_path,
>>   		     dpr->dpr_pid, pdp->prv[0] == '\0' ? "*" : pdp->prv,
>> -- 
>> 2.18.4
>>
>>
>> _______________________________________________
>> DTrace-devel mailing list
>> DTrace-devel at oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/dtrace-devel