Hi users/developers for Mesa,<div><br></div><div>In compiling some of the mesa project in the chromium situation I ran across a compile warning which looks valid and easily fixable.</div><div><br></div><div>I would like to pass along a solution in case it has value to you, either as part of the next release, or as a downstream fix for other users.</div>
<div><br></div><div>The problem shows up as a warning from GCC 4.3 and probably up.</div><div><br></div><div><span class="Apple-style-span" style="font-family: 'Courier New', courier, monotype; white-space: pre; font-size: medium; ">MesaLib/src/egl/main/eglconfig.c: In function _eglCompareConfigs:</span></div>
<meta charset="utf-8"><pre><span class="stdout" style="font-family: 'Courier New', courier, monotype; font-size: medium; ">MesaLib/src/egl/main/eglconfig.h:98: error: array subscript is below array bounds</span><span class="stdout">
</span></pre>This comes about because of the nexted inline functions _eglIndexConfig and _eglGetConfigKey which although in the case of the call site in CompareConfigs are called with valid attributes, the call is nested deeply enough that it appears GCC excercises it's option on not inlining the functions, and compiled without specific values of arguments in optimized mode (assert turns empty) they could access element -1 of the Storage array - hence the warning.<div>
<br></div><div>It seems to me that the smallest kind of fix to this is to move the assert down (since each call to eglIndexConfig is followed by an assert(idx>0) ) and return 0 instead of -1; providing equivalent protection in the debug case and less random behavior in the non-debug case (-01.patch)</div>
<div><br></div><div>Alternately, if it does not affect performance too much, adding "if (idx < 0) return 0;" between the assert and the return in _eglGetConfigKey would achieve deterministic performance and no potential illegal accesses.</div>
<div><br></div><div>A third alternative, which is more intrusive/larger but should not affect performance is to write a macro for the inner portion of the compare loop, and replace the local array with code that simply lists the attribute values in place. (see -02.patch)</div>
<meta charset="utf-8"><div><br></div><div><br></div><div>Alternately, you could disable the warning for the illegal access which would be a bit of a shame, since with the warning the compiler would warn you at compile time if you passed an illegal atribute. About the best time to get such a warning.</div>
<div><br></div><div>For what it's worth, your mileage may vary, etc,</div><div><br></div><div>Peter.</div><div><br></div><div><br></div><div><br></div>