From 53b01938ed864e2efaa9389b45e88284eebaaa63 Mon Sep 17 00:00:00 2001 From: Brian Paul Date: Mon, 26 Jan 2015 20:32:58 -0700 Subject: [PATCH] mesa: fix display list 8-byte alignment issue MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The _mesa_dlist_alloc() function is only guaranteed to return a pointer with 4-byte alignment. On 64-bit systems which don't support unaligned loads (e.g. SPARC or MIPS) this could lead to a bus error in the VBO code. The solution is to add a new _mesa_dlist_alloc_aligned() function which will return a pointer to an 8-byte aligned address on 64-bit systems. This is accomplished by inserting a 4-byte NOP instruction in the display list when needed. The only place this actually matters is the VBO code where we need to allocate a 'struct vbo_save_vertex_list' which needs to be 8-byte aligned (just as if it were malloc'd). The gears demo and others hit this bug. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88662 Cc: "10.4" Reviewed-by: José Fonseca --- src/mesa/main/dlist.c | 73 ++++++++++++++++++++++++++++++++++--- src/mesa/main/dlist.h | 3 ++ src/mesa/vbo/vbo_save_api.c | 5 ++- 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c index 138d3600699..7e5b83ce4b6 100644 --- a/src/mesa/main/dlist.c +++ b/src/mesa/main/dlist.c @@ -487,6 +487,7 @@ typedef enum /* The following three are meta instructions */ OPCODE_ERROR, /* raise compiled-in error */ OPCODE_CONTINUE, + OPCODE_NOP, /* No-op (used for 8-byte alignment */ OPCODE_END_OF_LIST, OPCODE_EXT_0 } OpCode; @@ -1012,13 +1013,16 @@ memdup(const void *src, GLsizei bytes) * Allocate space for a display list instruction (opcode + payload space). * \param opcode the instruction opcode (OPCODE_* value) * \param bytes instruction payload size (not counting opcode) - * \return pointer to allocated memory (the opcode space) + * \param align8 does the payload need to be 8-byte aligned? + * This is only relevant in 64-bit environments. + * \return pointer to allocated memory (the payload will be at pointer+1) */ static Node * -dlist_alloc(struct gl_context *ctx, OpCode opcode, GLuint bytes) +dlist_alloc(struct gl_context *ctx, OpCode opcode, GLuint bytes, bool align8) { const GLuint numNodes = 1 + (bytes + sizeof(Node) - 1) / sizeof(Node); const GLuint contNodes = 1 + POINTER_DWORDS; /* size of continue info */ + GLuint nopNode; Node *n; if (opcode < OPCODE_EXT_0) { @@ -1032,7 +1036,20 @@ dlist_alloc(struct gl_context *ctx, OpCode opcode, GLuint bytes) } } - if (ctx->ListState.CurrentPos + numNodes + contNodes > BLOCK_SIZE) { + if (sizeof(void *) > sizeof(Node) && align8 + && ctx->ListState.CurrentPos % 2 == 0) { + /* The opcode would get placed at node[0] and the payload would start + * at node[1]. But the payload needs to be at an even offset (8-byte + * multiple). + */ + nopNode = 1; + } + else { + nopNode = 0; + } + + if (ctx->ListState.CurrentPos + nopNode + numNodes + contNodes + > BLOCK_SIZE) { /* This block is full. Allocate a new block and chain to it */ Node *newblock; n = ctx->ListState.CurrentBlock + ctx->ListState.CurrentPos; @@ -1042,13 +1059,34 @@ dlist_alloc(struct gl_context *ctx, OpCode opcode, GLuint bytes) _mesa_error(ctx, GL_OUT_OF_MEMORY, "Building display list"); return NULL; } + + /* a fresh block should be 8-byte aligned on 64-bit systems */ + assert(((GLintptr) newblock) % sizeof(void *) == 0); + save_pointer(&n[1], newblock); ctx->ListState.CurrentBlock = newblock; ctx->ListState.CurrentPos = 0; + + /* Display list nodes are always 4 bytes. If we need 8-byte alignment + * we have to insert a NOP so that the payload of the real opcode lands + * on an even location: + * node[0] = OPCODE_NOP + * node[1] = OPCODE_x; + * node[2] = start of payload + */ + nopNode = sizeof(void *) > sizeof(Node) && align8; } n = ctx->ListState.CurrentBlock + ctx->ListState.CurrentPos; - ctx->ListState.CurrentPos += numNodes; + if (nopNode) { + assert(ctx->ListState.CurrentPos % 2 == 0); /* even value */ + n[0].opcode = OPCODE_NOP; + n++; + /* The "real" opcode will now be at an odd location and the payload + * will be at an even location. + */ + } + ctx->ListState.CurrentPos += nopNode + numNodes; n[0].opcode = opcode; @@ -1069,7 +1107,22 @@ dlist_alloc(struct gl_context *ctx, OpCode opcode, GLuint bytes) void * _mesa_dlist_alloc(struct gl_context *ctx, GLuint opcode, GLuint bytes) { - Node *n = dlist_alloc(ctx, (OpCode) opcode, bytes); + Node *n = dlist_alloc(ctx, (OpCode) opcode, bytes, false); + if (n) + return n + 1; /* return pointer to payload area, after opcode */ + else + return NULL; +} + + +/** + * Same as _mesa_dlist_alloc(), but return a pointer which is 8-byte + * aligned in 64-bit environments, 4-byte aligned otherwise. + */ +void * +_mesa_dlist_alloc_aligned(struct gl_context *ctx, GLuint opcode, GLuint bytes) +{ + Node *n = dlist_alloc(ctx, (OpCode) opcode, bytes, true); if (n) return n + 1; /* return pointer to payload area, after opcode */ else @@ -1119,7 +1172,7 @@ _mesa_dlist_alloc_opcode(struct gl_context *ctx, static inline Node * alloc_instruction(struct gl_context *ctx, OpCode opcode, GLuint nparams) { - return dlist_alloc(ctx, opcode, nparams * sizeof(Node)); + return dlist_alloc(ctx, opcode, nparams * sizeof(Node), false); } @@ -8897,6 +8950,9 @@ execute_list(struct gl_context *ctx, GLuint list) case OPCODE_CONTINUE: n = (Node *) get_pointer(&n[1]); break; + case OPCODE_NOP: + /* no-op */ + break; case OPCODE_END_OF_LIST: done = GL_TRUE; break; @@ -9947,6 +10003,9 @@ print_list(struct gl_context *ctx, GLuint list, const char *fname) fprintf(f, "DISPLAY-LIST-CONTINUE\n"); n = (Node *) get_pointer(&n[1]); break; + case OPCODE_NOP: + fprintf(f, "NOP\n"); + break; case OPCODE_END_OF_LIST: fprintf(f, "END-LIST %u\n", list); done = GL_TRUE; @@ -10097,6 +10156,8 @@ _mesa_init_display_list(struct gl_context *ctx) ctx->List.ListBase = 0; save_vtxfmt_init(&ctx->ListState.ListVtxfmt); + + InstSize[OPCODE_NOP] = 1; } diff --git a/src/mesa/main/dlist.h b/src/mesa/main/dlist.h index c57eb74da6d..6189632d478 100644 --- a/src/mesa/main/dlist.h +++ b/src/mesa/main/dlist.h @@ -60,6 +60,9 @@ extern void _mesa_compile_error( struct gl_context *ctx, GLenum error, const cha extern void *_mesa_dlist_alloc(struct gl_context *ctx, GLuint opcode, GLuint sz); +extern void * +_mesa_dlist_alloc_aligned(struct gl_context *ctx, GLuint opcode, GLuint bytes); + extern GLint _mesa_dlist_alloc_opcode( struct gl_context *ctx, GLuint sz, void (*execute)( struct gl_context *, void * ), void (*destroy)( struct gl_context *, void * ), diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c index 5055c221b6e..beef342bed8 100644 --- a/src/mesa/vbo/vbo_save_api.c +++ b/src/mesa/vbo/vbo_save_api.c @@ -375,11 +375,14 @@ _save_compile_vertex_list(struct gl_context *ctx) * being compiled. */ node = (struct vbo_save_vertex_list *) - _mesa_dlist_alloc(ctx, save->opcode_vertex_list, sizeof(*node)); + _mesa_dlist_alloc_aligned(ctx, save->opcode_vertex_list, sizeof(*node)); if (!node) return; + /* Make sure the pointer is aligned to the size of a pointer */ + assert((GLintptr) node % sizeof(void *) == 0); + /* Duplicate our template, increment refcounts to the storage structs: */ memcpy(node->attrsz, save->attrsz, sizeof(node->attrsz)); -- 2.30.2