[PATCH] uprobe: do not call populate_args() for pid probes

[PATCH] uprobe: do not call populate_args() for pid probes

[Kris Van Hees]

Only USDT (regular and is-enabled) probes need populate_args().

Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
---
 libdtrace/dt_prov_uprobe.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
index c63d58383..605ae8397 100644
--- a/libdtrace/dt_prov_uprobe.c
+++ b/libdtrace/dt_prov_uprobe.c
@@ -994,8 +994,10 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
 		 */
 	}
 
-	if (populate_args(dtp, psp, upp) < 0)
-		goto fail;
+	if (upp->flags & (PP_IS_ENABLED | PP_IS_USDT)) {
+		if (populate_args(dtp, psp, upp) < 0)
+			goto fail;
+	}
 
 	return uprp;
 
-- 
2.45.2

Re: [DTrace-devel] [PATCH] uprobe: do not call populate_args() for pid probes

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh@oracle.com>

FWIW, with every patch I wonder:  "Are there any test issues?" (Like, 
should we test something here?  Were we?  Etc.)  I assume in this 
particular case, problems were revealed by patch "Fix various memory 
leaks related to stapsdt and usdt probes" on tests
     test/unittest/usdt/tst.pidargmap.sh
     test/unittest/usdt/tst.pidargs.sh
     test/unittest/usdt/tst.pidprobes.sh
The present patch is simply to remediate those revealed problems. To me, 
that sort of information is useful in a commit message.

On 8/13/25 23:58, Kris Van Hees via DTrace-devel wrote:
> Only USDT (regular and is-enabled) probes need populate_args().
>
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> ---
>   libdtrace/dt_prov_uprobe.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> index c63d58383..605ae8397 100644
> --- a/libdtrace/dt_prov_uprobe.c
> +++ b/libdtrace/dt_prov_uprobe.c
> @@ -994,8 +994,10 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
>   		 */
>   	}
>   
> -	if (populate_args(dtp, psp, upp) < 0)
> -		goto fail;
> +	if (upp->flags & (PP_IS_ENABLED | PP_IS_USDT)) {
> +		if (populate_args(dtp, psp, upp) < 0)
> +			goto fail;
> +	}
>   
>   	return uprp;
>   

Re: [DTrace-devel] [PATCH] uprobe: do not call populate_args() for pid probes

[Kris Van Hees]

On Thu, Aug 14, 2025 at 12:47:25PM -0400, Eugene Loh wrote:
> Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
> 
> FWIW, with every patch I wonder:  "Are there any test issues?" (Like, should
> we test something here?  Were we?  Etc.)  I assume in this particular case,
> problems were revealed by patch "Fix various memory leaks related to stapsdt
> and usdt probes" on tests
>     test/unittest/usdt/tst.pidargmap.sh
>     test/unittest/usdt/tst.pidargs.sh
>     test/unittest/usdt/tst.pidprobes.sh
> The present patch is simply to remediate those revealed problems. To me,
> that sort of information is useful in a commit message.

I can add that in the future, though personally I don't find that as valuable
because anyone would have run a test prior to this patch and after the patch,
would notice that anyway.

But if it makes it easier to assess a patch, etc then I can add that in the
future.

> On 8/13/25 23:58, Kris Van Hees via DTrace-devel wrote:
> > Only USDT (regular and is-enabled) probes need populate_args().
> > 
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> > ---
> >   libdtrace/dt_prov_uprobe.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> > index c63d58383..605ae8397 100644
> > --- a/libdtrace/dt_prov_uprobe.c
> > +++ b/libdtrace/dt_prov_uprobe.c
> > @@ -994,8 +994,10 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
> >   		 */
> >   	}
> > -	if (populate_args(dtp, psp, upp) < 0)
> > -		goto fail;
> > +	if (upp->flags & (PP_IS_ENABLED | PP_IS_USDT)) {
> > +		if (populate_args(dtp, psp, upp) < 0)
> > +			goto fail;
> > +	}
> >   	return uprp;

Re: [DTrace-devel] [PATCH] uprobe: do not call populate_args() for pid probes

[Eugene Loh]

On 8/14/25 15:27, Kris Van Hees wrote:

> On Thu, Aug 14, 2025 at 12:47:25PM -0400, Eugene Loh wrote:
>> Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
>>
>> FWIW, with every patch I wonder:  "Are there any test issues?" (Like, should
>> we test something here?  Were we?  Etc.)  I assume in this particular case,
>> problems were revealed by patch "Fix various memory leaks related to stapsdt
>> and usdt probes" on tests
>>      test/unittest/usdt/tst.pidargmap.sh
>>      test/unittest/usdt/tst.pidargs.sh
>>      test/unittest/usdt/tst.pidprobes.sh
>> The present patch is simply to remediate those revealed problems. To me,
>> that sort of information is useful in a commit message.
> I can add that in the future, though personally I don't find that as valuable
> because anyone would have run a test prior to this patch and after the patch,
> would notice that anyway.
>
> But if it makes it easier to assess a patch, etc then I can add that in the
> future.

Cool, yes.  A person might run the full test suite before and after, but 
they might not.  Anyhow, it does make it easier to review the patch.

But the reason for this email is that I seem to be getting
         test/unittest/usdt/tst.stapsdt-notes-args.sh: FAIL: expected 
results differ.
         test/unittest/usdt/tst.stapsdt-notes-isenabled.sh: FAIL: 
expected results differ.
         test/unittest/usdt/tst.stapsdt-notes-lib.sh: FAIL: expected 
results differ.
         test/unittest/usdt/tst.stapsdt-notes.sh: FAIL: expected results 
differ.
with this patch.

>> On 8/13/25 23:58, Kris Van Hees via DTrace-devel wrote:
>>> Only USDT (regular and is-enabled) probes need populate_args().
>>>
>>> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
>>> ---
>>>    libdtrace/dt_prov_uprobe.c | 6 ++++--
>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
>>> index c63d58383..605ae8397 100644
>>> --- a/libdtrace/dt_prov_uprobe.c
>>> +++ b/libdtrace/dt_prov_uprobe.c
>>> @@ -994,8 +994,10 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
>>>    		 */
>>>    	}
>>> -	if (populate_args(dtp, psp, upp) < 0)
>>> -		goto fail;
>>> +	if (upp->flags & (PP_IS_ENABLED | PP_IS_USDT)) {
>>> +		if (populate_args(dtp, psp, upp) < 0)
>>> +			goto fail;
>>> +	}
>>>    	return uprp;

Re: [DTrace-devel] [PATCH] uprobe: do not call populate_args() for pid probes

[Kris Van Hees]

On Fri, Aug 15, 2025 at 02:13:33AM -0400, Eugene Loh wrote:
> On 8/14/25 15:27, Kris Van Hees wrote:
> 
> > On Thu, Aug 14, 2025 at 12:47:25PM -0400, Eugene Loh wrote:
> > > Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
> > > 
> > > FWIW, with every patch I wonder:  "Are there any test issues?" (Like, should
> > > we test something here?  Were we?  Etc.)  I assume in this particular case,
> > > problems were revealed by patch "Fix various memory leaks related to stapsdt
> > > and usdt probes" on tests
> > >      test/unittest/usdt/tst.pidargmap.sh
> > >      test/unittest/usdt/tst.pidargs.sh
> > >      test/unittest/usdt/tst.pidprobes.sh
> > > The present patch is simply to remediate those revealed problems. To me,
> > > that sort of information is useful in a commit message.
> > I can add that in the future, though personally I don't find that as valuable
> > because anyone would have run a test prior to this patch and after the patch,
> > would notice that anyway.
> > 
> > But if it makes it easier to assess a patch, etc then I can add that in the
> > future.
> 
> Cool, yes.  A person might run the full test suite before and after, but
> they might not.  Anyhow, it does make it easier to review the patch.
> 
> But the reason for this email is that I seem to be getting
>         test/unittest/usdt/tst.stapsdt-notes-args.sh: FAIL: expected results
> differ.
>         test/unittest/usdt/tst.stapsdt-notes-isenabled.sh: FAIL: expected
> results differ.
>         test/unittest/usdt/tst.stapsdt-notes-lib.sh: FAIL: expected results
> differ.
>         test/unittest/usdt/tst.stapsdt-notes.sh: FAIL: expected results
> differ.
> with this patch.

Oops - not sure how that passed on my system when testing.  Either way,
posting fix in a sec.
> 
> > > On 8/13/25 23:58, Kris Van Hees via DTrace-devel wrote:
> > > > Only USDT (regular and is-enabled) probes need populate_args().
> > > > 
> > > > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> > > > ---
> > > >    libdtrace/dt_prov_uprobe.c | 6 ++++--
> > > >    1 file changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
> > > > index c63d58383..605ae8397 100644
> > > > --- a/libdtrace/dt_prov_uprobe.c
> > > > +++ b/libdtrace/dt_prov_uprobe.c
> > > > @@ -994,8 +994,10 @@ static dt_probe_t *create_underlying(dtrace_hdl_t *dtp,
> > > >    		 */
> > > >    	}
> > > > -	if (populate_args(dtp, psp, upp) < 0)
> > > > -		goto fail;
> > > > +	if (upp->flags & (PP_IS_ENABLED | PP_IS_USDT)) {
> > > > +		if (populate_args(dtp, psp, upp) < 0)
> > > > +			goto fail;
> > > > +	}
> > > >    	return uprp;