[Mesa-dev] Fix a crash in update_texture_state() when requesting a fallback for external texture target

[Brian Paul]

On 10/12/2012 02:42 AM, Abdiel Janulgue wrote:
> On Wednesday, October 10, 2012 08:33:08 AM Brian Paul wrote:
>> On 10/10/2012 05:42 AM, Abdiel Janulgue wrote:
>>> I found a crash where updating the texture unit states ends up requesting
>>> a
>>> fallback texture for a  GL_TEXTURE_EXTERNAL_OES target and sets a null
>>> current texture object for the current unit.
>>>
>>> This is a fix by supporting  TEXTURE_EXTERNAL_INDEX target in
>>> _mesa_get_fallback_texture() and making sure the texture unit is not
>>> really
>>> used should an invalid  fallback be returned.
>>>
>>> Cheers,
>>> -abdiel
>>>
>>> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
>>> index e0494c9..7e2b6b5 100644
>>> --- a/src/mesa/main/texobj.c
>>> +++ b/src/mesa/main/texobj.c
>>> @@ -771,6 +771,9 @@ _mesa_get_fallback_texture(struct gl_context *ctx,
>>> gl_texture_index tex)
>>>
>>>             target = GL_TEXTURE_BUFFER;
>>>             break;
>>>
>>>          case TEXTURE_EXTERNAL_INDEX:
>>> +         dims = 2;
>>> +         target = GL_TEXTURE_EXTERNAL_OES;
>>> +         break;
>>>
>>>          default:
>>>             /* no-op */
>>>             return NULL;
>>>
>>> diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
>>> index 8ca9092..009b077 100644
>>> --- a/src/mesa/main/texstate.c
>>> +++ b/src/mesa/main/texstate.c
>>> @@ -600,6 +600,11 @@ update_texture_state( struct gl_context *ctx )
>>>
>>>                texTarget = (gl_texture_index) (ffs(enabledTargets) - 1);
>>>                texObj = _mesa_get_fallback_texture(ctx, texTarget);
>>>
>>> +
>>> +            if (!texObj)
>>> +               /* invalid fallback texture: make sure to disable texture
>>> unit */
>>> +               continue;
>>
>> So texObj should only be null here if _mesa_get_fallback_texture()
>> fails because of a bad/unsupported texture target or we run out of
>> memory for a 1-texel texture (pretty unlikely).
>>
>> So I think I'd like to see an assert(texObj) there followed by the
>> if/continue so that we get the assertion with debug builds but don't
>> crash with release builds.
>>
>> Looks OK otherwise.
>
> Thanks Brian. I've pushed the improved changes with your feedback to
> git at gitorious.org:mesa3d/mesa.git master branch

Thanks.  I'm going to tweak the comments and will commit it in a bit.

-Brian