Re: [Mesa-dev] [PATCH] st/mesa: add destroy_drawable interface

2017-07-24 Thread Marek Olšák
On Tue, Jul 25, 2017 at 12:20 AM, Charmaine Lee  wrote:
>
> Hi Marek,
>
> I have pushed the fix for Steam. Commit 
> bbc29393d3beaf6344c7188547b4ff61b63946ae
> should fix the problem.  Can you please try?

Thanks. Steam is working fine now.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] st/mesa: add destroy_drawable interface

2017-07-24 Thread Charmaine Lee

Hi Marek,

I have pushed the fix for Steam. Commit bbc29393d3beaf6344c7188547b4ff61b63946ae
should fix the problem.  Can you please try?

-Charmaine

From: Marek Olšák <mar...@gmail.com>
Sent: Monday, July 24, 2017 3:16:07 PM
To: Charmaine Lee
Cc: Christoph Haag; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH] st/mesa: add destroy_drawable interface

Hi Charmaine,

Which commits do I need to revert in 17.2 to unbreak Steam on Mesa? I
guess it's more than one.

Thanks,
Marek

On Sat, Jul 22, 2017 at 3:05 AM, Charmaine Lee <charmai...@vmware.com> wrote:
>
> 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  0x0023 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 , 
> 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 
> 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...@gma

Re: [Mesa-dev] [PATCH] st/mesa: add destroy_drawable interface

2017-07-24 Thread Marek Olšák
Hi Charmaine,

Which commits do I need to revert in 17.2 to unbreak Steam on Mesa? I
guess it's more than one.

Thanks,
Marek

On Sat, Jul 22, 2017 at 3:05 AM, Charmaine Lee <charmai...@vmware.com> wrote:
>
> 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  0x0023 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 , 
> 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 
> 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 
>&g

Re: [Mesa-dev] [PATCH] st/mesa: add destroy_drawable interface

2017-07-22 Thread Christoph Haag
It's the normal steam client and segfaults before it can show its gui.
Here is the trace: https://haagch.frickel.club/files/steam.trace
replaying this trace does not segfault, so maybe you need to 

Maybe I should mention that this is on an RX 480 with radeonsi.


On 22.07.2017 03:05, Charmaine Lee wrote:
> 
> 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  0x0023 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 , 
> 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 
> 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_tracke

Re: [Mesa-dev] [PATCH] st/mesa: add destroy_drawable interface

2017-07-21 Thread Charmaine Lee

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  0x0023 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 , 
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 
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.
> +*/
>

Re: [Mesa-dev] [PATCH] st/mesa: add destroy_drawable interface

2017-07-21 Thread Christoph Haag
This patch breaks steam for me. Segfault backtrace:

#0  0x0023 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 
#3  0xf6a448e5 in st_api_destroy_drawable (stapi=0xf7212bc0 , 
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 
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 
> Tested-by: Gert Wollny 
> ---
>  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 

Re: [Mesa-dev] [PATCH] st/mesa: add destroy_drawable interface

2017-07-21 Thread Emil Velikov
On 20 July 2017 at 19: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"
Please add proper tags, before merging.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101829
Fixes: 147d7fb772a ("st/mesa: add a winsys buffers list in st_context")

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] st/mesa: add destroy_drawable interface

2017-07-20 Thread Brian Paul
Looks good to me, but you might want to wait a day to see if there's any 
additional review feedback.


Reviewed-by: Brian Paul 


On 07/20/2017 12:26 PM, 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 
Tested-by: Gert Wollny 
---
  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(>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, >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 = >stvis;
 stfbi->flush_front = xmesa_st_framebuffer_flush_front;
 stfbi->validate = xmesa_st_framebuffer_validate;
+   stfbi->ID = p_atomic_inc_return(_stfbi_ID);
 p_atomic_set(>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(>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, >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 

[Mesa-dev] [PATCH] st/mesa: add destroy_drawable interface

2017-07-20 Thread Charmaine Lee
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 
Tested-by: Gert Wollny 
---
 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(>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, >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 = >stvis;
stfbi->flush_front = xmesa_st_framebuffer_flush_front;
stfbi->validate = xmesa_st_framebuffer_validate;
+   stfbi->ID = p_atomic_inc_return(_stfbi_ID);
p_atomic_set(>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(>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, >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"