panfrost: Fix gl_PointSize out of GL_POINTS
authorAlyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Tue, 2 Jun 2020 00:44:19 +0000 (20:44 -0400)
committerMarge Bot <eric+marge@anholt.net>
Wed, 3 Jun 2020 22:58:46 +0000 (22:58 +0000)
In this case, vs->writes_point_size is true as the VS writes
gl_PointSize, but panfrost_writes_points_size() is false as we are not
drawing points so the hardware doesn't process it. Thus the varying
descriptor is emitted but elements is never written. When the VS runs,
it will attempt to write to elements, a NULL pointer.

The behaviour is architecture-independent. On Midgard, the write
silently fails, hence why this bug was never noticed before. On Bifrost,
this raises an MMU fault.

The fix is to set the format to VARYING_DISCARD to ignore the write.

Noticed on Neverball.

Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5290>

src/gallium/drivers/panfrost/pan_cmdstream.c

index a40a1505817ac8caea38981298f3d78b89d4b3d5..608d1799f9785057b4e9367f0bfb6f27b7d66390 100644 (file)
@@ -1900,6 +1900,15 @@ panfrost_emit_varying_descriptor(struct panfrost_batch *batch,
         for (unsigned i = 0; i < vs->varying_count; i++) {
                 gl_varying_slot loc = vs->varyings_loc[i];
 
+                /* If we write gl_PointSize from the vertex shader but don't
+                 * consume it, no memory will be allocated for it, so if we
+                 * attempted to write anyway we would dereference a NULL
+                 * pointer on the GPU. Midgard seems fine with this; Bifrost
+                 * faults. */
+
+                if (loc == VARYING_SLOT_PSIZ && !panfrost_writes_point_size(ctx))
+                        ovs[i].format = MALI_VARYING_DISCARD;
+
                 bool captured = ((vs->so_mask & (1ll << loc)) ? true : false);
                 if (!captured)
                         continue;