Bug 69449

I looked just at source, didn't valgrind piglit, but the invalid read access seems to be done here:

void
program_resource_visitor::recursion(const glsl_type *t, char **name,
                                    size_t name_length, bool row_major,
                                    const glsl_type *record_type)
... 1st call ...
      for (unsigned i = 0; i < t->length; i++) {
...
         recursion(t->fields.structure[i].type, name, new_length,
                   t->fields.structure[i].row_major, record_type);
... 2nd call ...
      for (unsigned i = 0; i < t->length; i++) {
...
         recursion(t->fields.array, name, new_length,
                   t->fields.structure[i].row_major, record_type);


Access is to memory right after the allocation done here:
  const glsl_type *t =
  glsl_type::get_record_instance(fields, decl_count, this->name);
  -> t = new glsl_type(fields, num_fields, name);
    -> this->fields.structure = ralloc_array(this->mem_ctx,
                                         glsl_struct_field, length);
      -> return ralloc_size(ctx, size * count);
        -> type = ralloc_size(glsl_type::mem_ctx, size);
          -> void *block = calloc(1, size + sizeof(ralloc_header));


The allocated type and what's accessed is:
-------------------
struct glsl_type {
        GLenum                     gl_type;              /*     0     4 */
        enum glsl_base_type        base_type;            /*     4     4 */
        unsigned int               sampler_dimensionality:3; /*     8:29  4 */
        unsigned int               sampler_shadow:1;     /*     8:28  4 */
        unsigned int               sampler_array:1;      /*     8:27  4 */
        unsigned int               sampler_type:2;       /*     8:25  4 */
        unsigned int               interface_packing:2;  /*     8:23  4 */
        unsigned int               vector_elements:3;    /*     8:20  4 */
        unsigned int               matrix_columns:3;     /*     8:17  4 */

        /* XXX 17 bits hole, try to pack */
        /* XXX 4 bytes hole, try to pack */

        const char  *              name;                 /*    16     8 */
        unsigned int               length;               /*    24     4 */

        /* XXX 4 bytes hole, try to pack */

        union {
                const class glsl_type  * array;          /*           8 */
                const class glsl_type  * parameters;     /*           8 */
                class glsl_struct_field * structure;     /*           8 */
        } fields;
...
        /* size: 40, cachelines: 1, members: 168 */
        /* sum members: 32, holes: 2, sum holes: 8 */
        /* bit holes: 1, sum bit holes: 17 bits */
        /* last cacheline: 40 bytes */
};
struct glsl_struct_field {
        const class glsl_type  *   type;                 /*     0     8 */
        const char  *              name;                 /*     8     8 */
        bool                       row_major;            /*    16     1 */

        /* size: 24, cachelines: 1, members: 3 */
        /* padding: 7 */
        /* last cacheline: 24 bytes */
};
-----------------------
(Pahole output for 64-bit Mesa build on F19)

And the ralloc_header is:
-------------------------
struct ralloc_header
{
   /* A canary value used to determine whether a pointer is ralloc'd. */
   unsigned canary;

   struct ralloc_header *parent;

   /* The first child (head of a linked list) */
   struct ralloc_header *child;

   /* Linked list of siblings */
   struct ralloc_header *prev;
   struct ralloc_header *next;

   void (*destructor)(void *);
};
---------------------

I.e. assumably 4+4+5*8 = 48 bytes (4 bytes for padding).

48 + 40 byte glsl_type = 88 i.e. it *seems* like only single glsl_type item is allocated and access is to be for start of non-existing next one.


How in this code:
---------------------
      for (unsigned i = 0; i < t->length; i++) {
...
         recursion(t->fields.array, name, new_length,
                   t->fields.structure[i].row_major, record_type);
---------------------

t->length corresponds to number of items in t->fields.structure[], and how that information is kept in sync?


> ==19578== Invalid read of size 1
> ==19578==    at 0xA9E136A: program_resource_visitor::recursion(glsl_type const*, char**, unsigned long, bool, glsl_type const*) (link_uniforms.cpp:144)
...
> ==19578==  Address 0xc3dacc8 is 0 bytes after a block of size 88 alloc'd

I don't know why access is to offset 0 and only for 1 byte, as:
-  at glsl_type offset 0 is 'gl_type' member which is 4 bytes,
- 'structure' offset is 24 and size 8, and
- 1 byte 'row_major' offset is 16 and from glsl_struct_field struct start.

But if we instead make an assumption that either:
1. somehow Valgrind got the allocation size wrong, or
2. Mesa allocated glsl_type and then at some later point assigned
   its address to glsl_struct_field 'structure' member

trying to access 'row_major' in 3rd glsl_struct_field would be:
3*24+16 = 88 bytes.

I would assume 2) to give double free error from Valgrind (or Glibc) when Mesa tries to free structure record and glsl_type memory though.


Asking Valgrind to invoke Gdb and looking at assembly code at that point could help.



Btw. regarding the row_major struct item, as far as I can see, although it's passed as arg, it's not actually needed by the recursion() or different visit_field() members...

This should be fixed in master now [1].

Thanks.

[1] http://cgit.freedesktop.org/mesa/mesa/commit/?id=b3c04362b44a4eceb38c938ceb387a9c04d06973