<div dir="ltr">Friendly ping regarding this patch :-)<div><br></div><div style>--Myles</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jun 19, 2013 at 12:47 AM, Myles C. Maxfield <span dir="ltr"><<a href="mailto:myles.maxfield@gmail.com" target="_blank">myles.maxfield@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Any word on this?<div><br></div><div>Thanks,</div><div>Myles</div></div><div class="HOEnZb"><div class="h5">
<div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jun 17, 2013 at 12:09 PM, Myles C. Maxfield <span dir="ltr"><<a href="mailto:myles.maxfield@gmail.com" target="_blank">myles.maxfield@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.<div>
<br>
</div><div>Is the only way to replace "ASAN" with "Address Sanitizer" to change the subject of this email thread?</div><div><br></div><div>Anyway, here's a similar but modified patch:</div><div><br>
</div><div><div>From: Myles C. Maxfield <<a href="mailto:mymax@amazon.com" target="_blank">mymax@amazon.com</a>></div><div>Date: Mon, 17 Jun 2013 11:50:05 -0700</div><div>Subject: [PATCH] Appeasing Address Sanitizer</div>
<div><br></div>
<div>---</div><div> src/mesa/program/prog_parameter.c | 13 ++++++++++++-</div><div> 1 file changed, 12 insertions(+), 1 deletion(-)</div><div><br></div><div>diff --git a/src/mesa/program/prog_parameter.c b/src/mesa/program/prog_parameter.c</div>
<div>index 95b153e..1d46476 100644</div><div>--- a/src/mesa/program/prog_parameter.c</div><div>+++ b/src/mesa/program/prog_parameter.c</div><div>@@ -155,7 +155,18 @@ _mesa_add_parameter(struct gl_program_parameter_list *paramList,</div>
<div> p->Size = size;</div><div> p->DataType = datatype;</div><div><div> if (values) {</div><div>- COPY_4V(paramList->ParameterValues[oldNum + i], values);</div></div>
<div>+ if (size >= (i+1)*4) {</div><div>
<div>+ COPY_4V(paramList->ParameterValues[oldNum + i], values);</div></div><div>+ } else {</div><div><div>+ /* silence asan */</div></div><div>+ for (j = 0; j < 4; j++) {</div>
<div>+ if (i*4+j < size) {</div><div>+ paramList->ParameterValues[oldNum + i][j] = values[i*4+j];</div><div>+ } else {</div><div>+ paramList->ParameterValues[oldNum + i][j].f = 0.0f;</div>
<div>+ }</div><div>+ }</div><div><div>+ }</div><div> values += 4;</div><div> p->Initialized = GL_TRUE;</div><div> }</div></div><span><font color="#888888"><div>
-- </div><div>1.7.12.4 (Apple Git-37)</div>
</font></span></div></div><div><div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jun 17, 2013 at 8:13 AM, Brian Paul <span dir="ltr"><<a href="mailto:brianp@vmware.com" target="_blank">brianp@vmware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>On 06/14/2013 05:12 PM, Myles C. Maxfield wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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.<br>
<br>
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.<br>
<br>
Thanks,<br>
Myles C. Maxfield<br>
<br>
[1]<a href="https://code.google.com/p/address-sanitizer/" target="_blank">https://code.google.com/p/<u></u>address-sanitizer/</a><br>
<br>
diff --git a/src/mesa/program/prog_<u></u>parameter.c b/src/mesa/program/prog_<u></u>parameter.c<br>
index 2018fa5..63915fb 100644<br>
--- a/src/mesa/program/prog_<u></u>parameter.c<br>
+++ b/src/mesa/program/prog_<u></u>parameter.c<br>
@@ -158,7 +158,17 @@ _mesa_add_parameter(struct gl_program_parameter_list *paramList,<br>
p->DataType = datatype;<br>
p->Flags = flags;<br>
if (values) {<br>
- COPY_4V(paramList-><u></u>ParameterValues[oldNum + i], values);<br>
+ if (size & 3) {<br>
+ for (j = 0; j < size; j++) {<br>
+ paramList->ParameterValues[<u></u>oldNum + i][j] = values[j];<br>
+ }<br>
+ /* silence asan */<br>
+ for (j = size; j < 4; j++) {<br>
+ paramList->ParameterValues[<u></u>oldNum + i][j].f = 0;<br>
+ }<br>
+ } else {<br>
+ COPY_4V(paramList-><u></u>ParameterValues[oldNum + i], values);<br>
+ }<br>
values += 4;<br>
p->Initialized = GL_TRUE;<br>
}<br>
</blockquote>
<br></div></div>
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.<br>
<br>
In the commit message, let's not use "ASAN" since it's not obvious that it means Address Sanitizer.<span><font color="#888888"><br>
<br>
-Brian<br>
<br>
<br>
<br>
</font></span></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>