On Mon, 2016-06-06 at 16:02 -0700, Kenneth Graunke wrote:
> On Monday, June 6, 2016 3:56:09 PM PDT Mark Janes wrote:
> > 
> > Kenneth Graunke <kenn...@whitecape.org> writes:
> > 
> > > 
> > > The new ARB_vertex_attrib_64bit tests specify integer uniform
> > > values
> > > as hex, such as 0xc21620c5.  As an integer value, this is beyond
> > > LONG_MAX
> > > on 32-bit systems.  The intent is to parse it as an unsigned hex
> > > value and
> > > bitcast it.
> > > 
> > > However, we still need to handle parsing values with negative
> > > signs.
> > > 
> > > Using strtoll and truncating works.
> > > 
> > > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> > > ---
> > >  tests/shaders/shader_runner.c | 2 +-
> > >  tests/util/piglit-vbo.cpp     | 4 ++--
> > >  2 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tests/shaders/shader_runner.c
> > > b/tests/shaders/shader_runner.c
> > > index 94c7826..56fd97c 100644
> > > --- a/tests/shaders/shader_runner.c
> > > +++ b/tests/shaders/shader_runner.c
> > > @@ -1398,7 +1398,7 @@ get_ints(const char *line, int *ints,
> > > unsigned count)
> > >   unsigned i;
> > >  
> > >   for (i = 0; i < count; i++)
> > > -         ints[i] = strtol(line, (char **) &line, 0);
> > > +         ints[i] = strtoll(line, (char **) &line, 0);
> > >  }
> > >  
> > >  
> > > diff --git a/tests/util/piglit-vbo.cpp b/tests/util/piglit-
> > > vbo.cpp
> > > index fd7e72a..50e6731 100644
> > > --- a/tests/util/piglit-vbo.cpp
> > > +++ b/tests/util/piglit-vbo.cpp
> > > @@ -387,8 +387,8 @@ vertex_attrib_description::parse_datum(const
> > > char **text, void *data) const
> > >           break;
> > >   }
> > >   case GL_INT: {
> > > -         long value = (long) strtoll(*text, &endptr, 0);
> > > -         if (errno == ERANGE) {
> > > +         long long value = strtoll(*text, &endptr, 0);
> > > +         if (errno == ERANGE || (unsigned long long)
> > > value > 0xFFFFFFFFull) {
> >                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > ^^^^^^
> > with this check removed, the series corrects all 32 bit failures
> > introduced by b7eb469, and is
> > 
> > Tested-by: Mark Janes <mark.a.ja...@intel.com>
> Right, the value > 0xFFFFFFFFull bit is bogus - the sign bit gets
> extended.  I've just dropped this locally.  It removes a bit of
> sanity
> checking for bogus values written in .shader_test files, but we don't
> sanity check most values, either.

Ugh! Sorry for this bug.

With that change, it LGTM.

I'm still working in these tests so I will come with a follow-up patch
to handle the correct parsing of the values with negative sign.

Reviewed-by: Andres Gomez <ago...@igalia.com>
-- 
-- 
Br,

Andres


_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to