Ok so I'm thinking of adding a refcount to the variant to know if they are bound, and not garbage collect them in that case. Let me know what you think.
Stéphane 2012/1/26 Stéphane Marchesin <[email protected]>: > So actually it's a case of a use-after free. The variant is freed with > draw_llvm_destroy_variant and then reused through > llvm_pipeline_generic after free_gallium_state (and llvm) reused the > memory for something else. What prevents a variant bound to an fpme > from being freed by the garbage collection? > > Stéphane > > > 2012/1/26 Stéphane Marchesin <[email protected]>: >> I just took a look at it in gdb. Basically the jit_func pointer is >> corrupted by the free_gallivm_state function (in lp_bld_init.c). There >> is a comment to that effect already. It seems like the bug was always >> there but hidden because we regenerated state more than we had to. >> I'll keep digging... >> >> Stéphane >> >> >> 2012/1/26 Stéphane Marchesin <[email protected]>: >>> Hmm, I'll take a look later today. >>> >>> Stéphane >>> >>> 2012/1/26 Jose Fonseca <[email protected]>: >>>> Stephane, >>>> >>>> This commit caused a segmentation fault on glean texSwizzle test + >>>> llvmpipe: >>>> >>>> $ gdb --args glean --run results --overwrite --quick --tests texSwizzle >>>> (gdb) r >>>> Starting program: glean --run results --overwrite --quick --tests >>>> texSwizzle >>>> [Thread debugging using libthread_db enabled] >>>> >>>> Program received signal SIGSEGV, Segmentation fault. >>>> 0xfffffffffffffffc in ?? () >>>> (gdb) bt >>>> #0 0xfffffffffffffffc in ?? () >>>> #1 0x00007ffff6a26438 in llvm_pipeline_generic (middle=0x76e4a0, >>>> fetch_info=0x7fffffffd730, prim_info=0x7fffffffd700) >>>> at src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c:240 >>>> #2 0x00007ffff6a266fe in llvm_middle_end_linear_run (middle=0x76e4a0, >>>> start=0, count=4, prim_flags=0) >>>> at src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c:358 >>>> #3 0x00007ffff697bf23 in vsplit_segment_simple_linear (vsplit=0x76b670, >>>> flags=0, istart=0, icount=4) at >>>> src/gallium/auxiliary/draw/draw_pt_vsplit_tmp.h:237 >>>> #4 0x00007ffff697c228 in vsplit_run_linear (frontend=0x76b670, start=0, >>>> count=4) at src/gallium/auxiliary/draw/draw_split_tmp.h:61 >>>> #5 0x00007ffff697224e in draw_pt_arrays (draw=0x762510, prim=6, start=0, >>>> count=4) at src/gallium/auxiliary/draw/draw_pt.c:142 >>>> #6 0x00007ffff6972eb1 in draw_vbo (draw=0x762510, info=0x7fffffffd910) at >>>> src/gallium/auxiliary/draw/draw_pt.c:534 >>>> #7 0x00007ffff6689f67 in llvmpipe_draw_vbo (pipe=0x72fa10, >>>> info=0x7fffffffd910) at src/gallium/drivers/llvmpipe/lp_draw_arrays.c:85 >>>> #8 0x00007ffff68037f4 in st_draw_vbo (ctx=0x7c4b30, arrays=0x831c88, >>>> prims=0x7fffffffd9e0, nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', >>>> min_index=0, >>>> max_index=3, tfb_vertcount=0x0) at src/mesa/state_tracker/st_draw.c:1113 >>>> #9 0x00007ffff689d811 in vbo_draw_arrays (ctx=0x7c4b30, mode=6, start=0, >>>> count=4, numInstances=1) at src/mesa/vbo/vbo_exec_array.c:635 >>>> #10 0x00007ffff689d950 in vbo_exec_DrawArrays (mode=6, start=0, count=4) >>>> at src/mesa/vbo/vbo_exec_array.c:667 >>>> #11 0x0000000000458205 in GLEAN::TexSwizzleTest::TestSwizzles >>>> (this=0x6f10e0) at >>>> /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/ttexswizzle.cpp:293 >>>> #12 0x0000000000458558 in GLEAN::TexSwizzleTest::runOne (this=0x6f10e0, >>>> r=..., w=<optimized out>) >>>> at >>>> /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/ttexswizzle.cpp:387 >>>> #13 0x0000000000458ec7 in GLEAN::BaseTest<GLEAN::TexSwizzleResult>::run >>>> (this=0x6f10e0, environment=<optimized out>) >>>> at /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/tbase.h:317 >>>> #14 0x00000000004610b8 in main (argc=7, argv=0x7fffffffdec8) at >>>> /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/main.cpp:140 >>>> (gdb) >>>> >>>> Can you look into it? >>>> >>>> Jose >>>> >>>> >>>> ----- Original Message ----- >>>>> Module: Mesa >>>>> Branch: master >>>>> Commit: b6d3a435a0e0e53a9e8cc4c4249dc7c2f897a83d >>>>> URL: >>>>> >>>>> http://cgit.freedesktop.org/mesa/mesa/commit/?id=b6d3a435a0e0e53a9e8cc4c4249dc7c2f897a83d >>>>> >>>>> Author: Jakob Bornecrantz <[email protected]> >>>>> Date: Mon Jan 24 02:11:59 2011 +0100 >>>>> >>>>> draw: Only run prepare when state, prim and opt changes >>>>> >>>>> In bad applications like ipers which does a lot of draw calls with >>>>> no state changes this helps to greatly reduce time spent in prepare. >>>>> In ipers around 7% of CPU was spent in various prepare functions, >>>>> after this commit no prepare function show on the profile. >>>>> >>>>> This commit also has the added benefit of now grouping all pipelined >>>>> drawing into a single draw call if the driver uses vbuf_render. >>>>> >>>>> Reviewed-by: Stéphane Marchesin <[email protected]> >>>>> Tested-by: Stéphane Marchesin <[email protected]> >>>>> >>>>> --- >>>>> >>>>> src/gallium/auxiliary/draw/draw_context.c | 6 +++ >>>>> src/gallium/auxiliary/draw/draw_private.h | 8 ++++ >>>>> src/gallium/auxiliary/draw/draw_pt.c | 49 >>>>> ++++++++++++++++++++++++--- >>>>> src/gallium/auxiliary/draw/draw_pt.h | 2 +- >>>>> src/gallium/auxiliary/draw/draw_pt_vsplit.c | 11 ++++-- >>>>> 5 files changed, 66 insertions(+), 10 deletions(-) >>>>> >>>>> diff --git a/src/gallium/auxiliary/draw/draw_context.c >>>>> b/src/gallium/auxiliary/draw/draw_context.c >>>>> index 4ce4445..3c0b1aa 100644 >>>>> --- a/src/gallium/auxiliary/draw/draw_context.c >>>>> +++ b/src/gallium/auxiliary/draw/draw_context.c >>>>> @@ -355,6 +355,10 @@ draw_set_vertex_elements(struct draw_context >>>>> *draw, >>>>> { >>>>> assert(count <= PIPE_MAX_ATTRIBS); >>>>> >>>>> + /* We could improve this by only flushing the frontend and the >>>>> fetch part >>>>> + * of the middle. This would avoid recalculating the emit keys.*/ >>>>> + draw_do_flush( draw, DRAW_FLUSH_STATE_CHANGE ); >>>>> + >>>>> memcpy(draw->pt.vertex_element, elements, count * >>>>> sizeof(elements[0])); >>>>> draw->pt.nr_vertex_elements = count; >>>>> } >>>>> @@ -654,6 +658,8 @@ void draw_do_flush( struct draw_context *draw, >>>>> unsigned flags ) >>>>> >>>>> draw_pipeline_flush( draw, flags ); >>>>> >>>>> + draw_pt_flush( draw, flags ); >>>>> + >>>>> draw->flushing = FALSE; >>>>> } >>>>> } >>>>> diff --git a/src/gallium/auxiliary/draw/draw_private.h >>>>> b/src/gallium/auxiliary/draw/draw_private.h >>>>> index 1a0286d..c3eca97 100644 >>>>> --- a/src/gallium/auxiliary/draw/draw_private.h >>>>> +++ b/src/gallium/auxiliary/draw/draw_private.h >>>>> @@ -63,6 +63,7 @@ struct draw_stage; >>>>> struct vbuf_render; >>>>> struct tgsi_exec_machine; >>>>> struct tgsi_sampler; >>>>> +struct draw_pt_front_end; >>>>> >>>>> >>>>> /** >>>>> @@ -137,6 +138,12 @@ struct draw_context >>>>> /* Support prototype passthrough path: >>>>> */ >>>>> struct { >>>>> + /* Current active frontend */ >>>>> + struct draw_pt_front_end *frontend; >>>>> + unsigned prim; >>>>> + unsigned opt; >>>>> + unsigned eltSize; /* saved eltSize for flushing */ >>>>> + >>>>> struct { >>>>> struct draw_pt_middle_end *fetch_emit; >>>>> struct draw_pt_middle_end *fetch_shade_emit; >>>>> @@ -391,6 +398,7 @@ void draw_remove_extra_vertex_attribs(struct >>>>> draw_context *draw); >>>>> boolean draw_pt_init( struct draw_context *draw ); >>>>> void draw_pt_destroy( struct draw_context *draw ); >>>>> void draw_pt_reset_vertex_ids( struct draw_context *draw ); >>>>> +void draw_pt_flush( struct draw_context *draw, unsigned flags ); >>>>> >>>>> >>>>> /******************************************************************************* >>>>> diff --git a/src/gallium/auxiliary/draw/draw_pt.c >>>>> b/src/gallium/auxiliary/draw/draw_pt.c >>>>> index 9a017fd..025d539 100644 >>>>> --- a/src/gallium/auxiliary/draw/draw_pt.c >>>>> +++ b/src/gallium/auxiliary/draw/draw_pt.c >>>>> @@ -52,7 +52,7 @@ DEBUG_GET_ONCE_BOOL_OPTION(draw_no_fse, >>>>> "DRAW_NO_FSE", FALSE) >>>>> * - backend -- the vbuf_render provided by the driver. >>>>> */ >>>>> static boolean >>>>> -draw_pt_arrays(struct draw_context *draw, >>>>> +draw_pt_arrays(struct draw_context *draw, >>>>> unsigned prim, >>>>> unsigned start, >>>>> unsigned count) >>>>> @@ -106,17 +106,56 @@ draw_pt_arrays(struct draw_context *draw, >>>>> middle = draw->pt.middle.general; >>>>> } >>>>> >>>>> - frontend = draw->pt.front.vsplit; >>>>> + frontend = draw->pt.frontend; >>>>> + >>>>> + if (frontend ) { >>>>> + if (draw->pt.prim != prim || draw->pt.opt != opt) { >>>>> + /* In certain conditions switching primitives requires us >>>>> to flush >>>>> + * and validate the different stages. One example is when >>>>> smooth >>>>> + * lines are active but first drawn with triangles and then >>>>> with >>>>> + * lines. >>>>> + */ >>>>> + draw_do_flush( draw, DRAW_FLUSH_STATE_CHANGE ); >>>>> + frontend = NULL; >>>>> + } else if (draw->pt.eltSize != draw->pt.user.eltSize) { >>>>> + /* Flush draw state if eltSize changed. >>>>> + * This could be improved so only the frontend is flushed >>>>> since it >>>>> + * converts all indices to ushorts and the fetch part of >>>>> the middle >>>>> + * always perpares both linear and indexed. >>>>> + */ >>>>> + frontend->flush( frontend, DRAW_FLUSH_STATE_CHANGE ); >>>>> + frontend = NULL; >>>>> + } >>>>> + } >>>>> >>>>> - frontend->prepare( frontend, prim, middle, opt ); >>>>> + if (!frontend) { >>>>> + frontend = draw->pt.front.vsplit; >>>>> >>>>> - frontend->run(frontend, start, count); >>>>> + frontend->prepare( frontend, prim, middle, opt ); >>>>> >>>>> - frontend->finish( frontend ); >>>>> + draw->pt.frontend = frontend; >>>>> + draw->pt.eltSize = draw->pt.user.eltSize; >>>>> + draw->pt.prim = prim; >>>>> + draw->pt.opt = opt; >>>>> + } >>>>> + >>>>> + frontend->run( frontend, start, count ); >>>>> >>>>> return TRUE; >>>>> } >>>>> >>>>> +void draw_pt_flush( struct draw_context *draw, unsigned flags ) >>>>> +{ >>>>> + if (draw->pt.frontend) { >>>>> + draw->pt.frontend->flush( draw->pt.frontend, flags ); >>>>> + >>>>> + /* don't prepare if we only are flushing the backend */ >>>>> + if (!(flags & DRAW_FLUSH_BACKEND)) >>>>> + draw->pt.frontend = NULL; >>>>> + } >>>>> +} >>>>> + >>>>> + >>>>> >>>>> boolean draw_pt_init( struct draw_context *draw ) >>>>> { >>>>> diff --git a/src/gallium/auxiliary/draw/draw_pt.h >>>>> b/src/gallium/auxiliary/draw/draw_pt.h >>>>> index 9a45845..2c2efdc 100644 >>>>> --- a/src/gallium/auxiliary/draw/draw_pt.h >>>>> +++ b/src/gallium/auxiliary/draw/draw_pt.h >>>>> @@ -73,7 +73,7 @@ struct draw_pt_front_end { >>>>> unsigned start, >>>>> unsigned count ); >>>>> >>>>> - void (*finish)( struct draw_pt_front_end * ); >>>>> + void (*flush)( struct draw_pt_front_end *, unsigned flags ); >>>>> void (*destroy)( struct draw_pt_front_end * ); >>>>> }; >>>>> >>>>> diff --git a/src/gallium/auxiliary/draw/draw_pt_vsplit.c >>>>> b/src/gallium/auxiliary/draw/draw_pt_vsplit.c >>>>> index c19dcd9..0fed057 100644 >>>>> --- a/src/gallium/auxiliary/draw/draw_pt_vsplit.c >>>>> +++ b/src/gallium/auxiliary/draw/draw_pt_vsplit.c >>>>> @@ -178,11 +178,14 @@ static void vsplit_prepare(struct >>>>> draw_pt_front_end *frontend, >>>>> } >>>>> >>>>> >>>>> -static void vsplit_finish(struct draw_pt_front_end *frontend) >>>>> +static void vsplit_flush(struct draw_pt_front_end *frontend, >>>>> unsigned flags) >>>>> { >>>>> struct vsplit_frontend *vsplit = (struct vsplit_frontend *) >>>>> frontend; >>>>> - vsplit->middle->finish(vsplit->middle); >>>>> - vsplit->middle = NULL; >>>>> + >>>>> + if (!(flags & DRAW_FLUSH_BACKEND)) { >>>>> + vsplit->middle->finish(vsplit->middle); >>>>> + vsplit->middle = NULL; >>>>> + } >>>>> } >>>>> >>>>> >>>>> @@ -202,7 +205,7 @@ struct draw_pt_front_end *draw_pt_vsplit(struct >>>>> draw_context *draw) >>>>> >>>>> vsplit->base.prepare = vsplit_prepare; >>>>> vsplit->base.run = NULL; >>>>> - vsplit->base.finish = vsplit_finish; >>>>> + vsplit->base.flush = vsplit_flush; >>>>> vsplit->base.destroy = vsplit_destroy; >>>>> vsplit->draw = draw; >>>>> >>>>> >>>>> _______________________________________________ >>>>> mesa-commit mailing list >>>>> [email protected] >>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-commit >>>>> _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
