[Mesa-dev] [PATCH] mesa: prevent SIGSEGV passing GL_NONE to _mesa_ReadBuffer()

[Ahmed Allam]

On 01/23/2014 06:04 PM, Brian Paul wrote:
> On 01/23/2014 07:31 AM, Ahmed Allam wrote:
>> From: Ahmed Allam <ahmabdabd at hotmail.com>
>>
>>
>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=73956
>>
>> Signed-off-by: Ahmed Allam <ahmabdabd at hotmail.com>
>> ---
>>   src/mesa/main/buffers.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
>> index 2bdbf41..6c846cb 100644
>> --- a/src/mesa/main/buffers.c
>> +++ b/src/mesa/main/buffers.c
>> @@ -623,7 +623,9 @@ _mesa_ReadBuffer(GLenum buffer)
>>
>>      if (buffer == GL_NONE) {
>>         /* This is legal--it means that no buffer should be bound for
>> reading. */
>> -      srcBuffer = -1;
>> +      _mesa_error(ctx, GL_INVALID_ENUM,
>> +                           "glReadBuffer(buffer=0x%x)", buffer);
>> +     return;
>>      }
>>      else {
>>         /* general case / window-system framebuffer */
>
>
> According to the GL 3.2 spec, GL_NONE is a legal value for 
> glReadBuffer() and should not generate an error (as the comment says.) 
> And I just verified it with NVIDIA's driver.
>
> The spec says "ReadPixels generates an INVALID_OPERATION error if it 
> attempts to select a color buffer while READ_BUFFER is NONE". That's 
> what we do.  However, it looks like NVIDIA's driver is doing something 
> else.  It's actually doing glRead/CopyPixels w/out raising an error.
>
> Anyway, can you try this patch:
>
> diff --git a/src/mesa/state_tracker/st_cb_fbo.c 
> b/src/mesa/state_tracker/st_cb_fbo.c
> index 637f7ee..eca04b8 100644
> --- a/src/mesa/state_tracker/st_cb_fbo.c
> +++ b/src/mesa/state_tracker/st_cb_fbo.c
> @@ -700,7 +700,8 @@ st_ReadBuffer(struct gl_context *ctx, GLenum buffer)
>     (void) buffer;
>
>     /* add the renderbuffer on demand */
> -   st_manager_add_color_renderbuffer(st, fb, fb->_ColorReadBufferIndex);
> +   if (fb->_ColorReadBufferIndex >= 0)
> +      st_manager_add_color_renderbuffer(st, fb, 
> fb->_ColorReadBufferIndex);
>  }
>
>
> -Brian
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
Patch worked without any visible problems and without 
glReadBuffer(GL_NONE) producing any errors. I am starting to believe 
that this is the expected behavior as opposed to generating 
INVALID_OPERATION. Thanks.

Ahmed