[Mesa-dev] [PATCH] mesa: Add null pointer checks before dereferencing

[Brian Paul]

On 06/27/2013 04:51 PM, Anuj Phogat wrote:
> On Thu, Jun 27, 2013 at 2:59 PM, Brian Paul <brianp at vmware.com> wrote:
>> On 06/27/2013 03:31 PM, Kenneth Graunke wrote:
>>>
>>> On 06/27/2013 02:20 PM, Anuj Phogat wrote:
>>>>
>>>> Assertions are not sufficient to check for null pointers as they don't
>>>> show up in release builds. So, add explicit null pointer checks in the
>>>> code.
>>>>
>>>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>>>> ---
>>>>    src/mesa/program/prog_execute.c | 10 ++++++++++
>>>>    1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/src/mesa/program/prog_execute.c
>>>> b/src/mesa/program/prog_execute.c
>>>> index b902006..1bcbf6b 100644
>>>> --- a/src/mesa/program/prog_execute.c
>>>> +++ b/src/mesa/program/prog_execute.c
>>>> @@ -200,6 +200,8 @@ fetch_vector4(const struct prog_src_register *source,
>>>>    {
>>>>       const GLfloat *src = get_src_register_pointer(source, machine);
>>>>       ASSERT(src);
>>>> +   if (src == NULL)
>>>> +      return;
>>>>
>>>>       if (source->Swizzle == SWIZZLE_NOOP) {
>>>>          /* no swizzling */
>>>> @@ -303,6 +305,8 @@ fetch_vector1(const struct prog_src_register *source,
>>>>    {
>>>>       const GLfloat *src = get_src_register_pointer(source, machine);
>>>>       ASSERT(src);
>>>> +   if (src == NULL)
>>>> +      return;
>>>>
>>>>       result[0] = src[GET_SWZ(source->Swizzle, 0)];
>>>>
>>>> @@ -320,6 +324,9 @@ fetch_vector1ui(const struct prog_src_register
>>>> *source,
>>>>                    const struct gl_program_machine *machine)
>>>>    {
>>>>       const GLuint *src = (GLuint *) get_src_register_pointer(source,
>>>> machine);
>>>> +   ASSERT(src);
>>>> +   if (src == NULL)
>>>> +      return 0;
>>>>       return src[GET_SWZ(source->Swizzle, 0)];
>>>>    }
>>>>
>>>> @@ -1439,8 +1446,11 @@ _mesa_execute_program(struct gl_context * ctx,
>>>>             {
>>>>                const struct prog_src_register *source = &inst->SrcReg[0];
>>>>                const GLfloat *src = get_src_register_pointer(source,
>>>> machine);
>>>> +            ASSERT(src);
>>>>                GLfloat result[4];
>>>>                GLuint i;
>>>> +            if (src == NULL)
>>>> +               return GL_FALSE;
>>>>                for (i = 0; i < 4; i++) {
>>>>                   const GLuint swz = GET_SWZ(source->Swizzle, i);
>>>>                   if (swz == SWIZZLE_ZERO)
>>>>
>>>
>>> I don't like this.  I would just put an abort() below the _mesa_problem
>>> in get_src_register_pointer.
>>
>>
>> For release builds I don't think that we ever want to abort/exit.
>>
>> I think the only time get_src_register_pointer() can return NULL is for the
>> case of an invalid register file.  And we could instead return the ZeroVec
>> there.
> Yes. This is a better option for get_src_register_pointer(). But I
> think we can't return ZeroVec (a const float*) instead of NULL
> in get_dst_register_pointer(). Should we return dummyReg instead?

Probably.

-Brian