[Mesa-dev] [PATCH] draw: Ensure that prepare is always run after LLVM garbagge collection.

[Stéphane Marchesin]

2012/1/27 Jose Fonseca <jfonseca at vmware.com>:
>
>
> ----- Original Message -----
>> On Fri, Jan 27, 2012 at 07:28, Jose Fonseca <jfonseca at vmware.com>
>> wrote:
>> >
>> >
>> > ----- Original Message -----
>> >> On Fri, Jan 27, 2012 at 8:18 AM, Brian Paul
>> >> <brian.e.paul at gmail.com>
>> >> wrote:
>> >> > On Fri, Jan 27, 2012 at 7:40 AM,  <jfonseca at vmware.com> wrote:
>> >> >> From: José Fonseca <jfonseca at vmware.com>
>> >> >>
>> >> >> Should avoid dangling pointer derreference with
>> >> >>
>> >> >>  glean --run results --overwrite --quick --tests texSwizzle
>> >> >> ---
>> >> >>  src/gallium/auxiliary/draw/draw_llvm.c |    5 +++++
>> >> >>  1 files changed, 5 insertions(+), 0 deletions(-)
>> >> >>
>> >> >> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c
>> >> >> b/src/gallium/auxiliary/draw/draw_llvm.c
>> >> >> index e71c802..b81c725 100644
>> >> >> --- a/src/gallium/auxiliary/draw/draw_llvm.c
>> >> >> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
>> >> >> @@ -65,8 +65,13 @@ static void
>> >> >>  draw_llvm_garbage_collect_callback(void *cb_data)
>> >> >>  {
>> >> >>    struct draw_llvm *llvm = (struct draw_llvm *) cb_data;
>> >> >> +   struct draw_context *draw = llvm->draw;
>> >> >>    struct draw_llvm_variant_list_item *li;
>> >> >>
>> >> >> +   /* Ensure prepare will be run and shaders recompiled */
>> >> >> +   assert(!draw->suspend_flushing);
>> >> >> +   draw_do_flush(draw, DRAW_FLUSH_STATE_CHANGE);
>> >> >> +
>> >> >>    /* free all shader variants */
>> >> >>    li = first_elem(&llvm->vs_variants_list);
>> >> >>    while (!at_end(&llvm->vs_variants_list, li)) {
>> >> >
>> >> > Looks good to me.
>> >
>> > Thanks.
>> >
>> >> Should probably be tagged for the 8.0 branch too.
>> >
>> > Good point.
>> >
>> > Jose
>>
>> You also might want to remove the workaround in free_gallivm_state.
>> My
>> impression is that it works around that bug.
>>
>
> All those commented code in free_gallivm_state are workarounds for globals in LLVM which can never be safely destroyed. These workarounds need to be revisited for newer LLVM version, but I believe they are independent of the issue we saw here.  The fact that draw module crashed is because the underlying LLVM structures were properly destroyed.
>

Ok, it sounded to me like it might have worked around that bug,
because free_gallivm_state was the first one reusing the variant's
memory after it got freed in the garbage collection. But if you say
it's a coincidence then let's push the fix :)

Stéphane