Bug 65173 - segfault in _mesa_get_format_datatype and _mesa_get_color_read_type when state dumping with glretrace
segfault in _mesa_get_format_datatype and _mesa_get_color_read_type when stat...
Status: RESOLVED FIXED
Product: Mesa
Classification: Unclassified
Component: Mesa core
git
All All
: medium normal
Assigned To: mesa-dev
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-30 12:10 UTC by Vedran Rodic
Modified: 2013-06-03 00:12 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
fix (1.28 KB, text/plain)
2013-05-30 12:10 UTC, Vedran Rodic
Details
gdb full backtrace of the crash (3.96 KB, text/plain)
2013-05-30 12:11 UTC, Vedran Rodic
Details
fix-v2 (915 bytes, text/plain)
2013-05-30 21:51 UTC, Vedran Rodic
Details
patch which generates GL_INVALID_ENUM (3.07 KB, patch)
2013-05-31 14:56 UTC, Brian Paul
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Vedran Rodic 2013-05-30 12:10:37 UTC
Created attachment 80036 [details]
fix

when using glretrace to dump the GL state, sometimes _mesa_get_color_read_format and _mesa_get_color_read_type can access uninitialized ctx->ReadBuffer->_ColorReadBuffer

To reproduce run:

glretrace -D 137078 wine-preloader.137078.trace

trace file is here: 
http://mjesec.ffzg.hr/~vrodic/dota/wine-preloader.137078.trim.trace.bz2

I've attached a sample patch to fix the bug (probably incorrect) and a gdb backtrace .
Comment 1 Vedran Rodic 2013-05-30 12:11:10 UTC
Created attachment 80037 [details]
gdb full backtrace of the crash
Comment 2 Vedran Rodic 2013-05-30 12:13:46 UTC
nVidia proprietary driver doesn't crash with the same command.
Comment 3 Brian Paul 2013-05-30 15:09:15 UTC
Which version of Mesa, and which driver?

I tested with Mesa/master and the 9.1 branch and didn't see this problem.

However, I did find a problem when the libtxc_dxtn.so library wasn't present.  I'll post a patch for that to the mesa-dev list.
Comment 4 Vedran Rodic 2013-05-30 16:16:12 UTC
mesa git version, i'll check if libtxc_dxtn.so was present here

On Thu, May 30, 2013 at 5:09 PM,  <bugzilla-daemon@freedesktop.org> wrote:
> Comment # 3 on bug 65173 from Brian Paul
>
> Which version of Mesa, and which driver?
>
> I tested with Mesa/master and the 9.1 branch and didn't see this problem.
>
> However, I did find a problem when the libtxc_dxtn.so library wasn't
> present.
> I'll post a patch for that to the mesa-dev list.
>
> ________________________________
> You are receiving this mail because:
>
> You are the assignee for the bug.
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
Comment 5 Vedran Rodic 2013-05-30 17:01:19 UTC
I'm using glretrace from latest apitrace git, mesa from latest git.

I was able to reproduce this on nouveau driver, intel driver and gallium swrast driver.
Comment 6 Brian Paul 2013-05-30 17:12:07 UTC
Hmm, could we get another person (Jose?) to try this too?
Comment 7 Jose Fonseca 2013-05-30 21:35:01 UTC
(In reply to comment #6)
> Hmm, could we get another person (Jose?) to try this too?

Yep, it happens here too. ctx->ReadBuffer->_ColorReadBuffer is NULL in _mesa_get_color_read_type. This is the full stack backtrace (this time debug build):

Program received signal SIGSEGV, Segmentation fault.
_mesa_get_color_read_type (ctx=0xbed310) at src/mesa/main/framebuffer.c:905
905	   const GLenum data_type = _mesa_get_format_datatype(
(gdb) bt
#0  _mesa_get_color_read_type (ctx=0xbed310) at src/mesa/main/framebuffer.c:905
#1  0x00007ffff4ed67f8 in find_custom_value (ctx=0xbed310, d=0x7ffff608ca88, v=0x7fffffffb550) at src/mesa/main/get.c:696
#2  0x00007ffff4ed75ef in find_value (func=0x7ffff5a0328c "glGetIntegerv", pname=35738, p=0x7fffffffb548, v=0x7fffffffb550)
    at src/mesa/main/get.c:1099
#3  0x00007ffff4ed7d69 in _mesa_GetIntegerv (pname=35738, params=0x7fffffffce88) at src/mesa/main/get.c:1299
#4  0x000000000043255a in _glGetIntegerv (pname=35738, params=0x7fffffffce88)
    at /home/jfonseca/projects/apitrace/dispatch/glproc.hpp:4802
#5  0x000000000045bb28 in glstate::dumpParameters (json=..., context=...)
    at /home/jfonseca/projects/apitrace/retrace/glstate_params.cpp:14881
#6  0x000000000042e3ee in glstate::dumpCurrentContext (os=...) at /home/jfonseca/projects/apitrace/retrace/glstate.cpp:169
#7  0x000000000042c86a in GLDumper::dumpState (this=0x909d10, os=...)
    at /home/jfonseca/projects/apitrace/retrace/glretrace_main.cpp:445
#8  0x000000000041fb9d in retrace::retraceCall (call=0xbd8f40) at /home/jfonseca/projects/apitrace/retrace/retrace_main.cpp:197
#9  0x00000000004219ec in retrace::RelayRunner::runLeg (this=0x98a720, call=0xbd8f40)
    at /home/jfonseca/projects/apitrace/retrace/retrace_main.cpp:342
#10 0x00000000004218d2 in retrace::RelayRunner::runRace (this=0x98a720)
    at /home/jfonseca/projects/apitrace/retrace/retrace_main.cpp:313
#11 0x000000000041fe9a in retrace::RelayRace::run (this=0x7fffffffde30)
    at /home/jfonseca/projects/apitrace/retrace/retrace_main.cpp:480
#12 0x0000000000420072 in retrace::mainLoop () at /home/jfonseca/projects/apitrace/retrace/retrace_main.cpp:541
#13 0x0000000000420879 in main (argc=4, argv=0x7fffffffe068) at /home/jfonseca/projects/apitrace/retrace/retrace_main.cpp:804

The proposed patch fixes it.

I don't know if the ctx->ReadBuffer check is necessary but I suppose it doesn't hurt.

In summary: looks good to me.
Comment 8 Vedran Rodic 2013-05-30 21:51:51 UTC
Created attachment 80070 [details]
fix-v2

I'm happier with this new version of the patch, but I'm still not sure if returning 0 is a correct thing to do.
Comment 9 Jose Fonseca 2013-05-30 22:09:58 UTC
(In reply to comment #8)
> Created attachment 80070 [details]
> fix-v2
> 
> I'm happier with this new version of the patch, 

I'm afraid that version 2 assumes C99 and will break MSVC.

> but I'm still not sure if
> returning 0 is a correct thing to do.

Anything is better than crashing.

GL_READ_BUFFER is GL_NONE here, so it doesn't matter.

What does nVidia proprietary driver return for IMPLEMENTATION_COLOR_READ_{FORMAT,TYPE} on that call? That is, what is the output of

   glretrace -D 137078 wine-preloader.137078.trim.trace | grep IMPLEMENTATION_COLOR_READ

on nvidia?
Comment 10 Vedran Rodic 2013-05-30 22:28:44 UTC
>What does nVidia proprietary driver return for 
>IMPLEMENTATION_COLOR_READ_{FORMAT,TYPE} on that call? That is, what is the 
>output of
> glretrace -D 137078 wine-preloader.137078.trim.trace | grep 
>IMPLEMENTATION_COLOR_READ

Nothing, the part of the state dump where mesa version crashes looks like this (I've put stderr part in <stderr> tags):

 "GL_MAX_VERTEX_UNIFORM_COMPONENTS": 4096,
    "GL_MAX_VARYING_COMPONENTS": 60,
    "GL_MAX_VERTEX_TEXTURE_IMAGE_UNITS": 32,
    "GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS": 96,
    "GL_SHADING_LANGUAGE_VERSION": "3.30 NVIDIA via Cg compiler",
    "GL_CURRENT_PROGRAM": 18 <stderr>137078: glDebugOutputCallback: High severity API error 1282, GL_INVALID_OPERATION error generated. Invalid query on incomplete or unselected read framebuffer
137078: glDebugOutputCallback: High severity API error 1282, GL_INVALID_OPERATION error generated. Invalid query on incomplete or unselected read framebuffer</stderr>
,
    "GL_MAX_GEOMETRY_TEXTURE_IMAGE_UNITS": 32,
    "GL_TEXTURE_BUFFER": 0,
Comment 11 Jose Fonseca 2013-05-31 06:35:43 UTC
(In reply to comment #10)
> >What does nVidia proprietary driver return for 
> >IMPLEMENTATION_COLOR_READ_{FORMAT,TYPE} on that call? That is, what is the 
> >output of
> > glretrace -D 137078 wine-preloader.137078.trim.trace | grep 
> >IMPLEMENTATION_COLOR_READ
> 
> Nothing, the part of the state dump where mesa version crashes looks like
> this (I've put stderr part in <stderr> tags):
> 
>  "GL_MAX_VERTEX_UNIFORM_COMPONENTS": 4096,
>     "GL_MAX_VARYING_COMPONENTS": 60,
>     "GL_MAX_VERTEX_TEXTURE_IMAGE_UNITS": 32,
>     "GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS": 96,
>     "GL_SHADING_LANGUAGE_VERSION": "3.30 NVIDIA via Cg compiler",
>     "GL_CURRENT_PROGRAM": 18 <stderr>137078: glDebugOutputCallback: High
> severity API error 1282, GL_INVALID_OPERATION error generated. Invalid query
> on incomplete or unselected read framebuffer
> 137078: glDebugOutputCallback: High severity API error 1282,
> GL_INVALID_OPERATION error generated. Invalid query on incomplete or
> unselected read framebuffer</stderr>
> ,
>     "GL_MAX_GEOMETRY_TEXTURE_IMAGE_UNITS": 32,
>     "GL_TEXTURE_BUFFER": 0,

Thanks for checking. It seems it nvidia driver emits a GL_INVALID_OPERATION error.  We could do the same.

BTW, I did read the relevant specs for this query, but they don't really describe what happens in this situation.
Comment 12 Brian Paul 2013-05-31 14:56:50 UTC
Created attachment 80099 [details] [review]
patch which generates GL_INVALID_ENUM

How about this patch?  It generates GL_INVALID_OPERATION if there's no color read buffer.  It also cleans up both functions in general.
Comment 13 Jose Fonseca 2013-05-31 16:07:26 UTC
(In reply to comment #12)
> How about this patch?  It generates GL_INVALID_OPERATION if there's no color
> read buffer.  It also cleans up both functions in general.

Patch looks good to me!
Comment 14 Vedran Rodic 2013-06-02 10:49:27 UTC
If you were waiting for me to confirm the bug fixed, here's the confirmation :)
Comment 15 Brian Paul 2013-06-03 00:12:39 UTC
Thanks for testing.  Pushed as commit e20a2df4017ab10dd7199936948c6ac809bfacb6