Hi Christoph, Can you provide an apitrace of the test that crashes?
Thanks. -Charmaine ________________________________________ From: mesa-dev <mesa-dev-boun...@lists.freedesktop.org> on behalf of Christoph Haag <haagch+mesa...@frickel.club> Sent: Friday, July 21, 2017 4:52:41 PM To: mesa-dev@lists.freedesktop.org Subject: Re: [Mesa-dev] [PATCH] st/mesa: add destroy_drawable interface This patch breaks steam for me. Segfault backtrace: #0 0x00000023 in ?? () No symbol table info available. #1 0xf6b1a417 in _mesa_hash_table_search (ht=0x585bb7e8, key=0x5820ee88) at hash_table.c:246 __PRETTY_FUNCTION__ = "_mesa_hash_table_search" #2 0xf6a4488e in st_framebuffer_iface_remove (stfbi=0x5820ee88) at state_tracker/st_manager.c:545 entry = 0xf6bb3038 <swap_fences_unref+86> #3 0xf6a448e5 in st_api_destroy_drawable (stapi=0xf7212bc0 <st_gl_api>, stfbi=0x5820ee88) at state_tracker/st_manager.c:564 No locals. #4 0xf6bb2b64 in dri_destroy_buffer (dPriv=0x57d62560) at dri_drawable.c:186 drawable = 0x5820ee88 screen = 0x57e1a0a8 stapi = 0xf7212bc0 <st_gl_api> i = 7 #5 0xf6bb132c in dri_put_drawable (pdp=0x57d62560) at dri_util.c:642 No locals. #6 0xf6bb145c in driDestroyDrawable (pdp=0x57d62560) at dri_util.c:695 No locals. #7 0xf759249a in loader_dri3_drawable_fini (draw=0x581fb0f0) at loader_dri3_helper.c:109 i = 1478471608 #8 0xf758ca73 in dri3_destroy_drawable (base=0x581fb0d0) at dri3_glx.c:366 pdraw = 0x581fb0d0 #9 0xf7583b5a in driReleaseDrawables (gc=0x57e19ef8) at dri_common.c:452 priv = 0x57e082d0 pdraw = 0x581fb0d0 #10 0xf758c679 in dri3_bind_context (context=0x57e19ef8, old=0x57e19ef8, draw=165675047, read=165675047) at dri3_glx.c:223 pcp = 0x57e19ef8 psc = 0x57e07608 pdraw = 0x5866eb28 pread = 0x5866eb28 dri_draw = 0x0 dri_read = 0x0 #11 0xf754cd12 in MakeContextCurrent (dpy=0x57c16f30, draw=165675047, read=165675047, gc_user=0x57e19ef8) at glxcurrent.c:228 gc = 0x57e19ef8 oldGC = 0x57e19ef8 #12 0xf754ce75 in glXMakeCurrent (dpy=0x57c16f30, draw=165675047, gc=0x57e19ef8) at glxcurrent.c:274 No locals. #13 0xeb48addb in ?? () from /home/chris/.local/share/Steam/ubuntu12_32/vgui2_s.so No symbol table info available. #14 0xeb48dd0e in ?? () from /home/chris/.local/share/Steam/ubuntu12_32/vgui2_s.so No symbol table info available. #15 0xeb49d81d in ?? () from /home/chris/.local/share/Steam/ubuntu12_32/vgui2_s.so No symbol table info available. #16 0xefd74193 in ?? () from /home/chris/.local/share/Steam/ubuntu12_32/steamui.so No symbol table info available. #17 0xefd76946 in ?? () from /home/chris/.local/share/Steam/ubuntu12_32/steamui.so No symbol table info available. #18 0xefd77f16 in ?? () from /home/chris/.local/share/Steam/ubuntu12_32/steamui.so No symbol table info available. #19 0x5663d660 in RunSteam(int, char**, bool) () No symbol table info available. #20 0x5663e5f3 in ?? () No symbol table info available. #21 0x56629aac in ?? () No symbol table info available. #22 0xf799a1d3 in __libc_start_main () from /usr/lib32/libc.so.6 No symbol table info available. #23 0x5662d159 in _start () No symbol table info available. On 20.07.2017 20:26, Charmaine Lee wrote: > With this patch, the st manager will maintain a hash table for > the active framebuffer interface objects. A destroy_drawable interface > is added to allow the state tracker to notify the st manager to remove > the associated framebuffer interface object from the hash table, > so the associated framebuffer and its resources can be deleted > at framebuffers purge time. > > Fixes bug 101829 "read-after-free in st_framebuffer_validate" > > Tested-by: Brad King <brad.k...@kitware.com> > Tested-by: Gert Wollny <gw.foss...@gmail.com> > --- > src/gallium/include/state_tracker/st_api.h | 7 ++ > src/gallium/state_trackers/dri/dri_drawable.c | 6 +- > src/gallium/state_trackers/glx/xlib/xm_api.c | 5 ++ > src/gallium/state_trackers/glx/xlib/xm_st.c | 2 + > src/gallium/state_trackers/wgl/stw_st.c | 6 +- > src/mesa/state_tracker/st_manager.c | 95 > ++++++++++++++++++++++++++- > src/mesa/state_tracker/st_manager.h | 5 ++ > 7 files changed, 123 insertions(+), 3 deletions(-) > > diff --git a/src/gallium/include/state_tracker/st_api.h > b/src/gallium/include/state_tracker/st_api.h > index 30a4866..9b660f7 100644 > --- a/src/gallium/include/state_tracker/st_api.h > +++ b/src/gallium/include/state_tracker/st_api.h > @@ -552,6 +552,13 @@ struct st_api > * Get the currently bound context in the calling thread. > */ > struct st_context_iface *(*get_current)(struct st_api *stapi); > + > + /** > + * Notify the st manager the framebuffer interface object > + * is no longer valid. > + */ > + void (*destroy_drawable)(struct st_api *stapi, > + struct st_framebuffer_iface *stfbi); > }; > > /** > diff --git a/src/gallium/state_trackers/dri/dri_drawable.c > b/src/gallium/state_trackers/dri/dri_drawable.c > index 0cfdc30..c7df0f6 100644 > --- a/src/gallium/state_trackers/dri/dri_drawable.c > +++ b/src/gallium/state_trackers/dri/dri_drawable.c > @@ -169,6 +169,8 @@ void > dri_destroy_buffer(__DRIdrawable * dPriv) > { > struct dri_drawable *drawable = dri_drawable(dPriv); > + struct dri_screen *screen = drawable->screen; > + struct st_api *stapi = screen->st_api; > int i; > > pipe_surface_reference(&drawable->drisw_surface, NULL); > @@ -180,7 +182,9 @@ dri_destroy_buffer(__DRIdrawable * dPriv) > > swap_fences_unref(drawable); > > - drawable->base.ID = 0; > + /* Notify the st manager that this drawable is no longer valid */ > + stapi->destroy_drawable(stapi, &drawable->base); > + > FREE(drawable); > } > > diff --git a/src/gallium/state_trackers/glx/xlib/xm_api.c > b/src/gallium/state_trackers/glx/xlib/xm_api.c > index 881dd44..e4b1e9d 100644 > --- a/src/gallium/state_trackers/glx/xlib/xm_api.c > +++ b/src/gallium/state_trackers/glx/xlib/xm_api.c > @@ -595,6 +595,11 @@ xmesa_free_buffer(XMesaBuffer buffer) > */ > b->ws.drawable = 0; > > + /* Notify the st manager that the associated framebuffer interface > + * object is no longer valid. > + */ > + stapi->destroy_drawable(stapi, buffer->stfb); > + > /* XXX we should move the buffer to a delete-pending list and > destroy > * the buffer until it is no longer current. > */ > diff --git a/src/gallium/state_trackers/glx/xlib/xm_st.c > b/src/gallium/state_trackers/glx/xlib/xm_st.c > index 9e30efa..6a0f4aa 100644 > --- a/src/gallium/state_trackers/glx/xlib/xm_st.c > +++ b/src/gallium/state_trackers/glx/xlib/xm_st.c > @@ -273,6 +273,7 @@ xmesa_st_framebuffer_flush_front(struct st_context_iface > *stctx, > return ret; > } > > +static uint32_t xmesa_stfbi_ID = 0; > > struct st_framebuffer_iface * > xmesa_create_st_framebuffer(XMesaDisplay xmdpy, XMesaBuffer b) > @@ -302,6 +303,7 @@ xmesa_create_st_framebuffer(XMesaDisplay xmdpy, > XMesaBuffer b) > stfbi->visual = &xstfb->stvis; > stfbi->flush_front = xmesa_st_framebuffer_flush_front; > stfbi->validate = xmesa_st_framebuffer_validate; > + stfbi->ID = p_atomic_inc_return(&xmesa_stfbi_ID); > p_atomic_set(&stfbi->stamp, 1); > stfbi->st_manager_private = (void *) xstfb; > > diff --git a/src/gallium/state_trackers/wgl/stw_st.c > b/src/gallium/state_trackers/wgl/stw_st.c > index c2844b0..85a8b17 100644 > --- a/src/gallium/state_trackers/wgl/stw_st.c > +++ b/src/gallium/state_trackers/wgl/stw_st.c > @@ -256,7 +256,11 @@ stw_st_destroy_framebuffer_locked(struct > st_framebuffer_iface *stfb) > for (i = 0; i < ST_ATTACHMENT_COUNT; i++) > pipe_resource_reference(&stwfb->textures[i], NULL); > > - stwfb->base.ID = 0; > + /* Notify the st manager that the framebuffer interface is no > + * longer valid. > + */ > + stw_dev->stapi->destroy_drawable(stw_dev->stapi, &stwfb->base); > + > FREE(stwfb); > } > > diff --git a/src/mesa/state_tracker/st_manager.c > b/src/mesa/state_tracker/st_manager.c > index cb816de..ebc7ca8 100644 > --- a/src/mesa/state_tracker/st_manager.c > +++ b/src/mesa/state_tracker/st_manager.c > @@ -38,6 +38,7 @@ > #include "main/fbobject.h" > #include "main/renderbuffer.h" > #include "main/version.h" > +#include "util/hash_table.h" > #include "st_texture.h" > > #include "st_context.h" > @@ -59,6 +60,10 @@ > #include "util/u_surface.h" > #include "util/list.h" > > +struct hash_table; > +static struct hash_table *st_fbi_ht; /* framebuffer iface objects hash table > */ > + > +static mtx_t st_mutex = _MTX_INITIALIZER_NP; > > /** > * Map an attachment to a buffer index. > @@ -490,6 +495,76 @@ st_framebuffer_reference(struct st_framebuffer **ptr, > _mesa_reference_framebuffer((struct gl_framebuffer **) ptr, fb); > } > > + > +static uint32_t > +st_framebuffer_iface_hash(const void *key) > +{ > + return (uintptr_t)key; > +} > + > + > +static bool > +st_framebuffer_iface_equal(const void *a, const void *b) > +{ > + return (struct st_framebuffer_iface *)a == (struct st_framebuffer_iface > *)b; > +} > + > + > +static boolean > +st_framebuffer_iface_lookup(const struct st_framebuffer_iface *stfbi) > +{ > + struct hash_entry *entry; > + > + mtx_lock(&st_mutex); > + entry = _mesa_hash_table_search(st_fbi_ht, stfbi); > + mtx_unlock(&st_mutex); > + > + return entry != NULL; > +} > + > + > +static boolean > +st_framebuffer_iface_insert(struct st_framebuffer_iface *stfbi) > +{ > + struct hash_entry *entry; > + > + mtx_lock(&st_mutex); > + entry = _mesa_hash_table_insert(st_fbi_ht, stfbi, stfbi); > + mtx_unlock(&st_mutex); > + > + return entry != NULL; > +} > + > + > +static void > +st_framebuffer_iface_remove(struct st_framebuffer_iface *stfbi) > +{ > + struct hash_entry *entry; > + > + mtx_lock(&st_mutex); > + entry = _mesa_hash_table_search(st_fbi_ht, stfbi); > + if (!entry) > + goto unlock; > + > + _mesa_hash_table_remove(st_fbi_ht, entry); > + > +unlock: > + mtx_unlock(&st_mutex); > +} > + > + > +/** > + * The framebuffer interface object is no longer valid. > + * Remove the object from the framebuffer interface hash table. > + */ > +static void > +st_api_destroy_drawable(struct st_api *stapi, > + struct st_framebuffer_iface *stfbi) > +{ > + st_framebuffer_iface_remove(stfbi); > +} > + > + > /** > * Purge the winsys buffers list to remove any references to > * non-existing framebuffer interface objects. > @@ -506,7 +581,7 @@ st_framebuffers_purge(struct st_context *st) > * and unreference the framebuffer object, so its resources can be > * deleted. > */ > - if (stfb->iface->ID != stfb->iface_ID) { > + if (!st_framebuffer_iface_lookup(stfb->iface)) { > LIST_DEL(&stfb->head); > st_framebuffer_reference(&stfb, NULL); > } > @@ -810,6 +885,14 @@ st_framebuffer_reuse_or_create(struct st_context *st, > cur = st_framebuffer_create(st, stfbi); > > if (cur) { > + /* add the referenced framebuffer interface object to > + * the framebuffer interface object hash table. > + */ > + if (!st_framebuffer_iface_insert(stfbi)) { > + st_framebuffer_reference(&cur, NULL); > + return NULL; > + } > + > /* add to the context's winsys buffers list */ > LIST_ADD(&cur->head, &st->winsys_buffers); > > @@ -881,6 +964,8 @@ st_api_make_current(struct st_api *stapi, struct > st_context_iface *stctxi, > static void > st_api_destroy(struct st_api *stapi) > { > + _mesa_hash_table_destroy(st_fbi_ht, NULL); > + mtx_destroy(&st_mutex); > } > > /** > @@ -1015,10 +1100,18 @@ static const struct st_api st_gl_api = { > .create_context = st_api_create_context, > .make_current = st_api_make_current, > .get_current = st_api_get_current, > + .destroy_drawable = st_api_destroy_drawable, > }; > > struct st_api * > st_gl_api_create(void) > { > + /* Create a hash table for all the framebuffer interface objects */ > + > + mtx_init(&st_mutex, mtx_plain); > + st_fbi_ht = _mesa_hash_table_create(NULL, > + st_framebuffer_iface_hash, > + st_framebuffer_iface_equal); > + > return (struct st_api *) &st_gl_api; > } > diff --git a/src/mesa/state_tracker/st_manager.h > b/src/mesa/state_tracker/st_manager.h > index 68adf2f..c54f29e 100644 > --- a/src/mesa/state_tracker/st_manager.h > +++ b/src/mesa/state_tracker/st_manager.h > @@ -34,6 +34,7 @@ > > struct st_context; > struct st_framebuffer; > +struct st_framebuffer_interface; > > void > st_manager_flush_frontbuffer(struct st_context *st); > @@ -48,4 +49,8 @@ st_manager_add_color_renderbuffer(struct st_context *st, > struct gl_framebuffer * > void > st_framebuffer_reference(struct st_framebuffer **ptr, > struct st_framebuffer *stfb); > + > +void > +st_framebuffer_interface_destroy(struct st_framebuffer_interface *stfbi); > + > #endif /* ST_MANAGER_H */ > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev