From df43fb661b2030d9b833a42dd47b8d7bf58d73aa Mon Sep 17 00:00:00 2001 From: Brian Date: Tue, 6 May 2008 23:08:51 -0600 Subject: [PATCH] implement full reference counting for vertex/fragment programs Use _mesa_reference_vert/fragprog() wherever we assign program pointers. Fixes a memory corruption bug found with glean/api2 test. --- src/mesa/main/context.c | 26 ++++++---- src/mesa/main/mtypes.h | 4 +- src/mesa/main/state.c | 28 ++++++---- src/mesa/shader/program.c | 102 ++++++++++++++++++++++++------------- src/mesa/shader/program.h | 22 ++++++++ src/mesa/shader/shader_api.c | 8 +-- src/mesa/shader/slang/slang_link.c | 14 ++--- src/mesa/tnl/t_vp_build.c | 3 +- 8 files changed, 141 insertions(+), 66 deletions(-) diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c index 733aaad0309..d46c0cb74d9 100644 --- a/src/mesa/main/context.c +++ b/src/mesa/main/context.c @@ -149,8 +149,6 @@ int MESA_DEBUG_FLAGS = 0; /* ubyte -> float conversion */ GLfloat _mesa_ubyte_to_float_color_tab[256]; -static void -free_shared_state( GLcontext *ctx, struct gl_shared_state *ss ); /** @@ -420,12 +418,14 @@ alloc_shared_state( GLcontext *ctx ) #endif #if FEATURE_ARB_vertex_program - ss->DefaultVertexProgram = ctx->Driver.NewProgram(ctx, GL_VERTEX_PROGRAM_ARB, 0); + ss->DefaultVertexProgram = (struct gl_vertex_program *) + ctx->Driver.NewProgram(ctx, GL_VERTEX_PROGRAM_ARB, 0); if (!ss->DefaultVertexProgram) goto cleanup; #endif #if FEATURE_ARB_fragment_program - ss->DefaultFragmentProgram = ctx->Driver.NewProgram(ctx, GL_FRAGMENT_PROGRAM_ARB, 0); + ss->DefaultFragmentProgram = (struct gl_fragment_program *) + ctx->Driver.NewProgram(ctx, GL_FRAGMENT_PROGRAM_ARB, 0); if (!ss->DefaultFragmentProgram) goto cleanup; #endif @@ -502,12 +502,10 @@ cleanup: _mesa_DeleteHashTable(ss->Programs); #endif #if FEATURE_ARB_vertex_program - if (ss->DefaultVertexProgram) - ctx->Driver.DeleteProgram(ctx, ss->DefaultVertexProgram); + _mesa_reference_vertprog(ctx, &ss->DefaultVertexProgram, NULL); #endif #if FEATURE_ARB_fragment_program - if (ss->DefaultFragmentProgram) - ctx->Driver.DeleteProgram(ctx, ss->DefaultFragmentProgram); + _mesa_reference_fragprog(ctx, &ss->DefaultFragmentProgram, NULL); #endif #if FEATURE_ATI_fragment_shader if (ss->DefaultFragmentShader) @@ -714,10 +712,10 @@ free_shared_state( GLcontext *ctx, struct gl_shared_state *ss ) _mesa_DeleteHashTable(ss->Programs); #endif #if FEATURE_ARB_vertex_program - ctx->Driver.DeleteProgram(ctx, ss->DefaultVertexProgram); + _mesa_reference_vertprog(ctx, &ss->DefaultVertexProgram, NULL); #endif #if FEATURE_ARB_fragment_program - ctx->Driver.DeleteProgram(ctx, ss->DefaultFragmentProgram); + _mesa_reference_fragprog(ctx, &ss->DefaultFragmentProgram, NULL); #endif #if FEATURE_ATI_fragment_shader @@ -1251,6 +1249,14 @@ _mesa_free_context_data( GLcontext *ctx ) _mesa_unreference_framebuffer(&ctx->DrawBuffer); _mesa_unreference_framebuffer(&ctx->ReadBuffer); + _mesa_reference_vertprog(ctx, &ctx->VertexProgram.Current, NULL); + _mesa_reference_vertprog(ctx, &ctx->VertexProgram._Current, NULL); + _mesa_reference_vertprog(ctx, &ctx->VertexProgram._TnlProgram, NULL); + + _mesa_reference_fragprog(ctx, &ctx->FragmentProgram.Current, NULL); + _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._Current, NULL); + _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._TexEnvProgram, NULL); + _mesa_free_attrib_data(ctx); _mesa_free_lighting_data( ctx ); _mesa_free_eval_data( ctx ); diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index c8718a7f636..98fab69fd82 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2185,10 +2185,10 @@ struct gl_shared_state /*@{*/ struct _mesa_HashTable *Programs; /**< All vertex/fragment programs */ #if FEATURE_ARB_vertex_program - struct gl_program *DefaultVertexProgram; + struct gl_vertex_program *DefaultVertexProgram; #endif #if FEATURE_ARB_fragment_program - struct gl_program *DefaultFragmentProgram; + struct gl_fragment_program *DefaultFragmentProgram; #endif /*@}*/ diff --git a/src/mesa/main/state.c b/src/mesa/main/state.c index 5ff67b654e8..1c73c5c462b 100644 --- a/src/mesa/main/state.c +++ b/src/mesa/main/state.c @@ -978,50 +978,60 @@ update_program(GLcontext *ctx) * 3. Programs derived from fixed-function state. */ - ctx->FragmentProgram._Current = NULL; + _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._Current, NULL); if (shProg && shProg->LinkStatus) { /* Use shader programs */ /* XXX this isn't quite right, since we may have either a vertex * _or_ fragment shader (not always both). */ - ctx->VertexProgram._Current = shProg->VertexProgram; - ctx->FragmentProgram._Current = shProg->FragmentProgram; + _mesa_reference_vertprog(ctx, &ctx->VertexProgram._Current, + shProg->VertexProgram); + _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._Current, + shProg->FragmentProgram); } else { if (ctx->VertexProgram._Enabled) { /* use user-defined vertex program */ - ctx->VertexProgram._Current = ctx->VertexProgram.Current; + _mesa_reference_vertprog(ctx, &ctx->VertexProgram._Current, + ctx->VertexProgram.Current); } else if (ctx->VertexProgram._MaintainTnlProgram) { /* Use vertex program generated from fixed-function state. * The _Current pointer will get set in * _tnl_UpdateFixedFunctionProgram() later if appropriate. */ - ctx->VertexProgram._Current = NULL; + _mesa_reference_vertprog(ctx, &ctx->VertexProgram._Current, NULL); } else { /* no vertex program */ - ctx->VertexProgram._Current = NULL; + _mesa_reference_vertprog(ctx, &ctx->VertexProgram._Current, NULL); } if (ctx->FragmentProgram._Enabled) { /* use user-defined vertex program */ - ctx->FragmentProgram._Current = ctx->FragmentProgram.Current; + _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._Current, + ctx->FragmentProgram.Current); } else if (ctx->FragmentProgram._MaintainTexEnvProgram) { /* Use fragment program generated from fixed-function state. * The _Current pointer will get set in _mesa_UpdateTexEnvProgram() * later if appropriate. */ - ctx->FragmentProgram._Current = NULL; + _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._Current, NULL); } else { /* no fragment program */ - ctx->FragmentProgram._Current = NULL; + _mesa_reference_fragprog(ctx, &ctx->FragmentProgram._Current, NULL); } } + if (ctx->VertexProgram._Current) + assert(ctx->VertexProgram._Current->Base.Parameters); + if (ctx->FragmentProgram._Current) + assert(ctx->FragmentProgram._Current->Base.Parameters); + + ctx->FragmentProgram._Active = ctx->FragmentProgram._Enabled; if (ctx->FragmentProgram._MaintainTexEnvProgram && !ctx->FragmentProgram._Enabled) { diff --git a/src/mesa/shader/program.c b/src/mesa/shader/program.c index c539b527204..8166e7e935c 100644 --- a/src/mesa/shader/program.c +++ b/src/mesa/shader/program.c @@ -59,9 +59,9 @@ _mesa_init_program(GLcontext *ctx) ctx->VertexProgram.Enabled = GL_FALSE; ctx->VertexProgram.PointSizeEnabled = GL_FALSE; ctx->VertexProgram.TwoSideEnabled = GL_FALSE; - ctx->VertexProgram.Current = (struct gl_vertex_program *) ctx->Shared->DefaultVertexProgram; + _mesa_reference_vertprog(ctx, &ctx->VertexProgram.Current, + ctx->Shared->DefaultVertexProgram); assert(ctx->VertexProgram.Current); - ctx->VertexProgram.Current->Base.RefCount++; for (i = 0; i < MAX_NV_VERTEX_PROGRAM_PARAMS / 4; i++) { ctx->VertexProgram.TrackMatrix[i] = GL_NONE; ctx->VertexProgram.TrackMatrixTransform[i] = GL_IDENTITY_NV; @@ -70,7 +70,8 @@ _mesa_init_program(GLcontext *ctx) #if FEATURE_NV_fragment_program || FEATURE_ARB_fragment_program ctx->FragmentProgram.Enabled = GL_FALSE; - ctx->FragmentProgram.Current = (struct gl_fragment_program *) ctx->Shared->DefaultFragmentProgram; + _mesa_reference_fragprog(ctx, &ctx->FragmentProgram.Current, + ctx->Shared->DefaultFragmentProgram); assert(ctx->FragmentProgram.Current); ctx->FragmentProgram.Current->Base.RefCount++; #endif @@ -92,18 +93,10 @@ void _mesa_free_program_data(GLcontext *ctx) { #if FEATURE_NV_vertex_program || FEATURE_ARB_vertex_program - if (ctx->VertexProgram.Current) { - ctx->VertexProgram.Current->Base.RefCount--; - if (ctx->VertexProgram.Current->Base.RefCount <= 0) - ctx->Driver.DeleteProgram(ctx, &(ctx->VertexProgram.Current->Base)); - } + _mesa_reference_vertprog(ctx, &ctx->VertexProgram.Current, NULL); #endif #if FEATURE_NV_fragment_program || FEATURE_ARB_fragment_program - if (ctx->FragmentProgram.Current) { - ctx->FragmentProgram.Current->Base.RefCount--; - if (ctx->FragmentProgram.Current->Base.RefCount <= 0) - ctx->Driver.DeleteProgram(ctx, &(ctx->FragmentProgram.Current->Base)); - } + _mesa_reference_fragprog(ctx, &ctx->FragmentProgram.Current, NULL); #endif /* XXX probably move this stuff */ #if FEATURE_ATI_fragment_shader @@ -366,6 +359,59 @@ _mesa_lookup_program(GLcontext *ctx, GLuint id) /** + * Reference counting for vertex/fragment programs + */ +void +_mesa_reference_program(GLcontext *ctx, + struct gl_program **ptr, + struct gl_program *prog) +{ + assert(ptr); + if (*ptr && prog) { + /* sanity check */ + ASSERT((*ptr)->Target == prog->Target); + } + if (*ptr == prog) { + return; /* no change */ + } + if (*ptr) { + GLboolean deleteFlag; + + /*_glthread_LOCK_MUTEX((*ptr)->Mutex);*/ +#if 0 + printf("Program %p %u 0x%x Refcount-- to %d\n", + *ptr, (*ptr)->Id, (*ptr)->Target, (*ptr)->RefCount - 1); +#endif + ASSERT((*ptr)->RefCount > 0); + (*ptr)->RefCount--; + + deleteFlag = ((*ptr)->RefCount == 0); + /*_glthread_UNLOCK_MUTEX((*ptr)->Mutex);*/ + + if (deleteFlag) { + ASSERT(ctx); + ctx->Driver.DeleteProgram(ctx, *ptr); + } + + *ptr = NULL; + } + + assert(!*ptr); + if (prog) { + /*_glthread_LOCK_MUTEX(prog->Mutex);*/ + prog->RefCount++; +#if 0 + printf("Program %p %u 0x%x Refcount++ to %d\n", + prog, prog->Id, prog->Target, prog->RefCount); +#endif + /*_glthread_UNLOCK_MUTEX(prog->Mutex);*/ + } + + *ptr = prog; +} + + +/** * Return a copy of a program. * XXX Problem here if the program object is actually OO-derivation * made by a device driver. @@ -380,8 +426,9 @@ _mesa_clone_program(GLcontext *ctx, const struct gl_program *prog) return NULL; assert(clone->Target == prog->Target); + assert(clone->RefCount == 1); + clone->String = (GLubyte *) _mesa_strdup((char *) prog->String); - clone->RefCount = 1; clone->Format = prog->Format; clone->Instructions = _mesa_alloc_instructions(prog->NumInstructions); if (!clone->Instructions) { @@ -513,9 +560,9 @@ _mesa_BindProgram(GLenum target, GLuint id) /* Bind a default program */ newProg = NULL; if (target == GL_VERTEX_PROGRAM_ARB) /* == GL_VERTEX_PROGRAM_NV */ - newProg = ctx->Shared->DefaultVertexProgram; + newProg = &ctx->Shared->DefaultVertexProgram->Base; else - newProg = ctx->Shared->DefaultFragmentProgram; + newProg = &ctx->Shared->DefaultFragmentProgram->Base; } else { /* Bind a user program */ @@ -543,26 +590,16 @@ _mesa_BindProgram(GLenum target, GLuint id) return; } - /* unbind/delete oldProg */ - if (curProg->Id != 0) { - /* decrement refcount on previously bound fragment program */ - curProg->RefCount--; - /* and delete if refcount goes below one */ - if (curProg->RefCount <= 0) { - /* the program ID was already removed from the hash table */ - ctx->Driver.DeleteProgram(ctx, curProg); - } - } - /* bind newProg */ if (target == GL_VERTEX_PROGRAM_ARB) { /* == GL_VERTEX_PROGRAM_NV */ - ctx->VertexProgram.Current = (struct gl_vertex_program *) newProg; + _mesa_reference_vertprog(ctx, &ctx->VertexProgram.Current, + (struct gl_vertex_program *) newProg); } else if (target == GL_FRAGMENT_PROGRAM_NV || target == GL_FRAGMENT_PROGRAM_ARB) { - ctx->FragmentProgram.Current = (struct gl_fragment_program *) newProg; + _mesa_reference_fragprog(ctx, &ctx->FragmentProgram.Current, + (struct gl_fragment_program *) newProg); } - newProg->RefCount++; /* Never null pointers */ ASSERT(ctx->VertexProgram.Current); @@ -620,10 +657,7 @@ _mesa_DeletePrograms(GLsizei n, const GLuint *ids) } /* The ID is immediately available for re-use now */ _mesa_HashRemove(ctx->Shared->Programs, ids[i]); - prog->RefCount--; - if (prog->RefCount <= 0) { - ctx->Driver.DeleteProgram(ctx, prog); - } + _mesa_reference_program(ctx, &prog, NULL); } } } diff --git a/src/mesa/shader/program.h b/src/mesa/shader/program.h index ea2c8c30508..9a6883e6a55 100644 --- a/src/mesa/shader/program.h +++ b/src/mesa/shader/program.h @@ -86,6 +86,28 @@ _mesa_delete_program(GLcontext *ctx, struct gl_program *prog); extern struct gl_program * _mesa_lookup_program(GLcontext *ctx, GLuint id); +extern void +_mesa_reference_program(GLcontext *ctx, + struct gl_program **ptr, + struct gl_program *prog); + +static INLINE void +_mesa_reference_vertprog(GLcontext *ctx, + struct gl_vertex_program **ptr, + struct gl_vertex_program *prog) +{ + _mesa_reference_program(ctx, (struct gl_program **) ptr, + (struct gl_program *) prog); +} + +static INLINE void +_mesa_reference_fragprog(GLcontext *ctx, + struct gl_fragment_program **ptr, + struct gl_fragment_program *prog) +{ + _mesa_reference_program(ctx, (struct gl_program **) ptr, + (struct gl_program *) prog); +} extern struct gl_program * _mesa_clone_program(GLcontext *ctx, const struct gl_program *prog); diff --git a/src/mesa/shader/shader_api.c b/src/mesa/shader/shader_api.c index b0f79c29c1e..c319cef10a5 100644 --- a/src/mesa/shader/shader_api.c +++ b/src/mesa/shader/shader_api.c @@ -79,8 +79,7 @@ _mesa_clear_shader_program_data(GLcontext *ctx, /* to prevent a double-free in the next call */ shProg->VertexProgram->Base.Parameters = NULL; } - ctx->Driver.DeleteProgram(ctx, &shProg->VertexProgram->Base); - shProg->VertexProgram = NULL; + _mesa_reference_vertprog(ctx, &shProg->VertexProgram, NULL); } if (shProg->FragmentProgram) { @@ -88,8 +87,7 @@ _mesa_clear_shader_program_data(GLcontext *ctx, /* to prevent a double-free in the next call */ shProg->FragmentProgram->Base.Parameters = NULL; } - ctx->Driver.DeleteProgram(ctx, &shProg->FragmentProgram->Base); - shProg->FragmentProgram = NULL; + _mesa_reference_fragprog(ctx, &shProg->FragmentProgram, NULL); } if (shProg->Uniforms) { @@ -1100,6 +1098,8 @@ _mesa_link_program(GLcontext *ctx, GLuint program) return; } + FLUSH_VERTICES(ctx, _NEW_PROGRAM); + _slang_link(ctx, program, shProg); } diff --git a/src/mesa/shader/slang/slang_link.c b/src/mesa/shader/slang/slang_link.c index c8457fc483d..72fe9997c55 100644 --- a/src/mesa/shader/slang/slang_link.c +++ b/src/mesa/shader/slang/slang_link.c @@ -516,19 +516,19 @@ _slang_link(GLcontext *ctx, * changing src/dst registers after merging the uniforms and varying vars. */ if (vertProg) { - shProg->VertexProgram - = vertex_program(_mesa_clone_program(ctx, &vertProg->Base)); + _mesa_reference_vertprog(ctx, &shProg->VertexProgram, + vertex_program(_mesa_clone_program(ctx, &vertProg->Base))); } else { - shProg->VertexProgram = NULL; + _mesa_reference_vertprog(ctx, &shProg->VertexProgram, NULL); } if (fragProg) { - shProg->FragmentProgram - = fragment_program(_mesa_clone_program(ctx, &fragProg->Base)); + _mesa_reference_fragprog(ctx, &shProg->FragmentProgram, + fragment_program(_mesa_clone_program(ctx, &fragProg->Base))); } else { - shProg->FragmentProgram = NULL; + _mesa_reference_fragprog(ctx, &shProg->FragmentProgram, NULL); } if (shProg->VertexProgram) @@ -545,10 +545,12 @@ _slang_link(GLcontext *ctx, if (shProg->VertexProgram) { _mesa_free_parameter_list(shProg->VertexProgram->Base.Parameters); shProg->VertexProgram->Base.Parameters = shProg->Uniforms; + assert(shProg->Uniforms); } if (shProg->FragmentProgram) { _mesa_free_parameter_list(shProg->FragmentProgram->Base.Parameters); shProg->FragmentProgram->Base.Parameters = shProg->Uniforms; + assert(shProg->Uniforms); } if (shProg->VertexProgram) { diff --git a/src/mesa/tnl/t_vp_build.c b/src/mesa/tnl/t_vp_build.c index a7fd815a260..f254a4d7a4d 100644 --- a/src/mesa/tnl/t_vp_build.c +++ b/src/mesa/tnl/t_vp_build.c @@ -1573,7 +1573,8 @@ void _tnl_UpdateFixedFunctionProgram( GLcontext *ctx ) if (0) _mesa_printf("Found existing TNL program for key %x\n", hash); } - ctx->VertexProgram._Current = ctx->VertexProgram._TnlProgram; + _mesa_reference_vertprog(ctx, &ctx->VertexProgram._Current, + ctx->VertexProgram._TnlProgram); } /* Tell the driver about the change. Could define a new target for -- 2.11.0