[DTrace-devel] [PATCH 6/6] Add support for pid function "-" with absolute offset

[Eugene Loh]

On 1/9/25 14:12, Kris Van Hees wrote:

> On Fri, Dec 20, 2024 at 05:27:16PM -0500, eugene.loh--- via DTrace-devel wrote:
>> diff --git a/include/dtrace/pid.h b/include/dtrace/pid.h
>> @@ -47,6 +47,7 @@ typedef struct pid_probespec {
>>   	/*
>>   	 * Fields below this point do not apply to underlying probes.
>>   	 */
>> +	int pps_absoff;				/* use "-" for overlying probe function */
> Why use this?  All you do with it is record that func is "-", and then later
> in the pid provider, you actually use that to force the function name in the
> probe description to be "-".  But that function name in the probe desc is (in
> the pid provider) already coming from pps_fun, and the code was already
> assigning func to pps_fun, so it would be "-" anyway.  So the pps_absoff seems
> to serve no purpose, and you could just rely on pps_fun being "-" when needed.

Yeah, good point.  Let me see if I can remember/understand/explain 
what's going on here.

In essence, the problem is we have to pass two function names:  "-" (for 
the overlying probe) and the actual function name (for the underlying 
probe);  up until now, they were both the same.  I tossed around 
different ideas for this.  E.g., pass two function names in. Or, get the 
actual function name inside dt_prov_uprobe.  It seemed to make most 
sense (given the alternatives) to pass in the actual function name and 
add this flag to say when the overlying probe's function name should be 
"-".  Does that make sense?

Note that pps_fun is not "-".  Although we set it to func ("-"), the 
code that processes this case then fills pps_fun in with the actual 
function name.  Again, one option is not to do this until 
dt_prov_uprobe.c, but that introduces some other hassles.

>>   	pid_t pps_pid;				/* task PID */
>>   	uint64_t pps_nameoff;			/* offset to use for name */
>>   } pid_probespec_t;
>> diff --git a/libdtrace/dt_pid.c b/libdtrace/dt_pid.c
>> @@ -182,8 +181,50 @@ dt_pid_per_sym(dt_pid_probe_t *pp, const GElf_Sym *symp, const char *func)
>>   	psp->pps_fun = (char *) func;
>>   	psp->pps_nameoff = 0;
>>   	psp->pps_off = symp->st_value - pp->dpp_vaddr;
>> +	psp->pps_absoff = 0;
>>   
>> -	if (!isdash && gmatch("return", pp->dpp_name)) {
>> +	/*
>> +	 * The special function "-" means the probe name is an absolute
>> +	 * virtual address.
>> +	 */
>> +	if (strcmp("-", func) == 0) {
>> +		char *end;
>> +		GElf_Sym sym;
>> +
>> +		off = strtoull(pp->dpp_name, &end, 16);
>> +
>> +		psp->pps_absoff = 1;
>> +		psp->pps_nameoff = off;
>> +
>> +		if (dt_Plookup_by_addr(dtp, pid, off, (const char **)&psp->pps_fun, &sym)) {
>> +			rc = dt_pid_error(dtp, pcb, dpr, D_PROC_NAME,
>> +			     "failed to lookup 0x%lx in module '%s'", off, pp->dpp_mod);
>> +			if (psp->pps_fun != func && psp->pps_fun != NULL)
>> +				free(psp->pps_fun);
>> +			goto out;
>> +		}
>> +
>> +		psp->pps_prb = (char*)pp->dpp_name;  // make sure dt_prov_uprobe uses it
>> +		psp->pps_off = off - sym.st_value;   // make sure dt_prov_uprobe uses it // dump this
>> +		psp->pps_off = off - pp->dpp_vaddr;  // make sure dt_prov_uprobe uses it
>> +
>> +		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_OFFSETS) < 0)
>> +			rc = dt_pid_error(dtp, pcb, dpr, D_PROC_CREATEFAIL,
>> +			    "failed to create probes at '%s+0x%llx': %s",
>> +			    func, (unsigned long long)off, dtrace_errmsg(dtp, dtrace_errno(dtp)));
>> +		else
>> +			pp->dpp_nmatches++;
>> +		free(psp->pps_fun);
>> +		goto out;
>> +	}
>> +
>> +	if (gmatch("return", pp->dpp_name)) {
>>   		if (dt_pid_create_one_probe(pp->dpp_pr, dtp, psp, DTPPT_RETURN) < 0) {
>>   			rc = dt_pid_error(
>>   				dtp, pcb, dpr, D_PROC_CREATEFAIL,
>> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
>> index acf1c914f..ae4b262ac 100644
>> --- a/libdtrace/dt_prov_uprobe.c
>> +++ b/libdtrace/dt_prov_uprobe.c
>> @@ -735,7 +735,7 @@ static int provide_probe(dtrace_hdl_t *dtp, const pid_probespec_t *psp,
>>   	pd.id = DTRACE_IDNONE;
>>   	pd.prv = prv;
>>   	pd.mod = psp->pps_mod;
>> -	pd.fun = psp->pps_fun;
>> +	pd.fun = psp->pps_absoff ? "-" : psp->pps_fun;
>>   	pd.prb = prb;
>>   
>>   	/* Get (or create) the provider for the PID of the probe. */