From 35a15f02634a31c1517363d91aaef8f190e24687 Mon Sep 17 00:00:00 2001 From: Roland Scheidegger Date: Thu, 3 Dec 2009 23:15:38 +0100 Subject: [PATCH] gallium: fix reference counting functions to be strict-aliasing compliant Historically, parts of mesa code are not strict-aliasing safe, hence -fno-strict-aliasing is needed to compile (this got forgotten for scons builds for gallium, which indeed not only caused compiler warnings but also unexplicable crashes in non-debug builds). However, we should try to eliminate code not complying with strict-aliasing code at least for gallium. Hence change pipe_reference functions to make them strict-aliasing compliant. This adds a bit more complexity (especially for derived classes) but is the right thing to do, and it does in fact fix a segfault. --- src/gallium/auxiliary/pipebuffer/pb_buffer.h | 3 ++- src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c | 5 ++++- src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c | 3 +++ src/gallium/drivers/svga/svga_screen_texture.h | 3 ++- src/gallium/include/pipe/p_refcnt.h | 18 ++++++++---------- src/gallium/include/pipe/p_state.h | 9 ++++++--- src/gallium/include/pipe/p_video_state.h | 3 ++- 7 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/gallium/auxiliary/pipebuffer/pb_buffer.h b/src/gallium/auxiliary/pipebuffer/pb_buffer.h index 4ef3722..eb7e84b 100644 --- a/src/gallium/auxiliary/pipebuffer/pb_buffer.h +++ b/src/gallium/auxiliary/pipebuffer/pb_buffer.h @@ -237,8 +237,9 @@ pb_reference(struct pb_buffer **dst, { struct pb_buffer *old = *dst; - if (pipe_reference((struct pipe_reference**)dst, &src->base.reference)) + if (pipe_reference(&(*dst)->base.reference, &src->base.reference)) pb_destroy( old ); + *dst = src; } diff --git a/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c b/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c index 2f97368..a9375ab 100644 --- a/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c +++ b/src/gallium/auxiliary/pipebuffer/pb_buffer_fenced.c @@ -243,6 +243,7 @@ fenced_buffer_list_check_free_locked(struct fenced_buffer_list *fenced_list, struct pb_fence_ops *ops = fenced_list->ops; struct list_head *curr, *next; struct fenced_buffer *fenced_buf; + struct pb_buffer *pb_buf; struct pipe_fence_handle *prev_fence = NULL; curr = fenced_list->delayed.next; @@ -271,7 +272,9 @@ fenced_buffer_list_check_free_locked(struct fenced_buffer_list *fenced_list, fenced_buffer_remove_locked(fenced_list, fenced_buf); pipe_mutex_unlock(fenced_buf->mutex); - pb_reference((struct pb_buffer **)&fenced_buf, NULL); + pb_buf = &fenced_buf->base; + pb_reference(&pb_buf, NULL); + curr = next; next = curr->next; diff --git a/src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c b/src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c index 57d1ede..f0c88a0 100644 --- a/src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c +++ b/src/gallium/auxiliary/pipebuffer/pb_bufmgr_cache.c @@ -293,8 +293,11 @@ pb_cache_manager_create_buffer(struct pb_manager *_mgr, if(buf) { LIST_DEL(&buf->head); pipe_mutex_unlock(mgr->mutex); +#if 0 + /* XXX this didn't do anything right??? */ /* Increase refcount */ pb_reference((struct pb_buffer**)&buf, &buf->base); +#endif return &buf->base; } diff --git a/src/gallium/drivers/svga/svga_screen_texture.h b/src/gallium/drivers/svga/svga_screen_texture.h index 1cc4063..727f2c5 100644 --- a/src/gallium/drivers/svga/svga_screen_texture.h +++ b/src/gallium/drivers/svga/svga_screen_texture.h @@ -164,8 +164,9 @@ svga_sampler_view_reference(struct svga_sampler_view **ptr, struct svga_sampler_ { struct svga_sampler_view *old = *ptr; - if (pipe_reference((struct pipe_reference **)ptr, &v->reference)) + if (pipe_reference(&(*ptr)->reference, &v->reference)) svga_destroy_sampler_view_priv(old); + *ptr = v; } extern void diff --git a/src/gallium/include/pipe/p_refcnt.h b/src/gallium/include/pipe/p_refcnt.h index 1f9088b..f1875b6 100644 --- a/src/gallium/include/pipe/p_refcnt.h +++ b/src/gallium/include/pipe/p_refcnt.h @@ -59,30 +59,28 @@ pipe_is_referenced(struct pipe_reference *reference) /** - * Set 'ptr' to point to 'reference' and update reference counting. - * The old thing pointed to, if any, will be unreferenced first. - * 'reference' may be NULL. + * Update reference counting. + * The old thing pointed to, if any, will be unreferenced. + * Both 'ptr' and 'reference' may be NULL. */ static INLINE bool -pipe_reference(struct pipe_reference **ptr, struct pipe_reference *reference) +pipe_reference(struct pipe_reference *ptr, struct pipe_reference *reference) { bool destroy = FALSE; - if(*ptr != reference) { + if(ptr != reference) { /* bump the reference.count first */ if (reference) { assert(pipe_is_referenced(reference)); p_atomic_inc(&reference->count); } - if (*ptr) { - assert(pipe_is_referenced(*ptr)); - if (p_atomic_dec_zero(&(*ptr)->count)) { + if (ptr) { + assert(pipe_is_referenced(ptr)); + if (p_atomic_dec_zero(&ptr->count)) { destroy = TRUE; } } - - *ptr = reference; } return destroy; diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index 6de7af6..b9dfa1c 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -400,8 +400,9 @@ pipe_buffer_reference(struct pipe_buffer **ptr, struct pipe_buffer *buf) { struct pipe_buffer *old_buf = *ptr; - if (pipe_reference((struct pipe_reference **)ptr, &buf->reference)) + if (pipe_reference(&(*ptr)->reference, &buf->reference)) old_buf->screen->buffer_destroy(old_buf); + *ptr = buf; } static INLINE void @@ -409,8 +410,9 @@ pipe_surface_reference(struct pipe_surface **ptr, struct pipe_surface *surf) { struct pipe_surface *old_surf = *ptr; - if (pipe_reference((struct pipe_reference **)ptr, &surf->reference)) + if (pipe_reference(&(*ptr)->reference, &surf->reference)) old_surf->texture->screen->tex_surface_destroy(old_surf); + *ptr = surf; } static INLINE void @@ -418,8 +420,9 @@ pipe_texture_reference(struct pipe_texture **ptr, struct pipe_texture *tex) { struct pipe_texture *old_tex = *ptr; - if (pipe_reference((struct pipe_reference **)ptr, &tex->reference)) + if (pipe_reference(&(*ptr)->reference, &tex->reference)) old_tex->screen->texture_destroy(old_tex); + *ptr = tex; } diff --git a/src/gallium/include/pipe/p_video_state.h b/src/gallium/include/pipe/p_video_state.h index 4da26d6..b85f01c 100644 --- a/src/gallium/include/pipe/p_video_state.h +++ b/src/gallium/include/pipe/p_video_state.h @@ -56,8 +56,9 @@ pipe_video_surface_reference(struct pipe_video_surface **ptr, struct pipe_video_ { struct pipe_video_surface *old_surf = *ptr; - if (pipe_reference((struct pipe_reference **)ptr, &surf->reference)) + if (pipe_reference(&(*ptr)->reference, &surf->reference)) old_surf->screen->video_surface_destroy(old_surf); + *ptr = surf; } struct pipe_video_rect -- 2.7.4