[Mesa-dev] [PATCH v2 05/32] glsl: Zero per_vertex_accumulator::fields for valgrind & nir_serialize

Kenneth Graunke kenneth at whitecape.org
Fri Oct 20 23:14:32 UTC 2017


On Friday, October 20, 2017 4:11:20 PM PDT Kenneth Graunke wrote:
> On Friday, October 20, 2017 2:14:25 PM PDT Jason Ekstrand wrote:
> > On Thu, Oct 19, 2017 at 4:51 PM, Kenneth Graunke <kenneth at whitecape.org>
> > wrote:
> > 
> > > On Wednesday, October 18, 2017 10:31:53 PM PDT Jordan Justen wrote:
> > > > Signed-off-by: Jordan Justen <jordan.l.justen at 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.
> 
> Okay, that makes sense.  In that case, I would drop the fields()
> constructor in the initialization list and just do the memset.
> 
> Also, hope nobody adds a non-zero constructor to glsl_struct_field.

Or, alternatively, just make the glsl_struct_field constructor
initialize the whole struct with a memset, then leave this code be.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171020/24aa99dd/attachment.sig>


More information about the mesa-dev mailing list