panfrost: Fix NULL derefs in pan_cmdstream.c

Auditing nr_cbufs. Note we have to augment the blending logic a bit to
use the same 'no blend' case for missing RTs as we do for depth-only
passes.

Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9164>

diff --git a/src/gallium/drivers/panfrost/pan_cmdstream.c b/src/gallium/drivers/panfrost/pan_cmdstream.c
index 827f0ed..214aee2 100644 (file)
--- a/src/gallium/drivers/panfrost/pan_cmdstream.c
+++ b/src/gallium/drivers/panfrost/pan_cmdstream.c
@@ -245,15 +245,15 @@ static bool
 panfrost_fs_required(
                 struct panfrost_shader_state *fs,
                 struct panfrost_blend_final *blend,
-                unsigned rt_count)
+                struct pipe_framebuffer_state *state)
 {
         /* If we generally have side effects */
         if (fs->info.fs.sidefx)
                 return true;
 
         /* If colour is written we need to execute */
-        for (unsigned i = 0; i < rt_count; ++i) {
-                if (!blend[i].no_colour)
+        for (unsigned i = 0; i < state->nr_cbufs; ++i) {
+                if (!blend[i].no_colour && state->cbufs[i])
                         return true;
         }
 
@@ -292,20 +292,21 @@ panfrost_emit_bifrost_blend(struct panfrost_batch *batch,
                             void *rts)
 {
         unsigned rt_count = batch->key.nr_cbufs;
-
-        if (rt_count == 0) {
-                /* Disable blending for depth-only */
-                pan_pack(rts, BLEND, cfg) {
-                        cfg.enable = false;
-                        cfg.bifrost.internal.mode = MALI_BIFROST_BLEND_MODE_OFF;
-                }
-                return;
-        }
-
         const struct panfrost_device *dev = pan_device(batch->ctx->base.screen);
         struct panfrost_shader_state *fs = panfrost_get_shader_state(batch->ctx, PIPE_SHADER_FRAGMENT);
 
-        for (unsigned i = 0; i < rt_count; ++i) {
+        /* Always have at least one render target for depth-only passes */
+        for (unsigned i = 0; i < MAX2(rt_count, 1); ++i) {
+                /* Disable blending for unbacked render targets */
+                if (rt_count == 0 || !batch->key.cbufs[i]) {
+                        pan_pack(rts, BLEND, cfg) {
+                                cfg.enable = false;
+                                cfg.bifrost.internal.mode = MALI_BIFROST_BLEND_MODE_OFF;
+                        }
+
+                        continue;
+                }
+
                 pan_pack(rts + i * MALI_BLEND_LENGTH, BLEND, cfg) {
                         if (blend[i].no_colour) {
                                 cfg.enable = false;
@@ -374,21 +375,23 @@ panfrost_emit_midgard_blend(struct panfrost_batch *batch,
 {
         unsigned rt_count = batch->key.nr_cbufs;
 
-        if (rt_count == 0) {
-                /* Disable blending for depth-only */
-                pan_pack(rts, BLEND, cfg) {
-                        cfg.midgard.equation.color_mask = 0xf;
-                        cfg.midgard.equation.rgb.a = MALI_BLEND_OPERAND_A_SRC;
-                        cfg.midgard.equation.rgb.b = MALI_BLEND_OPERAND_B_SRC;
-                        cfg.midgard.equation.rgb.c = MALI_BLEND_OPERAND_C_ZERO;
-                        cfg.midgard.equation.alpha.a = MALI_BLEND_OPERAND_A_SRC;
-                        cfg.midgard.equation.alpha.b = MALI_BLEND_OPERAND_B_SRC;
-                        cfg.midgard.equation.alpha.c = MALI_BLEND_OPERAND_C_ZERO;
+        /* Always have at least one render target for depth-only passes */
+        for (unsigned i = 0; i < MAX2(rt_count, 1); ++i) {
+                /* Disable blending for unbacked render targets */
+                if (rt_count == 0 || !batch->key.cbufs[i]) {
+                        pan_pack(rts, BLEND, cfg) {
+                                cfg.midgard.equation.color_mask = 0xf;
+                                cfg.midgard.equation.rgb.a = MALI_BLEND_OPERAND_A_SRC;
+                                cfg.midgard.equation.rgb.b = MALI_BLEND_OPERAND_B_SRC;
+                                cfg.midgard.equation.rgb.c = MALI_BLEND_OPERAND_C_ZERO;
+                                cfg.midgard.equation.alpha.a = MALI_BLEND_OPERAND_A_SRC;
+                                cfg.midgard.equation.alpha.b = MALI_BLEND_OPERAND_B_SRC;
+                                cfg.midgard.equation.alpha.c = MALI_BLEND_OPERAND_C_ZERO;
+                        }
+
+                        continue;
                 }
-                return;
-        }
 
-        for (unsigned i = 0; i < rt_count; ++i) {
                 pan_pack(rts + i * MALI_BLEND_LENGTH, BLEND, cfg) {
                         if (blend[i].no_colour) {
                                 cfg.enable = false;
@@ -421,7 +424,7 @@ panfrost_emit_blend(struct panfrost_batch *batch, void *rts,
                 panfrost_emit_midgard_blend(batch, blend, rts);
 
         for (unsigned i = 0; i < batch->key.nr_cbufs; ++i) {
-                if (!blend[i].no_colour)
+                if (!blend[i].no_colour && batch->key.cbufs[i])
                         batch->draws |= (PIPE_CLEAR_COLOR0 << i);
         }
 }
@@ -433,10 +436,9 @@ panfrost_prepare_bifrost_fs_state(struct panfrost_context *ctx,
 {
         const struct panfrost_device *dev = pan_device(ctx->base.screen);
         struct panfrost_shader_state *fs = panfrost_get_shader_state(ctx, PIPE_SHADER_FRAGMENT);
-        unsigned rt_count = ctx->pipe_framebuffer.nr_cbufs;
         bool alpha_to_coverage = ctx->blend->base.alpha_to_coverage;
 
-        if (!panfrost_fs_required(fs, blend, rt_count)) {
+        if (!panfrost_fs_required(fs, blend, &ctx->pipe_framebuffer)) {
                 state->properties.uniform_buffer_count = 32;
                 state->properties.bifrost.shader_modifies_coverage = true;
                 state->properties.bifrost.allow_forward_pixel_to_kill = true;
@@ -449,8 +451,10 @@ panfrost_prepare_bifrost_fs_state(struct panfrost_context *ctx,
 
                 bool no_blend = true;
 
-                for (unsigned i = 0; i < rt_count; ++i)
-                        no_blend &= (!blend[i].load_dest | blend[i].no_colour);
+                for (unsigned i = 0; i < ctx->pipe_framebuffer.nr_cbufs; ++i) {
+                        no_blend &= (!blend[i].load_dest || blend[i].no_colour)
+                                || (!ctx->pipe_framebuffer.cbufs[i]);
+                }
 
                 state->properties.bifrost.allow_forward_pixel_to_kill =
                         !fs->info.fs.writes_depth &&
@@ -474,7 +478,7 @@ panfrost_prepare_midgard_fs_state(struct panfrost_context *ctx,
         unsigned rt_count = ctx->pipe_framebuffer.nr_cbufs;
         bool alpha_to_coverage = ctx->blend->base.alpha_to_coverage;
 
-        if (!panfrost_fs_required(fs, blend, rt_count)) {
+        if (!panfrost_fs_required(fs, blend, &ctx->pipe_framebuffer)) {
                 state->shader.shader = 0x1;
                 state->properties.midgard.work_register_count = 1;
                 state->properties.depth_source = MALI_DEPTH_SOURCE_FIXED_FUNCTION;
@@ -668,9 +672,13 @@ panfrost_emit_frag_shader_meta(struct panfrost_batch *batch)
         unsigned shader_offset = 0;
         struct panfrost_bo *shader_bo = NULL;
 
-        for (unsigned c = 0; c < ctx->pipe_framebuffer.nr_cbufs; ++c)
-                blend[c] = panfrost_get_blend_for_context(ctx, c, &shader_bo,
-                                                          &shader_offset);
+        for (unsigned c = 0; c < ctx->pipe_framebuffer.nr_cbufs; ++c) {
+                if (ctx->pipe_framebuffer.cbufs[c]) {
+                        blend[c] = panfrost_get_blend_for_context(ctx, c,
+                                        &shader_bo, &shader_offset);
+                }
+        }
+
         panfrost_emit_frag_shader(ctx, (struct mali_renderer_state_packed *) xfer.cpu, blend);
 
         if (!(dev->quirks & MIDGARD_SFBD))
@@ -979,7 +987,7 @@ panfrost_upload_rt_conversion_sysval(struct panfrost_batch *batch, unsigned rt,
         struct panfrost_context *ctx = batch->ctx;
         struct panfrost_device *dev = pan_device(ctx->base.screen);
 
-        if (rt < batch->key.nr_cbufs) {
+        if (rt < batch->key.nr_cbufs && batch->key.cbufs[rt]) {
                 enum pipe_format format = batch->key.cbufs[rt]->format;
                 uniform->u[0] = bifrost_get_blend_desc(dev, format, rt, 32) >> 32;
         } else {