On 2017-10-20 14:14:25, Jason Ekstrand wrote:
> On Thu, Oct 19, 2017 at 4:51 PM, Kenneth Graunke <kenn...@whitecape.org>
> wrote:
>
> > On Wednesday, October 18, 2017 10:31:53 PM PDT Jordan Justen wrote:
> > > Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com>
> > > ---
> > > src/compiler/glsl/builtin_variables.cpp | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/src/compiler/glsl/builtin_variables.cpp
> > b/src/compiler/glsl/builtin_variables.cpp
> > > index ea2d897cc8..d3cf12475b 100644
> > > --- a/src/compiler/glsl/builtin_variables.cpp
> > > +++ b/src/compiler/glsl/builtin_variables.cpp
> > > @@ -318,6 +318,7 @@ per_vertex_accumulator::per_vertex_accumulator()
> > > : fields(),
> > > num_fields(0)
> > > {
> > > + memset(fields, 0, sizeof(fields));
> > > }
> >
> > This shouldn't fix anything, we're calling the fields() constructor,
> > which should value-initialize every element of the fields array,
> > calling the constructor for glsl_struct_type on each element.
> >
> > That constructor is supposed to zero initialize items. Maybe it is
> > missing some of them?
> >
>
> No, the problem is that, when we do the user data check in the blob code,
> valgrind complains about the padding bytes in-between field elements. C++
> constructors don't initialize that data. That said, I agree that this is a
> tad sketchy. I just wish there were some way to shut up valgrind.
>
I'm now doubting the uninitialized padding explanation. I tried the
memset in the glsl_struct_field constructor as Ken mentioned, but I
also tried adding initializers for the fields in the default
constructor, and it fixed valgrind.
I didn't find anything mentioning that a default constructor on a c++
struct requires the fields to be explicitly initialized. I would think
that each field would be default initialized, the same as for a c++
class.
Anyway, here is alternate patch that fixes valgrind. Is this
preferable to a memset in the constructor? Is either Reviewed-by
worthy?
diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h
index b5e97e638b..0b4a66ca4d 100644
--- a/src/compiler/glsl_types.h
+++ b/src/compiler/glsl_types.h
@@ -1045,6 +1045,13 @@ struct glsl_struct_field {
}
glsl_struct_field()
+ : type(NULL), name(NULL), location(0), offset(0), xfb_buffer(0),
+ xfb_stride(0), interpolation(0), centroid(0),
+ sample(0), matrix_layout(0), patch(0),
+ precision(0), memory_read_only(0),
+ memory_write_only(0), memory_coherent(0), memory_volatile(0),
+ memory_restrict(0), image_format(0), explicit_xfb_buffer(0),
+ implicit_sized_array(0)
{
/* empty */
}
-Jordan
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
