[Piglit] [PATCH v2 1/2] test: fix core dump for depthstencil-render-miplevels

[Sergii Romantsov]

Hello, Tapani, Eric.

No worries, I'm OK with using this patch too.


Can it be treated as r-b?

On Wed, Jan 16, 2019 at 11:06 AM Tapani Pälli <tapani.palli at intel.com>
wrote:

>
>
> On 1/15/19 5:02 PM, Sergii Romantsov wrote:
> > Hello,
> >
> >     Yep, it seems valgrind outputs bunch of invalid reads and writes with
> >     this test.
> >
> > Seems any test with valgrind generates invalid reads and writes...
> > Probably it depends on macros-usage: probably they are not properly
> > handled by valgrind
>
> Yeah this may be but this time it points directly to the test code as well.
>
> >     It just seems a bit strange to first calculate a 'maximum'
> >     miplevel but then use even bigger value in the test :)
> >
> > I would say that max level is calculated properly, but in test also used
> > 0-level, which seems included as +1 to max_miplevel
> >
> >     Alternatively I guess one could change those loops to not exceed max
> >     level, use < operator instead of <= (?)
> >
> > If no more any objections - will update to '<'
>
> No worries, I'm OK with using this patch too.
>
> > On Tue, Jan 15, 2019 at 3:07 PM Tapani Pälli <tapani.palli at intel.com
> > <mailto:tapani.palli at intel.com>> wrote:
> >
> >
> >     On 1/14/19 12:51 PM, Sergii Romantsov wrote:
> >      > Usage test 'depthstencil-render-miplevels 200 s=z24_s8' causes
> >      > core dump on exit.
> >      > Fixed: array of pointers is allocated to size max_miplevel, but
> >      > used as max_miplevel + 1.
> >
> >     Yep, it seems valgrind outputs bunch of invalid reads and writes with
> >     this test.
> >
> >     Alternatively I guess one could change those loops to not exceed max
> >     level, use < operator instead of <= (?) I'm not sure if other
> >     calculations would match so some careful investigation would be
> >     required
> >     there. It just seems a bit strange to first calculate a 'maximum'
> >     miplevel but then use even bigger value in the test :)
> >
> >
> >      > CC: Eric Anholt <eric at anholt.net <mailto:eric at anholt.net>>
> >      > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108819
> >      > Fixes: 7a0e61d7792f (depthstencil-render-miplevels: Present the
> >     results in non-auto mode.)
> >      > Signed-off-by: Sergii Romantsov <sergii.romantsov at globallogic.com
> >     <mailto:sergii.romantsov at globallogic.com>>
> >      > ---
> >      >   tests/texturing/depthstencil-render-miplevels.cpp | 4 ++--
> >      >   1 file changed, 2 insertions(+), 2 deletions(-)
> >      >
> >      > diff --git a/tests/texturing/depthstencil-render-miplevels.cpp
> >     b/tests/texturing/depthstencil-render-miplevels.cpp
> >      > index 87af92f..9d4b42d 100644
> >      > --- a/tests/texturing/depthstencil-render-miplevels.cpp
> >      > +++ b/tests/texturing/depthstencil-render-miplevels.cpp
> >      > @@ -360,8 +360,8 @@ piglit_init(int argc, char **argv)
> >      >       piglit_require_extension("GL_ARB_depth_texture");
> >      >       piglit_require_extension("GL_ARB_texture_non_power_of_two");
> >      >
> >      > -     depth_miplevel_data = (float **)calloc(max_miplevel,
> >     sizeof(float *));
> >      > -     stencil_miplevel_data = (uint8_t **)calloc(max_miplevel,
> >      > +     depth_miplevel_data = (float **)calloc(max_miplevel + 1,
> >     sizeof(float *));
> >      > +     stencil_miplevel_data = (uint8_t **)calloc(max_miplevel + 1,
> >      >                                                  sizeof(uint8_t
> *));
> >      >
> >      >       /* argv[2]: buffer combination */
> >      >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20190118/01148ba1/attachment.html>