<div dir="ltr">Apologies folks, I had made those changes and somehow grabbed the wrong patch when I went to add the bug reference. Ugh.<div><br></div><div>This time for sure! I'll follow this with the -- real -- patch.</div>
<div><br></div><div>Courtney</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Apr 8, 2014 at 1:28 AM, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 04/07/2014 09:20 PM, Courtney Goeltzenleuchter wrote:<br>
> Decompressing ETC2 textures was causing intermitent segfault<br>
> by copying resulting 4x4 texel block to the destination texture<br>
> regardless of the size of the destination texture. Issue found<br>
> via application crash in GLBenchmark 3.0's Manhattan test.<br>
><br>
> v2: add more explanatory comments<br>
> v3: add bugzilla reference<br>
> v4: Correct cc syntax in commit log<br>
><br>
> Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=74988" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=74988</a><br>
> Cc: "9.2 10.0 10.1" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>><br>
> ---<br>
> src/mesa/main/texcompress_etc.c | 49 +++++++++++++++++++++--------------------<br>
> 1 file changed, 25 insertions(+), 24 deletions(-)<br>
><br>
> diff --git a/src/mesa/main/texcompress_etc.c b/src/mesa/main/texcompress_etc.c<br>
> index cbda689..39775e2 100644<br>
> --- a/src/mesa/main/texcompress_etc.c<br>
> +++ b/src/mesa/main/texcompress_etc.c<br>
> @@ -683,9 +683,10 @@ etc2_unpack_rgb8(uint8_t *dst_row,<br>
> etc2_rgb8_parse_block(&block, src,<br>
> false /* punchthrough_alpha */);<br>
><br>
> - for (j = 0; j < bh; j++) {<br>
> + /* be sure to stay within the bounds of the texture */<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_rgb8_fetch_texel(&block, i, j, dst,<br>
> false /* punchthrough_alpha */);<br>
> dst[3] = 255;<br>
> @@ -720,9 +721,9 @@ etc2_unpack_srgb8(uint8_t *dst_row,<br>
> etc2_rgb8_parse_block(&block, src,<br>
> false /* punchthrough_alpha */);<br>
><br>
> - for (j = 0; j < bh; j++) {<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_rgb8_fetch_texel(&block, i, j, dst,<br>
> false /* punchthrough_alpha */);<br>
> /* Convert to MESA_FORMAT_B8G8R8A8_SRGB */<br>
> @@ -763,9 +764,9 @@ etc2_unpack_rgba8(uint8_t *dst_row,<br>
> for (x = 0; x < width; x+= bw) {<br>
> etc2_rgba8_parse_block(&block, src);<br>
><br>
> - for (j = 0; j < bh; j++) {<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_rgba8_fetch_texel(&block, i, j, dst);<br>
> dst += comps;<br>
> }<br>
> @@ -800,9 +801,9 @@ etc2_unpack_srgb8_alpha8(uint8_t *dst_row,<br>
> for (x = 0; x < width; x+= bw) {<br>
> etc2_rgba8_parse_block(&block, src);<br>
><br>
> - for (j = 0; j < bh; j++) {<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_rgba8_fetch_texel(&block, i, j, dst);<br>
><br>
> /* Convert to MESA_FORMAT_B8G8R8A8_SRGB */<br>
> @@ -842,9 +843,9 @@ etc2_unpack_r11(uint8_t *dst_row,<br>
> for (x = 0; x < width; x+= bw) {<br>
> etc2_r11_parse_block(&block, src);<br>
><br>
> - for (j = 0; j < bh; j++) {<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps * comp_size;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_r11_fetch_texel(&block, i, j, dst);<br>
> dst += comps * comp_size;<br>
> }<br>
> @@ -878,10 +879,10 @@ etc2_unpack_rg11(uint8_t *dst_row,<br>
> /* red component */<br>
> etc2_r11_parse_block(&block, src);<br>
><br>
> - for (j = 0; j < bh; j++) {<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride +<br>
> x * comps * comp_size;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_r11_fetch_texel(&block, i, j, dst);<br>
> dst += comps * comp_size;<br>
> }<br>
> @@ -889,10 +890,10 @@ etc2_unpack_rg11(uint8_t *dst_row,<br>
> /* green component */<br>
> etc2_r11_parse_block(&block, src + 8);<br>
><br>
> - for (j = 0; j < bh; j++) {<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride +<br>
> x * comps * comp_size;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_r11_fetch_texel(&block, i, j, dst + comp_size);<br>
> dst += comps * comp_size;<br>
> }<br>
> @@ -925,10 +926,10 @@ etc2_unpack_signed_r11(uint8_t *dst_row,<br>
> for (x = 0; x < width; x+= bw) {<br>
> etc2_r11_parse_block(&block, src);<br>
><br>
> - for (j = 0; j < bh; j++) {<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride +<br>
> x * comps * comp_size;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_signed_r11_fetch_texel(&block, i, j, dst);<br>
> dst += comps * comp_size;<br>
> }<br>
> @@ -962,10 +963,10 @@ etc2_unpack_signed_rg11(uint8_t *dst_row,<br>
> /* red component */<br>
> etc2_r11_parse_block(&block, src);<br>
><br>
> - for (j = 0; j < bh; j++) {<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride +<br>
> x * comps * comp_size;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_signed_r11_fetch_texel(&block, i, j, dst);<br>
> dst += comps * comp_size;<br>
> }<br>
> @@ -973,10 +974,10 @@ etc2_unpack_signed_rg11(uint8_t *dst_row,<br>
> /* green component */<br>
> etc2_r11_parse_block(&block, src + 8);<br>
><br>
> - for (j = 0; j < bh; j++) {<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride +<br>
> x * comps * comp_size;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_signed_r11_fetch_texel(&block, i, j, dst + comp_size);<br>
> dst += comps * comp_size;<br>
> }<br>
> @@ -1006,9 +1007,9 @@ etc2_unpack_rgb8_punchthrough_alpha1(uint8_t *dst_row,<br>
> for (x = 0; x < width; x+= bw) {<br>
> etc2_rgb8_parse_block(&block, src,<br>
> true /* punchthrough_alpha */);<br>
> - for (j = 0; j < bh; j++) {<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_rgb8_fetch_texel(&block, i, j, dst,<br>
> true /* punchthrough_alpha */);<br>
> dst += comps;<br>
> @@ -1041,9 +1042,9 @@ etc2_unpack_srgb8_punchthrough_alpha1(uint8_t *dst_row,<br>
> for (x = 0; x < width; x+= bw) {<br>
> etc2_rgb8_parse_block(&block, src,<br>
> true /* punchthrough_alpha */);<br>
> - for (j = 0; j < bh; j++) {<br>
> + for (j = 0; j < bh && (j+y) < height; j++) {<br>
> uint8_t *dst = dst_row + (y + j) * dst_stride + x * comps;<br>
> - for (i = 0; i < bw; i++) {<br>
> + for (i = 0; i < bw && (i+x) < width; i++) {<br>
> etc2_rgb8_fetch_texel(&block, i, j, dst,<br>
> true /* punchthrough_alpha */);<br>
> /* Convert to MESA_FORMAT_B8G8R8A8_SRGB */<br>
><br>
<br>
</div></div>As is,<br>
Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
<br>
I do like Ian's suggestion (from 2014-02-14), where he suggested doing:<br>
<div class=""><br>
const unsigned h = MIN2(bh, height - y);<br>
const unsigned w = MIN2(bw, width - x);<br>
<br>
...<br>
<br>
</div> for (j = 0; j < h; j++) {<br>
<br>
...<br>
<br>
for (i = 0; i < w; i++) {<br>
<br>
That keeps the loop termination conditions as a simple comparison,<br>
rather than two comparisons, an add, and a logical-and.<br>
<br>
--Ken<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr">Courtney Goeltzenleuchter<br><div>LunarG</div><div><img src="http://media.lunarg.com/wp-content/themes/LunarG/images/logo.png" width="96" height="65"><br>
</div></div>
</div>