[DTrace-devel] [PATCH] Relocation processing for the ERROR program is done too early

[Kris Van Hees]

When we call dtrace_go(), we do something like this:

    dt_bpf_make_progs()
        dt_program_construct()        // just for ERROR
            dt_link()
                dt_link_construct()
    dt_bpf_gmap_create()
    dt_bpf_load_progs()               // other
            dt_link()
                dt_link_construct()

In dt_link_construct() we dive down and find dt_get_bvar().  One of the
relocations is to supply the value of STBSZ.  The first dt_link() is for
ERROR, while the subsequent calls in dt_bpf_load_progs() are for other
clauses -- that is, two separate versions of dt_get_bvar() are used.
Meanwhile, the value of STBSZ is not set until dt_bpf_gmap_create().
This means that the ERROR copy of dt_get_bvar() does not have STBSZ set
properly. This means that if ERROR accesses probeprov or probename,
dt_get_bvar() returns the beginning of the string table, which is a NUL
terminator.  Some other relocation values have similar issues.

Move the ERROR program construction to dt_bpf_load_progs().

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
---
 libdtrace/dt_bpf.c                          | 40 +++++++++++----------
 test/unittest/builtinvar/tst.probe_dtrace.d | 34 ++++++++++++++++++
 test/unittest/builtinvar/tst.probe_dtrace.r |  6 ++++
 3 files changed, 61 insertions(+), 19 deletions(-)
 create mode 100644 test/unittest/builtinvar/tst.probe_dtrace.d
 create mode 100644 test/unittest/builtinvar/tst.probe_dtrace.r

diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
index 70597d65..df427c86 100644
--- a/libdtrace/dt_bpf.c
+++ b/libdtrace/dt_bpf.c
@@ -1212,30 +1212,14 @@ int
 dt_bpf_make_progs(dtrace_hdl_t *dtp, uint_t cflags)
 {
 	dt_probe_t	*prp;
-	dtrace_difo_t	*dp;
-	dt_ident_t	*idp = dt_dlib_get_func(dtp, "dt_error");
-
-	assert(idp != NULL);
-
-	/*
-	 * First construct the ERROR probe program (to be included in probe
-	 * programs that may trigger a fault).
-	 *
-	 * After constructing the program, we need to patch up any calls to
-	 * dt_error because DTrace cannot handle faults in ERROR itself.
-	 */
-	dp = dt_program_construct(dtp, dtp->dt_error, cflags, idp);
-	if (dp == NULL)
-		return -1;
-
-	idp->di_flags |= DT_IDFLG_CGREG;	/* mark it as inline-ready */
-	dt_bpf_reloc_error_prog(dtp, dp);
 
 	/*
 	 * Now construct all the other programs.
 	 */
 	for (prp = dt_list_next(&dtp->dt_enablings); prp != NULL;
 	     prp = dt_list_next(prp)) {
+		dtrace_difo_t	*dp;
+
 		/* Already done. */
 		if (prp == dtp->dt_error)
 			continue;
@@ -1263,7 +1247,25 @@ int
 dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
 {
 	dt_probe_t	*prp;
+	dtrace_difo_t	*dp;
 	dtrace_optval_t	dest_ok = DTRACEOPT_UNSET;
+	dt_ident_t	*idp = dt_dlib_get_func(dtp, "dt_error");
+
+	assert(idp != NULL);
+
+	/*
+	 * First construct the ERROR probe program (to be linked in probe
+	 * programs that may trigger a fault).
+	 *
+	 * After constructing the program, we need to patch up any calls to
+	 * dt_error because DTrace cannot handle faults in ERROR itself (yet).
+	 */
+	dp = dt_program_construct(dtp, dtp->dt_error, cflags, idp);
+	if (dp == NULL)
+		return -1;
+
+	idp->di_flags |= DT_IDFLG_CGREG;	/* mark it as inline-ready */
+	dt_bpf_reloc_error_prog(dtp, dp);
 
 	/*
 	 * Determine whether we can allow destructive actions.
@@ -1272,10 +1274,10 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
 
 	for (prp = dt_list_next(&dtp->dt_enablings); prp != NULL;
 	     prp = dt_list_next(prp)) {
-		dtrace_difo_t	*dp = prp->difo;
 		int		fd;
 		int		rc = -1;
 
+		dp = prp->difo;
 		if (dp == NULL)
 			continue;
 
diff --git a/test/unittest/builtinvar/tst.probe_dtrace.d b/test/unittest/builtinvar/tst.probe_dtrace.d
new file mode 100644
index 00000000..d08067ad
--- /dev/null
+++ b/test/unittest/builtinvar/tst.probe_dtrace.d
@@ -0,0 +1,34 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * Licensed under the Universal Permissive License v 1.0 as shown at
+ * http://oss.oracle.com/licenses/upl.
+ */
+
+/*
+ * ASSERTION:
+ * Built-in variables listing provider, module, function, and name
+ * are correct from dtrace-provider probes.
+ *
+ * SECTION: Variables/Built-in Variables
+ */
+
+#pragma D option quiet
+
+/* Check build-in variables from the dtrace-provider probes. */
+BEGIN,ERROR,END
+{
+	printf("%s:%s:%s:%s\n", probeprov, probemod, probefunc, probename);
+}
+
+/* Cause the ERROR probe to fire. */
+BEGIN
+{
+	*((int*)0);
+}
+
+/* Cause the END probe to fire. */
+BEGIN,ERROR
+{
+	exit(0);
+}
diff --git a/test/unittest/builtinvar/tst.probe_dtrace.r b/test/unittest/builtinvar/tst.probe_dtrace.r
new file mode 100644
index 00000000..e6cccc88
--- /dev/null
+++ b/test/unittest/builtinvar/tst.probe_dtrace.r
@@ -0,0 +1,6 @@
+dtrace:::BEGIN
+dtrace:::ERROR
+dtrace:::END
+
+-- @@stderr --
+dtrace: error in dt_clause_3 for probe ID 1 (dtrace:::BEGIN): invalid address ({ptr}) at BPF pc NNN
-- 
2.45.2