* [PATCH 3/4] cg: reject clauses using return() by default
@ 2025-07-15 5:48 Kris Van Hees
2025-07-15 10:37 ` Nick Alcock
0 siblings, 1 reply; 4+ messages in thread
From: Kris Van Hees @ 2025-07-15 5:48 UTC (permalink / raw)
To: dtrace, dtrace-devel
Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
---
libdtrace/dt_cg.c | 10 +++++++++-
libdtrace/dt_errtags.h | 1 +
libdtrace/dtrace.h | 19 ++++++++++---------
3 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 738597ed..0607cd4e 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -875,6 +875,10 @@ dt_cg_call_clause(dtrace_hdl_t *dtp, dtrace_stmtdesc_t *sdp, dt_clause_arg_t *ar
*/
if (prp->prov->impl->reject_clause != NULL)
prp->prov->impl->reject_clause(prp, sdp->dtsd_clauseflags);
+ else if (sdp->dtsd_clauseflags & DT_CLSFLAG_RETURN)
+ xyerror(D_ACT_RETURN, "return() not allowed for %s:%s:%s:%s\n",
+ prp->desc->prv, prp->desc->mod, prp->desc->fun,
+ prp->desc->prb);
/*
* if (*dctx.act != act) // ldw %r0, [%r9 + DCTX_ACT]
@@ -1810,9 +1814,13 @@ dt_cg_clsflags(dt_pcb_t *pcb, dtrace_actkind_t kind, const dt_node_t *dnp)
{
int *cfp = &pcb->pcb_stmt->dtsd_clauseflags;
- if (DTRACEACT_ISDESTRUCTIVE(kind))
+ if (DTRACEACT_ISDESTRUCTIVE(kind)) {
*cfp |= DT_CLSFLAG_DESTRUCT;
+ if (kind == DTRACEACT_RETURN)
+ *cfp |= DT_CLSFLAG_RETURN;
+ }
+
if (kind == DTRACEACT_COMMIT) {
if (*cfp & (DT_CLSFLAG_DATAREC | DT_CLSFLAG_AGGREGATION))
dnerror(dnp, D_COMM_DREC, "commit( ) may "
diff --git a/libdtrace/dt_errtags.h b/libdtrace/dt_errtags.h
index b8648d17..65c59cba 100644
--- a/libdtrace/dt_errtags.h
+++ b/libdtrace/dt_errtags.h
@@ -158,6 +158,7 @@ typedef enum {
D_AGG_NULL, /* aggregation stmt has null effect */
D_AGG_SCALAR, /* aggregating function needs scalar */
D_ACT_SPEC, /* destructive action after speculate */
+ D_ACT_RETURN, /* return() not allowed for ... */
D_EXIT_SPEC, /* exit() action after speculate */
D_DREC_COMM, /* data action after commit() */
D_PRINTA_PROTO, /* printa() prototype mismatch */
diff --git a/libdtrace/dtrace.h b/libdtrace/dtrace.h
index 20672d3f..f0c1b8dc 100644
--- a/libdtrace/dtrace.h
+++ b/libdtrace/dtrace.h
@@ -154,15 +154,16 @@ typedef struct dtrace_stmtdesc {
} dtrace_stmtdesc_t;
/* dtsd clause flags */
-#define DT_CLSFLAG_DATAREC 1 /* data-recording */
-#define DT_CLSFLAG_SPECULATE 2 /* speculate */
-#define DT_CLSFLAG_COMMIT 4 /* commit */
-#define DT_CLSFLAG_COMMIT_DISCARD 8 /* commit/discard */
-#define DT_CLSFLAG_EXIT 16 /* exit */
-#define DT_CLSFLAG_DESTRUCT 32 /* destructive */
-#define DT_CLSFLAG_AGGREGATION 64 /* aggregation */
-#define DT_CLSFLAG_USDT_INCLUDE 128 /* could be used in USDT clause */
-#define DT_CLSFLAG_USDT_EXCLUDE 256 /* could not be used in USDT clause */
+#define DT_CLSFLAG_DATAREC 0x0001 /* data-recording */
+#define DT_CLSFLAG_SPECULATE 0x0002 /* speculate */
+#define DT_CLSFLAG_COMMIT 0x0004 /* commit */
+#define DT_CLSFLAG_COMMIT_DISCARD 0x0008 /* commit/discard */
+#define DT_CLSFLAG_EXIT 0x0010 /* exit */
+#define DT_CLSFLAG_DESTRUCT 0x0020 /* destructive */
+#define DT_CLSFLAG_RETURN 0x0040 /* aggregation */
+#define DT_CLSFLAG_AGGREGATION 0x0080 /* return action */
+#define DT_CLSFLAG_USDT_INCLUDE 0x0100 /* could be used in USDT clause */
+#define DT_CLSFLAG_USDT_EXCLUDE 0x0200 /* could not be used in USDT clause */
typedef int dtrace_stmt_f(dtrace_hdl_t *dtp, dtrace_prog_t *pgp,
dtrace_stmtdesc_t *sdp, void *data);
--
2.43.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 3/4] cg: reject clauses using return() by default
2025-07-15 5:48 [PATCH 3/4] cg: reject clauses using return() by default Kris Van Hees
@ 2025-07-15 10:37 ` Nick Alcock
2025-07-15 10:51 ` Nick Alcock
2025-07-15 15:37 ` Kris Van Hees
0 siblings, 2 replies; 4+ messages in thread
From: Nick Alcock @ 2025-07-15 10:37 UTC (permalink / raw)
To: Kris Van Hees; +Cc: dtrace, dtrace-devel
On 15 Jul 2025, Kris Van Hees verbalised:
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
Reviewed-by: Nick Alcock <nick.alcock@oracle.com>
> ---
> libdtrace/dt_cg.c | 10 +++++++++-
> libdtrace/dt_errtags.h | 1 +
> libdtrace/dtrace.h | 19 ++++++++++---------
> 3 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 738597ed..0607cd4e 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -875,6 +875,10 @@ dt_cg_call_clause(dtrace_hdl_t *dtp, dtrace_stmtdesc_t *sdp, dt_clause_arg_t *ar
> */
> if (prp->prov->impl->reject_clause != NULL)
> prp->prov->impl->reject_clause(prp, sdp->dtsd_clauseflags);
> + else if (sdp->dtsd_clauseflags & DT_CLSFLAG_RETURN)
> + xyerror(D_ACT_RETURN, "return() not allowed for %s:%s:%s:%s\n",
> + prp->desc->prv, prp->desc->mod, prp->desc->fun,
> + prp->desc->prb);
Ah, I guess these are the "default checks" given in the previous commit,
so you can just set up the reject_clause() if destructive mode is on,
and not need to check here at all.
> diff --git a/libdtrace/dtrace.h b/libdtrace/dtrace.h
> index 20672d3f..f0c1b8dc 100644
> --- a/libdtrace/dtrace.h
> +++ b/libdtrace/dtrace.h
> @@ -154,15 +154,16 @@ typedef struct dtrace_stmtdesc {
> } dtrace_stmtdesc_t;
>
> /* dtsd clause flags */
> -#define DT_CLSFLAG_DATAREC 1 /* data-recording */
> -#define DT_CLSFLAG_SPECULATE 2 /* speculate */
> -#define DT_CLSFLAG_COMMIT 4 /* commit */
> -#define DT_CLSFLAG_COMMIT_DISCARD 8 /* commit/discard */
> -#define DT_CLSFLAG_EXIT 16 /* exit */
> -#define DT_CLSFLAG_DESTRUCT 32 /* destructive */
> -#define DT_CLSFLAG_AGGREGATION 64 /* aggregation */
> -#define DT_CLSFLAG_USDT_INCLUDE 128 /* could be used in USDT clause */
> -#define DT_CLSFLAG_USDT_EXCLUDE 256 /* could not be used in USDT clause */
> +#define DT_CLSFLAG_DATAREC 0x0001 /* data-recording */
> +#define DT_CLSFLAG_SPECULATE 0x0002 /* speculate */
> +#define DT_CLSFLAG_COMMIT 0x0004 /* commit */
> +#define DT_CLSFLAG_COMMIT_DISCARD 0x0008 /* commit/discard */
> +#define DT_CLSFLAG_EXIT 0x0010 /* exit */
> +#define DT_CLSFLAG_DESTRUCT 0x0020 /* destructive */
> +#define DT_CLSFLAG_RETURN 0x0040 /* aggregation */
> +#define DT_CLSFLAG_AGGREGATION 0x0080 /* return action */
> +#define DT_CLSFLAG_USDT_INCLUDE 0x0100 /* could be used in USDT clause */
> +#define DT_CLSFLAG_USDT_EXCLUDE 0x0200 /* could not be used in USDT clause */
Oh yes please! I mean we all know our powers of two anyway, but this is
just cleaner. (Not sure why you stuffed DT_CLSFLAG_RETURN in the middle
like that, though.)
--
NULL && (void)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 3/4] cg: reject clauses using return() by default
2025-07-15 10:37 ` Nick Alcock
@ 2025-07-15 10:51 ` Nick Alcock
2025-07-15 15:37 ` Kris Van Hees
1 sibling, 0 replies; 4+ messages in thread
From: Nick Alcock @ 2025-07-15 10:51 UTC (permalink / raw)
To: Kris Van Hees; +Cc: dtrace, dtrace-devel
On 15 Jul 2025, Nick Alcock spake thusly:
> On 15 Jul 2025, Kris Van Hees verbalised:
>
>> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
>
> Reviewed-by: Nick Alcock <nick.alcock@oracle.com>
>
>> ---
>> libdtrace/dt_cg.c | 10 +++++++++-
>> libdtrace/dt_errtags.h | 1 +
>> libdtrace/dtrace.h | 19 ++++++++++---------
>> 3 files changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
>> index 738597ed..0607cd4e 100644
>> --- a/libdtrace/dt_cg.c
>> +++ b/libdtrace/dt_cg.c
>> @@ -875,6 +875,10 @@ dt_cg_call_clause(dtrace_hdl_t *dtp, dtrace_stmtdesc_t *sdp, dt_clause_arg_t *ar
>> */
>> if (prp->prov->impl->reject_clause != NULL)
>> prp->prov->impl->reject_clause(prp, sdp->dtsd_clauseflags);
>> + else if (sdp->dtsd_clauseflags & DT_CLSFLAG_RETURN)
>> + xyerror(D_ACT_RETURN, "return() not allowed for %s:%s:%s:%s\n",
>> + prp->desc->prv, prp->desc->mod, prp->desc->fun,
>> + prp->desc->prb);
>
> Ah, I guess these are the "default checks" given in the previous commit,
> so you can just set up the reject_clause() if destructive mode is on,
> and not need to check here at all.
No, nonsense -- dt_cg_clsflags turns on the destructive flag whenever it
turns on the return flag. Which is fine. (This behaviour is still
somewhat irregular, but I guess if anything can have special handling in
dt_cg_call_clause it would be something named return :) )
--
NULL && (void)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 3/4] cg: reject clauses using return() by default
2025-07-15 10:37 ` Nick Alcock
2025-07-15 10:51 ` Nick Alcock
@ 2025-07-15 15:37 ` Kris Van Hees
1 sibling, 0 replies; 4+ messages in thread
From: Kris Van Hees @ 2025-07-15 15:37 UTC (permalink / raw)
To: Nick Alcock; +Cc: Kris Van Hees, dtrace, dtrace-devel
On Tue, Jul 15, 2025 at 11:37:39AM +0100, Nick Alcock wrote:
> On 15 Jul 2025, Kris Van Hees verbalised:
>
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
>
> Reviewed-by: Nick Alcock <nick.alcock@oracle.com>
>
> > ---
> > libdtrace/dt_cg.c | 10 +++++++++-
> > libdtrace/dt_errtags.h | 1 +
> > libdtrace/dtrace.h | 19 ++++++++++---------
> > 3 files changed, 20 insertions(+), 10 deletions(-)
> >
> > diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> > index 738597ed..0607cd4e 100644
> > --- a/libdtrace/dt_cg.c
> > +++ b/libdtrace/dt_cg.c
> > @@ -875,6 +875,10 @@ dt_cg_call_clause(dtrace_hdl_t *dtp, dtrace_stmtdesc_t *sdp, dt_clause_arg_t *ar
> > */
> > if (prp->prov->impl->reject_clause != NULL)
> > prp->prov->impl->reject_clause(prp, sdp->dtsd_clauseflags);
> > + else if (sdp->dtsd_clauseflags & DT_CLSFLAG_RETURN)
> > + xyerror(D_ACT_RETURN, "return() not allowed for %s:%s:%s:%s\n",
> > + prp->desc->prv, prp->desc->mod, prp->desc->fun,
> > + prp->desc->prb);
>
> Ah, I guess these are the "default checks" given in the previous commit,
> so you can just set up the reject_clause() if destructive mode is on,
> and not need to check here at all.
Yes, this is where the comment block should start talking about default check.
The check has to be specific to DT_CLSFLAG_RETURN because we do not want to
just reject a clause based on whetehr destructive mode is on or not. That is
done elsewhere (but in the future could be added here - as a future change to
the -w patch Eugene posted).
> > diff --git a/libdtrace/dtrace.h b/libdtrace/dtrace.h
> > index 20672d3f..f0c1b8dc 100644
> > --- a/libdtrace/dtrace.h
> > +++ b/libdtrace/dtrace.h
> > @@ -154,15 +154,16 @@ typedef struct dtrace_stmtdesc {
> > } dtrace_stmtdesc_t;
> >
> > /* dtsd clause flags */
> > -#define DT_CLSFLAG_DATAREC 1 /* data-recording */
> > -#define DT_CLSFLAG_SPECULATE 2 /* speculate */
> > -#define DT_CLSFLAG_COMMIT 4 /* commit */
> > -#define DT_CLSFLAG_COMMIT_DISCARD 8 /* commit/discard */
> > -#define DT_CLSFLAG_EXIT 16 /* exit */
> > -#define DT_CLSFLAG_DESTRUCT 32 /* destructive */
> > -#define DT_CLSFLAG_AGGREGATION 64 /* aggregation */
> > -#define DT_CLSFLAG_USDT_INCLUDE 128 /* could be used in USDT clause */
> > -#define DT_CLSFLAG_USDT_EXCLUDE 256 /* could not be used in USDT clause */
> > +#define DT_CLSFLAG_DATAREC 0x0001 /* data-recording */
> > +#define DT_CLSFLAG_SPECULATE 0x0002 /* speculate */
> > +#define DT_CLSFLAG_COMMIT 0x0004 /* commit */
> > +#define DT_CLSFLAG_COMMIT_DISCARD 0x0008 /* commit/discard */
> > +#define DT_CLSFLAG_EXIT 0x0010 /* exit */
> > +#define DT_CLSFLAG_DESTRUCT 0x0020 /* destructive */
> > +#define DT_CLSFLAG_RETURN 0x0040 /* aggregation */
> > +#define DT_CLSFLAG_AGGREGATION 0x0080 /* return action */
> > +#define DT_CLSFLAG_USDT_INCLUDE 0x0100 /* could be used in USDT clause */
> > +#define DT_CLSFLAG_USDT_EXCLUDE 0x0200 /* could not be used in USDT clause */
>
> Oh yes please! I mean we all know our powers of two anyway, but this is
> just cleaner. (Not sure why you stuffed DT_CLSFLAG_RETURN in the middle
> like that, though.)
It just seemed like the better place, so USDT_INCLUDE and USDT_EXCLUDE are
nicely at 0x0100 and 0x0200.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-07-15 15:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-15 5:48 [PATCH 3/4] cg: reject clauses using return() by default Kris Van Hees
2025-07-15 10:37 ` Nick Alcock
2025-07-15 10:51 ` Nick Alcock
2025-07-15 15:37 ` 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