On 06/02/2015 01:42 PM, Eduardo Lima Mitev wrote: > lower_phis_to_scalar() pass recurses the instruction dependence graph to > determine if all the sources of a given instruction are scalarizable. > To prevent cycles, it temporary marks the phi instruction before recursing in, > then updates the entry with the resulting value. However, it does not consider > that the entry value may have changed after a recursion pass, hence causing > a use-after-free situation and a crash. > > This patch fixes this by reloading the entry corresponding to the 'phi' > after recursing and before updating its value. > > The crash can be reproduced ~20% of times with the dEQP test: > > dEQP-GLES3.functional.shaders.loops.while_constant_iterations.nested_sequence_fragment
This is the corresponding memory error as generated by valgrind: ==16154== Invalid write of size 8 ==16154== at 0x9B52472: should_lower_phi (nir_lower_phis_to_scalar.c:156) ==16154== by 0x9B52300: is_phi_src_scalarizable (nir_lower_phis_to_scalar.c:75) ==16154== by 0x9B5243F: should_lower_phi (nir_lower_phis_to_scalar.c:151) ==16154== by 0x9B52541: lower_phis_to_scalar_block (nir_lower_phis_to_scalar.c:184) ==16154== by 0x9B43A86: foreach_cf_node (nir.c:1995) ==16154== by 0x9B43A01: foreach_loop (nir.c:1981) ==16154== by 0x9B43ADA: foreach_cf_node (nir.c:1999) ==16154== by 0x9B43B4B: nir_foreach_block (nir.c:2014) ==16154== by 0x9B529C4: lower_phis_to_scalar_impl (nir_lower_phis_to_scalar.c:268) ==16154== by 0x9B52A2B: nir_lower_phis_to_scalar (nir_lower_phis_to_scalar.c:288) ==16154== by 0x9C59DB9: nir_optimize (brw_nir.c:41) ==16154== by 0x9C5A113: brw_create_nir (brw_nir.c:116) ==16154== by 0x9C685A2: brw_link_shader (brw_shader.cpp:301) ==16154== by 0x9A6CD75: _mesa_glsl_link_shader (ir_to_mesa.cpp:2964) ==16154== by 0x990D0C6: link_program (shaderapi.c:946) ==16154== by 0x990E1B2: _mesa_LinkProgram (shaderapi.c:1411) ==16154== by 0x723F076: shared_dispatch_stub_509 (glapi_mapi_tmp.h:19522) ==16154== by 0x102164E: glu::Program::link() (in /opt/devel/deqp/modules/gles3/deqp-gles3) ==16154== by 0x1022010: glu::ShaderProgram::ShaderProgram(glu::RenderContext const&, glu::ProgramSources const&) (in /opt/devel/deqp/modules/gles3/deqp-gles3) ==16154== by 0x12D8CDA: deqp::gls::ShaderRenderCase::init() (in /opt/devel/deqp/modules/gles3/deqp-gles3) ==16154== Address 0xd1a3880 is 64 bytes inside a block of size 168 free'd ==16154== at 0x4C29730: free (vg_replace_malloc.c:468) ==16154== by 0x9B173E2: unsafe_free (ralloc.c:255) ==16154== by 0x9B172CC: ralloc_free (ralloc.c:218) ==16154== by 0x9B1691F: _mesa_hash_table_rehash (hash_table.c:262) ==16154== by 0x9B1696A: hash_table_insert (hash_table.c:273) ==16154== by 0x9B16B70: _mesa_hash_table_insert (hash_table.c:342) ==16154== by 0x9B52416: should_lower_phi (nir_lower_phis_to_scalar.c:146) ==16154== by 0x9B52300: is_phi_src_scalarizable (nir_lower_phis_to_scalar.c:75) ==16154== by 0x9B5243F: should_lower_phi (nir_lower_phis_to_scalar.c:151) ==16154== by 0x9B52300: is_phi_src_scalarizable (nir_lower_phis_to_scalar.c:75) ==16154== by 0x9B5243F: should_lower_phi (nir_lower_phis_to_scalar.c:151) ==16154== by 0x9B52541: lower_phis_to_scalar_block (nir_lower_phis_to_scalar.c:184) ==16154== by 0x9B43A86: foreach_cf_node (nir.c:1995) ==16154== by 0x9B43A01: foreach_loop (nir.c:1981) ==16154== by 0x9B43ADA: foreach_cf_node (nir.c:1999) ==16154== by 0x9B43B4B: nir_foreach_block (nir.c:2014) ==16154== by 0x9B529C4: lower_phis_to_scalar_impl (nir_lower_phis_to_scalar.c:268) ==16154== by 0x9B52A2B: nir_lower_phis_to_scalar (nir_lower_phis_to_scalar.c:288) ==16154== by 0x9C59DB9: nir_optimize (brw_nir.c:41) ==16154== by 0x9C5A113: brw_create_nir (brw_nir.c:116) And this is a stacktrace (though not very helpful): Program terminated with signal 11, Segmentation fault. #0 0x00007f3f7e4a5963 in malloc_consolidate (av=av@entry=0x7f3f7e7cf620 <main_arena>) at malloc.c:4157 4157 malloc.c: No such file or directory. (gdb) bt #0 0x00007f3f7e4a5963 in malloc_consolidate (av=av@entry=0x7f3f7e7cf620 <main_arena>) at malloc.c:4157 #1 0x00007f3f7e4a6c98 in _int_malloc (av=av@entry=0x7f3f7e7cf620 <main_arena>, bytes=bytes@entry=1080) at malloc.c:3423 #2 0x00007f3f7e4a97dc in __libc_calloc (n=<optimized out>, elem_size=<optimized out>) at malloc.c:3219 #3 0x00007f3f7baa9f6f in ralloc_size (ctx=0x42acb20, size=1032) at ralloc.c:113 #4 0x00007f3f7baa9ffa in rzalloc_size (ctx=0x42acb20, size=1032) at ralloc.c:134 #5 0x00007f3f7baaa238 in rzalloc_array_size (ctx=0x42acb20, size=24, count=43) at ralloc.c:196 #6 0x00007f3f7baa97c7 in _mesa_hash_table_rehash (ht=0x42acb20, new_size_index=4) at hash_table.c:243 #7 0x00007f3f7baa996b in hash_table_insert (ht=0x42acb20, hash=3370383618, key=0x46a7b20, data=0x42ae350) at hash_table.c:273 #8 0x00007f3f7baa9b71 in _mesa_hash_table_insert (ht=0x42acb20, key=0x46a7b20, data=0x42ae350) at hash_table.c:342 #9 0x00007f3f7bafb7cf in validate_var_decl (var=0x46a7b20, is_global=false, state=0x7fffd86c3cb0) at nir/nir_validate.c:785 #10 0x00007f3f7bafbce9 in validate_function_impl (impl=0x42ae350, state=0x7fffd86c3cb0) at nir/nir_validate.c:863 #11 0x00007f3f7bafbef3 in validate_function_overload (overload=0x42ab390, state=0x7fffd86c3cb0) at nir/nir_validate.c:898 #12 0x00007f3f7bafbf63 in validate_function (func=0x42ab320, state=0x7fffd86c3cb0) at nir/nir_validate.c:907 #13 0x00007f3f7bafc33f in nir_validate_shader (shader=0x46a6310) at nir/nir_validate.c:979 #14 0x00007f3f7bbecdc6 in nir_optimize (nir=0x46a6310) at brw_nir.c:42 #15 0x00007f3f7bbed114 in brw_create_nir (brw=0x7f3f807d0040, shader_prog=0x42a9970, prog=0x46a6a60, stage=MESA_SHADER_FRAGMENT) at brw_nir.c:116 #16 0x00007f3f7bbfb5a3 in brw_link_shader (ctx=0x7f3f807d0040, shProg=0x42a9970) at brw_shader.cpp:301 #17 0x00007f3f7b9ffd76 in _mesa_glsl_link_shader (ctx=0x7f3f807d0040, prog=0x42a9970) at program/ir_to_mesa.cpp:2964 #18 0x00007f3f7b8a00c7 in link_program (ctx=0x7f3f807d0040, program=1) at main/shaderapi.c:946 #19 0x00007f3f7b8a11b3 in _mesa_LinkProgram (programObj=1) at main/shaderapi.c:1411 #20 0x00007f3f7dfcc077 in shared_dispatch_stub_509 (program=1) at shared-glapi/glapi_mapi_tmp.h:19522 #21 0x000000000102164f in glu::Program::link() () #22 0x0000000001022011 in glu::ShaderProgram::ShaderProgram(glu::RenderContext const&, glu::ProgramSources const&) () #23 0x00000000012d8cdb in deqp::gls::ShaderRenderCase::init() () #24 0x0000000000fd4bc2 in tcu::TestCaseWrapper::initTestCase(tcu::TestCase*) () #25 0x0000000000a9a75f in deqp::gles3::TestCaseWrapper::initTestCase(tcu::TestCase*) () #26 0x0000000000fd62f0 in tcu::TestExecutor::enterTestCase(tcu::TestCase*, char const*) () #27 0x0000000000fd6801 in tcu::TestExecutor::iterate() () #28 0x0000000000fad951 in tcu::App::iterate() () #29 0x0000000000a970c9 in main () > --- > src/glsl/nir/nir_lower_phis_to_scalar.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/src/glsl/nir/nir_lower_phis_to_scalar.c > b/src/glsl/nir/nir_lower_phis_to_scalar.c > index 4bdb800..a57d253 100644 > --- a/src/glsl/nir/nir_lower_phis_to_scalar.c > +++ b/src/glsl/nir/nir_lower_phis_to_scalar.c > @@ -153,6 +153,11 @@ should_lower_phi(nir_phi_instr *phi, struct > lower_phis_to_scalar_state *state) > break; > } > > + /* The hash table entry for 'phi' may have changed while recursing the > + * dependence graph, so we need to reset it */ > + entry = _mesa_hash_table_search(state->phi_table, phi); > + assert(entry); > + > entry->data = (void *)(intptr_t)scalarizable; > > return scalarizable; > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
