[Mesa-dev] [PATCH] gallium/{r600, radeonsi}: Fix segfault with color format

[Денис Паук]

Thank you, i have sent new patch version.

Current call sequence in backward order:

r600 =>
* r600_state_common.c::r600_translate_texformat and
r600_state_common.c::r600_translate_colorformat is called from
evergreen_state::r600_is_colorbuffer_format_supported and
r600_state::r600_is_colorbuffer_format_supported.

radeonsi =>
  * si_state::si_translate_colorformat =>
si_state::si_is_colorbuffer_format_supported =>
si_state::si_is_format_supported (In backward sequence.)
  * si_state::si_is_vertex_format_supported =>
si_state::si_is_format_supported

Looks as make sense to delete changes in si_translate_texformat will be
enough because we check format in si_is_sampler_format_supported.


On Mon, Sep 11, 2017 at 5:21 PM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:

> On 10.09.2017 20:52, Denis Pauk wrote:
>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102552
>> ---
>>   src/gallium/auxiliary/util/u_format.c        |  4 ++++
>>   src/gallium/drivers/r600/r600_state_common.c |  4 ++++
>>   src/gallium/drivers/radeonsi/si_state.c      | 13 ++++++++++++-
>>   3 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/auxiliary/util/u_format.c
>> b/src/gallium/auxiliary/util/u_format.c
>> index 3d281905ce..a6d42a428d 100644
>> --- a/src/gallium/auxiliary/util/u_format.c
>> +++ b/src/gallium/auxiliary/util/u_format.c
>> @@ -238,6 +238,10 @@ util_format_is_subsampled_422(enum pipe_format
>> format)
>>   boolean
>>   util_format_is_supported(enum pipe_format format, unsigned bind)
>>   {
>> +   if (format >= PIPE_FORMAT_COUNT) {
>> +      return FALSE;
>> +   }
>> +
>>      if (util_format_is_s3tc(format) && !util_format_s3tc_enabled) {
>>         return FALSE;
>>      }
>> diff --git a/src/gallium/drivers/r600/r600_state_common.c
>> b/src/gallium/drivers/r600/r600_state_common.c
>> index c1bce8304b..1515c28091 100644
>> --- a/src/gallium/drivers/r600/r600_state_common.c
>> +++ b/src/gallium/drivers/r600/r600_state_common.c
>> @@ -2284,6 +2284,8 @@ uint32_t r600_translate_texformat(struct
>> pipe_screen *screen,
>>                 format = PIPE_FORMAT_A4R4_UNORM;
>>         desc = util_format_description(format);
>> +       if (!desc)
>> +               goto out_unknown;
>>         /* Depth and stencil swizzling is handled separately. */
>>         if (desc->colorspace != UTIL_FORMAT_COLORSPACE_ZS) {
>> @@ -2650,6 +2652,8 @@ uint32_t r600_translate_colorformat(enum
>> chip_class chip, enum pipe_format forma
>>         const struct util_format_description *desc =
>> util_format_description(format);
>>         int channel = util_format_get_first_non_void_channel(format);
>>         bool is_float;
>> +       if (!desc)
>> +               return ~0U;
>>     #define HAS_SIZE(x,y,z,w) \
>>         (desc->channel[0].size == (x) && desc->channel[1].size == (y) && \
>> diff --git a/src/gallium/drivers/radeonsi/si_state.c
>> b/src/gallium/drivers/radeonsi/si_state.c
>> index ee070107fd..06fd5718fd 100644
>> --- a/src/gallium/drivers/radeonsi/si_state.c
>> +++ b/src/gallium/drivers/radeonsi/si_state.c
>> @@ -1292,6 +1292,8 @@ static void si_emit_db_render_state(struct
>> si_context *sctx, struct r600_atom *s
>>   static uint32_t si_translate_colorformat(enum pipe_format format)
>>   {
>>         const struct util_format_description *desc =
>> util_format_description(format);
>> +       if (!desc)
>> +               return V_028C70_COLOR_INVALID;
>>     #define HAS_SIZE(x,y,z,w) \
>>         (desc->channel[0].size == (x) && desc->channel[1].size == (y) && \
>> @@ -1442,6 +1444,9 @@ static uint32_t si_translate_texformat(struct
>> pipe_screen *screen,
>>         bool uniform = true;
>>         int i;
>>   +     if (!desc)
>> +               goto out_unknown;
>> +
>>         /* Colorspace (return non-RGB formats directly). */
>>         switch (desc->colorspace) {
>>         /* Depth stencil formats */
>> @@ -1796,7 +1801,11 @@ static unsigned si_tex_dim(struct si_screen
>> *sscreen, struct r600_texture *rtex,
>>     static bool si_is_sampler_format_supported(struct pipe_screen
>> *screen, enum pipe_format format)
>>   {
>> -       return si_translate_texformat(screen, format,
>> util_format_description(format),
>> +       struct util_format_description *desc =
>> util_format_description(format);
>> +       if (!desc)
>> +               return false;
>> +
>> +       return si_translate_texformat(screen, format, desc,
>>                                       util_format_get_first_non_void_channel(format))
>> != ~0U;
>>   }
>>   @@ -1925,6 +1934,8 @@ static unsigned si_is_vertex_format_supported(struct
>> pipe_screen *screen,
>>                           PIPE_BIND_VERTEX_BUFFER)) == 0);
>>         desc = util_format_description(format);
>> +       if (!desc)
>> +               return 0;
>>
>
> The two si_is_*_format_supported hunks are fine, but all the other places
> should never see an invalid pipe format, so you should remove those hunks
> (perhaps add an assertion).
>
> Thanks,
> Nicolai
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
>



-- 
Best regards,
                  Denis.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170912/d84871ce/attachment.html>