Bug 31837

Summary: Valgrind error due to alpha channel being left uninitialized when sampling 24bit RGB textures
Product: Mesa Reporter: Benoit Jacob <bjacob>
Component: Mesa coreAssignee: mesa-dev
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium    
Version: 7.8   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: patch

Description Benoit Jacob 2010-11-22 07:11:27 UTC
This comes from https://bugzilla.mozilla.org/show_bug.cgi?id=588918, see especially comments 16 and 18 there.

I have only tried with Mesa 7.8. Source code references below are against Mesa 7.8.2.

*** Description of the problem ***

Running under valgrind a program that does a glDrawArrays call using a GL_RGB8 texture, I get this error:

==14218== Conditional jump or move depends on uninitialised value(s)
==14218==    at 0x2111134F: _mesa_convert_colors (image.c:5421)
==14218==    by 0x21176CBA: convert_color_type (s_span.c:941)
==14218==    by 0x211784B4: _swrast_write_rgba_span (s_span.c:1226)
==14218==    by 0x21187818: general_triangle (s_tritemp.h:819)
==14218==    by 0x21153E6C: _tnl_render_triangles_verts (t_vb_rendertmp.h:184)
==14218==    by 0x211554B1: run_render (t_vb_render.c:320)
==14218==    by 0x2114D5F6: _tnl_run_pipeline (t_pipeline.c:153)
==14218==    by 0x2114E262: _tnl_draw_prims (t_draw.c:478)
==14218==    by 0x2114E340: _tnl_vbo_draw_prims (t_draw.c:384)
==14218==    by 0x21146FB0: vbo_exec_DrawArrays (vbo_exec_array.c:524)
==14218==    by 0x5A81AC8: mozilla::gl::GLContext::fDrawArrays(unsigned int,
int, int) (GLContext.h:1185)
==14218==    by 0x5A670B0: mozilla::WebGLContext::DrawArrays(unsigned int, int,
int) (WebGLContextGL.cpp:1075)
==14218==  Uninitialised value was created by a stack allocation
==14218==    at 0x211699D8: fetch_texel (prog_execute.c:373)

Stepping in GDB through fetch_texel(), I arrive at this location:
   opt_sample_rgb_2d at swrast/s_texfilter.c:1366

The code there does:

   for (k=0; k<n; k++) {
      GLint i = IFLOOR(texcoords[k][0] * width) & colMask;
      GLint j = IFLOOR(texcoords[k][1] * height) & rowMask;
      GLint pos = (j << shift) | i;
      GLubyte *texel = ((GLubyte *) img->Data) + 3*pos;
      rgba[k][RCOMP] = UBYTE_TO_FLOAT(texel[2]);
      rgba[k][GCOMP] = UBYTE_TO_FLOAT(texel[1]);
      rgba[k][BCOMP] = UBYTE_TO_FLOAT(texel[0]);
   }

It seems that it's leaving the alpha channel uninitialized.

When we look at the location of the first reported valgrind error using that
uninitialized value, _mesa_convert_colors (image.c:5421), the source code is:


   case GL_FLOAT:
      if (dstType == GL_UNSIGNED_BYTE) {
         const GLfloat (*src4)[4] = (const GLfloat (*)[4]) src;
         GLubyte (*dst1)[4] = (GLubyte (*)[4]) (useTemp ? tempBuffer : dst);
         GLuint i;
         for (i = 0; i < count; i++) {
            if (!mask || mask[i]) {
               UNCLAMPED_FLOAT_TO_UBYTE(dst1[i][RCOMP], src4[i][RCOMP]);
               UNCLAMPED_FLOAT_TO_UBYTE(dst1[i][GCOMP], src4[i][GCOMP]);
               UNCLAMPED_FLOAT_TO_UBYTE(dst1[i][BCOMP], src4[i][BCOMP]);
               UNCLAMPED_FLOAT_TO_UBYTE(dst1[i][ACOMP], src4[i][ACOMP]);
            }
         }
         if (useTemp)
            memcpy(dst, tempBuffer, count * 4 * sizeof(GLubyte));
      }


The error line 5421 is the one converting the alpha channel:
               UNCLAMPED_FLOAT_TO_UBYTE(dst1[i][ACOMP], src4[i][ACOMP]);

This confirms the theory that the valgrind error is caused by the alpha value here being uninitialized.
Comment 1 Benoit Jacob 2010-11-22 07:17:42 UTC
Created attachment 40478 [details] [review]
patch

I guess that this 1-line patch should fix it, but haven't tried.
Comment 2 Brian Paul 2010-11-22 08:05:28 UTC
That looks like the right fix.  I've committed it (6a0255122a7d7c0aa09bceacda90a5cea67d5ee2) to master and the 7.9 branch.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.