[Mesa-dev] [PATCH] gallium/{r600, radeonsi}: Fix segfault with color format (v2)
Denis Pauk
pauk.denis at gmail.com
Wed Sep 13 08:44:50 UTC 2017
This only example of code, that we can use for check is_format_supported
call. Its not real world code.
Best regards,
Denis.
On Sep 13, 2017 11:15 AM, "Nicolai Hähnle" <nhaehnle at gmail.com> wrote:
> On 13.09.2017 08:26, Denis Pauk wrote:
>
>> Little additional note:
>>
>> This checks related to possible issue with such kind of supported format
>> check in external code:
>>
>> ----
>> for(int i=0; i<PIPE_FORMAT_COUNT + 10; i++) {
>> pscreen->is_format_supported(pscreen, i, PIPE_TEXTURE_2D, 0,
>> PIPE_BIND_SAMPLER_VIEW);
>> }
>> -----
>>
>> When we have segfault with: 73, 78, 79, 80, 81,
>>
>> 86 (holes in enum pipe_format) and format value
>>
>> bigger than PIPE_FORMAT_COUNT.
>>
>
> Arguably, that code should call util_format_description() and skip the
> format is the result is NULL.
>
> And why on earth is that code looping until PIPE_FORMAT_COUNT + 10 in the
> first place? That never makes sense.
>
> Cheers,
> Nicolai
>
>
>
>>
>>
>> Best regards,
>> Denis.
>>
>> On Sep 13, 2017 7:54 AM, "Денис Паук" <pauk.denis at gmail.com <mailto:
>> pauk.denis at gmail.com>> wrote:
>>
>> Do you mean delete check in u_format.c:: util_format_is_supported?
>> Could you please explain more?
>>
>> On Wed, Sep 13, 2017 at 1:32 AM, Marek Olšák <maraeo at gmail.com
>> <mailto:maraeo at gmail.com>> wrote:
>>
>> On Wed, Sep 13, 2017 at 12:31 AM, Marek Olšák <maraeo at gmail.com
>> <mailto:maraeo at gmail.com>> wrote:
>> > I think we shouldn't be getting PIPE_FORMAT_COUNT in
>> > is_format_supported in the first place, and therefore drivers
>> don't
>> > have to work around it.
>>
>> Or any other invalid formats, for that matter.
>>
>> Marek
>>
>> >
>> > Marek
>> >
>> > On Tue, Sep 12, 2017 at 10:38 PM, Denis Pauk
>> <pauk.denis at gmail.com <mailto:pauk.denis at gmail.com>> wrote:
>> >> Bugzilla:
>> https://bugs.freedesktop.org/show_bug.cgi?id=102552
>> <https://bugs.freedesktop.org/show_bug.cgi?id=102552>
>> >>
>> >> v2: Patch cleanup proposed by Nicolai Hähnle.
>> >> * deleted changes in si_translate_texformat.
>> >>
>> >> Cc: Nicolai Hähnle <nhaehnle at gmail.com
>> <mailto:nhaehnle at gmail.com>>
>> >> Cc: Ilia Mirkin <imirkin at alum.mit.edu
>> <mailto:imirkin at alum.mit.edu>>
>> >> ---
>> >> src/gallium/auxiliary/util/u_format.c | 4 ++++
>> >> src/gallium/drivers/r600/r600_state_common.c | 4 ++++
>> >> src/gallium/drivers/radeonsi/si_state.c | 10 +++++++++-
>> >> 3 files changed, 17 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..f7ee24bdc6 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) && \
>> >> @@ -1796,7 +1798,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 +1931,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;
>> >>
>> >> /* There are no native 8_8_8 or 16_16_16 data
>> formats, and we currently
>> >> * select 8_8_8_8 and 16_16_16_16 instead. This
>> works reasonably well
>> >> --
>> >> 2.14.1
>> >>
>> >> _______________________________________________
>> >> mesa-dev mailing list
>> >> mesa-dev at lists.freedesktop.org
>> <mailto:mesa-dev at lists.freedesktop.org>
>> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
>>
>>
>>
>>
>> -- Best regards,
>> Denis.
>>
>>
>
> --
> Lerne, wie die Welt wirklich ist,
> Aber vergiss niemals, wie sie sein sollte.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170913/a912cc5f/attachment.html>
More information about the mesa-dev
mailing list