isl: Mark enum isl_channel_select packed so it becomes 1 byte.
authorKenneth Graunke <kenneth@whitecape.org>
Fri, 7 Jun 2019 00:36:09 +0000 (17:36 -0700)
committerKenneth Graunke <kenneth@whitecape.org>
Fri, 7 Jun 2019 18:09:44 +0000 (11:09 -0700)
I recently discovered that the following code lead to valgrind errors:

   struct isl_swizzle swizzle = ISL_SWIZZLE_IDENTITY;
   VALGRIND_CHECK_MEM_IS_DEFINED(&swizzle, sizeof(swizzle));

which is surprising, because struct isl_swizzle is simply:

   struct isl_swizzle {
      enum isl_channel_select r:4;
      enum isl_channel_select g:4;
      enum isl_channel_select b:4;
      enum isl_channel_select a:4;
   };

and the above code initializes all of them with a C99 initializer.
Iván Briano reminded me that C99 initializers don't necessarily zero
padding.  A quick inspection revealed that sizeof(struct isl_swizzle)
was 4 (rather than the expected 2).  Ian Romanick suggested changing
it to uint16_t, since this is essentially dicing up an unsigned, and
that worked.

This patch marks enum isl_channel_select packed, changing its size
from 4 bytes to 1 byte.  This then makes struct isl_swizzle 2 bytes,
with no bogus padding fields.  This eliminates valgrind undefined
memory warnings.

These isl_swizzle values become part of our BLORP blit program keys,
which are then hashed.  This undefined padding was being included in
the hashing, possibly leading to issues.  I originally saw this error
when running KHR-GL45.texture_size_promotion.functional in iris under
valgrind.

Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
src/intel/isl/isl.h

index 6790ba002ad2f40aa108c8b6b3e07b1569eee7b8..0218f05d17591dff2e9af7368f22b1b5c7a76254 100644 (file)
@@ -861,7 +861,7 @@ typedef uint8_t isl_channel_mask_t;
 /**
  * @brief A channel select (also known as texture swizzle) value
  */
-enum isl_channel_select {
+enum PACKED isl_channel_select {
    ISL_CHANNEL_SELECT_ZERO = 0,
    ISL_CHANNEL_SELECT_ONE = 1,
    ISL_CHANNEL_SELECT_RED = 4,