[DTrace-devel] [PATCH v3] Ensure curcpu values are scalarized when typecast

[Nick Alcock]

On 3 Aug 2022, Kris Van Hees spake thusly:

> On Wed, Aug 03, 2022 at 06:11:16PM +0100, Nick Alcock wrote:
>> On 3 Aug 2022, Kris Van Hees via DTrace-devel verbalised:
>> 
>> > -		if (val == NULL)
>> > -			return (uint64_t)NULL;	/* FIXME */
>> > +		if (val == NULL) {
>> > +			/*
>> > +			 * Typically, we would use 'return error(...);' but
>> > +			 * that confuses the verifier because it returns -1.
>> > +			 * So, instead, we explicitly return 0.
>> > +			 */
>> > +			error(dctx, DTRACEFLT_ILLOP, 0);
>> > +			return 0;
>> > +		}
>> 
>> Presumably 0 is in some way more special than -1? :)
>
> Absolutely.  Because we test for NULL pointers in the right places and thus
> the verifier is happy with that.

Oh of course it's a null pointer if it's 0.

>> I'd be a little concerned about propagation of this flag. Do we know it
>> propagates everywhere it needs to? What if you assign this thing to a
>> variable, what if you use it in a ternary, does it in general need to be
>> treated like alloca taint is?
>> 
>> (No matter what, you need to add it to the relevant places in
>> dt_node_printr: but it might be worth hunting for references to
>> DT_TOK_ALLOCA in dt_parser and seeing how many of these, if any, need to
>> have something similar done to them.)
>
> DPTR was already introduced in an earlier patch and propagates through the
> nodes.  So this is all already there.

Yeh, I misread: this is an idflg, so propagation is not a concern.

Only the printr for this new idflg is missing. -- oh wait no maybe you
don't need that, since unlike say DT_IDFLG_ALLOCA the flag cannot land
on variables, only ever on one function node (since nothing ever sets it
on anything else).

So... I guess it's OK:

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

-- 
NULL && (void)