[Mesa-dev] [PATCH v3 08/12] nir/from_ssa: Don't try to read an invalid instruction

[Connor Abbott]

That seems fine to me.

On Mon, Feb 16, 2015 at 4:23 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
>
> On Mon, Feb 16, 2015 at 1:03 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>>
>> On Mon, Feb 16, 2015 at 3:57 PM, Connor Abbott <cwabbott0 at gmail.com>
>> wrote:
>> > Now that we talked about this on the last version of this patch, this
>> > makes more sense. But I'd like to see something in the commit message
>> > explaining what the issue was (going too far and casting the exec_node
>> > embedded in the block to a nir_instr) and how you put the patch that
>> > changes the behavior of nir_instr_prev() to actually fix this next in
>> > order to avoid segfaults in the middle of the series.
>>
>> Whoops, I forgot... once you do that, you get my r-b for this and the
>> next patch.
>
>
> How about:
>
> Right now, the nir_instr_prev function function blindly looks up the
> previous element in the exec list and casts it to an instruction even if
> it's the tail sentinel.  The next commit will change this to return null if
> it's the first instruction.  Making this change first avoids getting a
> segfault between commits.  The only reason we never noticed is that, thanks
> to the way things are laid out in nir_block, the casted instruction's type
> was never parallal_copy.
>
>>
>> >
>> > On Mon, Feb 9, 2015 at 11:24 PM, Jason Ekstrand <jason at jlekstrand.net>
>> > wrote:
>> >> ---
>> >>  src/glsl/nir/nir_from_ssa.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/src/glsl/nir/nir_from_ssa.c b/src/glsl/nir/nir_from_ssa.c
>> >> index 3625237..7c50095 100644
>> >> --- a/src/glsl/nir/nir_from_ssa.c
>> >> +++ b/src/glsl/nir/nir_from_ssa.c
>> >> @@ -272,7 +272,7 @@ get_parallel_copy_at_end_of_block(nir_block *block)
>> >>     if (last_instr->type == nir_instr_type_jump)
>> >>        last_instr = nir_instr_prev(last_instr);
>> >>
>> >> -   if (last_instr->type == nir_instr_type_parallel_copy)
>> >> +   if (last_instr && last_instr->type == nir_instr_type_parallel_copy)
>> >>        return nir_instr_as_parallel_copy(last_instr);
>> >>     else
>> >>        return NULL;
>> >> --
>> >> 2.2.2
>> >>
>> >> _______________________________________________
>> >> mesa-dev mailing list
>> >> mesa-dev at lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>