* [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values
@ 2025-01-28 6:31 Kris Van Hees
2025-01-30 22:33 ` Eugene Loh
0 siblings, 1 reply; 5+ messages in thread
From: Kris Van Hees @ 2025-01-28 6:31 UTC (permalink / raw)
To: dtrace, dtrace-devel
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@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:
--
2.45.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values
2025-01-28 6:31 [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values Kris Van Hees
@ 2025-01-30 22:33 ` Eugene Loh
2025-01-30 23:03 ` Kris Van Hees
0 siblings, 1 reply; 5+ messages in thread
From: Eugene Loh @ 2025-01-30 22:33 UTC (permalink / raw)
To: Kris Van Hees, dtrace, dtrace-devel
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?
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.
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.
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@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:
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values
2025-01-30 22:33 ` Eugene Loh
@ 2025-01-30 23:03 ` Kris Van Hees
2025-01-30 23:43 ` Eugene Loh
0 siblings, 1 reply; 5+ messages in thread
From: Kris Van Hees @ 2025-01-30 23:03 UTC (permalink / raw)
To: Eugene Loh; +Cc: Kris Van Hees, dtrace, dtrace-devel
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@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:
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values
2025-01-30 23:03 ` Kris Van Hees
@ 2025-01-30 23:43 ` Eugene Loh
2025-01-31 0:13 ` Kris Van Hees
0 siblings, 1 reply; 5+ messages in thread
From: Eugene Loh @ 2025-01-30 23:43 UTC (permalink / raw)
To: Kris Van Hees; +Cc: dtrace, dtrace-devel
On 1/30/25 18:03, Kris Van Hees wrote:
> 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.
From the user's point of view, the choice is between:
*) the status quo, where an attempt to use one of the unimplemented
fields returns a rather nice error message that indicates what the
problem is
*) the proposed patch, where we "silently" return incorrect values,
leaving the user with incorrect results or wasting time debugging the
problem, filing a bug, etc.
So, there is hardly a balance here at all for users: the status quo is
far better.
From our test suite's point of view, there is a choice between less
coverage (some tests XFAIL) and "more coverage", but I question that
characterization since the "more coverage" case still doesn't check the
correctness of the output. Anyhow, to the extent that this is a
test-suite issue, let's fix it in the test suite instead of polluting
user-visible behavior. Let's not "game the test suite" by modifying
dtrace. The whole set of tst.psinfo*.d tests are in need of overhaul;
so let's improve test coverage by cleaning up the tests rather than
feeding users incorrect results.
If you'd like me to take a stab at cleaning up tst.psinfo*.d, let me know.
>> 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.
FWIW, test/unittest/builtinvar/tst.psinfo.d would also XPASS with this
patch.
>> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values
2025-01-30 23:43 ` Eugene Loh
@ 2025-01-31 0:13 ` Kris Van Hees
0 siblings, 0 replies; 5+ messages in thread
From: Kris Van Hees @ 2025-01-31 0:13 UTC (permalink / raw)
To: Eugene Loh; +Cc: Kris Van Hees, dtrace, dtrace-devel
Good point - I'll withdraw the patch. We can discuss about the best way
to handle the test issues in a later patch (series).
On Thu, Jan 30, 2025 at 06:43:17PM -0500, Eugene Loh wrote:
> On 1/30/25 18:03, Kris Van Hees wrote:
>
> > 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.
>
> From the user's point of view, the choice is between:
>
> *) the status quo, where an attempt to use one of the unimplemented fields
> returns a rather nice error message that indicates what the problem is
>
> *) the proposed patch, where we "silently" return incorrect values, leaving
> the user with incorrect results or wasting time debugging the problem,
> filing a bug, etc.
>
> So, there is hardly a balance here at all for users: the status quo is far
> better.
>
> From our test suite's point of view, there is a choice between less coverage
> (some tests XFAIL) and "more coverage", but I question that characterization
> since the "more coverage" case still doesn't check the correctness of the
> output. Anyhow, to the extent that this is a test-suite issue, let's fix it
> in the test suite instead of polluting user-visible behavior. Let's not
> "game the test suite" by modifying dtrace. The whole set of tst.psinfo*.d
> tests are in need of overhaul; so let's improve test coverage by cleaning
> up the tests rather than feeding users incorrect results.
>
> If you'd like me to take a stab at cleaning up tst.psinfo*.d, let me know.
>
> > > 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.
>
> FWIW, test/unittest/builtinvar/tst.psinfo.d would also XPASS with this
> patch.
>
> > > 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-01-31 0:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-28 6:31 [PATCH 6/6] procfs: populate pr_argc, pr_argv, and pr_envp with default values Kris Van Hees
2025-01-30 22:33 ` Eugene Loh
2025-01-30 23:03 ` Kris Van Hees
2025-01-30 23:43 ` Eugene Loh
2025-01-31 0:13 ` Kris Van Hees
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox