[Mesa-dev] [PATCH 2/4] glsl: Stop being clever with pointer arithmetic when fetching types.

Ian Romanick idr at freedesktop.org
Tue Jun 18 06:47:12 PDT 2013


On 06/18/2013 04:22 AM, Kenneth Graunke wrote:
> Currently, vector types are linked together closely: the glsl_type
> objects for float, vec2, vec3, and vec4 are all elements of the same
> array, in that exact order.  This makes it possible to obtain vector
> types via pointer arithmetic on the scalar type's convenience pointer.
> For example, float_type + (3 - 1) = vec3.

My recollection, is that this wasn't originally "clever" (aka bong 
hits).  Originally, we had something like

     const glsl_type *int_type[4];

int_type[0] pointed to the type for int, int_type[1] pointed to the type 
for ivec2, etc.  Many places in the code got the arithmetic wrong, so we 
added the ivec2_type fields, and so on, and so on... Fast forward a 
couple years, and now we have madness.

> However, relying on this is extremely fragile.  There's no particular
> reason the underlying type objects need to be stored in an array.  They
> could be individual class members, possibly with padding between them.
> Then the pointer arithmetic would break, and we'd get bad pointers to
> non-heap allocated data, causing subtle breakage that can't be detected
> by valgrind.  Cue insanity.
>
> Or someone could simply reorder the type variables, causing us to get
> the wrong type entirely.  Also cue insanity.
>
> Writing this explicitly is much safer.  With the new helper functions,
> it's a bit less code even.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>   src/glsl/glsl_types.cpp | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
> index a5491c5..d1d609a 100644
> --- a/src/glsl/glsl_types.cpp
> +++ b/src/glsl/glsl_types.cpp
> @@ -600,13 +600,13 @@ glsl_type::get_instance(unsigned base_type, unsigned rows, unsigned columns)
>      if (columns == 1) {
>         switch (base_type) {
>         case GLSL_TYPE_UINT:
> -	 return uint_type + (rows - 1);
> +	 return uvec(rows);
>         case GLSL_TYPE_INT:
> -	 return int_type + (rows - 1);
> +	 return ivec(rows);
>         case GLSL_TYPE_FLOAT:
> -	 return float_type + (rows - 1);
> +	 return vec(rows);
>         case GLSL_TYPE_BOOL:
> -	 return bool_type + (rows - 1);
> +	 return bvec(rows);
>         default:
>   	 return error_type;
>         }
>



More information about the mesa-dev mailing list