[DTrace-devel] [PATCH] Unify NULL pointer checking in dt_cg_check_notnull()
Eugene Loh
eugene.loh at oracle.com
Fri Jun 18 11:26:32 PDT 2021
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
What will be the ordering of patches? If this goes in first, I can
tweak dt_cg_act_[u]stack() to use it. If those patches go in first,
this patch should include those two functions.
What about DIVZERO in dt_cg_arithmetic_op()? How about extending the
new function to allow for use with DIVZERO?
On 6/18/21 2:07 PM, Kris Van Hees wrote:
> We need to generate code in multiple places to validate that a pointer
> value is not NULL. This patch introduces the dt_cg_check_notnull()
> function to do just that, Existing checks are replaced by a call to
> this function.
>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
> libdtrace/dt_cg.c | 40 +++++++++++++++++-----------------------
> 1 file changed, 17 insertions(+), 23 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 3d705a11..22f97404 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -612,6 +612,21 @@ dt_cg_probe_error(dt_pcb_t *pcb, uint32_t off, uint32_t fault, uint64_t illval)
> emit(dlp, BPF_RETURN());
> }
>
> +/*
> + * Generate code to validate that the value in the given register 'reg' is not
> + * the NULL pointer.
> + */
> +static void
> +dt_cg_check_notnull(dt_irlist_t *dlp, dt_regset_t *drp, int reg)
> +{
> + uint_t lbl_notnull = dt_irlist_label(dlp);
> +
> + emit(dlp, BPF_BRANCH_IMM(BPF_JNE, reg, 0, lbl_notnull));
> + dt_cg_probe_error(yypcb, -1, DTRACEFLT_BADADDR, 0);
> + emitl(dlp, lbl_notnull,
> + BPF_NOP());
> +}
> +
> /*
> * Check whether mst->fault indicates a fault was triggered. If so, abort the
> * current clause by means of a straight jump to the exit labal.
> @@ -3191,7 +3206,6 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>
> if (!(dnp->dn_flags & DT_NF_REF)) {
> uint_t ubit;
> - uint_t lbl_valid = dt_irlist_label(dlp);
>
> /*
> * Save and restore DT_NF_USERLAND across dt_cg_load():
> @@ -3202,12 +3216,7 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> dnp->dn_flags |=
> (dnp->dn_child->dn_flags & DT_NF_USERLAND);
>
> - /* if NULL pointer, report BADARR */
> - emit(dlp, BPF_BRANCH_IMM(BPF_JNE, dnp->dn_reg, 0,
> - lbl_valid));
> - dt_cg_probe_error(yypcb, -1, DTRACEFLT_BADADDR, 0);
> - emitl(dlp, lbl_valid,
> - BPF_NOP());
> + dt_cg_check_notnull(dlp, drp, dnp->dn_reg);
>
> /* FIXME: Does not handled signed or userland */
> emit(dlp, BPF_LOAD(dt_cg_load(dnp, ctfp, dnp->dn_type), dnp->dn_reg, dnp->dn_reg, 0));
> @@ -3293,25 +3302,10 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>
> case DT_TOK_PTR:
> case DT_TOK_DOT: {
> - uint_t lbl_valid = dt_irlist_label(dlp);
>
> assert(dnp->dn_right->dn_kind == DT_NODE_IDENT);
> dt_cg_node(dnp->dn_left, dlp, drp);
> -
> - /*
> - * If the lvalue is the NULL pointer, we must report a BADARR
> - * fault.
> - *
> - * if (left != 0) // jne %lreg, 0, valid
> - * goto valid;
> - * // (report BADADDR fault)
> - * valid:
> - */
> - emit(dlp, BPF_BRANCH_IMM(BPF_JNE, dnp->dn_left->dn_reg, 0,
> - lbl_valid));
> - dt_cg_probe_error(yypcb, -1, DTRACEFLT_BADADDR, 0);
> - emitl(dlp, lbl_valid,
> - BPF_NOP());
> + dt_cg_check_notnull(dlp, drp, dnp->dn_left->dn_reg);
>
> /*
> * If the left-hand side of PTR or DOT is a dynamic variable,
More information about the DTrace-devel
mailing list