On 07/10/2014 05:11 PM, Marek Olšák wrote:
From: Marek Olšák <marek.ol...@amd.com>

This happens when glGetMultisamplefv (or any other non-draw function) is
called, which doesn't invoke the VBO module to update _DrawArrays and
the pointer is invalid at that point.

However st/mesa still dereferences it to setup vertex buffers ==> crash.
---
  src/mesa/main/arrayobj.c   | 15 +++++++++++++++
  src/mesa/main/mtypes.h     | 14 ++++++++++++++
  src/mesa/vbo/vbo_context.h | 22 ++++------------------
  src/mesa/vbo/vbo_exec.c    | 15 ---------------
  4 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/src/mesa/main/arrayobj.c b/src/mesa/main/arrayobj.c
index efb9930..0f8f827 100644
--- a/src/mesa/main/arrayobj.c
+++ b/src/mesa/main/arrayobj.c
@@ -427,6 +427,21 @@ bind_vertex_array(struct gl_context *ctx, GLuint id, 
GLboolean genRequired)
        }
     }

+   if (ctx->Array.DrawMethod == DRAW_ARRAYS) {
+      /* The _DrawArrays pointer is pointing at the VAO being unbound and
+       * that VAO may be in the process if being deleted. If it's not going

s/if/of/


+       * to be deleted, this will have no effect, because the pointer needs
+       * to be updated by the VBO module anyway.
+       *
+       * Before the VBO module can update the pointer, we have to set it
+       * to NULL for drivers not to set up arrays which are not bound,
+       * or to prevent a crash if the VAO being unbound is going to be
+       * deleted.
+       */
+      ctx->Array._DrawArrays = NULL;
+      ctx->Array.DrawMethod = DRAW_NONE;
+   }
+
     ctx->NewState |= _NEW_ARRAY;
     _mesa_reference_vao(ctx, &ctx->Array.VAO, newObj);

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index a7126fd..b699021 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -1640,6 +1640,17 @@ struct gl_vertex_array_object
  };


+/** Used to signal when transitioning from one kind of drawing method
+ * to another.
+ */
+typedef enum {
+   DRAW_NONE,          /**< Initial value only */
+   DRAW_BEGIN_END,
+   DRAW_DISPLAY_LIST,
+   DRAW_ARRAYS
+} gl_draw_method;
+
+
  /**
   * Vertex array state
   */
@@ -1679,6 +1690,9 @@ struct gl_array_attrib
      * The array pointer is set up only by the VBO module.
      */
     const struct gl_client_array **_DrawArrays; /**< 0..VERT_ATTRIB_MAX-1 */
+
+   /** One of the DRAW_xxx flags, not consumed by drivers */
+   gl_draw_method DrawMethod;
  };


diff --git a/src/mesa/vbo/vbo_context.h b/src/mesa/vbo/vbo_context.h
index 1680e23..e224513 100644
--- a/src/mesa/vbo/vbo_context.h
+++ b/src/mesa/vbo/vbo_context.h
@@ -57,18 +57,6 @@
  #include "vbo_save.h"


-/** Used to signal when transitioning from one kind of drawing method
- * to another.
- */
-enum draw_method
-{
-   DRAW_NONE,          /**< Initial value only */
-   DRAW_BEGIN_END,
-   DRAW_DISPLAY_LIST,
-   DRAW_ARRAYS
-};
-
-
  struct vbo_context {
     struct gl_client_array currval[VBO_ATTRIB_MAX];

@@ -83,8 +71,6 @@ struct vbo_context {
      * is responsible for initiating any fallback actions required:
      */
     vbo_draw_func draw_prims;
-
-   enum draw_method last_draw_method;
  };


@@ -122,11 +108,11 @@ get_program_mode( struct gl_context *ctx )
   * that arrays may be changing.
   */
  static inline void
-vbo_draw_method(struct vbo_context *vbo, enum draw_method method)
+vbo_draw_method(struct vbo_context *vbo, gl_draw_method method)
  {
-   if (vbo->last_draw_method != method) {
-      struct gl_context *ctx = vbo->exec.ctx;
+   struct gl_context *ctx = vbo->exec.ctx;

+   if (ctx->Array.DrawMethod != method) {
        switch (method) {
        case DRAW_ARRAYS:
           ctx->Array._DrawArrays = vbo->exec.array.inputs;
@@ -142,7 +128,7 @@ vbo_draw_method(struct vbo_context *vbo, enum draw_method 
method)
        }

        ctx->NewDriverState |= ctx->DriverFlags.NewArray;
-      vbo->last_draw_method = method;
+      ctx->Array.DrawMethod = method;
     }
  }

diff --git a/src/mesa/vbo/vbo_exec.c b/src/mesa/vbo/vbo_exec.c
index bd2b1b1..eb90350 100644
--- a/src/mesa/vbo/vbo_exec.c
+++ b/src/mesa/vbo/vbo_exec.c
@@ -82,21 +82,6 @@ void vbo_exec_invalidate_state( struct gl_context *ctx, 
GLuint new_state )

     if (!exec->validating && new_state & (_NEW_PROGRAM|_NEW_ARRAY)) {
        exec->array.recalculate_inputs = GL_TRUE;
-
-      /* If we ended up here because a VAO was deleted, the _DrawArrays
-       * pointer which pointed to the VAO might be invalid now, so set it
-       * to NULL.  This prevents crashes in driver functions like Clear
-       * where driver state validation might occur, but the vbo module is
-       * still in an invalid state.
-       *
-       * Drivers should skip vertex array state validation if _DrawArrays
-       * is NULL.  It also has no effect on performance, because attrib
-       * bindings will be recalculated anyway.
-       */
-      if (vbo->last_draw_method == DRAW_ARRAYS) {
-         ctx->Array._DrawArrays = NULL;
-         vbo->last_draw_method = DRAW_NONE;
-      }
     }

     if (new_state & _NEW_EVAL)


Reviewed-by: Brian Paul <bri...@vmware.com>


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to