[Mesa-dev] [PATCH 1/2] intel/isl: Add bounds-checking assertions in isl_format_get_layout

[Jason Ekstrand]

On Wed, Jun 6, 2018 at 10:25 AM, Lionel Landwerlin <
lionel.g.landwerlin at intel.com> wrote:

> On 06/06/18 18:17, Jason Ekstrand wrote:
>
>> We add two assertions instead of one because the first assertion that
>> format != ISL_FORMAT_UNSUPPORTED is more descriptive and checks for a
>> real but unsupported enumerant while the second ensures that they don't
>> pass in garbage values.  We also update some other helpers to use
>> isl_format_get_layout instead of using the table directly so that they
>> get bounds checking too.
>>
>> Cc: mesa-stable at lists.freedesktop.org
>> ---
>>   src/intel/isl/isl.h | 32 +++++++++++++++++++++-----------
>>   1 file changed, 21 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/intel/isl/isl.h b/src/intel/isl/isl.h
>> index 00cfe31fc04..6800b1d76a6 100644
>> --- a/src/intel/isl/isl.h
>> +++ b/src/intel/isl/isl.h
>> @@ -389,6 +389,9 @@ enum isl_format {
>>      ISL_FORMAT_GEN9_CCS_64BPP,
>>      ISL_FORMAT_GEN9_CCS_128BPP,
>>   +   /* An upper bound on the supported format enumerations */
>> +   ISL_NUM_FORMATS,
>> +
>>      /* Hardware doesn't understand this out-of-band value */
>>      ISL_FORMAT_UNSUPPORTED =                             UINT16_MAX,
>>   };
>> @@ -1423,6 +1426,8 @@ isl_device_get_sample_counts(struct isl_device
>> *dev);
>>   static inline const struct isl_format_layout * ATTRIBUTE_CONST
>>   isl_format_get_layout(enum isl_format fmt)
>>   {
>> +   assert(fmt != ISL_FORMAT_UNSUPPORTED);
>> +   assert(fmt < ISL_NUM_FORMATS);
>>      return &isl_format_layouts[fmt];
>>   }
>>   @@ -1431,7 +1436,7 @@ bool isl_format_is_valid(enum isl_format);
>>   static inline const char * ATTRIBUTE_CONST
>>   isl_format_get_name(enum isl_format fmt)
>>   {
>> -   return isl_format_layouts[fmt].name;
>> +   return isl_format_get_layout(fmt)->name;
>>   }
>>     bool isl_format_supports_rendering(const struct gen_device_info
>> *devinfo,
>> @@ -1546,7 +1551,7 @@ isl_format_block_is_1x1x1(enum isl_format fmt)
>>   static inline bool
>>   isl_format_is_srgb(enum isl_format fmt)
>>   {
>> -   return isl_format_layouts[fmt].colorspace == ISL_COLORSPACE_SRGB;
>> +   return isl_format_get_layout(fmt)->colorspace == ISL_COLORSPACE_SRGB;
>>   }
>>     enum isl_format isl_format_srgb_to_linear(enum isl_format fmt);
>> @@ -1556,20 +1561,25 @@ isl_format_is_rgb(enum isl_format fmt)
>>   {
>>      if (isl_format_is_yuv(fmt))
>>         return false;
>> -   return isl_format_layouts[fmt].channels.r.bits > 0 &&
>> -          isl_format_layouts[fmt].channels.g.bits > 0 &&
>> -          isl_format_layouts[fmt].channels.b.bits > 0 &&
>> -          isl_format_layouts[fmt].channels.a.bits == 0;
>> +
>> +   const struct isl_format_layout *fmtl = isl_format_get_layout(fmt);
>> +
>> +   return fmtl->channels.r.bits > 0 &&
>> +          fmtl->channels.g.bits > 0 &&
>> +          fmtl->channels.b.bits > 0 &&
>> +          fmtl->channels.a.bits == 0;
>>   }
>>     static inline bool
>>   isl_format_is_rgbx(enum isl_format fmt)
>>   {
>> -   return isl_format_layouts[fmt].channels.r.bits > 0 &&
>> -          isl_format_layouts[fmt].channels.g.bits > 0 &&
>> -          isl_format_layouts[fmt].channels.b.bits > 0 &&
>> -          isl_format_layouts[fmt].channels.a.bits > 0 &&
>> -          isl_format_layouts[fmt].channels.a.type == ISL_VOID;
>> +   const struct isl_format_layout *fmtl = isl_format_get_layout(fmt);
>> +
>> +   return fmtl->channels.r.bits > 0 &&
>> +          fmtl->channels.g.bits > 0 &&
>> +          fmtl->channels.b.bits > 0 &&
>> +          fmtl->channels.a.bits > 0 &&
>> +          fmtl->channels.a.type == ISL_VOID;
>>   }
>>     enum isl_format isl_format_rgb_to_rgba(enum isl_format rgb)
>> ATTRIBUTE_CONST;
>>
> Do you want to use it in isl_buffer_fill_image_param() too?
>

Good call.  I've made that change locally.


> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
>

Thanks!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180606/88de9ebf/attachment-0001.html>