[PATCH] trace: print alloca pointers as actual pointer values

[PATCH] trace: print alloca pointers as actual pointer values

[Kris Van Hees]

Because alloca pointers are stored internally as ofssets into the
scratchmem area, they were printed as small integers.  They are
now printed as actual pointer values into kernel space.

Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
---
 libdtrace/dt_cg.c                          | 19 +++++++++--------
 test/unittest/actions/trace/tst.alloca.d   | 24 ++++++++++++++++++++++
 test/unittest/actions/trace/tst.alloca.r   |  1 +
 test/unittest/actions/trace/tst.alloca.r.p | 11 ++++++++++
 4 files changed, 46 insertions(+), 9 deletions(-)
 create mode 100644 test/unittest/actions/trace/tst.alloca.d
 create mode 100644 test/unittest/actions/trace/tst.alloca.r
 create mode 100755 test/unittest/actions/trace/tst.alloca.r.p

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index cd9e7f4e9..7af3dd44b 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -1687,16 +1687,17 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
 		align = vtype.dtdt_align;
 
 		/*
-		 * A DEREF of a REF node does not get resolved in dt_cg_node()
-		 * because the ref node already holds the pointer.  But for
-		 * alloca pointers, that will be the offset into scratchmem so
-		 * we still need to turn it into a real pointer here.
+		 * Alloca pointers are stored as an offset into scratchmem, so
+		 * they need to be converted into real pointers before we go on.
+		 * If the alloca pointer is a REF or ref-by-value is requested,
+		 * we need to do bounds checking before turning the alloca
+		 * pointer into a real pointer.
 		 */
-		if (dnp->dn_kind == DT_NODE_OP1 &&
-		    dnp->dn_op == DT_TOK_DEREF && (dnp->dn_flags & DT_NF_REF) &&
-		    (dnp->dn_child->dn_flags & DT_NF_ALLOCA)) {
-			dt_cg_alloca_access_check(dlp, drp, dnp->dn_reg,
-						  DT_ISIMM, size);
+		if (dnp->dn_flags & DT_NF_ALLOCA) {
+			if ((dnp->dn_flags & DT_NF_REF) || (arg & DT_NF_REF))
+				dt_cg_alloca_access_check(dlp, drp, dnp->dn_reg,
+							  DT_ISIMM, size);
+
 			dt_cg_alloca_ptr(dlp, drp, dnp->dn_reg, dnp->dn_reg);
 			not_null = 1;
 		}
diff --git a/test/unittest/actions/trace/tst.alloca.d b/test/unittest/actions/trace/tst.alloca.d
new file mode 100644
index 000000000..d2ff5152d
--- /dev/null
+++ b/test/unittest/actions/trace/tst.alloca.d
@@ -0,0 +1,24 @@
+#pragma D option quiet
+
+BEGIN
+{
+	arr = (int *)alloca(5 * sizeof(int));
+	idx = 4;
+	arr[0] = 1;
+	arr[1] = 22;
+	arr[2] = 333;
+	arr[3] = 4444;
+	arr[4] = 55555;
+	trace(arr);
+	trace(" ");
+	trace(*arr);
+	trace(" ");
+	trace(arr + 2);
+	trace(" ");
+	trace(*(arr + 2));
+	trace(" ");
+	trace(arr + idx);
+	trace(" ");
+	trace(*(arr + idx));
+	exit(0);
+}
diff --git a/test/unittest/actions/trace/tst.alloca.r b/test/unittest/actions/trace/tst.alloca.r
new file mode 100644
index 000000000..e9bbf2f5d
--- /dev/null
+++ b/test/unittest/actions/trace/tst.alloca.r
@@ -0,0 +1 @@
+OK 1 OK 333 OK 55555
diff --git a/test/unittest/actions/trace/tst.alloca.r.p b/test/unittest/actions/trace/tst.alloca.r.p
new file mode 100755
index 000000000..8515861ad
--- /dev/null
+++ b/test/unittest/actions/trace/tst.alloca.r.p
@@ -0,0 +1,11 @@
+#!/usr/bin/gawk -f
+
+{
+	$1 = $1 > 0x7fffffff ? "OK" : "BAD";
+	$3 = $3 > 0x7fffffff ? "OK" : "BAD";
+	$5 = $5 > 0x7fffffff ? "OK" : "BAD";
+}
+
+{
+	print;
+}
-- 
2.45.2

Re: [PATCH] trace: print alloca pointers as actual pointer values

[Eugene Loh]

On 8/29/25 14:46, Kris Van Hees wrote:

> Because alloca pointers are stored internally as ofssets into the

s/ofssets/offsets/

For the test, how about converting the trace() to printf("%x")? Makes 
the script a little simpler and the hex output might feel more intuitive 
to a human reader if/when it ever comes to that.

Most of all, the test fails wherever I tried it.  I think the problem is 
that the BPF verifier doesn't like adding a map_value pointer and an 
unbounded value... even if we do not dereference the result!  So, in 
this patch, drop the DT_NF_REF checks, instead calling 
dt_cg_alloca_access_check() everytime.

I wrote a longer explanation of this.  Here it is, in case it's helpful.

The BPF verifier complains something like this:

     [...]
     r8 = *(u64 *)(r10 -8) R8_w=fp-88          r8 = dctx
     r8 = *(u64 *)(r8 +72) R8_w=map_value(...) r8 = dctx->gvar
     r8 = *(u64 *)(r8 + 0) R8_w=inv(id=0)      r8 = arr
     r7 = *(u64 *)(r10 -8) R7_w=fp-88          r7 = dctx
     r7 = *(u64 *)(r7 +40) R7_w=map_value(...) r7 = dctx->scratchmem
     r8 += r7              math between map_value pointer and register 
with unbounded min value is not allowed

So what has happened is that we assign r8=arr (first gvar), whose value 
is small since it's an alloca().  Then, we go to the new dt_cg.c code:

     if ((dnp->dn_flags & DT_NF_REF) || (arg & DT_NF_REF))
         dt_cg_alloca_access_check(dlp, drp, dnp->dn_reg, DT_ISIMM, size);
     dt_cg_alloca_ptr(dlp, drp, dnp->dn_reg, dnp->dn_reg);

We do NOT call the dt_cg_alloca_access_check(), but drop directly into 
dt_cg_alloca_ptr().  It allocates %r7 and does something like this:

     dt_cg_access_dctx(%r7, dlp, drp, DCTX_SCRATCHMEM);
     emit(dlp,  BPF_ALU64_REG(BPF_ADD, %r8, %r7));

Anyhow, that last add is forbidden, because %r7 is dctx->scratchmem and 
%r8 is unbounded.

I guess the problem is that the dt_cg_alloca_access_check() is not being 
called, and the BPF verifier doesn't like that, even though we are not 
dereferencing the pointer.

Maybe drop the &DT_NF_REF conditions in the new code introduced by the 
patch?

> scratchmem area, they were printed as small integers.  They are
> now printed as actual pointer values into kernel space.
>
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> ---
>   libdtrace/dt_cg.c                          | 19 +++++++++--------
>   test/unittest/actions/trace/tst.alloca.d   | 24 ++++++++++++++++++++++
>   test/unittest/actions/trace/tst.alloca.r   |  1 +
>   test/unittest/actions/trace/tst.alloca.r.p | 11 ++++++++++
>   4 files changed, 46 insertions(+), 9 deletions(-)
>   create mode 100644 test/unittest/actions/trace/tst.alloca.d
>   create mode 100644 test/unittest/actions/trace/tst.alloca.r
>   create mode 100755 test/unittest/actions/trace/tst.alloca.r.p
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index cd9e7f4e9..7af3dd44b 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1687,16 +1687,17 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
>   		align = vtype.dtdt_align;
>   
>   		/*
> -		 * A DEREF of a REF node does not get resolved in dt_cg_node()
> -		 * because the ref node already holds the pointer.  But for
> -		 * alloca pointers, that will be the offset into scratchmem so
> -		 * we still need to turn it into a real pointer here.
> +		 * Alloca pointers are stored as an offset into scratchmem, so
> +		 * they need to be converted into real pointers before we go on.
> +		 * If the alloca pointer is a REF or ref-by-value is requested,
> +		 * we need to do bounds checking before turning the alloca
> +		 * pointer into a real pointer.
>   		 */
> -		if (dnp->dn_kind == DT_NODE_OP1 &&
> -		    dnp->dn_op == DT_TOK_DEREF && (dnp->dn_flags & DT_NF_REF) &&
> -		    (dnp->dn_child->dn_flags & DT_NF_ALLOCA)) {
> -			dt_cg_alloca_access_check(dlp, drp, dnp->dn_reg,
> -						  DT_ISIMM, size);
> +		if (dnp->dn_flags & DT_NF_ALLOCA) {
> +			if ((dnp->dn_flags & DT_NF_REF) || (arg & DT_NF_REF))
> +				dt_cg_alloca_access_check(dlp, drp, dnp->dn_reg,
> +							  DT_ISIMM, size);
> +
>   			dt_cg_alloca_ptr(dlp, drp, dnp->dn_reg, dnp->dn_reg);
>   			not_null = 1;
>   		}
> diff --git a/test/unittest/actions/trace/tst.alloca.d b/test/unittest/actions/trace/tst.alloca.d
> new file mode 100644
> index 000000000..d2ff5152d
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.alloca.d
> @@ -0,0 +1,24 @@
> +#pragma D option quiet
> +
> +BEGIN
> +{
> +	arr = (int *)alloca(5 * sizeof(int));
> +	idx = 4;
> +	arr[0] = 1;
> +	arr[1] = 22;
> +	arr[2] = 333;
> +	arr[3] = 4444;
> +	arr[4] = 55555;
> +	trace(arr);
> +	trace(" ");
> +	trace(*arr);
> +	trace(" ");
> +	trace(arr + 2);
> +	trace(" ");
> +	trace(*(arr + 2));
> +	trace(" ");
> +	trace(arr + idx);
> +	trace(" ");
> +	trace(*(arr + idx));
> +	exit(0);
> +}
> diff --git a/test/unittest/actions/trace/tst.alloca.r b/test/unittest/actions/trace/tst.alloca.r
> new file mode 100644
> index 000000000..e9bbf2f5d
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.alloca.r
> @@ -0,0 +1 @@
> +OK 1 OK 333 OK 55555
> diff --git a/test/unittest/actions/trace/tst.alloca.r.p b/test/unittest/actions/trace/tst.alloca.r.p
> new file mode 100755
> index 000000000..8515861ad
> --- /dev/null
> +++ b/test/unittest/actions/trace/tst.alloca.r.p
> @@ -0,0 +1,11 @@
> +#!/usr/bin/gawk -f
> +
> +{
> +	$1 = $1 > 0x7fffffff ? "OK" : "BAD";
> +	$3 = $3 > 0x7fffffff ? "OK" : "BAD";
> +	$5 = $5 > 0x7fffffff ? "OK" : "BAD";
> +}
> +
> +{
> +	print;
> +}

Re: [PATCH] trace: print alloca pointers as actual pointer values

[Kris Van Hees]

On Fri, Aug 29, 2025 at 11:20:47PM -0400, Eugene Loh wrote:
> On 8/29/25 14:46, Kris Van Hees wrote:
> 
> > Because alloca pointers are stored internally as ofssets into the
> 
> s/ofssets/offsets/
> 
> For the test, how about converting the trace() to printf("%x")? Makes the
> script a little simpler and the hex output might feel more intuitive to a
> human reader if/when it ever comes to that.

The reason for using trace is that it is the most basic action that calls
dt_cg_store_val().  That is what we are exercising here.

> Most of all, the test fails wherever I tried it.  I think the problem is
> that the BPF verifier doesn't like adding a map_value pointer and an
> unbounded value... even if we do not dereference the result!  So, in this
> patch, drop the DT_NF_REF checks, instead calling
> dt_cg_alloca_access_check() everytime.

Ah, tehe joy of fixing one problem only to uncover another.  Dropping the
DT_NF_REF checks breaks other tests.  I'll have to see what needs to be done
to ensure that all tests pass correctly with a variant of this patch.

> I wrote a longer explanation of this.  Here it is, in case it's helpful.
> 
> The BPF verifier complains something like this:
> 
>     [...]
>     r8 = *(u64 *)(r10 -8) R8_w=fp-88          r8 = dctx
>     r8 = *(u64 *)(r8 +72) R8_w=map_value(...) r8 = dctx->gvar
>     r8 = *(u64 *)(r8 + 0) R8_w=inv(id=0)      r8 = arr
>     r7 = *(u64 *)(r10 -8) R7_w=fp-88          r7 = dctx
>     r7 = *(u64 *)(r7 +40) R7_w=map_value(...) r7 = dctx->scratchmem
>     r8 += r7              math between map_value pointer and register with
> unbounded min value is not allowed
> 
> So what has happened is that we assign r8=arr (first gvar), whose value is
> small since it's an alloca().  Then, we go to the new dt_cg.c code:
> 
>     if ((dnp->dn_flags & DT_NF_REF) || (arg & DT_NF_REF))
>         dt_cg_alloca_access_check(dlp, drp, dnp->dn_reg, DT_ISIMM, size);
>     dt_cg_alloca_ptr(dlp, drp, dnp->dn_reg, dnp->dn_reg);
> 
> We do NOT call the dt_cg_alloca_access_check(), but drop directly into
> dt_cg_alloca_ptr().  It allocates %r7 and does something like this:
> 
>     dt_cg_access_dctx(%r7, dlp, drp, DCTX_SCRATCHMEM);
>     emit(dlp,  BPF_ALU64_REG(BPF_ADD, %r8, %r7));
> 
> Anyhow, that last add is forbidden, because %r7 is dctx->scratchmem and %r8
> is unbounded.
> 
> I guess the problem is that the dt_cg_alloca_access_check() is not being
> called, and the BPF verifier doesn't like that, even though we are not
> dereferencing the pointer.
> 
> Maybe drop the &DT_NF_REF conditions in the new code introduced by the
> patch?

I think that is more likely that we need to scalarize the alloca pointer if
it not a REF, so that the arithmetic will be allowed (the verifier will no
longer know it is a map pointer).

> > scratchmem area, they were printed as small integers.  They are
> > now printed as actual pointer values into kernel space.
> > 
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> > ---
> >   libdtrace/dt_cg.c                          | 19 +++++++++--------
> >   test/unittest/actions/trace/tst.alloca.d   | 24 ++++++++++++++++++++++
> >   test/unittest/actions/trace/tst.alloca.r   |  1 +
> >   test/unittest/actions/trace/tst.alloca.r.p | 11 ++++++++++
> >   4 files changed, 46 insertions(+), 9 deletions(-)
> >   create mode 100644 test/unittest/actions/trace/tst.alloca.d
> >   create mode 100644 test/unittest/actions/trace/tst.alloca.r
> >   create mode 100755 test/unittest/actions/trace/tst.alloca.r.p
> > 
> > diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> > index cd9e7f4e9..7af3dd44b 100644
> > --- a/libdtrace/dt_cg.c
> > +++ b/libdtrace/dt_cg.c
> > @@ -1687,16 +1687,17 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
> >   		align = vtype.dtdt_align;
> >   		/*
> > -		 * A DEREF of a REF node does not get resolved in dt_cg_node()
> > -		 * because the ref node already holds the pointer.  But for
> > -		 * alloca pointers, that will be the offset into scratchmem so
> > -		 * we still need to turn it into a real pointer here.
> > +		 * Alloca pointers are stored as an offset into scratchmem, so
> > +		 * they need to be converted into real pointers before we go on.
> > +		 * If the alloca pointer is a REF or ref-by-value is requested,
> > +		 * we need to do bounds checking before turning the alloca
> > +		 * pointer into a real pointer.
> >   		 */
> > -		if (dnp->dn_kind == DT_NODE_OP1 &&
> > -		    dnp->dn_op == DT_TOK_DEREF && (dnp->dn_flags & DT_NF_REF) &&
> > -		    (dnp->dn_child->dn_flags & DT_NF_ALLOCA)) {
> > -			dt_cg_alloca_access_check(dlp, drp, dnp->dn_reg,
> > -						  DT_ISIMM, size);
> > +		if (dnp->dn_flags & DT_NF_ALLOCA) {
> > +			if ((dnp->dn_flags & DT_NF_REF) || (arg & DT_NF_REF))
> > +				dt_cg_alloca_access_check(dlp, drp, dnp->dn_reg,
> > +							  DT_ISIMM, size);
> > +
> >   			dt_cg_alloca_ptr(dlp, drp, dnp->dn_reg, dnp->dn_reg);
> >   			not_null = 1;
> >   		}
> > diff --git a/test/unittest/actions/trace/tst.alloca.d b/test/unittest/actions/trace/tst.alloca.d
> > new file mode 100644
> > index 000000000..d2ff5152d
> > --- /dev/null
> > +++ b/test/unittest/actions/trace/tst.alloca.d
> > @@ -0,0 +1,24 @@
> > +#pragma D option quiet
> > +
> > +BEGIN
> > +{
> > +	arr = (int *)alloca(5 * sizeof(int));
> > +	idx = 4;
> > +	arr[0] = 1;
> > +	arr[1] = 22;
> > +	arr[2] = 333;
> > +	arr[3] = 4444;
> > +	arr[4] = 55555;
> > +	trace(arr);
> > +	trace(" ");
> > +	trace(*arr);
> > +	trace(" ");
> > +	trace(arr + 2);
> > +	trace(" ");
> > +	trace(*(arr + 2));
> > +	trace(" ");
> > +	trace(arr + idx);
> > +	trace(" ");
> > +	trace(*(arr + idx));
> > +	exit(0);
> > +}
> > diff --git a/test/unittest/actions/trace/tst.alloca.r b/test/unittest/actions/trace/tst.alloca.r
> > new file mode 100644
> > index 000000000..e9bbf2f5d
> > --- /dev/null
> > +++ b/test/unittest/actions/trace/tst.alloca.r
> > @@ -0,0 +1 @@
> > +OK 1 OK 333 OK 55555
> > diff --git a/test/unittest/actions/trace/tst.alloca.r.p b/test/unittest/actions/trace/tst.alloca.r.p
> > new file mode 100755
> > index 000000000..8515861ad
> > --- /dev/null
> > +++ b/test/unittest/actions/trace/tst.alloca.r.p
> > @@ -0,0 +1,11 @@
> > +#!/usr/bin/gawk -f
> > +
> > +{
> > +	$1 = $1 > 0x7fffffff ? "OK" : "BAD";
> > +	$3 = $3 > 0x7fffffff ? "OK" : "BAD";
> > +	$5 = $5 > 0x7fffffff ? "OK" : "BAD";
> > +}
> > +
> > +{
> > +	print;
> > +}