Bug 104803 - SIGSEGV in state_tracker/st_glsl_to_tgsi_temprename.cpp
Summary: SIGSEGV in state_tracker/st_glsl_to_tgsi_temprename.cpp
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Gallium/swr (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
Depends on:
Reported: 2018-01-26 15:18 UTC by Brad King
Modified: 2018-01-30 04:11 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:

apitrace of test that crashes (149.53 KB, application/gzip)
2018-01-26 15:18 UTC, Brad King
apitrace of test recorded before change that crashes (213.20 KB, application/gzip)
2018-01-26 16:08 UTC, Brad King
Prelimiary fix (2.39 KB, patch)
2018-01-26 21:56 UTC, Gert Wollny
Details | Splinter Review

Description Brad King 2018-01-26 15:18:16 UTC
Created attachment 136975 [details]
apitrace of test that crashes

Some VTK tests started crashing with nightly Mesa recently.  It bisects to commit 807e2539e5 (mesa/st/glsl_to_tgsi: Add tracking of ifelse writes in register merging).

Running a test with current `master` (commit d3ce493b34), gdb reports this stack trace:

Thread 1 "vtkRenderingLIC" received signal SIGSEGV, Segmentation fault.
0x00007ffff3c74542 in (anonymous namespace)::prog_scope::begin (this=0x0) at state_tracker/st_glsl_to_tgsi_temprename.cpp:437
437        return scope_begin;
(gdb) where
#0  0x00007ffff3c74542 in (anonymous namespace)::prog_scope::begin (this=0x0) at state_tracker/st_glsl_to_tgsi_temprename.cpp:437
#1  0x00007ffff3c7432f in (anonymous namespace)::prog_scope::contains_range_of (this=0x0, other=...) at state_tracker/st_glsl_to_tgsi_temprename.cpp:365
#2  0x00007ffff3c75048 in (anonymous namespace)::temp_comp_access::get_required_lifetime (this=0x555556116ed0) at state_tracker/st_glsl_to_tgsi_temprename.cpp:803
#3  0x00007ffff3c748dc in (anonymous namespace)::temp_access::get_required_lifetime (this=0x555556116ed0) at state_tracker/st_glsl_to_tgsi_temprename.cpp:527
#4  0x00007ffff3c755b7 in (anonymous namespace)::access_recorder::get_required_lifetimes (this=0x7fffffffb6c0, lifetimes=0x555555c9ead0) at state_tracker/st_glsl_to_tgsi_temprename.cpp:933
#5  0x00007ffff3c75f3a in get_temp_registers_required_lifetimes (mem_ctx=0x555555ca56b0, instructions=0x5555561159d0, ntemps=60, lifetimes=0x555555c9ead0) at state_tracker/st_glsl_to_tgsi_temprename.cpp:1120
#6  0x00007ffff3c6b3a4 in glsl_to_tgsi_visitor::merge_registers (this=0x555556110500) at state_tracker/st_glsl_to_tgsi.cpp:5316
#7  0x00007ffff3c6fb29 in get_mesa_program_tgsi (ctx=0x5555559308a0, shader_program=0x555555c3aee0, shader=0x55555610f5f0) at state_tracker/st_glsl_to_tgsi.cpp:6767
#8  0x00007ffff3c704c5 in st_link_shader (ctx=0x5555559308a0, prog=0x555555c3aee0) at state_tracker/st_glsl_to_tgsi.cpp:7065
#9  0x00007ffff3c95b77 in _mesa_glsl_link_shader (ctx=0x5555559308a0, prog=0x555555c3aee0) at program/ir_to_mesa.cpp:3126
#10 0x00007ffff3b446c3 in link_program (no_error=false, shProg=0x555555c3aee0, ctx=0x5555559308a0) at main/shaderapi.c:1203
#11 link_program_error (ctx=0x5555559308a0, shProg=0x555555c3aee0) at main/shaderapi.c:1281
#12 0x00007ffff3b45cad in _mesa_LinkProgram (programObj=9) at main/shaderapi.c:1773
...vtk code...

I've also attached an apitrace.

For reference I configure Mesa with:

./autogen.sh \
  --prefix="$prefix" \
  --enable-debug \
  --disable-dri \
  --disable-egl \
  --disable-gbm \
  --disable-gles1 \
  --disable-gles2 \
  --disable-shared-glapi \
  --with-platforms=x11 \
  --enable-glx=gallium-xlib \
  --enable-gallium-osmesa \
  --with-gallium-drivers=swrast \
  --enable-gallium-llvm=yes \
     LLVM_CONFIG=/usr/bin/llvm-config-3.8 \
Comment 1 Brian Paul 2018-01-26 16:00:25 UTC
Hi Brad,
I configured and built Mesa as you described, but I can't repro the crash with your trace.  I tested both ToT and 807e2539e5 with softpipe and llvmpipe.

I'm using gcc/g++ 5.4.0, FWIW.
Comment 2 Brad King 2018-01-26 16:08:23 UTC
Created attachment 136976 [details]
apitrace of test recorded before change that crashes

Thanks Brian.  For another try, here is an apitrace of the test that is more complete by running it with a version of Mesa from just before the breakage.

Also, here is valgrind memcheck output from the segfault:

==27717== Invalid read of size 4
==27717==    at 0x8B2D542: (anonymous namespace)::prog_scope::begin() const (st_glsl_to_tgsi_temprename.cpp:437)
==27717==    by 0x8B2D32E: (anonymous namespace)::prog_scope::contains_range_of((anonymous namespace)::prog_scope const&) const (st_glsl_to_tgsi_temprename.cpp:365)                                                                                                                       
==27717==    by 0x8B2E047: (anonymous namespace)::temp_comp_access::get_required_lifetime() (st_glsl_to_tgsi_temprename.cpp:803)
==27717==    by 0x8B2D8DB: (anonymous namespace)::temp_access::get_required_lifetime() (st_glsl_to_tgsi_temprename.cpp:527)
==27717==    by 0x8B2E5B6: (anonymous namespace)::access_recorder::get_required_lifetimes(lifetime*) (st_glsl_to_tgsi_temprename.cpp:933)
==27717==    by 0x8B2EF39: get_temp_registers_required_lifetimes(void*, exec_list*, int, lifetime*) (st_glsl_to_tgsi_temprename.cpp:1120)
==27717==    by 0x8B243A3: glsl_to_tgsi_visitor::merge_registers() (st_glsl_to_tgsi.cpp:5316)
==27717==    by 0x8B28B28: get_mesa_program_tgsi(gl_context*, gl_shader_program*, gl_linked_shader*) (st_glsl_to_tgsi.cpp:6767)
==27717==    by 0x8B294C4: st_link_shader (st_glsl_to_tgsi.cpp:7065)
==27717==    by 0x8B4EB76: _mesa_glsl_link_shader (ir_to_mesa.cpp:3126)
==27717==    by 0x89FD6C2: link_program (shaderapi.c:1203)
==27717==    by 0x89FD6C2: link_program_error (shaderapi.c:1281)
==27717==    by 0x89FECAC: _mesa_LinkProgram (shaderapi.c:1773)
==27717==  Address 0xc is not stack'd, malloc'd or (recently) free'd

That "Address 0xc" looks suspicious.
Comment 3 Brian Paul 2018-01-26 16:15:38 UTC
OK, that trace triggers the crash.  I'll take a quick look, but it'll probably be up to Gert to investigate and fix.
Comment 4 Brian Paul 2018-01-26 16:30:58 UTC
This patch seems to fix the crash, but I'm not sure it's actually correct.  Gert will have to take a look.

diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
index 76b3f43..54802b3 100644
--- a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
+++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp
@@ -791,7 +791,8 @@ lifetime temp_comp_access::get_required_lifetime()
    const prog_scope *conditional = enclosing_scope_first_write->enclosing_conditional();
    if (conditional && !conditional->contains_range_of(*last_read_scope) &&
        (conditional->is_switchcase_scope_in_loop() ||
-        conditional_ifelse_write_in_loop())) {
+        conditional_ifelse_write_in_loop()) &&
+       conditional->outermost_loop()) {
          keep_for_full_loop = true;
          enclosing_scope_first_write = conditional->outermost_loop();
@@ -800,6 +801,8 @@ lifetime temp_comp_access::get_required_lifetime()
     * required first read before write scope, and last read scope.
    const prog_scope *enclosing_scope = enclosing_scope_first_read;
+   assert(enclosing_scope);
+   assert(enclosing_scope_first_write);
    if (enclosing_scope_first_write->contains_range_of(*enclosing_scope))
       enclosing_scope = enclosing_scope_first_write;
Comment 5 Gert Wollny 2018-01-26 21:10:11 UTC
@Brian I can also reproduce the crash, and was able to add a test case for it, but I AFAICS  your fix is just a workaround, and the culprit is somewhere else.
Comment 6 Gert Wollny 2018-01-26 21:56:33 UTC
Created attachment 136978 [details] [review]
Prelimiary fix

Hi Brad, 

this patch fixes the segfault in the trace you posted, could you test the patch to see if it fixes also the other tests?

Comment 7 Brad King 2018-01-29 12:45:16 UTC
Gert, yes that fixes all the tests.  Thanks!
Comment 8 Brian Paul 2018-01-30 04:11:33 UTC
Fixed with commit 6a7d1ca2c49ae06bbb323936f1e1c17ba7e2c9a1

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.