[DTrace-devel] [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values

Kris Van Hees kris.van.hees at oracle.com
Thu Jan 30 23:03:46 UTC 2025


On Thu, Jan 30, 2025 at 05:33:45PM -0500, Eugene Loh wrote:
> My main question is how it is better to provide incorrect results than it is
> not to have implemented this functionality?  How about just waiting until we
> implement this functionality?  Isn't it better that a user is told that we
> haven't implemented this stuff rather than give them results that are
> incorrect?

That is always a balance.  Ideally, we should have tests for each independent
member of each translator, so that we can do more fine grained testing.  But
it is not sensible that we do not end up testing any translator members because
some are not implemented, whereas providing a default value for them (which we
e.g. also do for members in the io translators).  That is the precedent I am
depending on here.

> It is odd that XFAIL is being lifted when we are producing incorrect output,
> but I suppose that is the burden of some other test/s.  So, okay.

Yes, we need more fine grained tests.

> Meanwhile, XFAIL is being lifted for
> test/unittest/builtinvar/tst.psinfo-bug21974606.d
> even though that change should probably actually appear in an earlier patch,
> "procfs: implement d_execargs() for pr_psargs translator support", where it
> starts to XPASS since it depends only on ps_args and not on any of the
> fields handled in the current patch.

Hm, yes, forgot to move that one.  Will do.
> 
> On 1/28/25 01:31, Kris Van Hees wrote:
> > The pr_argc, pr_argv, and pr_envp fields in psinfo are not implemented
> > yet, so it makes sense to set them to 0 rather than not providing any
> > translator for them.
> > 
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> >   dlibs/aarch64/5.11/procfs.d                       | 8 +++-----
> >   dlibs/aarch64/5.12/procfs.d                       | 8 +++-----
> >   dlibs/aarch64/5.14/procfs.d                       | 8 +++-----
> >   dlibs/aarch64/5.16/procfs.d                       | 8 +++-----
> >   dlibs/aarch64/5.2/procfs.d                        | 8 +++-----
> >   dlibs/aarch64/5.6/procfs.d                        | 8 +++-----
> >   dlibs/aarch64/6.1/procfs.d                        | 8 +++-----
> >   dlibs/aarch64/6.10/procfs.d                       | 8 +++-----
> >   dlibs/x86_64/5.11/procfs.d                        | 8 +++-----
> >   dlibs/x86_64/5.12/procfs.d                        | 8 +++-----
> >   dlibs/x86_64/5.14/procfs.d                        | 8 +++-----
> >   dlibs/x86_64/5.16/procfs.d                        | 8 +++-----
> >   dlibs/x86_64/5.2/procfs.d                         | 8 +++-----
> >   dlibs/x86_64/5.6/procfs.d                         | 8 +++-----
> >   dlibs/x86_64/6.1/procfs.d                         | 8 +++-----
> >   dlibs/x86_64/6.10/procfs.d                        | 8 +++-----
> >   libdtrace/procfs.d.in                             | 8 +++-----
> >   test/unittest/builtinvar/tst.psinfo-bug21974606.d | 1 -
> >   test/unittest/builtinvar/tst.psinfo-bug22561297.d | 4 +---
> >   test/unittest/builtinvar/tst.psinfo1.d            | 1 -
> >   20 files changed, 52 insertions(+), 90 deletions(-)
> > 
> > diff --git a/dlibs/aarch64/5.11/procfs.d b/dlibs/aarch64/5.11/procfs.d
> > index 9c06fe1f..52b2bbe2 100644
> > --- a/dlibs/aarch64/5.11/procfs.d
> > +++ b/dlibs/aarch64/5.11/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/5.12/procfs.d b/dlibs/aarch64/5.12/procfs.d
> > index 9c06fe1f..52b2bbe2 100644
> > --- a/dlibs/aarch64/5.12/procfs.d
> > +++ b/dlibs/aarch64/5.12/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/5.14/procfs.d b/dlibs/aarch64/5.14/procfs.d
> > index 2824d137..8c05e299 100644
> > --- a/dlibs/aarch64/5.14/procfs.d
> > +++ b/dlibs/aarch64/5.14/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/5.16/procfs.d b/dlibs/aarch64/5.16/procfs.d
> > index daf30745..e52ab29a 100644
> > --- a/dlibs/aarch64/5.16/procfs.d
> > +++ b/dlibs/aarch64/5.16/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/5.2/procfs.d b/dlibs/aarch64/5.2/procfs.d
> > index 3594e5e9..4a95dfd1 100644
> > --- a/dlibs/aarch64/5.2/procfs.d
> > +++ b/dlibs/aarch64/5.2/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/5.6/procfs.d b/dlibs/aarch64/5.6/procfs.d
> > index 9c06fe1f..52b2bbe2 100644
> > --- a/dlibs/aarch64/5.6/procfs.d
> > +++ b/dlibs/aarch64/5.6/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/6.1/procfs.d b/dlibs/aarch64/6.1/procfs.d
> > index 2d52f079..4881aa5b 100644
> > --- a/dlibs/aarch64/6.1/procfs.d
> > +++ b/dlibs/aarch64/6.1/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/aarch64/6.10/procfs.d b/dlibs/aarch64/6.10/procfs.d
> > index 2d52f079..4881aa5b 100644
> > --- a/dlibs/aarch64/6.10/procfs.d
> > +++ b/dlibs/aarch64/6.10/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.11/procfs.d b/dlibs/x86_64/5.11/procfs.d
> > index 7679db2e..96e55dc1 100644
> > --- a/dlibs/x86_64/5.11/procfs.d
> > +++ b/dlibs/x86_64/5.11/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.12/procfs.d b/dlibs/x86_64/5.12/procfs.d
> > index 7679db2e..96e55dc1 100644
> > --- a/dlibs/x86_64/5.12/procfs.d
> > +++ b/dlibs/x86_64/5.12/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.14/procfs.d b/dlibs/x86_64/5.14/procfs.d
> > index 3a348ebc..8dbf3c01 100644
> > --- a/dlibs/x86_64/5.14/procfs.d
> > +++ b/dlibs/x86_64/5.14/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.16/procfs.d b/dlibs/x86_64/5.16/procfs.d
> > index daf30745..e52ab29a 100644
> > --- a/dlibs/x86_64/5.16/procfs.d
> > +++ b/dlibs/x86_64/5.16/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.2/procfs.d b/dlibs/x86_64/5.2/procfs.d
> > index 6ad926ee..23e05c2c 100644
> > --- a/dlibs/x86_64/5.2/procfs.d
> > +++ b/dlibs/x86_64/5.2/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/5.6/procfs.d b/dlibs/x86_64/5.6/procfs.d
> > index 7679db2e..96e55dc1 100644
> > --- a/dlibs/x86_64/5.6/procfs.d
> > +++ b/dlibs/x86_64/5.6/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/6.1/procfs.d b/dlibs/x86_64/6.1/procfs.d
> > index 2d52f079..4881aa5b 100644
> > --- a/dlibs/x86_64/6.1/procfs.d
> > +++ b/dlibs/x86_64/6.1/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/dlibs/x86_64/6.10/procfs.d b/dlibs/x86_64/6.10/procfs.d
> > index 2d52f079..4881aa5b 100644
> > --- a/dlibs/x86_64/6.10/procfs.d
> > +++ b/dlibs/x86_64/6.10/procfs.d
> > @@ -143,11 +143,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* Not implemented yet. */
> > +	pr_argv = 0;			/* Not implemented yet. */
> > +	pr_envp = 0;			/* Not implemented yet. */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/libdtrace/procfs.d.in b/libdtrace/procfs.d.in
> > index 827d6b81..d4433be4 100644
> > --- a/libdtrace/procfs.d.in
> > +++ b/libdtrace/procfs.d.in
> > @@ -181,11 +181,9 @@ translator psinfo_t < struct task_struct *T > {
> >   	pr_fname = T->comm;
> >   	pr_psargs = d_execargs(T);
> >   	pr_wstat = 0;
> > -/*
> > -	pr_argc = get_psinfo(T)->__psinfo(argc);
> > -	pr_argv = (uintptr_t)get_psinfo(T)->__psinfo(argv);
> > -	pr_envp = (uintptr_t)get_psinfo(T)->__psinfo(envp);
> > - */
> > +	pr_argc = 0;			/* not implemented yet */
> > +	pr_argv = 0;			/* not implemented yet */
> > +	pr_envp = 0;			/* not implemented yet */
> >   	pr_dmodel = PR_MODEL_LP64;
> > diff --git a/test/unittest/builtinvar/tst.psinfo-bug21974606.d b/test/unittest/builtinvar/tst.psinfo-bug21974606.d
> > index 68d9503d..03857e83 100644
> > --- a/test/unittest/builtinvar/tst.psinfo-bug21974606.d
> > +++ b/test/unittest/builtinvar/tst.psinfo-bug21974606.d
> > @@ -4,7 +4,6 @@
> >    * Licensed under the Universal Permissive License v 1.0 as shown at
> >    * http://oss.oracle.com/licenses/upl.
> >    */
> > -/* @@xfail: dtv2 */
> >   /*
> >    * ASSERTION:
> > diff --git a/test/unittest/builtinvar/tst.psinfo-bug22561297.d b/test/unittest/builtinvar/tst.psinfo-bug22561297.d
> > index b9efd0ec..ffccf469 100644
> > --- a/test/unittest/builtinvar/tst.psinfo-bug22561297.d
> > +++ b/test/unittest/builtinvar/tst.psinfo-bug22561297.d
> > @@ -4,11 +4,9 @@
> >    * Licensed under the Universal Permissive License v 1.0 as shown at
> >    * http://oss.oracle.com/licenses/upl.
> >    */
> > -/* @@xfail: dtv2 */
> >   /*
> > - * ASSERTION:
> > - * To print psinfo structure values from profile.
> > + * ASSERTION: To print psinfo structure values from profile.
> >    *
> >    * SECTION: Variables/Built-in Variables
> >    */
> > diff --git a/test/unittest/builtinvar/tst.psinfo1.d b/test/unittest/builtinvar/tst.psinfo1.d
> > index 9e6d5053..7c451598 100644
> > --- a/test/unittest/builtinvar/tst.psinfo1.d
> > +++ b/test/unittest/builtinvar/tst.psinfo1.d
> > @@ -4,7 +4,6 @@
> >    * Licensed under the Universal Permissive License v 1.0 as shown at
> >    * http://oss.oracle.com/licenses/upl.
> >    */
> > -/* @@xfail: dtv2 */
> >   /*
> >    * ASSERTION:



More information about the DTrace-devel mailing list