* [PATCH v4] trace: print alloca pointers as actual pointer values
@ 2025-09-16 18:26 Kris Van Hees
2025-09-16 22:53 ` Eugene Loh
2025-09-17 15:03 ` Nick Alcock
0 siblings, 2 replies; 6+ messages in thread
From: Kris Van Hees @ 2025-09-16 18:26 UTC (permalink / raw)
To: dtrace, dtrace-devel
Because alloca pointers are stored internally as offsets 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 | 46 ++++++++++++++++------
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, 71 insertions(+), 11 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 7d542521..1f120e1d 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -1687,18 +1687,42 @@ 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.
+ * An alloca pointer value either has the ALLOCA flag set, or
+ * the value node is a DEREF of an alloca pointer child.
+ * 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 not, we should scalarize it so that the BPF verifier does
+ * not complain.
*/
- 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);
- dt_cg_alloca_ptr(dlp, drp, dnp->dn_reg, dnp->dn_reg);
- not_null = 1;
+ if ((dnp->dn_flags & DT_NF_ALLOCA) ||
+ (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))) {
+ 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;
+ } else {
+ int reg;
+
+ dt_regset_xalloc(drp, BPF_REG_0);
+ emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_FP, DT_STK_DCTX));
+ if ((reg = dt_regset_alloc(drp)) == -1)
+ longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
+ emit(dlp, BPF_LOAD(BPF_DW, reg, BPF_REG_0, DCTX_SCRATCHMEM));
+ emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_0, DCTX_MST));
+ emit(dlp, BPF_STORE(BPF_DW, BPF_REG_0, DMST_SCALARIZER, reg));
+ emit(dlp, BPF_LOAD(BPF_DW, reg, BPF_REG_0, DMST_SCALARIZER));
+ dt_regset_free(drp, BPF_REG_0);
+ emit(dlp, BPF_ALU64_REG(BPF_ADD, dnp->dn_reg, reg));
+ dt_regset_free(drp, reg);
+ }
}
}
diff --git a/test/unittest/actions/trace/tst.alloca.d b/test/unittest/actions/trace/tst.alloca.d
new file mode 100644
index 00000000..d2ff5152
--- /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 00000000..e9bbf2f5
--- /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 00000000..8515861a
--- /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.43.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v4] trace: print alloca pointers as actual pointer values
2025-09-16 18:26 [PATCH v4] trace: print alloca pointers as actual pointer values Kris Van Hees
@ 2025-09-16 22:53 ` Eugene Loh
2025-09-17 15:03 ` Nick Alcock
1 sibling, 0 replies; 6+ messages in thread
From: Eugene Loh @ 2025-09-16 22:53 UTC (permalink / raw)
To: Kris Van Hees, dtrace, dtrace-devel
Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
On 9/16/25 14:26, Kris Van Hees wrote:
> Because alloca pointers are stored internally as offsets 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 | 46 ++++++++++++++++------
> 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, 71 insertions(+), 11 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 7d542521..1f120e1d 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1687,18 +1687,42 @@ 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.
> + * An alloca pointer value either has the ALLOCA flag set, or
> + * the value node is a DEREF of an alloca pointer child.
> + * 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 not, we should scalarize it so that the BPF verifier does
> + * not complain.
> */
> - 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);
> - dt_cg_alloca_ptr(dlp, drp, dnp->dn_reg, dnp->dn_reg);
> - not_null = 1;
> + if ((dnp->dn_flags & DT_NF_ALLOCA) ||
> + (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))) {
> + 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;
> + } else {
> + int reg;
> +
> + dt_regset_xalloc(drp, BPF_REG_0);
> + emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_FP, DT_STK_DCTX));
> + if ((reg = dt_regset_alloc(drp)) == -1)
> + longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> + emit(dlp, BPF_LOAD(BPF_DW, reg, BPF_REG_0, DCTX_SCRATCHMEM));
> + emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_0, DCTX_MST));
> + emit(dlp, BPF_STORE(BPF_DW, BPF_REG_0, DMST_SCALARIZER, reg));
> + emit(dlp, BPF_LOAD(BPF_DW, reg, BPF_REG_0, DMST_SCALARIZER));
> + dt_regset_free(drp, BPF_REG_0);
> + emit(dlp, BPF_ALU64_REG(BPF_ADD, dnp->dn_reg, reg));
> + dt_regset_free(drp, reg);
> + }
> }
> }
>
> diff --git a/test/unittest/actions/trace/tst.alloca.d b/test/unittest/actions/trace/tst.alloca.d
> new file mode 100644
> index 00000000..d2ff5152
> --- /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 00000000..e9bbf2f5
> --- /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 00000000..8515861a
> --- /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;
> +}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v4] trace: print alloca pointers as actual pointer values
2025-09-16 18:26 [PATCH v4] trace: print alloca pointers as actual pointer values Kris Van Hees
2025-09-16 22:53 ` Eugene Loh
@ 2025-09-17 15:03 ` Nick Alcock
2025-09-17 15:29 ` Kris Van Hees
1 sibling, 1 reply; 6+ messages in thread
From: Nick Alcock @ 2025-09-17 15:03 UTC (permalink / raw)
To: Kris Van Hees; +Cc: dtrace, dtrace-devel
On 16 Sep 2025, Kris Van Hees verbalised:
> Because alloca pointers are stored internally as offsets 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>
So... the change is to generate real pointers for children of alloca
nodes too, and scalarize more aggressively? I don't really see what the
second part has to do with this change (and more specifically why it
didn't cause problems before now).
--
NULL && (void)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] trace: print alloca pointers as actual pointer values
2025-09-17 15:03 ` Nick Alcock
@ 2025-09-17 15:29 ` Kris Van Hees
2025-09-18 13:01 ` Nick Alcock
0 siblings, 1 reply; 6+ messages in thread
From: Kris Van Hees @ 2025-09-17 15:29 UTC (permalink / raw)
To: Nick Alcock; +Cc: Kris Van Hees, dtrace, dtrace-devel
On Wed, Sep 17, 2025 at 04:03:33PM +0100, Nick Alcock wrote:
> On 16 Sep 2025, Kris Van Hees verbalised:
>
> > Because alloca pointers are stored internally as offsets 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>
>
> So... the change is to generate real pointers for children of alloca
> nodes too, and scalarize more aggressively? I don't really see what the
> second part has to do with this change (and more specifically why it
> didn't cause problems before now).
It doesn't cause problems - it is just not what should happen. Since an alloca
pointer is a pointer into kernel space one would expect (and legacy behaviour
shows this) that printing it would give us an address value in kernel space.
Without this patchm we print the *offset* into the scratch memory area, which
is certainly not a valid pointer in kernel space.
Because of how code generation is done, it unfortunately requires slightly more
complex logic to know which values to convert into actual pointer values in
order to store them in the trace output buffer. An extra complication is that,
if we are storing the pointer value into the trace event buffer, some versions
of the verifier will complain if it is a value that is marked as an address
into a BPF map. That is why it needs to be scalarized.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] trace: print alloca pointers as actual pointer values
2025-09-17 15:29 ` Kris Van Hees
@ 2025-09-18 13:01 ` Nick Alcock
2025-09-18 14:45 ` Kris Van Hees
0 siblings, 1 reply; 6+ messages in thread
From: Nick Alcock @ 2025-09-18 13:01 UTC (permalink / raw)
To: Kris Van Hees; +Cc: Nick Alcock, dtrace, dtrace-devel
On 17 Sep 2025, Kris Van Hees spake thusly:
> On Wed, Sep 17, 2025 at 04:03:33PM +0100, Nick Alcock wrote:
>> On 16 Sep 2025, Kris Van Hees verbalised:
>>
>> > Because alloca pointers are stored internally as offsets 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>
>>
>> So... the change is to generate real pointers for children of alloca
>> nodes too, and scalarize more aggressively? I don't really see what the
>> second part has to do with this change (and more specifically why it
>> didn't cause problems before now).
>
> It doesn't cause problems - it is just not what should happen. Since an alloca
> pointer is a pointer into kernel space one would expect (and legacy behaviour
> shows this) that printing it would give us an address value in kernel space.
> Without this patchm we print the *offset* into the scratch memory area, which
> is certainly not a valid pointer in kernel space.
Ooh true!
> Because of how code generation is done, it unfortunately requires slightly more
> complex logic to know which values to convert into actual pointer values in
> order to store them in the trace output buffer. An extra complication is that,
> if we are storing the pointer value into the trace event buffer, some versions
> of the verifier will complain if it is a value that is marked as an address
> into a BPF map. That is why it needs to be scalarized.
Aha, this is saying "if this is not being dereferenced, but instead we
want its literal value" (e.g. for pointer printing) "scalarize it to
lose its mapdom". Presumably we know that the result of such a REF
operation will never be dereferenced, since after scalarization we can't
dereference it?
--
NULL && (void)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] trace: print alloca pointers as actual pointer values
2025-09-18 13:01 ` Nick Alcock
@ 2025-09-18 14:45 ` Kris Van Hees
0 siblings, 0 replies; 6+ messages in thread
From: Kris Van Hees @ 2025-09-18 14:45 UTC (permalink / raw)
To: Nick Alcock; +Cc: Kris Van Hees, dtrace, dtrace-devel
On Thu, Sep 18, 2025 at 02:01:48PM +0100, Nick Alcock wrote:
> On 17 Sep 2025, Kris Van Hees spake thusly:
>
> > On Wed, Sep 17, 2025 at 04:03:33PM +0100, Nick Alcock wrote:
> >> On 16 Sep 2025, Kris Van Hees verbalised:
> >>
> >> > Because alloca pointers are stored internally as offsets 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>
> >>
> >> So... the change is to generate real pointers for children of alloca
> >> nodes too, and scalarize more aggressively? I don't really see what the
> >> second part has to do with this change (and more specifically why it
> >> didn't cause problems before now).
> >
> > It doesn't cause problems - it is just not what should happen. Since an alloca
> > pointer is a pointer into kernel space one would expect (and legacy behaviour
> > shows this) that printing it would give us an address value in kernel space.
> > Without this patchm we print the *offset* into the scratch memory area, which
> > is certainly not a valid pointer in kernel space.
>
> Ooh true!
>
> > Because of how code generation is done, it unfortunately requires slightly more
> > complex logic to know which values to convert into actual pointer values in
> > order to store them in the trace output buffer. An extra complication is that,
> > if we are storing the pointer value into the trace event buffer, some versions
> > of the verifier will complain if it is a value that is marked as an address
> > into a BPF map. That is why it needs to be scalarized.
>
> Aha, this is saying "if this is not being dereferenced, but instead we
> want its literal value" (e.g. for pointer printing) "scalarize it to
> lose its mapdom". Presumably we know that the result of such a REF
> operation will never be dereferenced, since after scalarization we can't
> dereference it?
Yes.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-18 14:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-16 18:26 [PATCH v4] trace: print alloca pointers as actual pointer values Kris Van Hees
2025-09-16 22:53 ` Eugene Loh
2025-09-17 15:03 ` Nick Alcock
2025-09-17 15:29 ` Kris Van Hees
2025-09-18 13:01 ` Nick Alcock
2025-09-18 14:45 ` 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