From: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Date: Fri, 5 Jul 2019 12:51:23 +0000 (+0200)
Subject: st/mesa: verify that vertex buffer offset isn't negative
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=a9655f36fe9088816973c19df9399268ef6bfcd8;p=mesa.git

st/mesa: verify that vertex buffer offset isn't negative

For drivers supporting PIPE_CAP_SIGNED_VERTEX_BUFFER_OFFSET the buffer_offset value
will be interpreted as an signed int.

An example of application code causing a negative offset:

            float b[] = { ... }; // 3 float for pos, 3 for color
            glBufferData(GL_ARRAY_BUFFER, ..., b, ...);
            glVertexAttribPointer(0, 3, GL_FLOAT, GL_FALSE, 6 * sizeof(float), 0);
            glVertexAttribPointer(1, 3, GL_FLOAT, GL_FALSE, 6 * sizeof(float), &b[3]);
                                                                                ^
                                                                    should be 3 * sizeof(float)

The offset is a ptr so when interpreted as a signed int it can be negative.

This commit adds a verification that (int) buffer_offset is not negative - this would
indicate an application bug. Since it's too late to emit a GL_INVALID_VALUE error,
we replace the negative offset by 0 and emit a debug message.

Signed-off-by: Marek Olšák <marek.olsak@amd.com>
---

diff --git a/src/mesa/state_tracker/st_atom_array.c b/src/mesa/state_tracker/st_atom_array.c
index 89a0923fbbe..1662050bf04 100644
--- a/src/mesa/state_tracker/st_atom_array.c
+++ b/src/mesa/state_tracker/st_atom_array.c
@@ -410,6 +410,18 @@ st_setup_arrays(struct st_context *st,
          vbuffer[bufidx].buffer.resource = stobj ? stobj->buffer : NULL;
          vbuffer[bufidx].is_user_buffer = false;
          vbuffer[bufidx].buffer_offset = _mesa_draw_binding_offset(binding);
+         if (st->has_signed_vertex_buffer_offset) {
+            /* 'buffer_offset' will be interpreted as an signed int, so make sure
+             * the user supplied offset is not negative (application bug).
+             */
+            if ((int) vbuffer[bufidx].buffer_offset < 0) {
+               assert ((int) vbuffer[bufidx].buffer_offset >= 0);
+               /* Fallback if assert are disabled: we can't disable this attribute
+                * since other parts expects it (e.g: velements, vp_variant), so
+                * use a non-buggy offset value instead */
+               vbuffer[bufidx].buffer_offset = 0;
+            }
+         }
       } else {
          /* Set the binding */
          const void *ptr = (const void *)_mesa_draw_binding_offset(binding);
diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c
index a94ffe26eba..fada76437b2 100644
--- a/src/mesa/state_tracker/st_context.c
+++ b/src/mesa/state_tracker/st_context.c
@@ -637,6 +637,8 @@ st_create_context_priv(struct gl_context *ctx, struct pipe_context *pipe,
       screen->get_param(screen, PIPE_CAP_INDEP_BLEND_FUNC);
    st->needs_rgb_dst_alpha_override =
       screen->get_param(screen, PIPE_CAP_RGB_OVERRIDE_DST_ALPHA_BLEND);
+   st->has_signed_vertex_buffer_offset =
+      screen->get_param(screen, PIPE_CAP_SIGNED_VERTEX_BUFFER_OFFSET);
 
    st->has_hw_atomics =
       screen->get_shader_param(screen, PIPE_SHADER_FRAGMENT,
diff --git a/src/mesa/state_tracker/st_context.h b/src/mesa/state_tracker/st_context.h
index ceb28d5f5c5..93387df7cca 100644
--- a/src/mesa/state_tracker/st_context.h
+++ b/src/mesa/state_tracker/st_context.h
@@ -145,6 +145,7 @@ struct st_context
    boolean has_indep_blend_func;
    boolean needs_rgb_dst_alpha_override;
    boolean can_bind_const_buffer_as_vertex;
+   boolean has_signed_vertex_buffer_offset;
 
    /**
     * If a shader can be created when we get its source.