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

[Myles C. Maxfield]

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/20130617/42115ecc/attachment.html>