[DTrace-devel] [PATCH 02/14] Replace not-NULL test with more general runtime test

[Nick Alcock]

On 2 May 2023, eugene loh outgrape:

> From: Eugene Loh <eugene.loh at oracle.com>
>
> When we dereference a pointer, it should generally be safe to
> dereference D (including alloca) pointers, even if a more robust
> mechanism is needed to guard against dereferencing unsafe pointers
> in the more general case.  We do, however, check whether a pointer
> is NULL.
>
> There are times, however, when even a D pointer cannot safely be
> dereferenced.  This will especially be true once NULL string pointers
> will be supported.

Err, why not soup up the not-null check to handle null strings, then?
(And what other cases are there?)

> So drop the NULL-pointer check and implement dereferencing with a
> more general scalar load.

Reviewed-by: Nick Alcock <nick.alcock at oracle.com>

with one question below (but with my reviewed-by because the answer is
almost certainly "of course it's fine, you fool" in which case this
commit seems like a good idea).

The historical background here is that in the past alloca pointers had a
unique nonzero null representation which needed special checking. They
don't any more, so a great many of these notnull checks can probably be
generalized.

> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_cg.c                   | 6 +-----
>  test/unittest/pointers/tst.basic2.d | 3 +--
>  2 files changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 483603ef..c1fd46c0 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -5884,7 +5884,6 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  			uint_t	op;
>  			ssize_t	size;
>  
> -			dt_cg_check_notnull(dlp, drp, dnp->dn_reg);
>  			op = dt_cg_ldsize(dnp, ctfp, dnp->dn_type, &size);
>  
>  			/*
> @@ -5899,10 +5898,7 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  						 dnp->dn_reg);
>  			}
>  
> -			if (dnp->dn_child->dn_flags & (DT_NF_ALLOCA | DT_NF_DPTR))
> -				emit(dlp, BPF_LOAD(op, dnp->dn_reg, dnp->dn_reg, 0));
> -			else
> -				dt_cg_load_scalar(dnp, op, size, dlp, drp);
> +			dt_cg_load_scalar(dnp, op, size, dlp, drp);

This does turn a simple load asm instruction into a helper call for
things for which a load instruction should always work (if non-null!).
Do we mind? Is this always even possible?

-- 
NULL && (void)