Created attachment 80037 [details]
gdb full backtrace of the crash

nVidia proprietary driver doesn't crash with the same command.

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.

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
>

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.

Hmm, could we get another person (Jose?) to try this too?

(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.

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.

(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?

>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,

(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.

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.

(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!

If you were waiting for me to confirm the bug fixed, here's the confirmation :)

Thanks for testing.  Pushed as commit e20a2df4017ab10dd7199936948c6ac809bfacb6