I took a closer look at your patch and I think it's incorrect.
Note that the 'values' pointer is incremented by 4 in each loop
iteration and size is decremented by 4. So accessing values[i*4+j] will
eventually go out of bounds.
I think something like this would work.
diff --git a/src/mesa/program/prog_parameter.c
b/src/mesa/program/prog_parameter.c
index 95b153e..1b16feb 100644
--- a/src/mesa/program/prog_parameter.c
+++ b/src/mesa/program/prog_parameter.c
@@ -155,7 +155,17 @@ _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 >= 4) {
+ COPY_4V(paramList->ParameterValues[oldNum + i], values);
+ }
+ else {
+ for (j = 0; j < size; j++) {
+ paramList->ParameterValues[oldNum + i][j] = values[j];
+ }
+ for (; j < 4; j++) {
+ paramList->ParameterValues[oldNum + i][j] = 0.0f;
+ }
+ }
values += 4;
p->Initialized = GL_TRUE;
}
-Brian
On 06/19/2013 01:47 AM, Myles C. Maxfield wrote:
Any word on this?
Thanks,
Myles
On Mon, Jun 17, 2013 at 12:09 PM, Myles C. Maxfield
<myles.maxfi...@gmail.com <mailto:myles.maxfi...@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 <my...@amazon.com <mailto:my...@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 <bri...@vmware.com
<mailto:bri...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
