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

[Eugene Loh]

On 1/9/25 15:52, Eugene Loh wrote:

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

Hmm.  Having said all that, maybe another (better?!) way of solving this 
problem is to add another probe type to pid_probetype_t.  Maybe 
DTPPT_ABSOFFSETS.  After all, we already have this problem of a 
"special" name for the overlying probe while requiring the "real" name 
for the underlying probe:  we have that problem for probe names.  There, 
we solve the problem by checking the pid_probetype_t.  So maybe that 
same approach should be used here, for probe function.

Let me know.  If you agree/prefer, I can revise the patch.

>
>>>       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. */