[Mesa-dev] [PATCH] prog_parameter.c ASAN Patch

[Myles C. Maxfield]

Friendly ping regarding this patch :-)

--Myles


On Wed, Jun 19, 2013 at 12:47 AM, Myles C. Maxfield <
myles.maxfield at gmail.com> wrote:

> Any word on this?
>
> Thanks,
> Myles
>
>
> On Mon, Jun 17, 2013 at 12:09 PM, Myles C. Maxfield <
> myles.maxfield at gmail.com> wrote:
>
>> Sure. I was under the impression that |size| couldn't be both greater
>> than 4 and a non-multiple of 4, but I've reworked the patch to incorporate
>> this and to be a little more straightforward.
>>
>> Is the only way to replace "ASAN" with "Address Sanitizer" to change the
>> subject of this email thread?
>>
>> Anyway, here's a similar but modified patch:
>>
>> From: Myles C. Maxfield <mymax at amazon.com>
>> Date: Mon, 17 Jun 2013 11:50:05 -0700
>> Subject: [PATCH] Appeasing Address Sanitizer
>>
>> ---
>>  src/mesa/program/prog_parameter.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/program/prog_parameter.c
>> b/src/mesa/program/prog_parameter.c
>> index 95b153e..1d46476 100644
>> --- a/src/mesa/program/prog_parameter.c
>> +++ b/src/mesa/program/prog_parameter.c
>> @@ -155,7 +155,18 @@ _mesa_add_parameter(struct gl_program_parameter_list
>> *paramList,
>>           p->Size = size;
>>           p->DataType = datatype;
>>           if (values) {
>> -            COPY_4V(paramList->ParameterValues[oldNum + i], values);
>> +            if (size >= (i+1)*4) {
>> +                COPY_4V(paramList->ParameterValues[oldNum + i], values);
>> +            } else {
>> +                /* silence asan */
>> +                for (j = 0; j < 4; j++) {
>> +                    if (i*4+j < size) {
>> +                        paramList->ParameterValues[oldNum + i][j] =
>> values[i*4+j];
>> +                    } else {
>> +                        paramList->ParameterValues[oldNum + i][j].f =
>> 0.0f;
>> +                    }
>> +                }
>> +            }
>>              values += 4;
>>              p->Initialized = GL_TRUE;
>>           }
>> --
>> 1.7.12.4 (Apple Git-37)
>>
>>
>> On Mon, Jun 17, 2013 at 8:13 AM, Brian Paul <brianp at vmware.com> wrote:
>>
>>> On 06/14/2013 05:12 PM, Myles C. Maxfield wrote:
>>>
>>>> Sorry for the triple post; I received a bounce email the first time and
>>>> got sent to the spam folder the second time, so I'm trying a third time.
>>>>
>>>> Hello, all. I was running Mesa with Address Sanitizer [1] turned on,
>>>> and found one place where ASAN pointed out a read-before-initialized
>>>> problem. In particular, in _mesa_add_parameter, in prog_parameter.c,
>>>> |values| represents an array holding a variable number of values. These
>>>> values get copied out of the array 4 at a time with the COPY_4V macro,
>>>> however, the array might only contain a single element. In this case, ASAN
>>>> reports a read-before-initialize because the last 3 of the 4 elements
>>>> haven't been written to yet. I was hoping to contribute a patch that will
>>>> silence this problem that ASAN reports. I'm happy to incorporate any
>>>> feedback anyone has into this patch.
>>>>
>>>> Thanks,
>>>> Myles C. Maxfield
>>>>
>>>> [1]https://code.google.com/p/**address-sanitizer/<https://code.google.com/p/address-sanitizer/>
>>>>
>>>> diff --git a/src/mesa/program/prog_**parameter.c
>>>> b/src/mesa/program/prog_**parameter.c
>>>> index 2018fa5..63915fb 100644
>>>> --- a/src/mesa/program/prog_**parameter.c
>>>> +++ b/src/mesa/program/prog_**parameter.c
>>>> @@ -158,7 +158,17 @@ _mesa_add_parameter(struct
>>>> gl_program_parameter_list *paramList,
>>>>            p->DataType = datatype;
>>>>            p->Flags = flags;
>>>>            if (values) {
>>>> -            COPY_4V(paramList->**ParameterValues[oldNum + i], values);
>>>> +            if (size & 3) {
>>>> +              for (j = 0; j < size; j++) {
>>>> +                paramList->ParameterValues[**oldNum + i][j] =
>>>> values[j];
>>>> +              }
>>>> +              /* silence asan */
>>>> +              for (j = size; j < 4; j++) {
>>>> +                paramList->ParameterValues[**oldNum + i][j].f = 0;
>>>> +              }
>>>> +            } else {
>>>> +              COPY_4V(paramList->**ParameterValues[oldNum + i],
>>>> values);
>>>> +            }
>>>>               values += 4;
>>>>               p->Initialized = GL_TRUE;
>>>>            }
>>>>
>>>
>>> The value of 'size' can actually be greater than 4 (IIRC, and the
>>> function comment are still correct).  For example, for a matrix, size=16.
>>>  So the first for-loop should be fixed, just to be safe.
>>>
>>> In the commit message, let's not use "ASAN" since it's not obvious that
>>> it means Address Sanitizer.
>>>
>>> -Brian
>>>
>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130628/32b4178d/attachment.html>