Bug 37351 - [i965 bisected] Stencil read fail
Summary: [i965 bisected] Stencil read fail
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: All All
: medium normal
Assignee: Eric Anholt
QA Contact:
URL:
Whiteboard:
Keywords:
: 37425 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-05-19 03:06 UTC by Ian Romanick
Modified: 2011-05-23 08:01 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Ian Romanick 2011-05-19 03:06:58 UTC
Something about

commit 73f07004811a2522b45179c74cd9a6d6d2e5c578
Author: Eric Anholt <eric@anholt.net>
Date:   Fri Apr 15 19:58:21 2011 -0700

    intel: Use mesa core's R8, RG88, R16, RG1616 RB accessors.
    
    Fixes:
    ARB_texture_rg/fbo-alphatest-formats
    
    Reviewed-by: Brian Paul <brianp@vmware.com>

causes many stencil related tests to fail.  I wanted to cherry-pick this patch to 7.10 since it fixes a known bug.  However, this causes segfaults in numerous piglit tests, including fdo23670-drawpix_stencil, fbo-blit-d24s8, and stencil-drawpixels.  I see the same behavior on master (note: I build without floating-point texture support, FWIW).  fdo23670-drawpix_stencil crashes with:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff63825ac in put_row_s8 (ctx=0x686320, s8rb=0x9c46b0, count=20, x=50, 
    y=50, values=0x7fffffff9960, mask=0x0) at main/depthstencil.c:475
475	               dst[i] = (dst[i] & 0xffffff) | (src[i] << 24);
Missing separate debuginfos, use: debuginfo-install expat-2.0.1-10.fc13.x86_64 freeglut-2.6.0-5.fc14.x86_64 glibc-2.13-1.x86_64 libgcc-4.5.1-4.fc14.x86_64 libjpeg-turbo-1.1.0-2.fc14.x86_64 libstdc++-4.5.1-4.fc14.x86_64 libtiff-3.9.4-1.fc14.x86_64 mesa-libGLU-7.9-5.fc14.x86_64 zlib-1.2.5-2.fc14.x86_64
(gdb) bt
#0  0x00007ffff63825ac in put_row_s8 (ctx=0x686320, s8rb=0x9c46b0, count=20, 
    x=50, y=50, values=0x7fffffff9960, mask=0x0) at main/depthstencil.c:475
#1  0x00007ffff62bfa40 in _swrast_write_stencil_span (ctx=0x686320, n=20, 
    x=50, y=50, stencil=0x7fffffff9960 '\001' <repeats 20 times>)
    at swrast/s_stencil.c:1121
#2  0x00007ffff62b0c07 in draw_stencil_pixels (ctx=0x686320, x=50, y=50, 
    width=20, height=20, type=5121, unpack=0x695fa8, pixels=0x7fffffffdaf0)
    at swrast/s_drawpix.c:347
#3  0x00007ffff62b1e74 in _swrast_DrawPixels (ctx=0x686320, x=50, y=50, 
    width=20, height=20, format=6401, type=5121, unpack=0x695fa8, 
    pixels=0x7fffffffdaf0) at swrast/s_drawpix.c:735
#4  0x00007ffff610676b in intelDrawPixels (ctx=0x686320, x=50, y=50, width=20, 
    height=20, format=6401, type=5121, unpack=0x695fa8, pixels=0x7fffffffdaf0)
    at intel_pixel_draw.c:51
#5  0x00007ffff6383edb in _mesa_DrawPixels (width=20, height=20, format=6401, 
    type=5121, pixels=0x7fffffffdaf0) at main/drawpix.c:96
#6  0x0000000000427ca4 in piglit_display ()
#7  0x0000000000427d55 in display ()
#8  0x0000003e42e20f75 in ?? () from /usr/lib64/libglut.so.3
#9  0x0000003e42e24ab9 in fgEnumWindows () from /usr/lib64/libglut.so.3
#10 0x0000003e42e214da in glutMainLoopEvent () from /usr/lib64/libglut.so.3
#11 0x0000003e42e21db5 in glutMainLoop () from /usr/lib64/libglut.so.3
#12 0x0000000000427efa in main ()

dst is non-NULL, but it points at memory that cannot be read.  This seems to be caused by RowStride being negative.  This is caused by the following code in intel_renderbuffer_map:

   /* Flip orientation if it's the window system buffer */
   if (!rb->Name) {
      rb->Data += rb->RowStride * (irb->region->height - 1) * irb->region->cpp;
      rb->RowStride = -rb->RowStride;
   }

Since all the software accessors flip the Y coordinate, this code seems bogus.  Removing this code eliminates the crashes, but the tests in question still fail (they pass without this patch).

What I really don't understand is why changing the R and RG accessors impacts depth and stencil.
Comment 1 Ian Romanick 2011-05-19 03:37:19 UTC
I take some of the previous comment back.  Further bisecting shows that:

commit 6ab9889a2704304a45b4da5b28840af08f6f42c5
Author: Eric Anholt <eric@anholt.net>
Date:   Fri Apr 15 12:58:17 2011 -0700

    mesa: Use _mesa_get_format_bytes to refactor out the RB get_pointer_*
    
    Reviewed-by: Brian Paul <brianp@vmware.com>

is the real problem.  Of course, it's still not clear to me why this fails.

(gdb) print *dsrb
$2 = {Magic = -1430532899, Mutex = {__data = {__lock = 0, __count = 0, 
      __owner = 0, __nusers = 0, __kind = 0, __spins = 0, __list = {
        __prev = 0x0, __next = 0x0}}, __size = '\000' <repeats 39 times>, 
    __align = 0}, ClassID = 305419896, Name = 0, RefCount = 4, Width = 100, 
  Height = 100, RowStride = -128, Purgeable = 0 '\000', 
  InternalFormat = 34041, _BaseFormat = 34041, Format = 34, 
  NumSamples = 0 '\000', DataType = 34042, Data = 0x7ffff2e40600, 
  Wrapped = 0x9c3600, Delete = 0x7ffff60f5463 <intel_delete_renderbuffer>, 
  AllocStorage = 0x7ffff60f5a19 <intel_alloc_window_storage>, 
  GetPointer = 0x7ffff61f0cd8 <get_pointer_generic>, 
  GetRow = 0x7ffff61f16f2 <get_row_uint>, 
  GetValues = 0x7ffff61f1797 <get_values_uint>, 
  PutRow = 0x7ffff61f1866 <put_row_uint>, PutRowRGB = 0, 
  PutMonoRow = 0x7ffff61f195a <put_mono_row_uint>, 
  PutValues = 0x7ffff61f1a4b <put_values_uint>, 
  PutMonoValues = 0x7ffff61f1b2f <put_mono_values_uint>}
(gdb) print format_info[34]
$3 = {Name = MESA_FORMAT_S8_Z24, 
  StrName = 0x7ffff6416896 "MESA_FORMAT_S8_Z24", BaseFormat = 34041, 
  DataType = 5125, RedBits = 0 '\000', GreenBits = 0 '\000', 
  BlueBits = 0 '\000', AlphaBits = 0 '\000', LuminanceBits = 0 '\000', 
  IntensityBits = 0 '\000', IndexBits = 0 '\000', DepthBits = 24 '\030', 
  StencilBits = 8 '\b', BlockWidth = 1 '\001', BlockHeight = 1 '\001', 
  BytesPerBlock = 4 '\004'}

So get_pointer_generic should produce the same result as the old get_pointer_uint.
Comment 2 Jose Fonseca 2011-05-20 17:33:15 UTC
*** Bug 37425 has been marked as a duplicate of this bug. ***
Comment 3 Adam Jackson 2011-05-23 08:01:37 UTC
commit e8b1c6d6f55f5be3bef25084fdd8b6127517e137
Author: Adam Jackson <ajax@redhat.com>
Date:   Fri May 20 18:21:15 2011 -0400

    mesa: Fix return type of  _mesa_get_format_bytes() (#37351)
    
    Despite that negative values aren't sensible here, making this unsigned
    is dangerous.  Consider get_pointer_generic, which computes a value of
    the form:
    
        void *base + (int x * int stride + int y) * unsigned bpp
    
    The usual arithmetic conversions will coerce the (x*stride + y)
    subexpression to unsigned.  Since stride can be negative, this is
    disastrous.
    
    Fixes at least the following piglit tests on Ironlake:
    
        fbo/fbo-blit-d24s8
        spec/ARB_depth_texture/fbo-clear-formats
        spec/EXT_packed_depth_stencil/fbo-clear-formats
    
    NOTE: This is a candidate for the 7.10 branch.

    Reviewed-by: Chad Versace <chad.versace@intel.com>
    Signed-off-by: Adam Jackson <ajax@redhat.com>


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.