[PATCH] mesa: handle PBO access error in display list mode

Yuanhan Liu yuanhan.liu at linux.intel.com
Sat Oct 15 18:13:10 PDT 2011


While dealing with pbo data in display list mode, it does check the pbo
access error at unpack_image. But it could not generate it, as it is
in display list compile time. If invalid PBO access found, NULL then
would be returned. While at the execution time, we can't detect if we
met a such error as the data is not stored as PBO access anymore. The
code would treat it as a _normal_ NULL pixel data.

That's how the error is missed. Here I introduced a in-file macro
BAD_ACCESS to mark that we meet a PBO access error at compile time, and
we would like to handle it at the execution time. This would make the
error defer recognizable.

Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
---
 src/mesa/main/dlist.c |   82 ++++++++++++++++++++++++++++++++-----------------
 1 files changed, 54 insertions(+), 28 deletions(-)

diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c
index 1099670..95e3235 100644
--- a/src/mesa/main/dlist.c
+++ b/src/mesa/main/dlist.c
@@ -612,6 +612,7 @@ ext_opcode_print(struct gl_context *ctx, Node *node)
 }


+static inline void free_image(void *data);
 /**
  * Delete the named display list, but don't remove from hash table.
  * \param dlist - display list pointer
@@ -644,55 +645,55 @@ _mesa_delete_list(struct gl_context *ctx, struct
gl_display_list *dlist)
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_DRAW_PIXELS:
-            free(n[5].data);
+            free_image(n[5].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_BITMAP:
-            free(n[7].data);
+            free_image(n[7].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_COLOR_TABLE:
-            free(n[6].data);
+            free_image(n[6].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_COLOR_SUB_TABLE:
-            free(n[6].data);
+            free_image(n[6].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_CONVOLUTION_FILTER_1D:
-            free(n[6].data);
+            free_image(n[6].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_CONVOLUTION_FILTER_2D:
-            free(n[7].data);
+            free_image(n[7].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_POLYGON_STIPPLE:
-            free(n[1].data);
+            free_image(n[1].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_TEX_IMAGE1D:
-            free(n[8].data);
+            free_image(n[8].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_TEX_IMAGE2D:
-            free(n[9].data);
+            free_image(n[9].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_TEX_IMAGE3D:
-            free(n[10].data);
+            free_image(n[10].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_TEX_SUB_IMAGE1D:
-            free(n[7].data);
+            free_image(n[7].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_TEX_SUB_IMAGE2D:
-            free(n[9].data);
+            free_image(n[9].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_TEX_SUB_IMAGE3D:
-            free(n[11].data);
+            free_image(n[11].data);
             n += InstSize[n[0].opcode];
             break;
          case OPCODE_COMPRESSED_TEX_IMAGE_1D:
@@ -869,6 +870,7 @@ translate_id(GLsizei n, GLenum type, const GLvoid * list)



+#define BAD_ACCESS	((GLvoid *)-1)

 /**********************************************************************/
 /*****                        Public                              *****/
@@ -881,6 +883,9 @@ translate_id(GLsizei n, GLenum type, const GLvoid * list)
  * arguments aren't error-checked until the command is actually executed
  * (not when they're compiled).
  * But if we run out of memory, GL_OUT_OF_MEMORY will be recorded.
+ *
+ * Return BAD_ACCESS if invalid pbo access found. This would make the
+ * error can be detected at display list execution time.
  */
 static GLvoid *
 unpack_image(struct gl_context *ctx, GLuint dimensions,
@@ -939,8 +944,29 @@ unpack_image(struct gl_context *ctx, GLuint dimensions,
       }
       return image;
    }
-   /* bad access! */
-   return NULL;
+
+   return BAD_ACCESS;
+}
+
+/*
+ * Here to detect the BAD_ACCESS error triggered at unpack_image
+ */
+static inline GLboolean
+is_valid_access(struct gl_context *ctx, const GLvoid *data, const char *func)
+{
+   if (data == BAD_ACCESS) {
+      _mesa_error(ctx, GL_INVALID_OPERATION, func);
+      return GL_FALSE;
+   }
+
+   return GL_TRUE;
+}
+
+static inline void
+free_image(GLvoid *data)
+{
+   if (data != BAD_ACCESS)
+      free(data);
 }

 /**
@@ -7538,7 +7564,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             CALL_BindTexture(ctx->Exec, (n[1].e, n[2].ui));
             break;
          case OPCODE_BITMAP:
-            {
+            if (is_valid_access(ctx, n[7].data, "glBitmap")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_Bitmap(ctx->Exec, ((GLsizei) n[1].i, (GLsizei) n[2].i,
@@ -7669,7 +7695,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             CALL_ColorMaterial(ctx->Exec, (n[1].e, n[2].e));
             break;
          case OPCODE_COLOR_TABLE:
-            {
+            if (is_valid_access(ctx, n[6].data, "glColorTable")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_ColorTable(ctx->Exec, (n[1].e, n[2].e, n[3].i, n[4].e,
@@ -7700,7 +7726,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             }
             break;
          case OPCODE_COLOR_SUB_TABLE:
-            {
+            if (is_valid_access(ctx, n[6].data, "glColorSubTable")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_ColorSubTable(ctx->Exec, (n[1].e, n[2].i, n[3].i,
@@ -7709,7 +7735,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             }
             break;
          case OPCODE_CONVOLUTION_FILTER_1D:
-            {
+            if (is_valid_access(ctx, n[6].data, "glConvolutionFilter1D")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_ConvolutionFilter1D(ctx->Exec, (n[1].e, n[2].i, n[3].i,
@@ -7719,7 +7745,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             }
             break;
          case OPCODE_CONVOLUTION_FILTER_2D:
-            {
+            if (is_valid_access(ctx, n[7].data, "glConvolutionFilter2D")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_ConvolutionFilter2D(ctx->Exec, (n[1].e, n[2].i, n[3].i,
@@ -7814,7 +7840,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             CALL_DrawBuffer(ctx->Exec, (n[1].e));
             break;
          case OPCODE_DRAW_PIXELS:
-            {
+            if (is_valid_access(ctx, n[5].data, "glDrawPixels")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_DrawPixels(ctx->Exec, (n[1].i, n[2].i, n[3].e, n[4].e,
@@ -8001,7 +8027,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             CALL_PolygonMode(ctx->Exec, (n[1].e, n[2].e));
             break;
          case OPCODE_POLYGON_STIPPLE:
-            {
+            if (is_valid_access(ctx, n[1].data, "glPolygonStipple")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_PolygonStipple(ctx->Exec, ((GLubyte *) n[1].data));
@@ -8110,7 +8136,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             }
             break;
          case OPCODE_TEX_IMAGE1D:
-            {
+            if (is_valid_access(ctx, n[8].data, "glTexImage1D")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_TexImage1D(ctx->Exec, (n[1].e,      /* target */
@@ -8125,7 +8151,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             }
             break;
          case OPCODE_TEX_IMAGE2D:
-            {
+            if (is_valid_access(ctx, n[9].data, "glTexImage2D")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_TexImage2D(ctx->Exec, (n[1].e,      /* target */
@@ -8141,7 +8167,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             }
             break;
          case OPCODE_TEX_IMAGE3D:
-            {
+            if (is_valid_access(ctx, n[10].data, "glTexImage3D")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_TexImage3D(ctx->Exec, (n[1].e,      /* target */
@@ -8158,7 +8184,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             }
             break;
          case OPCODE_TEX_SUB_IMAGE1D:
-            {
+            if (is_valid_access(ctx, n[7].data, "glTexSubImage1D")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_TexSubImage1D(ctx->Exec, (n[1].e, n[2].i, n[3].i,
@@ -8168,7 +8194,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             }
             break;
          case OPCODE_TEX_SUB_IMAGE2D:
-            {
+            if (is_valid_access(ctx, n[9].data, "glTexSubImage2D")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_TexSubImage2D(ctx->Exec, (n[1].e, n[2].i, n[3].i,
@@ -8179,7 +8205,7 @@ execute_list(struct gl_context *ctx, GLuint list)
             }
             break;
          case OPCODE_TEX_SUB_IMAGE3D:
-            {
+            if (is_valid_access(ctx, n[11].data, "glTexSubImage3D")) {
                const struct gl_pixelstore_attrib save = ctx->Unpack;
                ctx->Unpack = ctx->DefaultPacking;
                CALL_TexSubImage3D(ctx->Exec, (n[1].e, n[2].i, n[3].i,
-- 
1.7.3.1


More information about the mesa-dev mailing list