On Thu, Aug 4, 2016 at 12:22 AM, Martin Peres <martin.pe...@linux.intel.com> wrote: > > > On 03/08/16 16:54, Nicolas Boichat wrote: >> >> In the case where dri2_initialize is called with a TestOnly display, >> the display is not actually initialized, so dri2_egl_display always >> fails, and we cannot do any reference counting. >> >> Fixes piglit spec@egl_khr_create_context@verify gl flavor (reproducible >> with LIBGL_ALWAYS_SOFTWARE=1) and spec@egl_khr_fence_sync@conformance. >> >> Fixes: 9ee683f877 (egl/dri2: Add reference count for dri2_egl_display) >> Cc: "12.0" <mesa-sta...@lists.freedesktop.org> >> Reported-by: Michel Dänzer <mic...@daenzer.net> >> Signed-off-by: Nicolas Boichat <drink...@chromium.org> >> --- >> >> Compile-tested only, please give it a spin, thanks! > > Still crashes, same backtrace before and after the patch:
Actually, I was thinking about this bug: https://bugs.freedesktop.org/show_bug.cgi?id=97136, which should be spec@egl_khr_create_context@verify gl flavor? Did you try that test? I probably should not have mentionned spec@egl_khr_fence_sync@conformance in the commit message... > > Program received signal SIGSEGV, Segmentation fault. > 0x00007ffff3b90de9 in wl_proxy_destroy () from > /usr/lib/libwayland-client.so.0 > (gdb) bt > #0 0x00007ffff3b90de9 in wl_proxy_destroy () from > /usr/lib/libwayland-client.so.0 > #1 0x00007ffff679bc4e in wl_registry_destroy (wl_registry=<optimized out>) > at /usr/include/wayland-client-protocol.h:1047 > #2 dri2_display_release (disp=disp@entry=0x7913f0) at > drivers/dri2/egl_dri2.c:896 > #3 0x00007ffff679c031 in dri2_terminate (drv=<optimized out>, > disp=0x7913f0) at drivers/dri2/egl_dri2.c:932 > #4 0x00007ffff6790f7d in eglTerminate (dpy=0x7913f0) at main/eglapi.c:532 > #5 0x00000000004016f5 in init_display (platform=12760, > out_dpy=0x7fffffffe3a0) at > /home/mupuf/programmation/piglit/tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c:136 > #6 0x000000000040285f in init_other_display (out_other_dpy=0x7fffffffe3d8, > orig_dpy=0x617c50) at > /home/mupuf/programmation/piglit/tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c:956 > #7 0x0000000000402917 in test_eglCreateSyncKHR_wrong_display_same_thread > (test_data=0x0) at > /home/mupuf/programmation/piglit/tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c:1006 > #8 0x00007ffff75e10aa in piglit_run_selected_subtests > (all_subtests=0x404380 <fence_sync_subtests>, selected_subtests=0x0, > num_selected_subtests=0, previous_result=PIGLIT_SKIP) at > /home/mupuf/programmation/piglit/tests/util/piglit-util.c:756 > #9 0x000000000040319d in main (argc=1, argv=0x7fffffffe578) at > /home/mupuf/programmation/piglit/tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c:1435 > > If you cannot reproduce, this is something I could be persuaded to look > into. Not easy for me to reproduce, but... Looking that the test source code: https://cgit.freedesktop.org/piglit/tree/tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c Do you know why we end up in the error path in init_display? My guess is that eglInitialize->dri2_initialize->dri2_initialize_wayland fails after setting disp->DriverData, so the display refcount is == 0, but the display is not null, leading to the crash in egl_terminate. I just spotted this patch for x11: https://patchwork.freedesktop.org/patch/101934/ platform_wayland needs to be modified in a similar way. Or, better, we need to refactor dri2_initialize_* functions to avoid touching _EGLDisplay. For the record, Emil spotted this issue when I submitted the offending patch, and I haven't followed up ,-( Best, > > >> >> src/egl/drivers/dri2/egl_dri2.c | 29 +++++++++-------------------- >> 1 file changed, 9 insertions(+), 20 deletions(-) >> >> diff --git a/src/egl/drivers/dri2/egl_dri2.c >> b/src/egl/drivers/dri2/egl_dri2.c >> index a5cab68..8cdca6a 100644 >> --- a/src/egl/drivers/dri2/egl_dri2.c >> +++ b/src/egl/drivers/dri2/egl_dri2.c >> @@ -788,45 +788,34 @@ dri2_initialize(_EGLDriver *drv, _EGLDisplay *disp) >> if (disp->Options.UseFallback) >> return EGL_FALSE; >> >> + /* Nothing to initialize for a test only display */ >> + if (disp->Options.TestOnly) >> + return EGL_TRUE; >> + >> switch (disp->Platform) { >> #ifdef HAVE_SURFACELESS_PLATFORM >> case _EGL_PLATFORM_SURFACELESS: >> - if (disp->Options.TestOnly) >> - ret = EGL_TRUE; >> - else >> - ret = dri2_initialize_surfaceless(drv, disp); >> + ret = dri2_initialize_surfaceless(drv, disp); >> break; >> #endif >> #ifdef HAVE_X11_PLATFORM >> case _EGL_PLATFORM_X11: >> - if (disp->Options.TestOnly) >> - ret = EGL_TRUE; >> - else >> - ret = dri2_initialize_x11(drv, disp); >> + ret = dri2_initialize_x11(drv, disp); >> break; >> #endif >> #ifdef HAVE_DRM_PLATFORM >> case _EGL_PLATFORM_DRM: >> - if (disp->Options.TestOnly) >> - ret = EGL_TRUE; >> - else >> - ret = dri2_initialize_drm(drv, disp); >> + ret = dri2_initialize_drm(drv, disp); >> break; >> #endif >> #ifdef HAVE_WAYLAND_PLATFORM >> case _EGL_PLATFORM_WAYLAND: >> - if (disp->Options.TestOnly) >> - ret = EGL_TRUE; >> - else >> - ret = dri2_initialize_wayland(drv, disp); >> + ret = dri2_initialize_wayland(drv, disp); >> break; >> #endif >> #ifdef HAVE_ANDROID_PLATFORM >> case _EGL_PLATFORM_ANDROID: >> - if (disp->Options.TestOnly) >> - ret = EGL_TRUE; >> - else >> - ret = dri2_initialize_android(drv, disp); >> + ret = dri2_initialize_android(drv, disp); >> break; >> #endif >> default: >> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
