broadcom/vc4: Fix use-after-free trying to mix a quad and tile clear.
authorEric Anholt <eric@anholt.net>
Mon, 18 Sep 2017 22:17:31 +0000 (15:17 -0700)
committerEric Anholt <eric@anholt.net>
Mon, 18 Sep 2017 23:16:00 +0000 (16:16 -0700)
The blitter will bind just the depth buffer, which flushes the current job
if we had both a color and depth/stencil.  If the clear was doing partial
depth/stencil (quad-based) and color (tile-based), we'd go on to try to
set up the rest of the tile clear in the now flushed job.

Instead, move the partial clear up before we start setting up the job for
the current FBO state, and re-fetch the job if we're continuing on to a
tile-based clear.  Fixes valgrind failures in fbo-depthtex.

Fixes: 9421a6065c4e ("vc4: Fix fallback to quad clears of depth in GLX.")
src/gallium/drivers/vc4/vc4_draw.c

index 2074931c46a911a38f1c268dcecad5c351f52ccd..fdf983dae7f37d4fd5ebee07c43a61d9d4bb13cf 100644 (file)
@@ -502,6 +502,37 @@ vc4_clear(struct pipe_context *pctx, unsigned buffers,
         struct vc4_context *vc4 = vc4_context(pctx);
         struct vc4_job *job = vc4_get_job_for_fbo(vc4);
 
+        if (buffers & PIPE_CLEAR_DEPTHSTENCIL) {
+                struct vc4_resource *rsc =
+                        vc4_resource(vc4->framebuffer.zsbuf->texture);
+                unsigned zsclear = buffers & PIPE_CLEAR_DEPTHSTENCIL;
+
+                /* Clearing ZS will clear both Z and stencil, so if we're
+                 * trying to clear just one then we need to draw a quad to do
+                 * it instead.  We need to do this before setting up
+                 * tile-based clears in vc4->job, because the blitter may
+                 * submit the current job.
+                 */
+                if ((zsclear == PIPE_CLEAR_DEPTH ||
+                     zsclear == PIPE_CLEAR_STENCIL) &&
+                    (rsc->initialized_buffers & ~(zsclear | job->cleared)) &&
+                    util_format_is_depth_and_stencil(vc4->framebuffer.zsbuf->format)) {
+                        perf_debug("Partial clear of Z+stencil buffer, "
+                                   "drawing a quad instead of fast clearing\n");
+                        vc4_blitter_save(vc4);
+                        util_blitter_clear(vc4->blitter,
+                                           vc4->framebuffer.width,
+                                           vc4->framebuffer.height,
+                                           1,
+                                           zsclear,
+                                           NULL, depth, stencil);
+                        buffers &= ~zsclear;
+                        if (!buffers)
+                                return;
+                        job = vc4_get_job_for_fbo(vc4);
+                }
+        }
+
         /* We can't flag new buffers for clearing once we've queued draws.  We
          * could avoid this by using the 3d engine to clear.
          */
@@ -537,29 +568,6 @@ vc4_clear(struct pipe_context *pctx, unsigned buffers,
         if (buffers & PIPE_CLEAR_DEPTHSTENCIL) {
                 struct vc4_resource *rsc =
                         vc4_resource(vc4->framebuffer.zsbuf->texture);
-                unsigned zsclear = buffers & PIPE_CLEAR_DEPTHSTENCIL;
-
-                /* Clearing ZS will clear both Z and stencil, so if we're
-                 * trying to clear just one then we need to draw a quad to do
-                 * it instead.
-                 */
-                if ((zsclear == PIPE_CLEAR_DEPTH ||
-                     zsclear == PIPE_CLEAR_STENCIL) &&
-                    (rsc->initialized_buffers & ~(zsclear | job->cleared)) &&
-                    util_format_is_depth_and_stencil(vc4->framebuffer.zsbuf->format)) {
-                        perf_debug("Partial clear of Z+stencil buffer, "
-                                   "drawing a quad instead of fast clearing\n");
-                        vc4_blitter_save(vc4);
-                        util_blitter_clear(vc4->blitter,
-                                           vc4->framebuffer.width,
-                                           vc4->framebuffer.height,
-                                           1,
-                                           zsclear,
-                                           NULL, depth, stencil);
-                        buffers &= ~zsclear;
-                        if (!buffers)
-                                return;
-                }
 
                 /* Though the depth buffer is stored with Z in the high 24,
                  * for this field we just need to store it in the low 24.
@@ -571,7 +579,7 @@ vc4_clear(struct pipe_context *pctx, unsigned buffers,
                 if (buffers & PIPE_CLEAR_STENCIL)
                         job->clear_stencil = stencil;
 
-                rsc->initialized_buffers |= zsclear;
+                rsc->initialized_buffers |= (buffers & PIPE_CLEAR_DEPTHSTENCIL);
         }
 
         job->draw_min_x = 0;