[Mesa-dev] [PATCH 07/16] radeonsi: add reference count to si_compute

[Marek Olšák]

On Mon, Aug 21, 2017 at 8:36 AM, Gert Wollny <gw.fossdev at gmail.com> wrote:
> Am Sonntag, den 20.08.2017, 15:48 +0200 schrieb Marek Olšák:
>> The patch seems OK to me.
>
> Hmm,
>
> you're right in that I was wrong about "program", I still need learn to
> think in patches.
>
> However, when I look at si_delete_compute_state where
> si_compute_reference is called:
>
>
>>+   si_compute_reference(&program, NULL);
>>+}
> ...
>>+static inline void
>>+si_compute_reference(struct si_compute **dst, struct si_compute *src)
>>+{
>>+     if (pipe_reference(&(*dst)->reference, &src->reference))
>>+       si_destroy_compute(*dst);
>
> then src will be a NULL pointer and de-referencing it is most likely
> already undefined behavior.
>
> Anyway, in the end the address of a member is taken (i.e.
> src+offsetof(src, reference), and passed as a pointer, and this pointer
>  is later de-referenced in
>
>   pipe_reference_described
>
> if it is not 0.
>
> For now this runs through because "reference" is the first element in
> the si_compute struct, and hence its offset is 0 and &src->reference
> will also be 0, but the moment someone decides to reorder si_compute
> this may no longer the case and the code would break.
>
> At the least it should be commented that the element reference within
> the structure si_compute must always be at offset 0.

You're right. All pipe_reference declarations should always be first.
And that's why they are always first. It's an unwritten rule in
Gallium.

Marek