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

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 <[email protected]>
> > > 
> > > 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 <[email protected]>
> > > > ---
> > > >    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;