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

[Sergii Romantsov]

Ping?

On Fri, Jan 18, 2019 at 6:15 PM Sergii Romantsov <sergii.romantsov at gmail.com>
wrote:

> 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/20190305/ee82e329/attachment.html>