[DTrace-devel] [PATCH v2 16/17] cg: support casts of pointers to integers

[Nick Alcock]

On 15 Mar 2022, Eugene Loh via DTrace-devel said:

> First of all, how about a quick return:
>         if (!dt_node_is_scalar(dst))
>                 return;
> Thereafter, no more dst-is-scalar checks.
>
> Also, there should be test cases.

There are. tst.alloca-alignment.d fails without this, because it does

        trace((uint64_t) b - (uint64_t) a);

which triggers verifier failures without this commit.

> E.g., consider:
> test1: trace((int)alloca(8)); printf("success\n"); exit(0)}
> test2: trace((unsigned long)alloca(8)); printf("success\n"); exit(0)}
>
> With Solaris I get:
> test1: -1965031400success
> test2: -103600976035808success
>
> With legacy DT on Linux I get:
> test1: -1001582568success
> test2: -111296403427296success
>
> With DT on BPF without this patch I get:
> test1: BPF: 198: (67) r7 <<= 32    BPF: R7 pointer arithmetic with <<= operator prohibited
> test2: 18446694915642859384success
>
> With DT on BPF with this patch I get:
> test1: BPF: 198: (67) r7 <<= 32    BPF: R7 pointer arithmetic with <<= operator prohibited
> test2: 18446694915643383672success
>
> As far as these tests go, the patch makes no difference.  So, I think you should add tests like these but you also need a test
> that's sensitive to what you are trying to do with this patch.

That looks like a different bug.

> And why does that BPF verifier problem occur even with your patch?  I
> think because the code tries type casting with the <<, which you
> cannot use on a pointer.  So maybe you have to switch the logic up
> some.  First check if the src is a pointer. (With the quick return, we
> already know the dst is a scalar.)  If src is a pointer, do the
> scalarize operation.  Then, if necessary, also do the shift left/right
> magic.

That would only help with the single case of shifting. What we probably
want to do is for arithmetic ops that are prohibited by the verifier
to imply a uintptr_t cast, maybe? But since we can hardly descalarize it
afterwards maybe we should make it a compile-time error to do crazy
things like this with pointers without casting first. It depends whether
we want D to implicitly cast pointers to integers for us or be strict
about it. It more or less has to be strict about not doing things like
this with pointers now: the only question is whether we turn things into
integers to help the user or whether we force the user to do it to make
the user be clearer about types.