[Mesa-dev] [PATCH] intel: fix null 1D texture handling

[Yuanhan Liu]

On Thu, Sep 08, 2011 at 01:16:23PM -0700, Ian Romanick wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 09/08/2011 03:16 AM, Yuanhan Liu wrote:
> > If user call glTexImage1D with width = 0 and height = 0(the last
> > level) like this: glTexImage1D(GL_TEXTURE_1D, level, interfomart,
> > 0, 0, format, type, pixel)
> > 
> > It would generate a SIGSEGV fault. As i945_miptree_layout_2d didn't
> > handle this special case. More info are commented in line in this
> > patch.
> > 
> > This would fix the oglc divzero(basic.texQOrWEqualsZero) and 
> > divzero(basic.texTrivialPrim) test case fail.
> > 
> > Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
> 
> This feels like a band-aid.  Can this problem also occur on pre-i945
> hardware (that use i915_miptree_layout_2d)?
Finally find a 915gm machine. But stuff totally broken on that machine.
That give me no chance to test it. Will test it when broken fixed.

> 
> > --- src/mesa/drivers/dri/intel/intel_tex_layout.c |   12
> > ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/intel/intel_tex_layout.c
> > b/src/mesa/drivers/dri/intel/intel_tex_layout.c index
> > 9d81523..4ec71c2 100644 ---
> > a/src/mesa/drivers/dri/intel/intel_tex_layout.c +++
> > b/src/mesa/drivers/dri/intel/intel_tex_layout.c @@ -61,6 +61,18 @@
> > void i945_miptree_layout_2d(struct intel_context *intel, GLuint
> > width = mt->width0; GLuint height = mt->height0;
> > 
> > +   if (width == 0 && _mesa_get_texture_dimensions(mt->target) ==
> > 1) {
> 
> Does the dimensionality of the texture target actually matter?  It

In fact, it's not about the texture target, it's about the way how to 
detect it is a null texture(width=0, height=0) or not. A user might call 
glTexImage1D(1D, ... width=0, ..height=0 ..), but mesa would set the width
to 1 later at _mesa_TexImage1D. That result i945_miptree_layout_2d would
return a mt with total_height = 2 = ALIGN(height = 1, 2).

Here is how the oglc divzero(basic.texQOrWEqualsZero) trigger this
segfalut error:

It calls:
    glTexImage1D(GL_TEXTURE_1D, 0, 3, 8, 0, GL_RGB, GL_FLOAT, pixel);
    glTexImage1D(GL_TEXTURE_1D, 1, 3, 4, 0, GL_RGB, GL_FLOAT, pixel);
    glTexImage1D(GL_TEXTURE_1D, 2, 3, 2, 0, GL_RGB, GL_FLOAT, pixel);
    glTexImage1D(GL_TEXTURE_1D, 3, 3, 1, 0, GL_RGB, GL_FLOAT, pixel);
    glTexImage1D(GL_TEXTURE_1D, 4, 3, 0, 0, GL_RGB, GL_FLOAT, pixel);

At the first glTexImage1D call, it would generate a mt with last_level
equal to 3. This would make the later three glTexImage1D call safety.

But when it calling the last glTexImage1D call, the intelTexImage didn't
find a match mt for it, as the level is 4, but the prev mt just with
last_level equal to 3. It then will try to allocate a new mt by calling
intel_miptree_create_for_teximage. It should return NULL as this a NULL
texture. But i915_miptree_layout_2d would set the mt->total_height to 2
instead of 0, that make intel_miptree_create_for_teximage return a
non-NULL mt.

In the same intelTexImage function, it then would call 
texImage->Data = intel_miptree_image_map(..) to prepare for submitting
data. Then it would trigger a segfalt error as it's going to access
mt->level[4].x_offset[] at intel_miptree_get_image_offset, as we didn't
setup info for level[4] before.


> seems like we would want to bailout whenever width (or height) is zero.
> If the dimensionality does matter, calling
> _mesa_get_texture_dimensions is wrong.  It will return 2 for
> GL_TEXTURE_1D_ARRAY, and I think we want to treat 1D texture arrays
> the same as non-array 1D textures.
Thanks for inforamtion.

> 
> > +      /* +       * This is the _real_ last level of 1D texture
> > with width equal +       * to 0. Just simply set total_height to 0
> > to indicate this is +       * a NULL texture. Or it will get a
> > value of ALIGN(1, aligh_h), +       * which equals to 2, as mesa
> > would set the value of _height_ +       * to 1 in 1D texture. +
> > */ +      mt->total_height = 0; +      return; +   } + 
> > mt->total_width = mt->width0;
> 
> I see that mt->total_height gets set to zero below.  I'm move that
> assignment and the mt->total_width = mt->width0 assignment above the
> 'if (width == 0)' condition.

Yes.

While writting this email, I thought another patch to fix this issue.
Will send it later.

Thanks,
Yuanhan Liu
> 
> > intel_get_texture_alignment_unit(mt->format, &align_w, &align_h);
> > 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
> 
> iEYEARECAAYFAk5pIpcACgkQX1gOwKyEAw9RogCcDK8vXR2bfcjHS4sb24SS8g54
> 8nQAnRcISXoA+oeNkNqoxEvgzOpmJ8Wa
> =nT5n
> -----END PGP SIGNATURE-----